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

Handle sudo connection in the account view #516

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Nov 29, 2023

Fixes #445

Summary of changes

  • expand webadmin.views.my_account to also detect whether the current context was created via sudo
  • If the current session was created with sudo, disable the ability to update experimenter properties and password in the UI

To test this PR, follow the authentication workflow described in #445 (comment) i.e.

  1. create a session using --sudo via the CLI
  2. use the session key as the username/password to authenticate using OMERO.web
  3. got to the My Account view i.e. click on the user profile then User settings

Without this PR, the Save and Change my password button should be active. Clicking on Save should launch the 500 error page with a server SecurityViolation of type Current user is not admin for the given user(s). Clicking on Change my password will ask for the current user password which is not known in this workflow since the session was created using the principal password.

With this PR, both buttons should be hidden.

Expand webadmin.views.my_account to also detect whether the current
context was created via sudo
If the current session was created with sudo, disable the ability to
update experimenter properties and password in the UI
@sbesson sbesson requested review from knabar and will-moore November 29, 2023 14:57
Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Looks good. Reproduced the bug error without this PR. With this PR looks as expected:

Screenshot 2023-11-29 at 15 36 04

Copy link
Member

@knabar knabar left a comment

Choose a reason for hiding this comment

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

I feel we need to go a small step further; having an editable form without a "Save" button seems confusing.

Some options, not necessarily better:

  • disable (instead of remove) the "Save" button
  • add some explanatory text on why there is no "Save" button
  • Make the form inactive (e.g. by adding the inert property, see https://stackoverflow.com/a/73500364/34171)

@knabar knabar added this to the 5.24.0 milestone Dec 1, 2023
@sbesson
Copy link
Member Author

sbesson commented Dec 12, 2023

Thanks @knabar, the approach of the last commit makes sense to me. Passing back to @will-moore and @pwalczysko for another round of review.

@will-moore
Copy link
Member

Looks good - form is "inert":

Screenshot 2023-12-12 at 14 45 56

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

👍

@pwalczysko
Copy link
Member

confirming, lgtm

@knabar knabar merged commit 9b9de7e into ome:master Dec 19, 2023
9 checks passed
@sbesson sbesson deleted the myaccount_sudoerId branch May 15, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

updateMyAccount SecurityViolation
4 participants