Skip to content

Allow users with ManageUsers permission to edit local user's email addresses #6183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danimo wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow users with ManageUsers permission to edit local user's email addresses #6183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cc3879
Allow users with ManageUsers permission to edit local user's email ad…
danimo Jan 19, 2026
1a4fd66
Merge branch 'master' into feat/edit_email
danimo Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions app/controllers/api/v1/users_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,13 @@ def valid_domain?
end

def permitted_params
is_admin = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call
is_user_manager = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call

return %i[password avatar language role_id invite_token] if external_auth? && !is_admin
permitted = %i[password avatar language role_id invite_token]
permitted.push(:name) if is_user_manager || !external_auth?
permitted.push(:email) if is_user_manager
Copy link
Copy Markdown
Collaborator

@farhatahmad farhatahmad Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing the admin to change an external users email might be problematic - especially if things aren't configured correctly

Copy link
Copy Markdown
Collaborator

@farhatahmad farhatahmad Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that this should be a local user thing but I imagine that's not what most people would need this for

Copy link
Copy Markdown
Contributor Author

@danimo danimo Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing the admin to change an external users email might be problematic - especially if things aren't configured correctly

It depends a bit if email is the identifying key (or if the email fallback is activated). Anyway, if you feel like it is warranted, I can add a note in the documentation.


%i[name password avatar language role_id invite_token]
permitted
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/components/users/user/forms/UpdateUserForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export default function UpdateUserForm({ user }) {
return (
<Form methods={methods} onSubmit={updateUserAPI.mutate}>
<FormControl field={fields.name} type="text" readOnly={user.external_account && !PermissionChecker.hasManageUsers(currentUser)} />
<FormControl field={fields.email} type="email" readOnly />
<FormControl field={fields.email} type="email" readOnly={!PermissionChecker.hasManageUsers(currentUser)} />
<FormSelect field={fields.language} variant="dropdown">
{
Object.keys(locales || {}).map((code) => <Option key={code} value={code}>{locales[code]}</Option>)
Expand Down
Loading