admins can change external users's names (#5965)

* admins can change external users's names

* Revert "admins can change external users's names"

This reverts commit bf02581.

* admins can change an external user's name

* - `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

* added method for permitted params for better readability

app/controllers/api/v1/users_controller.rb

@@ -169,11 +169,7 @@ def create_user_params
       end
 
       def update_user_params
-        @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)
-                                end
+        @update_user_params ||= params.require(:user).permit(permitted_params)
       end
 
       def change_password_params
@@ -198,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

app/javascript/components/users/user/forms/UpdateUserForm.jsx

@@ -70,7 +70,7 @@ export default function UpdateUserForm({ user }) {
 
   return (
     <Form methods={methods} onSubmit={updateUserAPI.mutate}>
-      <FormControl field={fields.name} type="text" readOnly={currentUser.external_account} />
+      <FormControl field={fields.name} type="text" readOnly={user.external_account && !PermissionChecker.hasManageUsers(currentUser)} />
       <FormControl field={fields.email} type="email" readOnly />
       <FormSelect field={fields.language} variant="dropdown">
         {
@@ -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,

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)

app/serializers/user_serializer.rb

@@ -19,14 +19,18 @@
 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
 
   def language
     object.language.tr('_', '-')
   end
 
+  def external_account
+    object.external_id?
+  end
+
   def avatar
     user_avatar(object)
   end

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