diff --git a/client/pages/login.vue b/client/pages/login.vue index 6ec67b00..f7579dd6 100644 --- a/client/pages/login.vue +++ b/client/pages/login.vue @@ -109,7 +109,7 @@ export default { return this.$store.state.user.user }, openidAuthUri() { - return `${process.env.serverUrl}/auth/openid?callback=${location.toString()}` + return `${process.env.serverUrl}/auth/openid?callback=${location.href.split('?').shift()}` }, openIDButtonText() { return this.authFormData?.authOpenIDButtonText || 'Login with OpenId' @@ -238,6 +238,15 @@ export default { }) }, updateLoginVisibility(authMethods) { + if (this.$route.query?.error) { + this.error = this.$route.query.error + + // Remove error query string + const newurl = new URL(location.href) + newurl.searchParams.delete('error') + window.history.replaceState({ path: newurl.href }, '', newurl.href) + } + if (authMethods.includes('local') || !authMethods.length) { this.login_local = true } else { @@ -257,8 +266,8 @@ export default { } }, async mounted() { - if (new URLSearchParams(window.location.search).get('setToken')) { - localStorage.setItem('token', new URLSearchParams(window.location.search).get('setToken')) + if (this.$route.query?.setToken) { + localStorage.setItem('token', this.$route.query.setToken) } if (localStorage.getItem('token')) { if (await this.checkAuth()) return // if valid user no need to check status diff --git a/server/Auth.js b/server/Auth.js index f504e315..06db47a8 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -101,9 +101,10 @@ class Auth { }, async (tokenset, userinfo, done) => { Logger.debug(`[Auth] openid callback userinfo=`, userinfo) + let failureMessage = 'Unauthorized' if (!userinfo.sub) { Logger.error(`[Auth] openid callback invalid userinfo, no sub`) - return done(null, null) + return done(null, null, failureMessage) } // First check for matching user by sub @@ -116,7 +117,8 @@ class Auth { // 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? + // TODO: Message isn't actually returned to the user yet. Need to override the passport authenticated callback + failureMessage = 'A matching user was found but is already matched with another user from your auth provider' user = null } } else if (Database.serverSettings.authOpenIDMatchExistingBy === 'username' && userinfo.preferred_username) { @@ -125,7 +127,8 @@ class Auth { // 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? + // TODO: Message isn't actually returned to the user yet. Need to override the passport authenticated callback + failureMessage = 'A matching user was found but is already matched with another user from your auth provider' user = null } } @@ -147,8 +150,11 @@ class Auth { } if (!user?.isActive) { + if (user && !user.isActive) { + failureMessage = 'Unauthorized' + } // deny login - done(null, null) + done(null, null, failureMessage) return } @@ -366,7 +372,7 @@ class Auth { if (req.session[sessionKey].mobile) { return passport.authenticate('openid-client', { redirect_uri: 'audiobookshelf://oauth' })(req, res, next) } else { - return passport.authenticate('openid-client')(req, res, next) + return passport.authenticate('openid-client', { failureRedirect: '/login?error=Unauthorized&autoLaunch=0' })(req, res, next) } }, // on a successfull login: read the cookies and react like the client requested (callback or json) diff --git a/server/controllers/MiscController.js b/server/controllers/MiscController.js index 6d7507cf..11adf3e9 100644 --- a/server/controllers/MiscController.js +++ b/server/controllers/MiscController.js @@ -652,7 +652,7 @@ class MiscController { Logger.warn(`[MiscController] Invalid value for ${key}. Expected boolean`) continue } - } else if (updatedValueType !== null && updatedValueType !== 'string') { + } else if (settingsUpdate[key] !== null && updatedValueType !== 'string') { Logger.warn(`[MiscController] Invalid value for ${key}. Expected string or null`) continue } diff --git a/server/objects/settings/ServerSettings.js b/server/objects/settings/ServerSettings.js index afde1ddf..df5e71f1 100644 --- a/server/objects/settings/ServerSettings.js +++ b/server/objects/settings/ServerSettings.js @@ -58,11 +58,6 @@ class ServerSettings { // Active auth methodes this.authActiveAuthMethods = ['local'] - // google-oauth20 settings - this.authGoogleOauth20ClientID = null - this.authGoogleOauth20ClientSecret = null - this.authGoogleOauth20CallbackURL = null - // openid settings this.authOpenIDIssuerURL = null this.authOpenIDAuthorizationURL = null @@ -118,9 +113,6 @@ class ServerSettings { this.buildNumber = settings.buildNumber || 0 // Added v2.4.5 this.authActiveAuthMethods = settings.authActiveAuthMethods || ['local'] - this.authGoogleOauth20ClientID = settings.authGoogleOauth20ClientID || null - this.authGoogleOauth20ClientSecret = settings.authGoogleOauth20ClientSecret || null - this.authGoogleOauth20CallbackURL = settings.authGoogleOauth20CallbackURL || null this.authOpenIDIssuerURL = settings.authOpenIDIssuerURL || null this.authOpenIDAuthorizationURL = settings.authOpenIDAuthorizationURL || null @@ -139,16 +131,6 @@ class ServerSettings { this.authActiveAuthMethods = ['local'] } - // remove uninitialized methods - // GoogleOauth20 - if (this.authActiveAuthMethods.includes('google-oauth20') && ( - !this.authGoogleOauth20ClientID || - !this.authGoogleOauth20ClientSecret || - !this.authGoogleOauth20CallbackURL - )) { - this.authActiveAuthMethods.splice(this.authActiveAuthMethods.indexOf('google-oauth20', 0), 1) - } - // remove uninitialized methods // OpenID if (this.authActiveAuthMethods.includes('openid') && ( @@ -226,9 +208,6 @@ class ServerSettings { version: this.version, buildNumber: this.buildNumber, authActiveAuthMethods: this.authActiveAuthMethods, - authGoogleOauth20ClientID: this.authGoogleOauth20ClientID, // Do not return to client - authGoogleOauth20ClientSecret: this.authGoogleOauth20ClientSecret, // Do not return to client - authGoogleOauth20CallbackURL: this.authGoogleOauth20CallbackURL, authOpenIDIssuerURL: this.authOpenIDIssuerURL, authOpenIDAuthorizationURL: this.authOpenIDAuthorizationURL, authOpenIDTokenURL: this.authOpenIDTokenURL, @@ -247,8 +226,6 @@ class ServerSettings { toJSONForBrowser() { const json = this.toJSON() delete json.tokenSecret - delete json.authGoogleOauth20ClientID - delete json.authGoogleOauth20ClientSecret delete json.authOpenIDClientID delete json.authOpenIDClientSecret return json @@ -261,9 +238,6 @@ class ServerSettings { get authenticationSettings() { return { authActiveAuthMethods: this.authActiveAuthMethods, - authGoogleOauth20ClientID: this.authGoogleOauth20ClientID, // Do not return to client - authGoogleOauth20ClientSecret: this.authGoogleOauth20ClientSecret, // Do not return to client - authGoogleOauth20CallbackURL: this.authGoogleOauth20CallbackURL, authOpenIDIssuerURL: this.authOpenIDIssuerURL, authOpenIDAuthorizationURL: this.authOpenIDAuthorizationURL, authOpenIDTokenURL: this.authOpenIDTokenURL,