From 840811b46460d08690dd59c24dcc24daafb2587f Mon Sep 17 00:00:00 2001 From: advplyr Date: Sat, 4 Nov 2023 15:36:43 -0500 Subject: [PATCH] Replace passport openidconnect plugin with openid-client, add JWKS and logout URL server settings, use email and email_verified instead of username --- client/pages/config/authentication.vue | 8 ++ package-lock.json | 91 ++++++++++++++++------- package.json | 2 +- server/Auth.js | 59 ++++++++++----- server/objects/settings/ServerSettings.js | 9 +++ 5 files changed, 125 insertions(+), 44 deletions(-) diff --git a/client/pages/config/authentication.vue b/client/pages/config/authentication.vue index 317364a2..13867ef3 100644 --- a/client/pages/config/authentication.vue +++ b/client/pages/config/authentication.vue @@ -23,6 +23,10 @@ + + + + @@ -97,6 +101,10 @@ export default { this.$toast.error('Userinfo URL required') isValid = false } + if (!this.newAuthSettings.authOpenIDJwksURL) { + this.$toast.error('JWKS URL required') + isValid = false + } if (!this.newAuthSettings.authOpenIDClientID) { this.$toast.error('Client ID required') isValid = false diff --git a/package-lock.json b/package-lock.json index 55149176..dd2b8339 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,10 +17,10 @@ "htmlparser2": "^8.0.1", "node-tone": "^1.0.1", "nodemailer": "^6.9.2", + "openid-client": "^5.6.1", "passport": "^0.6.0", "passport-google-oauth20": "^2.0.0", "passport-jwt": "^4.0.1", - "passport-openidconnect": "^0.1.1", "sequelize": "^6.32.1", "socket.io": "^4.5.4", "sqlite3": "^5.1.6", @@ -1343,6 +1343,14 @@ "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", "optional": true }, + "node_modules/jose": { + "version": "4.15.4", + "resolved": "https://registry.npmjs.org/jose/-/jose-4.15.4.tgz", + "integrity": "sha512-W+oqK4H+r5sITxfxpSU+MMdr/YSWGvgZMQDIsNoBDGGy4i7GBPTtvFKibQzW06n3U3TqHjhvBJsirShsEJ6eeQ==", + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/jsonwebtoken": { "version": "9.0.0", "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.0.tgz", @@ -1883,6 +1891,14 @@ "node": ">=0.10.0" } }, + "node_modules/object-hash": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", + "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==", + "engines": { + "node": ">= 6" + } + }, "node_modules/object-inspect": { "version": "1.12.2", "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.12.2.tgz", @@ -1891,6 +1907,14 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/oidc-token-hash": { + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/oidc-token-hash/-/oidc-token-hash-5.0.3.tgz", + "integrity": "sha512-IF4PcGgzAr6XXSff26Sk/+P4KZFJVuHAJZj3wgO3vX2bMdNVp/QXTP3P7CEm9V1IdG8lDLY3HhiqpsE/nOwpPw==", + "engines": { + "node": "^10.13.0 || >=12.0.0" + } + }, "node_modules/on-finished": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.4.1.tgz", @@ -1918,6 +1942,20 @@ "wrappy": "1" } }, + "node_modules/openid-client": { + "version": "5.6.1", + "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-5.6.1.tgz", + "integrity": "sha512-PtrWsY+dXg6y8mtMPyL/namZSYVz8pjXz3yJiBNZsEdCnu9miHLB4ELVC85WvneMKo2Rg62Ay7NkuCpM0bgiLQ==", + "dependencies": { + "jose": "^4.15.1", + "lru-cache": "^6.0.0", + "object-hash": "^2.2.0", + "oidc-token-hash": "^5.0.3" + }, + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/p-map": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/p-map/-/p-map-4.0.0.tgz", @@ -1997,22 +2035,6 @@ "url": "https://github.com/sponsors/jaredhanson" } }, - "node_modules/passport-openidconnect": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/passport-openidconnect/-/passport-openidconnect-0.1.1.tgz", - "integrity": "sha512-r0QJiWEzwCg2MeCIXVP5G6YxVRqnEsZ2HpgKRthZ9AiQHJrgGUytXpsdcGF9BRwd3yMrEesb/uG/Yxb86rrY0g==", - "dependencies": { - "oauth": "0.9.x", - "passport-strategy": "1.x.x" - }, - "engines": { - "node": ">= 0.6.0" - }, - "funding": { - "type": "github", - "url": "https://github.com/sponsors/jaredhanson" - } - }, "node_modules/passport-strategy": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz", @@ -3929,6 +3951,11 @@ "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", "optional": true }, + "jose": { + "version": "4.15.4", + "resolved": "https://registry.npmjs.org/jose/-/jose-4.15.4.tgz", + "integrity": "sha512-W+oqK4H+r5sITxfxpSU+MMdr/YSWGvgZMQDIsNoBDGGy4i7GBPTtvFKibQzW06n3U3TqHjhvBJsirShsEJ6eeQ==" + }, "jsonwebtoken": { "version": "9.0.0", "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.0.tgz", @@ -4330,11 +4357,21 @@ "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", "integrity": "sha512-rJgTQnkUnH1sFw8yT6VSU3zD3sWmu6sZhIseY8VX+GRu3P6F7Fu+JNDoXfklElbLJSnc3FUQHVe4cU5hj+BcUg==" }, + "object-hash": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", + "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==" + }, "object-inspect": { "version": "1.12.2", "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.12.2.tgz", "integrity": "sha512-z+cPxW0QGUp0mcqcsgQyLVRDoXFQbXOwBaqyF7VIgI4TWNQsDHrBpUQslRmIfAoYWdYzs6UlKJtB2XJpTaNSpQ==" }, + "oidc-token-hash": { + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/oidc-token-hash/-/oidc-token-hash-5.0.3.tgz", + "integrity": "sha512-IF4PcGgzAr6XXSff26Sk/+P4KZFJVuHAJZj3wgO3vX2bMdNVp/QXTP3P7CEm9V1IdG8lDLY3HhiqpsE/nOwpPw==" + }, "on-finished": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.4.1.tgz", @@ -4356,6 +4393,17 @@ "wrappy": "1" } }, + "openid-client": { + "version": "5.6.1", + "resolved": "https://registry.npmjs.org/openid-client/-/openid-client-5.6.1.tgz", + "integrity": "sha512-PtrWsY+dXg6y8mtMPyL/namZSYVz8pjXz3yJiBNZsEdCnu9miHLB4ELVC85WvneMKo2Rg62Ay7NkuCpM0bgiLQ==", + "requires": { + "jose": "^4.15.1", + "lru-cache": "^6.0.0", + "object-hash": "^2.2.0", + "oidc-token-hash": "^5.0.3" + } + }, "p-map": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/p-map/-/p-map-4.0.0.tgz", @@ -4409,15 +4457,6 @@ "utils-merge": "1.x.x" } }, - "passport-openidconnect": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/passport-openidconnect/-/passport-openidconnect-0.1.1.tgz", - "integrity": "sha512-r0QJiWEzwCg2MeCIXVP5G6YxVRqnEsZ2HpgKRthZ9AiQHJrgGUytXpsdcGF9BRwd3yMrEesb/uG/Yxb86rrY0g==", - "requires": { - "oauth": "0.9.x", - "passport-strategy": "1.x.x" - } - }, "passport-strategy": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz", diff --git a/package.json b/package.json index a3b4a7c5..d4e9c209 100644 --- a/package.json +++ b/package.json @@ -39,10 +39,10 @@ "htmlparser2": "^8.0.1", "node-tone": "^1.0.1", "nodemailer": "^6.9.2", + "openid-client": "^5.6.1", "passport": "^0.6.0", "passport-google-oauth20": "^2.0.0", "passport-jwt": "^4.0.1", - "passport-openidconnect": "^0.1.1", "sequelize": "^6.32.1", "socket.io": "^4.5.4", "sqlite3": "^5.1.6", diff --git a/server/Auth.js b/server/Auth.js index b7ea59c4..fa5020a0 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -5,8 +5,9 @@ const LocalStrategy = require('./libs/passportLocal') const JwtStrategy = require('passport-jwt').Strategy const ExtractJwt = require('passport-jwt').ExtractJwt const GoogleStrategy = require('passport-google-oauth20').Strategy -const OpenIDConnectStrategy = require('passport-openidconnect') +const OpenIDClient = require('openid-client') const Database = require('./Database') +const Logger = require('./Logger') /** * @class Class for handling all the authentication related functionality. @@ -62,20 +63,33 @@ class Auth { // Check if we should load the openid strategy if (global.ServerSettings.authActiveAuthMethods.includes("openid")) { - passport.use(new OpenIDConnectStrategy({ + const openIdIssuerClient = new OpenIDClient.Issuer({ issuer: global.ServerSettings.authOpenIDIssuerURL, - authorizationURL: global.ServerSettings.authOpenIDAuthorizationURL, - tokenURL: global.ServerSettings.authOpenIDTokenURL, - userInfoURL: global.ServerSettings.authOpenIDUserInfoURL, - clientID: global.ServerSettings.authOpenIDClientID, - clientSecret: global.ServerSettings.authOpenIDClientSecret, - callbackURL: '/auth/openid/callback', - scope: ["openid", "email", "profile"], - skipUserProfile: false - }, async (issuer, profile, done) => { - // TODO: do we want to create the users which does not exist? + 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 + }) + const openIdClientStrategy = new OpenIDClient.Strategy({ + client: openIdClient, + params: { + redirect_uri: '/auth/openid/callback', + scope: 'openid profile email' + } + }, async (tokenset, userinfo, done) => { + // TODO: Here is where to lookup the Abs user or register a new Abs user + Logger.debug(`[Auth] openid callback userinfo=`, userinfo) - const user = await Database.userModel.getUserByUsername(profile.username) + let user = null + // TODO: Temporary lookup existing user by email. May be replaced by a setting to toggle this or use name + if (userinfo.email && userinfo.email_verified) { + user = await Database.userModel.getUserByEmail(userinfo.email) + // TODO: If using existing user then save userinfo.sub on user + } if (!user?.isActive) { // deny login @@ -85,7 +99,12 @@ class Auth { // permit login return done(null, user) - })) + }) + // The strategy name is set to the issuer hostname by default but didnt' see a way to override this + // @see https://github.com/panva/node-openid-client/blob/a84d022f195f82ca1c97f8f6b2567ebcef8738c3/lib/passport_strategy.js#L75 + openIdClientStrategy.name = 'openid-client' + + passport.use(openIdClientStrategy) } // Load the JwtStrategy (always) -> for bearer token auth @@ -99,7 +118,7 @@ class Auth { process.nextTick(function () { // only store id to session return cb(null, JSON.stringify({ - "id": user.id, + id: user.id, })) }) }) @@ -216,7 +235,13 @@ class Auth { // openid strategy login route (this redirects to the configured openid login provider) router.get('/auth/openid', (req, res, next) => { - const auth_func = passport.authenticate('openidconnect') + // This is a (temporary?) hack to not have to get the full redirect URL from the user + // it uses the URL made in this request and adds the relative URL /auth/openid/callback + const strategy = passport._strategy('openid-client') + strategy._params.redirect_uri = new URL(`${req.protocol}://${req.get('host')}/auth/openid/callback`).toString() + + + const auth_func = passport.authenticate('openid-client') // params (isRest, callback) to a cookie that will be send to the client this.paramsToCookies(req, res) auth_func(req, res, next) @@ -224,7 +249,7 @@ class Auth { // openid strategy callback route (this receives the token from the configured openid login provider) router.get('/auth/openid/callback', - passport.authenticate('openidconnect'), + passport.authenticate('openid-client'), // on a successfull login: read the cookies and react like the client requested (callback or json) this.handleLoginSuccessBasedOnCookie.bind(this)) diff --git a/server/objects/settings/ServerSettings.js b/server/objects/settings/ServerSettings.js index 71e2e05d..781943b4 100644 --- a/server/objects/settings/ServerSettings.js +++ b/server/objects/settings/ServerSettings.js @@ -68,6 +68,8 @@ class ServerSettings { this.authOpenIDAuthorizationURL = '' this.authOpenIDTokenURL = '' this.authOpenIDUserInfoURL = '' + this.authOpenIDJwksURL = '' + this.authOpenIDLogoutURL = '' this.authOpenIDClientID = '' this.authOpenIDClientSecret = '' this.authOpenIDButtonText = 'Login with OpenId' @@ -122,6 +124,8 @@ class ServerSettings { 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.authOpenIDButtonText = settings.authOpenIDButtonText || 'Login with OpenId' @@ -148,6 +152,7 @@ class ServerSettings { this.authOpenIDAuthorizationURL === '' || this.authOpenIDTokenURL === '' || this.authOpenIDUserInfoURL === '' || + this.authOpenIDJwksURL === '' || this.authOpenIDClientID === '' || this.authOpenIDClientSecret === '' )) { @@ -224,6 +229,8 @@ class ServerSettings { authOpenIDAuthorizationURL: this.authOpenIDAuthorizationURL, authOpenIDTokenURL: this.authOpenIDTokenURL, authOpenIDUserInfoURL: this.authOpenIDUserInfoURL, + authOpenIDJwksURL: this.authOpenIDJwksURL, + authOpenIDLogoutURL: this.authOpenIDLogoutURL, authOpenIDClientID: this.authOpenIDClientID, // Do not return to client authOpenIDClientSecret: this.authOpenIDClientSecret, // Do not return to client authOpenIDButtonText: this.authOpenIDButtonText, @@ -251,6 +258,8 @@ class ServerSettings { authOpenIDAuthorizationURL: this.authOpenIDAuthorizationURL, authOpenIDTokenURL: this.authOpenIDTokenURL, authOpenIDUserInfoURL: this.authOpenIDUserInfoURL, + authOpenIDJwksURL: this.authOpenIDJwksURL, + authOpenIDLogoutURL: this.authOpenIDLogoutURL, authOpenIDClientID: this.authOpenIDClientID, // Do not return to client authOpenIDClientSecret: this.authOpenIDClientSecret, // Do not return to client authOpenIDButtonText: this.authOpenIDButtonText,