From 237fe84c54341b788c99a099b1f6018698074c2f Mon Sep 17 00:00:00 2001 From: advplyr Date: Fri, 10 Nov 2023 16:11:51 -0600 Subject: [PATCH] Add new API endpoint for updating auth-settings and update passport auth strategies --- client/pages/config/authentication.vue | 20 ++- server/Auth.js | 196 +++++++++++++--------- server/controllers/MiscController.js | 88 +++++++++- server/objects/settings/ServerSettings.js | 68 ++++---- server/routers/ApiRouter.js | 2 + 5 files changed, 255 insertions(+), 119 deletions(-) diff --git a/client/pages/config/authentication.vue b/client/pages/config/authentication.vue index 0da486c1..9ea8172a 100644 --- a/client/pages/config/authentication.vue +++ b/client/pages/config/authentication.vue @@ -199,13 +199,19 @@ export default { if (this.enableOpenIDAuth) this.newAuthSettings.authActiveAuthMethods.push('openid') this.savingSettings = true - const success = await this.$store.dispatch('updateServerSettings', this.newAuthSettings) - this.savingSettings = false - if (success) { - this.$toast.success('Server settings updated') - } else { - this.$toast.error('Failed to update server settings') - } + this.$axios + .$patch('/api/auth-settings', this.newAuthSettings) + .then((data) => { + this.$store.commit('setServerSettings', data.serverSettings) + this.$toast.success('Server settings updated') + }) + .catch((error) => { + console.error('Failed to update server settings', error) + this.$toast.error('Failed to update server settings') + }) + .finally(() => { + this.savingSettings = false + }) }, init() { this.newAuthSettings = { diff --git a/server/Auth.js b/server/Auth.js index eeb7ad47..15e36576 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -36,7 +36,12 @@ class Auth { async initPassportJs() { // Check if we should load the local strategy (username + password login) if (global.ServerSettings.authActiveAuthMethods.includes("local")) { - passport.use(new LocalStrategy(this.localAuthCheckUserPw.bind(this))) + this.initAuthStrategyPassword() + } + + // Check if we should load the openid strategy + if (global.ServerSettings.authActiveAuthMethods.includes("openid")) { + this.initAuthStrategyOpenID() } // Check if we should load the google-oauth20 strategy @@ -62,84 +67,6 @@ class Auth { }).bind(this))) } - // Check if we should load the openid strategy - if (global.ServerSettings.authActiveAuthMethods.includes("openid")) { - const openIdIssuerClient = new OpenIDClient.Issuer({ - issuer: global.ServerSettings.authOpenIDIssuerURL, - authorization_endpoint: global.ServerSettings.authOpenIDAuthorizationURL, - token_endpoint: global.ServerSettings.authOpenIDTokenURL, - userinfo_endpoint: global.ServerSettings.authOpenIDUserInfoURL, - jwks_uri: global.ServerSettings.authOpenIDJwksURL - }).Client - const openIdClient = new openIdIssuerClient({ - client_id: global.ServerSettings.authOpenIDClientID, - client_secret: global.ServerSettings.authOpenIDClientSecret - }) - passport.use('openid-client', new OpenIDClient.Strategy({ - client: openIdClient, - params: { - redirect_uri: '/auth/openid/callback', - scope: 'openid profile email' - } - }, async (tokenset, userinfo, done) => { - Logger.debug(`[Auth] openid callback userinfo=`, userinfo) - - if (!userinfo.sub) { - Logger.error(`[Auth] openid callback invalid userinfo, no sub`) - return done(null, null) - } - - // First check for matching user by sub - let user = await Database.userModel.getUserByOpenIDSub(userinfo.sub) - if (!user) { - // Optionally match existing by email or username based on server setting "authOpenIDMatchExistingBy" - if (Database.serverSettings.authOpenIDMatchExistingBy === 'email' && userinfo.email && userinfo.email_verified) { - Logger.info(`[Auth] openid: User not found, checking existing with email "${userinfo.email}"`) - user = await Database.userModel.getUserByEmail(userinfo.email) - // Check that user is not already matched - if (user?.authOpenIDSub) { - Logger.warn(`[Auth] openid: User found with email "${userinfo.email}" but is already matched with sub "${user.authOpenIDSub}"`) - // TODO: Show some error log? - user = null - } - } else if (Database.serverSettings.authOpenIDMatchExistingBy === 'username' && userinfo.preferred_username) { - Logger.info(`[Auth] openid: User not found, checking existing with username "${userinfo.preferred_username}"`) - user = await Database.userModel.getUserByUsername(userinfo.preferred_username) - // Check that user is not already matched - if (user?.authOpenIDSub) { - Logger.warn(`[Auth] openid: User found with username "${userinfo.preferred_username}" but is already matched with sub "${user.authOpenIDSub}"`) - // TODO: Show some error log? - user = null - } - } - - // If existing user was matched and isActive then save sub to user - if (user?.isActive) { - Logger.info(`[Auth] openid: New user found matching existing user "${user.username}"`) - user.authOpenIDSub = userinfo.sub - await Database.userModel.updateFromOld(user) - } else if (user && !user.isActive) { - Logger.warn(`[Auth] openid: New user found matching existing user "${user.username}" but that user is deactivated`) - } - - // Optionally auto register the user - if (!user && Database.serverSettings.authOpenIDAutoRegister) { - Logger.info(`[Auth] openid: Auto-registering user with sub "${userinfo.sub}"`, userinfo) - user = await Database.userModel.createUserFromOpenIdUserInfo(userinfo, this) - } - } - - if (!user?.isActive) { - // deny login - done(null, null) - return - } - - // permit login - return done(null, user) - })) - } - // Load the JwtStrategy (always) -> for bearer token auth passport.use(new JwtStrategy({ jwtFromRequest: ExtractJwt.fromExtractors([ExtractJwt.fromAuthHeaderAsBearerToken(), ExtractJwt.fromUrlQueryParameter('token')]), @@ -167,6 +94,117 @@ class Auth { }).bind(this)) } + /** + * Passport use LocalStrategy + */ + initAuthStrategyPassword() { + passport.use(new LocalStrategy(this.localAuthCheckUserPw.bind(this))) + } + + /** + * Passport use OpenIDClient.Strategy + */ + initAuthStrategyOpenID() { + const openIdIssuerClient = new OpenIDClient.Issuer({ + issuer: global.ServerSettings.authOpenIDIssuerURL, + authorization_endpoint: global.ServerSettings.authOpenIDAuthorizationURL, + token_endpoint: global.ServerSettings.authOpenIDTokenURL, + userinfo_endpoint: global.ServerSettings.authOpenIDUserInfoURL, + jwks_uri: global.ServerSettings.authOpenIDJwksURL + }).Client + const openIdClient = new openIdIssuerClient({ + client_id: global.ServerSettings.authOpenIDClientID, + client_secret: global.ServerSettings.authOpenIDClientSecret + }) + passport.use('openid-client', new OpenIDClient.Strategy({ + client: openIdClient, + params: { + redirect_uri: '/auth/openid/callback', + scope: 'openid profile email' + } + }, async (tokenset, userinfo, done) => { + Logger.debug(`[Auth] openid callback userinfo=`, userinfo) + + if (!userinfo.sub) { + Logger.error(`[Auth] openid callback invalid userinfo, no sub`) + return done(null, null) + } + + // First check for matching user by sub + let user = await Database.userModel.getUserByOpenIDSub(userinfo.sub) + if (!user) { + // Optionally match existing by email or username based on server setting "authOpenIDMatchExistingBy" + if (Database.serverSettings.authOpenIDMatchExistingBy === 'email' && userinfo.email && userinfo.email_verified) { + Logger.info(`[Auth] openid: User not found, checking existing with email "${userinfo.email}"`) + user = await Database.userModel.getUserByEmail(userinfo.email) + // Check that user is not already matched + if (user?.authOpenIDSub) { + Logger.warn(`[Auth] openid: User found with email "${userinfo.email}" but is already matched with sub "${user.authOpenIDSub}"`) + // TODO: Show some error log? + user = null + } + } else if (Database.serverSettings.authOpenIDMatchExistingBy === 'username' && userinfo.preferred_username) { + Logger.info(`[Auth] openid: User not found, checking existing with username "${userinfo.preferred_username}"`) + user = await Database.userModel.getUserByUsername(userinfo.preferred_username) + // Check that user is not already matched + if (user?.authOpenIDSub) { + Logger.warn(`[Auth] openid: User found with username "${userinfo.preferred_username}" but is already matched with sub "${user.authOpenIDSub}"`) + // TODO: Show some error log? + user = null + } + } + + // If existing user was matched and isActive then save sub to user + if (user?.isActive) { + Logger.info(`[Auth] openid: New user found matching existing user "${user.username}"`) + user.authOpenIDSub = userinfo.sub + await Database.userModel.updateFromOld(user) + } else if (user && !user.isActive) { + Logger.warn(`[Auth] openid: New user found matching existing user "${user.username}" but that user is deactivated`) + } + + // Optionally auto register the user + if (!user && Database.serverSettings.authOpenIDAutoRegister) { + Logger.info(`[Auth] openid: Auto-registering user with sub "${userinfo.sub}"`, userinfo) + user = await Database.userModel.createUserFromOpenIdUserInfo(userinfo, this) + } + } + + if (!user?.isActive) { + // deny login + done(null, null) + return + } + + // permit login + return done(null, user) + })) + } + + /** + * Unuse strategy + * + * @param {string} name + */ + unuseAuthStrategy(name) { + passport.unuse(name) + } + + /** + * Use strategy + * + * @param {string} name + */ + useAuthStrategy(name) { + if (name === 'openid') { + this.initAuthStrategyOpenID() + } else if (name === 'local') { + this.initAuthStrategyPassword() + } else { + Logger.error('[Auth] Invalid auth strategy ' + name) + } + } + /** * Stores the client's choice how the login callback should happen in temp cookies * diff --git a/server/controllers/MiscController.js b/server/controllers/MiscController.js index 9b58188f..6d7507cf 100644 --- a/server/controllers/MiscController.js +++ b/server/controllers/MiscController.js @@ -129,7 +129,7 @@ class MiscController { return res.sendStatus(403) } const settingsUpdate = req.body - if (!settingsUpdate || !isObject(settingsUpdate)) { + if (!isObject(settingsUpdate)) { return res.status(400).send('Invalid settings update object') } @@ -604,5 +604,91 @@ class MiscController { } return res.json(Database.serverSettings.authenticationSettings) } + + /** + * PATCH: api/auth-settings + * @this import('../routers/ApiRouter') + * + * @param {import('express').Request} req + * @param {import('express').Response} res + */ + async updateAuthSettings(req, res) { + if (!req.user.isAdminOrUp) { + Logger.error(`[MiscController] Non-admin user "${req.user.username}" attempted to update auth settings`) + return res.sendStatus(403) + } + + const settingsUpdate = req.body + if (!isObject(settingsUpdate)) { + return res.status(400).send('Invalid auth settings update object') + } + + let hasUpdates = false + + const currentAuthenticationSettings = Database.serverSettings.authenticationSettings + const originalAuthMethods = [...currentAuthenticationSettings.authActiveAuthMethods] + + // TODO: Better validation of auth settings once auth settings are separated from server settings + for (const key in currentAuthenticationSettings) { + if (settingsUpdate[key] === undefined) continue + + if (key === 'authActiveAuthMethods') { + let updatedAuthMethods = settingsUpdate[key]?.filter?.((authMeth) => Database.serverSettings.supportedAuthMethods.includes(authMeth)) + if (Array.isArray(updatedAuthMethods) && updatedAuthMethods.length) { + updatedAuthMethods.sort() + currentAuthenticationSettings[key].sort() + if (updatedAuthMethods.join() !== currentAuthenticationSettings[key].join()) { + Logger.debug(`[MiscController] Updating auth settings key "authActiveAuthMethods" from "${currentAuthenticationSettings[key].join()}" to "${updatedAuthMethods.join()}"`) + Database.serverSettings[key] = updatedAuthMethods + hasUpdates = true + } + } else { + Logger.warn(`[MiscController] Invalid value for authActiveAuthMethods`) + } + } else { + const updatedValueType = typeof settingsUpdate[key] + if (['authOpenIDAutoLaunch', 'authOpenIDAutoRegister'].includes(key)) { + if (updatedValueType !== 'boolean') { + Logger.warn(`[MiscController] Invalid value for ${key}. Expected boolean`) + continue + } + } else if (updatedValueType !== null && updatedValueType !== 'string') { + Logger.warn(`[MiscController] Invalid value for ${key}. Expected string or null`) + continue + } + let updatedValue = settingsUpdate[key] + if (updatedValue === '') updatedValue = null + let currentValue = currentAuthenticationSettings[key] + if (currentValue === '') currentValue = null + + if (updatedValue !== currentValue) { + Logger.debug(`[MiscController] Updating auth settings key "${key}" from "${currentValue}" to "${updatedValue}"`) + Database.serverSettings[key] = updatedValue + hasUpdates = true + } + } + } + + if (hasUpdates) { + // Use/unuse auth methods + Database.serverSettings.supportedAuthMethods.forEach((authMethod) => { + if (originalAuthMethods.includes(authMethod) && !Database.serverSettings.authActiveAuthMethods.includes(authMethod)) { + // Auth method has been removed + Logger.info(`[MiscController] Disabling active auth method "${authMethod}"`) + this.auth.unuseAuthStrategy(authMethod) + } else if (!originalAuthMethods.includes(authMethod) && Database.serverSettings.authActiveAuthMethods.includes(authMethod)) { + // Auth method has been added + Logger.info(`[MiscController] Enabling active auth method "${authMethod}"`) + this.auth.useAuthStrategy(authMethod) + } + }) + + await Database.updateServerSettings() + } + + res.json({ + serverSettings: Database.serverSettings.toJSONForBrowser() + }) + } } module.exports = new MiscController() \ No newline at end of file diff --git a/server/objects/settings/ServerSettings.js b/server/objects/settings/ServerSettings.js index 05a64d06..afde1ddf 100644 --- a/server/objects/settings/ServerSettings.js +++ b/server/objects/settings/ServerSettings.js @@ -59,19 +59,19 @@ class ServerSettings { this.authActiveAuthMethods = ['local'] // google-oauth20 settings - this.authGoogleOauth20ClientID = '' - this.authGoogleOauth20ClientSecret = '' - this.authGoogleOauth20CallbackURL = '' + this.authGoogleOauth20ClientID = null + this.authGoogleOauth20ClientSecret = null + this.authGoogleOauth20CallbackURL = null // openid settings - this.authOpenIDIssuerURL = '' - this.authOpenIDAuthorizationURL = '' - this.authOpenIDTokenURL = '' - this.authOpenIDUserInfoURL = '' - this.authOpenIDJwksURL = '' - this.authOpenIDLogoutURL = '' - this.authOpenIDClientID = '' - this.authOpenIDClientSecret = '' + this.authOpenIDIssuerURL = null + this.authOpenIDAuthorizationURL = null + this.authOpenIDTokenURL = null + this.authOpenIDUserInfoURL = null + this.authOpenIDJwksURL = null + this.authOpenIDLogoutURL = null + this.authOpenIDClientID = null + this.authOpenIDClientSecret = null this.authOpenIDButtonText = 'Login with OpenId' this.authOpenIDAutoLaunch = false this.authOpenIDAutoRegister = false @@ -118,18 +118,18 @@ class ServerSettings { this.buildNumber = settings.buildNumber || 0 // Added v2.4.5 this.authActiveAuthMethods = settings.authActiveAuthMethods || ['local'] - this.authGoogleOauth20ClientID = settings.authGoogleOauth20ClientID || '' - this.authGoogleOauth20ClientSecret = settings.authGoogleOauth20ClientSecret || '' - this.authGoogleOauth20CallbackURL = settings.authGoogleOauth20CallbackURL || '' + this.authGoogleOauth20ClientID = settings.authGoogleOauth20ClientID || null + this.authGoogleOauth20ClientSecret = settings.authGoogleOauth20ClientSecret || null + this.authGoogleOauth20CallbackURL = settings.authGoogleOauth20CallbackURL || null - this.authOpenIDIssuerURL = settings.authOpenIDIssuerURL || '' - this.authOpenIDAuthorizationURL = settings.authOpenIDAuthorizationURL || '' - this.authOpenIDTokenURL = settings.authOpenIDTokenURL || '' - this.authOpenIDUserInfoURL = settings.authOpenIDUserInfoURL || '' - this.authOpenIDJwksURL = settings.authOpenIDJwksURL || '' - this.authOpenIDLogoutURL = settings.authOpenIDLogoutURL || '' - this.authOpenIDClientID = settings.authOpenIDClientID || '' - this.authOpenIDClientSecret = settings.authOpenIDClientSecret || '' + this.authOpenIDIssuerURL = settings.authOpenIDIssuerURL || null + this.authOpenIDAuthorizationURL = settings.authOpenIDAuthorizationURL || null + this.authOpenIDTokenURL = settings.authOpenIDTokenURL || null + this.authOpenIDUserInfoURL = settings.authOpenIDUserInfoURL || null + this.authOpenIDJwksURL = settings.authOpenIDJwksURL || null + this.authOpenIDLogoutURL = settings.authOpenIDLogoutURL || null + this.authOpenIDClientID = settings.authOpenIDClientID || null + this.authOpenIDClientSecret = settings.authOpenIDClientSecret || null this.authOpenIDButtonText = settings.authOpenIDButtonText || 'Login with OpenId' this.authOpenIDAutoLaunch = !!settings.authOpenIDAutoLaunch this.authOpenIDAutoRegister = !!settings.authOpenIDAutoRegister @@ -142,9 +142,9 @@ class ServerSettings { // remove uninitialized methods // GoogleOauth20 if (this.authActiveAuthMethods.includes('google-oauth20') && ( - this.authGoogleOauth20ClientID === '' || - this.authGoogleOauth20ClientSecret === '' || - this.authGoogleOauth20CallbackURL === '' + !this.authGoogleOauth20ClientID || + !this.authGoogleOauth20ClientSecret || + !this.authGoogleOauth20CallbackURL )) { this.authActiveAuthMethods.splice(this.authActiveAuthMethods.indexOf('google-oauth20', 0), 1) } @@ -152,13 +152,13 @@ class ServerSettings { // remove uninitialized methods // OpenID if (this.authActiveAuthMethods.includes('openid') && ( - this.authOpenIDIssuerURL === '' || - this.authOpenIDAuthorizationURL === '' || - this.authOpenIDTokenURL === '' || - this.authOpenIDUserInfoURL === '' || - this.authOpenIDJwksURL === '' || - this.authOpenIDClientID === '' || - this.authOpenIDClientSecret === '' + !this.authOpenIDIssuerURL || + !this.authOpenIDAuthorizationURL || + !this.authOpenIDTokenURL || + !this.authOpenIDUserInfoURL || + !this.authOpenIDJwksURL || + !this.authOpenIDClientID || + !this.authOpenIDClientSecret )) { this.authActiveAuthMethods.splice(this.authActiveAuthMethods.indexOf('openid', 0), 1) } @@ -254,6 +254,10 @@ class ServerSettings { return json } + get supportedAuthMethods() { + return ['local', 'openid'] + } + get authenticationSettings() { return { authActiveAuthMethods: this.authActiveAuthMethods, diff --git a/server/routers/ApiRouter.js b/server/routers/ApiRouter.js index c4c0df5e..8c97d59b 100644 --- a/server/routers/ApiRouter.js +++ b/server/routers/ApiRouter.js @@ -35,6 +35,7 @@ const Series = require('../objects/entities/Series') class ApiRouter { constructor(Server) { + /** @type {import('../Auth')} */ this.auth = Server.auth this.playbackSessionManager = Server.playbackSessionManager this.abMergeManager = Server.abMergeManager @@ -310,6 +311,7 @@ class ApiRouter { this.router.delete('/genres/:genre', MiscController.deleteGenre.bind(this)) this.router.post('/validate-cron', MiscController.validateCronExpression.bind(this)) this.router.get('/auth-settings', MiscController.getAuthSettings.bind(this)) + this.router.patch('/auth-settings', MiscController.updateAuthSettings.bind(this)) this.router.post('/watcher/update', MiscController.updateWatchedPath.bind(this)) }