feat: Ability to configure OIDC claims source (#888)

Closes #884
plankanban · Sep 20, 2024 · 1217969 · 1217969
1 parent e410e21
commit 1217969
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 29 deletions.
diff --git a/docker-compose-dev.yml b/docker-compose-dev.yml
@@ -37,6 +37,7 @@ services:
       # - OIDC_RESPONSE_MODE=fragment
       # - OIDC_USE_DEFAULT_RESPONSE_MODE=true
       # - OIDC_ADMIN_ROLES=admin
+      # - OIDC_CLAIMS_SOURCE=userinfo
       # - OIDC_EMAIL_ATTRIBUTE=email
       # - OIDC_NAME_ATTRIBUTE=name
       # - OIDC_USERNAME_ATTRIBUTE=preferred_username

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -44,6 +44,7 @@ services:
       # - OIDC_RESPONSE_MODE=fragment
       # - OIDC_USE_DEFAULT_RESPONSE_MODE=true
       # - OIDC_ADMIN_ROLES=admin
+      # - OIDC_CLAIMS_SOURCE=userinfo
       # - OIDC_EMAIL_ATTRIBUTE=email
       # - OIDC_NAME_ATTRIBUTE=name
       # - OIDC_USERNAME_ATTRIBUTE=preferred_username

diff --git a/server/.env.sample b/server/.env.sample
@@ -35,6 +35,7 @@ SECRET_KEY=notsecretkey
 # OIDC_RESPONSE_MODE=fragment
 # OIDC_USE_DEFAULT_RESPONSE_MODE=true
 # OIDC_ADMIN_ROLES=admin
+# OIDC_CLAIMS_SOURCE=userinfo
 # OIDC_EMAIL_ATTRIBUTE=email
 # OIDC_NAME_ATTRIBUTE=name
 # OIDC_USERNAME_ATTRIBUTE=preferred_username

diff --git a/server/api/controllers/access-tokens/exchange-using-oidc.js b/server/api/controllers/access-tokens/exchange-using-oidc.js
@@ -6,8 +6,8 @@ const Errors = {
   INVALID_CODE_OR_NONCE: {
     invalidCodeOrNonce: 'Invalid code or nonce',
   },
-  INVALID_USERINFO_SIGNATURE: {
-    invalidUserinfoSignature: 'Invalid signature on userinfo due to client misconfiguration',
+  INVALID_USERINFO_CONFIGURATION: {
+    invalidUserinfoConfiguration: 'Invalid userinfo configuration',
   },
   EMAIL_ALREADY_IN_USE: {
     emailAlreadyInUse: 'Email already in use',
@@ -40,7 +40,7 @@ module.exports = {
     invalidCodeOrNonce: {
       responseType: 'unauthorized',
     },
-    invalidUserinfoSignature: {
+    invalidUserinfoConfiguration: {
       responseType: 'unauthorized',
     },
     emailAlreadyInUse: {
@@ -63,7 +63,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('invalidUserinfoConfiguration', () => Errors.INVALID_USERINFO_CONFIGURATION)
       .intercept('emailAlreadyInUse', () => Errors.EMAIL_ALREADY_IN_USE)
       .intercept('usernameAlreadyInUse', () => Errors.USERNAME_ALREADY_IN_USE)
       .intercept('missingValues', () => Errors.MISSING_VALUES);

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
@@ -12,7 +12,7 @@ module.exports = {
 
   exits: {
     invalidCodeOrNonce: {},
-    invalidUserinfoSignature: {},
+    invalidUserinfoConfiguration: {},
     missingValues: {},
     emailAlreadyInUse: {},
     usernameAlreadyInUse: {},
@@ -21,9 +21,9 @@ module.exports = {
   async fn(inputs) {
     const client = sails.hooks.oidc.getClient();
 
-    let userInfo;
+    let tokenSet;
     try {
-      const tokenSet = await client.callback(
+      tokenSet = await client.callback(
         sails.config.custom.oidcRedirectUri,
         {
           iss: sails.config.custom.oidcIssuer,
@@ -33,23 +33,36 @@ module.exports = {
           nonce: inputs.nonce,
         },
       );
-      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}`);
+    } catch (error) {
+      sails.log.warn(`Error while exchanging OIDC code: ${error}`);
       throw 'invalidCodeOrNonce';
     }
 
+    let claims;
+    if (sails.config.custom.oidcClaimsSource === 'id_token') {
+      claims = tokenSet.claims();
+    } else {
+      try {
+        claims = await client.userinfo(tokenSet);
+      } catch (error) {
+        let errorText;
+        if (
+          error instanceof SyntaxError &&
+          error.message.includes('Unexpected token e in JSON at position 0')
+        ) {
+          errorText = 'response is signed';
+        } else {
+          errorText = error.toString();
+        }
+
+        sails.log.warn(`Error while fetching OIDC userinfo: ${errorText}`);
+        throw 'invalidUserinfoConfiguration';
+      }
+    }
+
     if (
-      !userInfo[sails.config.custom.oidcEmailAttribute] ||
-      !userInfo[sails.config.custom.oidcNameAttribute]
+      !claims[sails.config.custom.oidcEmailAttribute] ||
+      !claims[sails.config.custom.oidcNameAttribute]
     ) {
       throw 'missingValues';
     }
@@ -58,23 +71,23 @@ module.exports = {
     if (sails.config.custom.oidcAdminRoles.includes('*')) {
       isAdmin = true;
     } else {
-      const roles = userInfo[sails.config.custom.oidcRolesAttribute];
+      const roles = claims[sails.config.custom.oidcRolesAttribute];
       if (Array.isArray(roles)) {
         // Use a Set here to avoid quadratic time complexity
-        const userRoles = new Set(userInfo[sails.config.custom.oidcRolesAttribute]);
+        const userRoles = new Set(claims[sails.config.custom.oidcRolesAttribute]);
         isAdmin = sails.config.custom.oidcAdminRoles.findIndex((role) => userRoles.has(role)) > -1;
       }
     }
 
     const values = {
       isAdmin,
-      email: userInfo[sails.config.custom.oidcEmailAttribute],
+      email: claims[sails.config.custom.oidcEmailAttribute],
       isSso: true,
-      name: userInfo[sails.config.custom.oidcNameAttribute],
+      name: claims[sails.config.custom.oidcNameAttribute],
       subscribeToOwnCards: false,
     };
     if (!sails.config.custom.oidcIgnoreUsername) {
-      values.username = userInfo[sails.config.custom.oidcUsernameAttribute];
+      values.username = claims[sails.config.custom.oidcUsernameAttribute];
     }
 
     let user;
@@ -84,7 +97,7 @@ module.exports = {
     // concurrently with logging in via OIDC.
     let identityProviderUser = await IdentityProviderUser.findOne({
       issuer: sails.config.custom.oidcIssuer,
-      sub: userInfo.sub,
+      sub: claims.sub,
     });
 
     if (identityProviderUser) {
@@ -108,7 +121,7 @@ module.exports = {
       identityProviderUser = await IdentityProviderUser.create({
         userId: user.id,
         issuer: sails.config.custom.oidcIssuer,
-        sub: userInfo.sub,
+        sub: claims.sub,
       });
     }
 

diff --git a/server/api/helpers/utils/send-webhooks.js b/server/api/helpers/utils/send-webhooks.js
@@ -130,8 +130,8 @@ async function sendWebhook(webhook, event, data, user) {
         `Webhook ${webhook.url} failed with status ${response.status} and message: ${message}`,
       );
     }
-  } catch (e) {
-    sails.log.error(`Webhook ${webhook.url} failed with error message: ${e.message}`);
+  } catch (error) {
+    sails.log.error(`Webhook ${webhook.url} failed with error: ${error}`);
   }
 }
 

diff --git a/server/config/custom.js b/server/config/custom.js
@@ -52,6 +52,7 @@ module.exports.custom = {
   oidcResponseMode: process.env.OIDC_RESPONSE_MODE || 'fragment',
   oidcUseDefaultResponseMode: process.env.OIDC_USE_DEFAULT_RESPONSE_MODE === 'true',
   oidcAdminRoles: process.env.OIDC_ADMIN_ROLES ? process.env.OIDC_ADMIN_ROLES.split(',') : [],
+  oidcClaimsSource: process.env.OIDC_CLAIMS_SOURCE || 'userinfo',
   oidcEmailAttribute: process.env.OIDC_EMAIL_ATTRIBUTE || 'email',
   oidcNameAttribute: process.env.OIDC_NAME_ATTRIBUTE || 'name',
   oidcUsernameAttribute: process.env.OIDC_USERNAME_ATTRIBUTE || 'preferred_username',