From ad2966c5d62138aca79743c1a7750cbe4c00eb4e Mon Sep 17 00:00:00 2001
From: aleb_the_flash <45729124+lebaudantoine@users.noreply.github.com>
Date: Tue, 16 Jul 2024 12:19:27 +0200
Subject: [PATCH] feat: Improve OIDC support for strict providers (#824)

---
 .../access-tokens/exchange-using-oidc.js            |  7 +++++++
 server/api/controllers/show-config.js               | 13 +++++++++----
 .../helpers/users/get-or-create-one-using-oidc.js   |  5 +++++
 server/api/hooks/oidc/index.js                      | 11 +++++++++--
 server/config/custom.js                             |  4 ++++
 5 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/server/api/controllers/access-tokens/exchange-using-oidc.js b/server/api/controllers/access-tokens/exchange-using-oidc.js
index 469d4462a..7eb375f68 100644
--- 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
index d1dc6f676..2740cb7ba 100644
--- 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
index c54b20955..b7bd837c8 100644
--- 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
index 6dbeddd12..e4a66c4f4 100644
--- 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
index ac8b9276e..8971dc118 100644
--- 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',