feat: Improve OIDC support for strict providers (#824)

plankanban · Jul 16, 2024 · ad2966c · ad2966c
1 parent 8d74cc1
commit ad2966c
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 6 deletions.
diff --git a/server/api/controllers/access-tokens/exchange-using-oidc.js b/server/api/controllers/access-tokens/exchange-using-oidc.js
@@ -13,6 +13,9 @@ const Errors = {
   MISSING_VALUES: {
     missingValues: 'Unable to retrieve required values (email, name)',
   },
+  INVALID_USERINFO_SIGNATURE: {
+    invalidUserInfoSignature: "Invalid signature on userInfo due to client misconfiguration"
+  }
 };
 
 module.exports = {
@@ -40,6 +43,9 @@ module.exports = {
     missingValues: {
       responseType: 'unprocessableEntity',
     },
+    invalidUserInfoSignature: {
+      responseType: 'unauthorized',
+    },
   },
 
   async fn(inputs) {
@@ -51,6 +57,7 @@ module.exports = {
         sails.log.warn(`Invalid code or nonce! (IP: ${remoteAddress})`);
         return Errors.INVALID_CODE_OR_NONCE;
       })
+      .intercept('invalidUserInfoSignature', () => Errors.INVALID_USERINFO_SIGNATURE)
       .intercept('emailAlreadyInUse', () => Errors.EMAIL_ALREADY_IN_USE)
       .intercept('usernameAlreadyInUse', () => Errors.USERNAME_ALREADY_IN_USE)
       .intercept('missingValues', () => Errors.MISSING_VALUES);

diff --git a/server/api/controllers/show-config.js b/server/api/controllers/show-config.js
@@ -4,11 +4,16 @@ module.exports = {
     if (sails.hooks.oidc.isActive()) {
       const oidcClient = sails.hooks.oidc.getClient();
 
+      const authorizationParameters = {
+        scope: sails.config.custom.oidcScopes,
+      }
+
+      if(!sails.config.custom.oidcDefaultResponseMode) {
+        authorizationParameters.response_mode = sails.config.custom.oidcResponseMode
+      }
+
       oidc = {
-        authorizationUrl: oidcClient.authorizationUrl({
-          scope: sails.config.custom.oidcScopes,
-          response_mode: 'fragment',
-        }),
+        authorizationUrl: oidcClient.authorizationUrl(authorizationParameters),
         endSessionUrl: oidcClient.issuer.end_session_endpoint ? oidcClient.endSessionUrl({}) : null,
         isEnforced: sails.config.custom.oidcEnforced,
       };

diff --git a/server/api/helpers/users/get-or-create-one-using-oidc.js b/server/api/helpers/users/get-or-create-one-using-oidc.js
@@ -11,6 +11,7 @@ module.exports = {
   },
 
   exits: {
+    invalidUserInfoSignature: {},
     invalidCodeOrNonce: {},
     missingValues: {},
     emailAlreadyInUse: {},
@@ -34,6 +35,10 @@ module.exports = {
       );
       userInfo = await client.userinfo(tokenSet);
     } catch (e) {
+      if (e instanceof SyntaxError && e.message.includes('Unexpected token e in JSON at position 0')) {
+        sails.log.warn('Error while exchanging OIDC code: userInfo response is signed.');
+        throw 'invalidUserInfoSignature';
+      }
       sails.log.warn(`Error while exchanging OIDC code: ${e}`);
       throw 'invalidCodeOrNonce';
     }

diff --git a/server/api/hooks/oidc/index.js b/server/api/hooks/oidc/index.js
@@ -25,12 +25,19 @@ module.exports = function defineOidcHook(sails) {
 
       const issuer = await openidClient.Issuer.discover(sails.config.custom.oidcIssuer);
 
-      client = new issuer.Client({
+      const metadata = {
         client_id: sails.config.custom.oidcClientId,
         client_secret: sails.config.custom.oidcClientSecret,
         redirect_uris: [sails.config.custom.oidcRedirectUri],
         response_types: ['code'],
-      });
+        userinfo_signed_response_alg: sails.config.custom.oidcUserinfoSignedResponseAlg,
+      }
+
+      if (sails.config.custom.oidcIdTokenSignedResponseAlg) {
+        metadata.id_token_signed_response_alg = sails.config.custom.oidcIdTokenSignedResponseAlg
+      }
+
+      client = new issuer.Client(metadata);
     },
 
     getClient() {

diff --git a/server/config/custom.js b/server/config/custom.js
@@ -39,7 +39,11 @@ module.exports.custom = {
   oidcIssuer: process.env.OIDC_ISSUER,
   oidcClientId: process.env.OIDC_CLIENT_ID,
   oidcClientSecret: process.env.OIDC_CLIENT_SECRET,
+  oidcIdTokenSignedResponseAlg: process.env.OIDC_ID_TOKEN_SIGNED_RESPONSE_ALG,
+  oidcUserinfoSignedResponseAlg: process.env.OIDC_USERINFO_SIGNED_RESPONSE_ALG,
   oidcScopes: process.env.OIDC_SCOPES || 'openid email profile',
+  oidcResponseMode: process.env.OIDC_RESPONSE_MODE || 'fragment',
+  oidcDefaultResponseMode: process.env.OIDC_DEFAULT_RESPONSE_MODE === 'true',
   oidcAdminRoles: process.env.OIDC_ADMIN_ROLES ? process.env.OIDC_ADMIN_ROLES.split(',') : [],
   oidcEmailAttribute: process.env.OIDC_EMAIL_ATTRIBUTE || 'email',
   oidcNameAttribute: process.env.OIDC_NAME_ATTRIBUTE || 'name',