feat: Add ability to map OIDC attributes and ignore username

Closes #554
plankanban · Jan 25, 2024 · 634d6ce · 634d6ce
1 parent 31d4d5f
commit 634d6ce
Show file tree

Hide file tree

Showing 13 changed files with 112 additions and 72 deletions.
diff --git a/client/src/components/UserSettingsModal/AccountPane/AccountPane.jsx b/client/src/components/UserSettingsModal/AccountPane/AccountPane.jsx
@@ -24,6 +24,7 @@ const AccountPane = React.memo(
     organization,
     language,
     isLocked,
+    isUsernameLocked,
     isAvatarUpdating,
     usernameUpdateForm,
     emailUpdateForm,
@@ -104,7 +105,7 @@ const AccountPane = React.memo(
           value={language || 'auto'}
           onChange={handleLanguageChange}
         />
-        {!isLocked && (
+        {(!isLocked || !isUsernameLocked) && (
           <>
             <Divider horizontal section>
               <Header as="h4">
@@ -113,56 +114,62 @@ const AccountPane = React.memo(
                 })}
               </Header>
             </Divider>
-            <div className={styles.action}>
-              <UserUsernameEditPopup
-                usePasswordConfirmation
-                defaultData={usernameUpdateForm.data}
-                username={username}
-                isSubmitting={usernameUpdateForm.isSubmitting}
-                error={usernameUpdateForm.error}
-                onUpdate={onUsernameUpdate}
-                onMessageDismiss={onUsernameUpdateMessageDismiss}
-              >
-                <Button className={styles.actionButton}>
-                  {t('action.editUsername', {
-                    context: 'title',
-                  })}
-                </Button>
-              </UserUsernameEditPopup>
-            </div>
-            <div className={styles.action}>
-              <UserEmailEditPopup
-                usePasswordConfirmation
-                defaultData={emailUpdateForm.data}
-                email={email}
-                isSubmitting={emailUpdateForm.isSubmitting}
-                error={emailUpdateForm.error}
-                onUpdate={onEmailUpdate}
-                onMessageDismiss={onEmailUpdateMessageDismiss}
-              >
-                <Button className={styles.actionButton}>
-                  {t('action.editEmail', {
-                    context: 'title',
-                  })}
-                </Button>
-              </UserEmailEditPopup>
-            </div>
-            <div className={styles.action}>
-              <UserPasswordEditPopup
-                usePasswordConfirmation
-                defaultData={passwordUpdateForm.data}
-                isSubmitting={passwordUpdateForm.isSubmitting}
-                error={passwordUpdateForm.error}
-                onUpdate={onPasswordUpdate}
-                onMessageDismiss={onPasswordUpdateMessageDismiss}
-              >
-                <Button className={styles.actionButton}>
-                  {t('action.editPassword', {
-                    context: 'title',
-                  })}
-                </Button>
-              </UserPasswordEditPopup>
-            </div>
+            {!isUsernameLocked && (
+              <div className={styles.action}>
+                <UserUsernameEditPopup
+                  defaultData={usernameUpdateForm.data}
+                  username={username}
+                  isSubmitting={usernameUpdateForm.isSubmitting}
+                  error={usernameUpdateForm.error}
+                  usePasswordConfirmation={!isLocked} // FIXME: hack
+                  onUpdate={onUsernameUpdate}
+                  onMessageDismiss={onUsernameUpdateMessageDismiss}
+                >
+                  <Button className={styles.actionButton}>
+                    {t('action.editUsername', {
+                      context: 'title',
+                    })}
+                  </Button>
+                </UserUsernameEditPopup>
+              </div>
+            )}
+            {!isLocked && (
+              <>
+                <div className={styles.action}>
+                  <UserEmailEditPopup
+                    usePasswordConfirmation
+                    defaultData={emailUpdateForm.data}
+                    email={email}
+                    isSubmitting={emailUpdateForm.isSubmitting}
+                    error={emailUpdateForm.error}
+                    onUpdate={onEmailUpdate}
+                    onMessageDismiss={onEmailUpdateMessageDismiss}
+                  >
+                    <Button className={styles.actionButton}>
+                      {t('action.editEmail', {
+                        context: 'title',
+                      })}
+                    </Button>
+                  </UserEmailEditPopup>
+                </div>
+                <div className={styles.action}>
+                  <UserPasswordEditPopup
+                    usePasswordConfirmation
+                    defaultData={passwordUpdateForm.data}
+                    isSubmitting={passwordUpdateForm.isSubmitting}
+                    error={passwordUpdateForm.error}
+                    onUpdate={onPasswordUpdate}
+                    onMessageDismiss={onPasswordUpdateMessageDismiss}
+                  >
+                    <Button className={styles.actionButton}>
+                      {t('action.editPassword', {
+                        context: 'title',
+                      })}
+                    </Button>
+                  </UserPasswordEditPopup>
+                </div>
+              </>
+            )}
           </>
         )}
       </Tab.Pane>
@@ -179,6 +186,7 @@ AccountPane.propTypes = {
   organization: PropTypes.string,
   language: PropTypes.string,
   isLocked: PropTypes.bool.isRequired,
+  isUsernameLocked: PropTypes.bool.isRequired,
   isAvatarUpdating: PropTypes.bool.isRequired,
   /* eslint-disable react/forbid-prop-types */
   usernameUpdateForm: PropTypes.object.isRequired,

diff --git a/client/src/components/UserSettingsModal/UserSettingsModal.jsx b/client/src/components/UserSettingsModal/UserSettingsModal.jsx
@@ -18,6 +18,7 @@ const UserSettingsModal = React.memo(
     language,
     subscribeToOwnCards,
     isLocked,
+    isUsernameLocked,
     isAvatarUpdating,
     usernameUpdateForm,
     emailUpdateForm,
@@ -50,6 +51,7 @@ const UserSettingsModal = React.memo(
             organization={organization}
             language={language}
             isLocked={isLocked}
+            isUsernameLocked={isUsernameLocked}
             isAvatarUpdating={isAvatarUpdating}
             usernameUpdateForm={usernameUpdateForm}
             emailUpdateForm={emailUpdateForm}
@@ -108,6 +110,7 @@ UserSettingsModal.propTypes = {
   language: PropTypes.string,
   subscribeToOwnCards: PropTypes.bool.isRequired,
   isLocked: PropTypes.bool.isRequired,
+  isUsernameLocked: PropTypes.bool.isRequired,
   isAvatarUpdating: PropTypes.bool.isRequired,
   /* eslint-disable react/forbid-prop-types */
   usernameUpdateForm: PropTypes.object.isRequired,

diff --git a/client/src/components/UsersModal/Item/ActionsStep.jsx b/client/src/components/UsersModal/Item/ActionsStep.jsx
@@ -136,13 +136,15 @@ const ActionsStep = React.memo(
                 context: 'title',
               })}
             </Menu.Item>
+            {!user.isUsernameLocked && (
+              <Menu.Item className={styles.menuItem} onClick={handleEditUsernameClick}>
+                {t('action.editUsername', {
+                  context: 'title',
+                })}
+              </Menu.Item>
+            )}
             {!user.isLocked && (
               <>
-                <Menu.Item className={styles.menuItem} onClick={handleEditUsernameClick}>
-                  {t('action.editUsername', {
-                    context: 'title',
-                  })}
-                </Menu.Item>
                 <Menu.Item className={styles.menuItem} onClick={handleEditEmailClick}>
                   {t('action.editEmail', {
                     context: 'title',

diff --git a/client/src/components/UsersModal/Item/Item.jsx b/client/src/components/UsersModal/Item/Item.jsx
@@ -19,6 +19,7 @@ const Item = React.memo(
     isAdmin,
     isLocked,
     isRoleLocked,
+    isUsernameLocked,
     isDeletionLocked,
     emailUpdateForm,
     passwordUpdateForm,
@@ -61,6 +62,7 @@ const Item = React.memo(
               phone,
               isAdmin,
               isLocked,
+              isUsernameLocked,
               isDeletionLocked,
               emailUpdateForm,
               passwordUpdateForm,
@@ -95,6 +97,7 @@ Item.propTypes = {
   isAdmin: PropTypes.bool.isRequired,
   isLocked: PropTypes.bool.isRequired,
   isRoleLocked: PropTypes.bool.isRequired,
+  isUsernameLocked: PropTypes.bool.isRequired,
   isDeletionLocked: PropTypes.bool.isRequired,
   /* eslint-disable react/forbid-prop-types */
   emailUpdateForm: PropTypes.object.isRequired,

diff --git a/client/src/components/UsersModal/UsersModal.jsx b/client/src/components/UsersModal/UsersModal.jsx
@@ -112,6 +112,7 @@ const UsersModal = React.memo(
                   isAdmin={item.isAdmin}
                   isLocked={item.isLocked}
                   isRoleLocked={item.isRoleLocked}
+                  isUsernameLocked={item.isUsernameLocked}
                   isDeletionLocked={item.isDeletionLocked}
                   emailUpdateForm={item.emailUpdateForm}
                   passwordUpdateForm={item.passwordUpdateForm}

diff --git a/client/src/containers/UserSettingsModalContainer.js b/client/src/containers/UserSettingsModalContainer.js
@@ -16,6 +16,7 @@ const mapStateToProps = (state) => {
     language,
     subscribeToOwnCards,
     isLocked,
+    isUsernameLocked,
     isAvatarUpdating,
     emailUpdateForm,
     passwordUpdateForm,
@@ -32,6 +33,7 @@ const mapStateToProps = (state) => {
     language,
     subscribeToOwnCards,
     isLocked,
+    isUsernameLocked,
     isAvatarUpdating,
     emailUpdateForm,
     passwordUpdateForm,

diff --git a/client/src/models/User.js b/client/src/models/User.js
@@ -46,6 +46,7 @@ export default class extends BaseModel {
     isAdmin: attr(),
     isLocked: attr(),
     isRoleLocked: attr(),
+    isUsernameLocked: attr(),
     isDeletionLocked: attr(),
     deletedAt: attr(),
     createdAt: attr({

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -44,7 +44,11 @@ services:
       # - OIDC_CLIENT_SECRET=
       # - OIDC_SCOPES=openid email profile
       # - OIDC_ADMIN_ROLES=admin
+      # - OIDC_EMAIL_ATTRIBUTE=email
+      # - OIDC_NAME_ATTRIBUTE=name
+      # - OIDC_USERNAME_ATTRIBUTE=preferred_username
       # - OIDC_ROLES_ATTRIBUTE=groups
+      # - OIDC_IGNORE_USERNAME=true
       # - OIDC_IGNORE_ROLES=true
     depends_on:
       - postgres

diff --git a/server/.env.sample b/server/.env.sample
@@ -27,7 +27,11 @@ SECRET_KEY=notsecretkey
 # OIDC_CLIENT_SECRET=
 # OIDC_SCOPES=openid email profile
 # OIDC_ADMIN_ROLES=admin
+# OIDC_EMAIL_ATTRIBUTE=email
+# OIDC_NAME_ATTRIBUTE=name
+# OIDC_USERNAME_ATTRIBUTE=preferred_username
 # OIDC_ROLES_ATTRIBUTE=groups
+# OIDC_IGNORE_USERNAME=true
 # OIDC_IGNORE_ROLES=true
 
 ## Do not edit this

diff --git a/server/api/controllers/users/update-username.js b/server/api/controllers/users/update-username.js
@@ -53,11 +53,7 @@ module.exports = {
   async fn(inputs) {
     const { currentUser } = this.req;
 
-    if (inputs.id === currentUser.id) {
-      if (!inputs.currentPassword) {
-        throw Errors.INVALID_CURRENT_PASSWORD;
-      }
-    } else if (!currentUser.isAdmin) {
+    if (inputs.id !== currentUser.id && !currentUser.isAdmin) {
       throw Errors.USER_NOT_FOUND; // Forbidden
     }
 
@@ -67,15 +63,18 @@ module.exports = {
       throw Errors.USER_NOT_FOUND;
     }
 
-    if (user.email === sails.config.custom.defaultAdminEmail || user.isSso) {
+    if (user.email === sails.config.custom.defaultAdminEmail) {
       throw Errors.NOT_ENOUGH_RIGHTS;
     }
 
-    if (
-      inputs.id === currentUser.id &&
-      !bcrypt.compareSync(inputs.currentPassword, user.password)
-    ) {
-      throw Errors.INVALID_CURRENT_PASSWORD;
+    if (user.isSso) {
+      if (!sails.config.custom.oidcIgnoreUsername) {
+        throw Errors.NOT_ENOUGH_RIGHTS;
+      }
+    } else if (inputs.id === currentUser.id) {
+      if (!inputs.currentPassword || !bcrypt.compareSync(inputs.currentPassword, user.password)) {
+        throw Errors.INVALID_CURRENT_PASSWORD;
+      }
     }
 
     const values = _.pick(inputs, ['username']);

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
@@ -38,7 +38,10 @@ module.exports = {
       throw 'invalidCodeOrNonce';
     }
 
-    if (!userInfo.email || !userInfo.name) {
+    if (
+      !userInfo[sails.config.custom.oidcEmailAttribute] ||
+      !userInfo[sails.config.custom.oidcNameAttribute]
+    ) {
       throw 'missingValues';
     }
 
@@ -56,12 +59,14 @@ module.exports = {
 
     const values = {
       isAdmin,
-      email: userInfo.email,
+      email: userInfo[sails.config.custom.oidcEmailAttribute],
       isSso: true,
-      name: userInfo.name,
-      username: userInfo.preferred_username,
+      name: userInfo[sails.config.custom.oidcNameAttribute],
       subscribeToOwnCards: false,
     };
+    if (!sails.config.custom.oidcIgnoreUsername) {
+      values.username = userInfo[sails.config.custom.oidcUsernameAttribute];
+    }
 
     let user;
     // This whole block technically needs to be executed in a transaction
@@ -95,7 +100,10 @@ module.exports = {
       });
     }
 
-    const updateFieldKeys = ['email', 'isSso', 'name', 'username'];
+    const updateFieldKeys = ['email', 'isSso', 'name'];
+    if (!sails.config.custom.oidcIgnoreUsername) {
+      updateFieldKeys.push('username');
+    }
     if (!sails.config.custom.oidcIgnoreRoles) {
       updateFieldKeys.push('isAdmin');
     }

diff --git a/server/api/models/User.js b/server/api/models/User.js
@@ -116,6 +116,7 @@ module.exports = {
       ..._.omit(this, ['password', 'isSso', 'avatar', 'passwordChangedAt']),
       isLocked: this.isSso || isDefaultAdmin,
       isRoleLocked: (this.isSso && !sails.config.custom.oidcIgnoreRoles) || isDefaultAdmin,
+      isUsernameLocked: (this.isSso && !sails.config.custom.oidcIgnoreUsername) || isDefaultAdmin,
       isDeletionLocked: isDefaultAdmin,
       avatarUrl:
         this.avatar &&

diff --git a/server/config/custom.js b/server/config/custom.js
@@ -38,7 +38,11 @@ module.exports.custom = {
   oidcClientSecret: process.env.OIDC_CLIENT_SECRET,
   oidcScopes: process.env.OIDC_SCOPES || 'openid email profile',
   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',
+  oidcUsernameAttribute: process.env.OIDC_USERNAME_ATTRIBUTE || 'preferred_username',
   oidcRolesAttribute: process.env.OIDC_ROLES_ATTRIBUTE || 'groups',
+  oidcIgnoreUsername: process.env.OIDC_IGNORE_USERNAME === 'true',
   oidcIgnoreRoles: process.env.OIDC_IGNORE_ROLES === 'true',
 
   // TODO: move client base url to environment variable?