Skip to content
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.

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

Fix : Ignore error messages for admin created accounts #16132

Merged
merged 2 commits into from May 23, 2023

Conversation

jvanbraekel
Copy link
Contributor

Always create account when admin request it, ignoring errors messages from 'check_registration_allowed'.
Fix the issue #16067
Side effect : Bypass the 'challenge' option of the authenticator if set, when an admin create an account (locally).

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1.Reproduce the test steps included in the bug/issue report.
  1. Check that the new user account is create successfully.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Always create account when admin request it, ignoring errors messages from 'check_registration_allowed'
@github-actions github-actions bot added this to the 23.1 milestone May 22, 2023
@dannon
Copy link
Member

dannon commented May 22, 2023

@jvanbraekel Thank you for this, it looks good to me.

One minor tweak might be to add informative text to the admin interface user creation area just saying when user creation is disabled. We should already have the relevant configuration values on the client side, and it could just say something like "Note that user creation is disabled, but you can bypass this as an administrator" -- just to make sure admins know the state of things prior to creating a new user.

@jvanbraekel
Copy link
Contributor Author

At the moment, the form/action for user creation by admin is likely the same than the user self-registration one. In ours local instance we get the default ' registration_warning_message' (from lib/galaxy/config/schemas/config_schema.yml) which warn against multiple account creation. That looks strange in the admin interface, replacing it by the informative text you suggest would make more sense, but I don't known how to do that . Lastly, this message is valid only for local user management, as some external authentication provider may not authorize user creation, even as galaxy admin.

@dannon
Copy link
Member

dannon commented May 23, 2023

@jvanbraekel Yeah, agreed that it looks strange to reuse the default form here from the admin interface -- we discussed this in a recent meeting as well, along with some enhancements to make it more clear that it's an admin action that may or may not be bypassing user_creation configuration. I'll go ahead and merge this as-is for now and we can try to follow up on enhancing that

@dannon dannon merged commit eb29c29 into galaxyproject:dev May 23, 2023
37 of 39 checks passed
@github-actions
Copy link

This PR was merged without a "kind/" label, please correct.

@jvanbraekel jvanbraekel deleted the patch-2 branch September 11, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants