From 6dc9e4ed99060dddd8b4744881499d630bc44628 Mon Sep 17 00:00:00 2001
From: Maksim Eltyshev <meltyshev@gmail.com>
Date: Wed, 25 Oct 2023 23:39:34 +0200
Subject: [PATCH] fix: Disable role change when OIDC roles are not ignored

---
 client/src/components/UsersModal/Item/ActionsStep.jsx  |  2 +-
 client/src/components/UsersModal/Item/Item.jsx         | 10 ++++++----
 client/src/components/UsersModal/UsersModal.jsx        |  3 ++-
 client/src/models/User.js                              |  3 ++-
 server/api/controllers/users/update.js                 |  4 ++++
 .../api/helpers/users/get-or-create-one-using-oidc.js  |  8 +++-----
 server/api/models/User.js                              |  7 ++++---
 server/config/custom.js                                |  2 +-
 server/db/seeds/default.js                             |  4 ----
 9 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/client/src/components/UsersModal/Item/ActionsStep.jsx b/client/src/components/UsersModal/Item/ActionsStep.jsx
index 88c089ac..f8a2959b 100644
--- a/client/src/components/UsersModal/Item/ActionsStep.jsx
+++ b/client/src/components/UsersModal/Item/ActionsStep.jsx
@@ -155,7 +155,7 @@ const ActionsStep = React.memo(
                 </Menu.Item>
               </>
             )}
-            {!user.isLockedAdmin && (
+            {!user.isDeletionLocked && (
               <Menu.Item className={styles.menuItem} onClick={handleDeleteClick}>
                 {t('action.deleteUser', {
                   context: 'title',
diff --git a/client/src/components/UsersModal/Item/Item.jsx b/client/src/components/UsersModal/Item/Item.jsx
index da73c5c8..80db3568 100755
--- a/client/src/components/UsersModal/Item/Item.jsx
+++ b/client/src/components/UsersModal/Item/Item.jsx
@@ -18,7 +18,8 @@ const Item = React.memo(
     phone,
     isAdmin,
     isLocked,
-    isLockedAdmin,
+    isRoleLocked,
+    isDeletionLocked,
     emailUpdateForm,
     passwordUpdateForm,
     usernameUpdateForm,
@@ -48,7 +49,7 @@ const Item = React.memo(
         <Table.Cell>{username || '-'}</Table.Cell>
         <Table.Cell>{email}</Table.Cell>
         <Table.Cell>
-          <Radio toggle checked={isAdmin} disabled={isLockedAdmin} onChange={handleIsAdminChange} />
+          <Radio toggle checked={isAdmin} disabled={isRoleLocked} onChange={handleIsAdminChange} />
         </Table.Cell>
         <Table.Cell textAlign="right">
           <ActionsPopup
@@ -60,7 +61,7 @@ const Item = React.memo(
               phone,
               isAdmin,
               isLocked,
-              isLockedAdmin,
+              isDeletionLocked,
               emailUpdateForm,
               passwordUpdateForm,
               usernameUpdateForm,
@@ -93,7 +94,8 @@ Item.propTypes = {
   phone: PropTypes.string,
   isAdmin: PropTypes.bool.isRequired,
   isLocked: PropTypes.bool.isRequired,
-  isLockedAdmin: PropTypes.bool.isRequired,
+  isRoleLocked: PropTypes.bool.isRequired,
+  isDeletionLocked: PropTypes.bool.isRequired,
   /* eslint-disable react/forbid-prop-types */
   emailUpdateForm: PropTypes.object.isRequired,
   passwordUpdateForm: PropTypes.object.isRequired,
diff --git a/client/src/components/UsersModal/UsersModal.jsx b/client/src/components/UsersModal/UsersModal.jsx
index 4e7452e8..0b55778e 100755
--- a/client/src/components/UsersModal/UsersModal.jsx
+++ b/client/src/components/UsersModal/UsersModal.jsx
@@ -111,7 +111,8 @@ const UsersModal = React.memo(
                   phone={item.phone}
                   isAdmin={item.isAdmin}
                   isLocked={item.isLocked}
-                  isLockedAdmin={item.isLockedAdmin}
+                  isRoleLocked={item.isRoleLocked}
+                  isDeletionLocked={item.isDeletionLocked}
                   emailUpdateForm={item.emailUpdateForm}
                   passwordUpdateForm={item.passwordUpdateForm}
                   usernameUpdateForm={item.usernameUpdateForm}
diff --git a/client/src/models/User.js b/client/src/models/User.js
index cce2a78a..84555308 100755
--- a/client/src/models/User.js
+++ b/client/src/models/User.js
@@ -45,7 +45,8 @@ export default class extends BaseModel {
     subscribeToOwnCards: attr(),
     isAdmin: attr(),
     isLocked: attr(),
-    isLockedAdmin: attr(),
+    isRoleLocked: attr(),
+    isDeletionLocked: attr(),
     deletedAt: attr(),
     createdAt: attr({
       getDefault: () => new Date(),
diff --git a/server/api/controllers/users/update.js b/server/api/controllers/users/update.js
index c876efbd..161ae78c 100755
--- a/server/api/controllers/users/update.js
+++ b/server/api/controllers/users/update.js
@@ -73,6 +73,10 @@ module.exports = {
       delete inputs.name;
       /* eslint-enable no-param-reassign */
     } else if (user.isSso) {
+      if (!sails.config.custom.oidcIgnoreRoles) {
+        delete inputs.isAdmin; // eslint-disable-line no-param-reassign
+      }
+
       delete inputs.name; // eslint-disable-line no-param-reassign
     }
 
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 ef3f3dce..6a862022 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
@@ -90,11 +90,9 @@ module.exports = {
       });
     }
 
-    const updateFieldKeys = ['email', 'isAdmin', 'isSso', 'name', 'username'];
-
-    if (sails.config.custom.oidcIgnoreRoles) {
-      // Remove isAdmin from updateFieldKeys
-      updateFieldKeys.splice(updateFieldKeys.indexOf('isAdmin'), 1);
+    const updateFieldKeys = ['email', 'isSso', 'name', 'username'];
+    if (!sails.config.custom.oidcIgnoreRoles) {
+      updateFieldKeys.push('isAdmin');
     }
 
     const updateValues = {};
diff --git a/server/api/models/User.js b/server/api/models/User.js
index 5a16fe23..9bf8a298 100755
--- a/server/api/models/User.js
+++ b/server/api/models/User.js
@@ -110,12 +110,13 @@ module.exports = {
   tableName: 'user_account',
 
   customToJSON() {
-    const isLockedAdmin = this.email === sails.config.custom.defaultAdminEmail;
+    const isDefaultAdmin = this.email === sails.config.custom.defaultAdminEmail;
 
     return {
       ..._.omit(this, ['password', 'isSso', 'avatar', 'passwordChangedAt']),
-      isLockedAdmin,
-      isLocked: this.isSso || isLockedAdmin,
+      isLocked: this.isSso || isDefaultAdmin,
+      isRoleLocked: (this.isSso && !sails.config.custom.oidcIgnoreRoles) || isDefaultAdmin,
+      isDeletionLocked: isDefaultAdmin,
       avatarUrl:
         this.avatar &&
         `${sails.config.custom.userAvatarsUrl}/${this.avatar.dirname}/square-100.${this.avatar.extension}`,
diff --git a/server/config/custom.js b/server/config/custom.js
index 73e2427e..cbbc89ba 100644
--- a/server/config/custom.js
+++ b/server/config/custom.js
@@ -39,7 +39,7 @@ module.exports.custom = {
   oidcScopes: process.env.OIDC_SCOPES || 'openid email profile',
   oidcAdminRoles: process.env.OIDC_ADMIN_ROLES ? process.env.OIDC_ADMIN_ROLES.split(',') : [],
   oidcRolesAttribute: process.env.OIDC_ROLES_ATTRIBUTE || 'groups',
-  oidcIgnoreRoles : process.env.OIDC_IGNORE_ROLES || false,
+  oidcIgnoreRoles: process.env.OIDC_IGNORE_ROLES === 'true',
 
   // TODO: move client base url to environment variable?
   oidcRedirectUri: `${
diff --git a/server/db/seeds/default.js b/server/db/seeds/default.js
index 20624e50..137cf569 100644
--- a/server/db/seeds/default.js
+++ b/server/db/seeds/default.js
@@ -34,10 +34,6 @@ exports.seed = async (knex) => {
       createdAt: new Date().toISOString(),
     });
   } catch (error) {
-    if (Object.keys(data).length === 0) {
-      return;
-    }
-
     await knex('user_account').update(data).where('email', process.env.DEFAULT_ADMIN_EMAIL);
   }
 };