From bf02581d7b6d1fc9de30aef1b15d431d42fe200f Mon Sep 17 00:00:00 2001 From: Ali Hadi Mazeh Date: Thu, 14 Nov 2024 16:18:14 -0500 Subject: [PATCH 1/5] admins can change external users's names --- app/controllers/api/v1/users_controller.rb | 4 +++- db/schema.rb | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 6661d71a91..7073380dad 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -169,7 +169,9 @@ def create_user_params end def update_user_params - @update_user_params ||= if external_auth? + @update_user_params ||= if external_auth? && current_user.role.name.eql?('Administrator') + params.require(:user).permit(:name) + elsif external_auth? params.require(:user).permit(:password, :avatar, :language, :role_id, :invite_token) else params.require(:user).permit(:name, :password, :avatar, :language, :role_id, :invite_token) diff --git a/db/schema.rb b/db/schema.rb index dd09a891c4..4f439f27d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2023_12_18_154727) do +ActiveRecord::Schema[7.1].define(version: 2023_07_05_183747) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -193,12 +193,12 @@ t.uuid "role_id" t.string "language", null: false t.string "reset_digest" - t.datetime "reset_sent_at", precision: nil + t.datetime "reset_sent_at" t.boolean "verified", default: false t.string "verification_digest" - t.datetime "verification_sent_at", precision: nil + t.datetime "verification_sent_at" t.string "session_token" - t.datetime "session_expiry", precision: nil + t.datetime "session_expiry" t.integer "status", default: 0 t.index ["email", "provider"], name: "index_users_on_email_and_provider", unique: true t.index ["reset_digest"], name: "index_users_on_reset_digest", unique: true From 4be5646e9a8d094fa4110c8eed001d178679f656 Mon Sep 17 00:00:00 2001 From: Ali Hadi Mazeh Date: Tue, 19 Nov 2024 14:04:04 -0500 Subject: [PATCH 2/5] Revert "admins can change external users's names" This reverts commit bf02581d7b6d1fc9de30aef1b15d431d42fe200f. --- app/controllers/api/v1/users_controller.rb | 4 +--- db/schema.rb | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 7073380dad..6661d71a91 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -169,9 +169,7 @@ def create_user_params end def update_user_params - @update_user_params ||= if external_auth? && current_user.role.name.eql?('Administrator') - params.require(:user).permit(:name) - elsif external_auth? + @update_user_params ||= if external_auth? params.require(:user).permit(:password, :avatar, :language, :role_id, :invite_token) else params.require(:user).permit(:name, :password, :avatar, :language, :role_id, :invite_token) diff --git a/db/schema.rb b/db/schema.rb index 4f439f27d1..dd09a891c4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2023_07_05_183747) do +ActiveRecord::Schema[7.1].define(version: 2023_12_18_154727) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -193,12 +193,12 @@ t.uuid "role_id" t.string "language", null: false t.string "reset_digest" - t.datetime "reset_sent_at" + t.datetime "reset_sent_at", precision: nil t.boolean "verified", default: false t.string "verification_digest" - t.datetime "verification_sent_at" + t.datetime "verification_sent_at", precision: nil t.string "session_token" - t.datetime "session_expiry" + t.datetime "session_expiry", precision: nil t.integer "status", default: 0 t.index ["email", "provider"], name: "index_users_on_email_and_provider", unique: true t.index ["reset_digest"], name: "index_users_on_reset_digest", unique: true From 19084c49481edc6dd1fa50c46888e3e5cdec717a Mon Sep 17 00:00:00 2001 From: Ali Hadi Mazeh Date: Tue, 19 Nov 2024 15:42:23 -0500 Subject: [PATCH 3/5] admins can change an external user's name --- app/controllers/api/v1/users_controller.rb | 6 +++++- spec/controllers/users_controller_spec.rb | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 6661d71a91..b10d9e4125 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -169,7 +169,11 @@ def create_user_params end def update_user_params - @update_user_params ||= if external_auth? + is_admin = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call + + @update_user_params ||= if external_auth? && is_admin + params.require(:user).permit(:name) + elsif external_auth? params.require(:user).permit(:password, :avatar, :language, :role_id, :invite_token) else params.require(:user).permit(:name, :password, :avatar, :language, :role_id, :invite_token) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index d24017fc3b..61d94a428f 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -463,6 +463,21 @@ expect(user.role_id).to eq(updated_params[:role_id]) end + + it 'allows a user with ManageUser permissions to edit an external users name' do + sign_in_user(user_with_manage_users_permission) + + external_user = create(:user, external_id: 'external-id') + updated_params = { + name: 'New External Name' + } + + patch :update, params: { id: external_user.id, user: updated_params } + + external_user.reload + + expect(external_user.name).to eq(updated_params[:name]) + end end describe '#destroy' do From 509ee971c75706c778080965d2fe04536090a2c1 Mon Sep 17 00:00:00 2001 From: Ali Hadi Mazeh Date: Wed, 20 Nov 2024 11:44:51 -0500 Subject: [PATCH 4/5] - `UpdateUserForm` checks if user is an external and if the current user signed in doesn't have Manage Users permissions - `external_account` method & attribute moved from current user serializer to user serializer --- .../components/users/user/forms/UpdateUserForm.jsx | 3 ++- app/serializers/current_user_serializer.rb | 6 +----- app/serializers/user_serializer.rb | 6 +++++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/javascript/components/users/user/forms/UpdateUserForm.jsx b/app/javascript/components/users/user/forms/UpdateUserForm.jsx index 740f8d420f..d1db18bd43 100644 --- a/app/javascript/components/users/user/forms/UpdateUserForm.jsx +++ b/app/javascript/components/users/user/forms/UpdateUserForm.jsx @@ -70,7 +70,7 @@ export default function UpdateUserForm({ user }) { return (
- + { @@ -102,6 +102,7 @@ UpdateUserForm.propTypes = { name: PropTypes.string.isRequired, email: PropTypes.string.isRequired, provider: PropTypes.string.isRequired, + external_account: PropTypes.bool.isRequired, role: PropTypes.shape({ id: PropTypes.string.isRequired, name: PropTypes.string.isRequired, diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index c29c5c16e4..e3d3c59241 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -17,16 +17,12 @@ # frozen_string_literal: true class CurrentUserSerializer < UserSerializer - attributes :signed_in, :permissions, :status, :external_account, :super_admin + attributes :signed_in, :permissions, :status, :super_admin def signed_in true end - def external_account - object.external_id? - end - def permissions RolePermission.joins(:permission) .where(role_id: object.role_id) diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index ff2d2c7339..4fbeb92c8e 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -19,7 +19,7 @@ class UserSerializer < ApplicationSerializer include Avatarable - attributes :id, :name, :email, :provider, :language, :avatar, :verified, :created_at + attributes :id, :name, :email, :provider, :language, :avatar, :verified, :created_at, :external_account belongs_to :role @@ -27,6 +27,10 @@ def language object.language.tr('_', '-') end + def external_account + object.external_id? + end + def avatar user_avatar(object) end From d033678354467a41f3c9b40152f99facbc994e8e Mon Sep 17 00:00:00 2001 From: Ali Hadi Mazeh Date: Mon, 25 Nov 2024 16:11:24 -0500 Subject: [PATCH 5/5] added method for permitted params for better readability --- app/controllers/api/v1/users_controller.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index b10d9e4125..09c860e4ed 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -169,15 +169,7 @@ def create_user_params end def update_user_params - is_admin = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call - - @update_user_params ||= if external_auth? && is_admin - params.require(:user).permit(:name) - elsif external_auth? - params.require(:user).permit(:password, :avatar, :language, :role_id, :invite_token) - else - params.require(:user).permit(:name, :password, :avatar, :language, :role_id, :invite_token) - end + @update_user_params ||= params.require(:user).permit(permitted_params) end def change_password_params @@ -202,6 +194,14 @@ def valid_domain? end false end + + def permitted_params + is_admin = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call + + return %i[password avatar language role_id invite_token] if external_auth? && !is_admin + + %i[name password avatar language role_id invite_token] + end end end end