From 8b10c20465f832b78b0f80bd93c9f99d4e67c7fd Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Mon, 22 May 2023 15:53:03 +0200 Subject: [PATCH] move most of the code to strapi/ee --- .../core/admin/ee/server/services/passport.js | 17 ++++++++-- .../passport => ee/server}/utils/sso-lock.js | 33 +++++++++++-------- packages/core/admin/server/services/auth.js | 11 +++---- .../services/passport/local-strategy.js | 9 ++--- 4 files changed, 41 insertions(+), 29 deletions(-) rename packages/core/admin/{server/services/passport => ee/server}/utils/sso-lock.js (55%) diff --git a/packages/core/admin/ee/server/services/passport.js b/packages/core/admin/ee/server/services/passport.js index 86d9d8d07d..f84c6cd918 100644 --- a/packages/core/admin/ee/server/services/passport.js +++ b/packages/core/admin/ee/server/services/passport.js @@ -5,14 +5,25 @@ const { features } = require('@strapi/strapi/lib/utils/ee'); const createLocalStrategy = require('../../../server/services/passport/local-strategy'); const sso = require('./passport/sso'); +const { isSsoLocked } = require('../utils/sso-lock'); + +const localStrategyMiddleware = async ([error, user, message], done) => { + if (await isSsoLocked(user)) { + return done(error, null, { + message: 'Login not allowed, please contact your administrator', + }); + } + + return done(error, user, message); +}; const getPassportStrategies = () => { - const localStrategy = createLocalStrategy(strapi); - if (!features.isEnabled('sso')) { - return [localStrategy]; + return [createLocalStrategy(strapi)]; } + const localStrategy = createLocalStrategy(strapi, localStrategyMiddleware); + if (!strapi.isLoaded) { sso.syncProviderRegistryWithConfig(); } diff --git a/packages/core/admin/server/services/passport/utils/sso-lock.js b/packages/core/admin/ee/server/utils/sso-lock.js similarity index 55% rename from packages/core/admin/server/services/passport/utils/sso-lock.js rename to packages/core/admin/ee/server/utils/sso-lock.js index c24d605cb6..c3b42b582b 100644 --- a/packages/core/admin/server/services/passport/utils/sso-lock.js +++ b/packages/core/admin/ee/server/utils/sso-lock.js @@ -1,36 +1,41 @@ 'use strict'; +const { features } = require('@strapi/strapi/ee'); +const { isEmpty } = require('lodash/fp'); + const isSsoLocked = async (user) => { - if (!strapi.EE || !user) { + if (!features.isEnabled('sso') || !user) { // TODO: we should be calling strapi.features.isEnabled("sso") but that's EE code. Should we load it dynamically when EE is enabled? Or add EE code to override this strategy? return false; } - // TODO: if user object has roles === undefined, we need to query for it. [] should be fine, it means we got the roles object but they don't have any - + // check if any roles are locked const adminStore = await strapi.store({ type: 'core', name: 'admin' }); const { providers } = await adminStore.get({ key: 'auth' }); const lockedRoles = providers.authenticationDisabled || []; + if (isEmpty(lockedRoles)) { + return false; + } + + // Ensure we have user.roles and get them if we don't have them + let roles = user.roles; + if (!user.roles) { + const u = await strapi.query('admin::user').findOne({ + where: { id: user.id }, + populate: ['roles'], + }); + roles = u.roles; + } // Check for roles that have blocked const isLocked = lockedRoles.some((lockedId) => // lockedRoles will be a string to avoid issues with frontend and bigints - user.roles?.some((role) => lockedId === role.id.toString()) + roles?.some((role) => lockedId === role.id.toString()) ); return isLocked; }; -const userPopulateForSso = () => { - if (!strapi.EE) { - // TODO: we should be calling strapi.features.isEnabled("sso") but that's EE code. Should we load it dynamically when EE is enabled? Or add EE code to override this strategy? - return undefined; - } - - return ['roles']; -}; - module.exports = { isSsoLocked, - userPopulateForSso, }; diff --git a/packages/core/admin/server/services/auth.js b/packages/core/admin/server/services/auth.js index 4a6b9bbde9..b3c5487c38 100644 --- a/packages/core/admin/server/services/auth.js +++ b/packages/core/admin/server/services/auth.js @@ -5,7 +5,7 @@ const _ = require('lodash'); const { getAbsoluteAdminUrl } = require('@strapi/utils'); const { ApplicationError } = require('@strapi/utils').errors; const { getService } = require('../utils'); -const { isSsoLocked, userPopulateForSso } = require('./passport/utils/sso-lock'); +const { isSsoLocked } = require('../../ee/server/utils/sso-lock'); /** * hashes a password @@ -31,7 +31,6 @@ const validatePassword = (password, hash) => bcrypt.compare(password, hash); const checkCredentials = async ({ email, password }) => { const user = await strapi.query('admin::user').findOne({ where: { email }, - populate: userPopulateForSso(), }); if (!user || !user.password) { @@ -59,11 +58,10 @@ const checkCredentials = async ({ email, password }) => { * @param {string} param.email user email for which to reset the password */ const forgotPassword = async ({ email } = {}) => { - const user = await strapi - .query('admin::user') - .findOne({ where: { email, isActive: true }, populate: userPopulateForSso() }); + const user = await strapi.query('admin::user').findOne({ where: { email, isActive: true } }); if (!user || (await isSsoLocked(user))) { + // TODO: this needs to be removed and overidden in /ee/ return; } @@ -104,9 +102,10 @@ const forgotPassword = async ({ email } = {}) => { const resetPassword = async ({ resetPasswordToken, password } = {}) => { const matchingUser = await strapi .query('admin::user') - .findOne({ where: { resetPasswordToken, isActive: true }, populate: userPopulateForSso() }); + .findOne({ where: { resetPasswordToken, isActive: true } }); if (!matchingUser || isSsoLocked(matchingUser)) { + // TODO: this needs to be removed and overidden in /ee/ throw new ApplicationError(); } diff --git a/packages/core/admin/server/services/passport/local-strategy.js b/packages/core/admin/server/services/passport/local-strategy.js index a157a6a151..c87d3c5f3e 100644 --- a/packages/core/admin/server/services/passport/local-strategy.js +++ b/packages/core/admin/server/services/passport/local-strategy.js @@ -2,9 +2,8 @@ const { toLower } = require('lodash/fp'); const { Strategy: LocalStrategy } = require('passport-local'); -const { isSsoLocked } = require('./utils/sso-lock'); -const createLocalStrategy = (strapi) => { +const createLocalStrategy = (strapi, middleware) => { return new LocalStrategy( { usernameField: 'email', @@ -18,10 +17,8 @@ const createLocalStrategy = (strapi) => { password, }) .then(async ([error, user, message]) => { - if (await isSsoLocked(user)) { - return done(error, null, { - message: 'Login not allowed, please contact your administrator', - }); + if (middleware) { + return middleware([error, user, message], done); } return done(error, user, message);