-
Notifications
You must be signed in to change notification settings - Fork 20
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
JMX Auth UX #116
JMX Auth UX #116
Conversation
Includes some refactoring to share auth-related logic within service(s)
To test: use a recent (now relies on changes in cryostatio/cryostat-legacy#242) ContainerJFR backend and run it like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine for me. I'm a bit confused about how the delete button is supposed to work. Is it to delete saved credentials for the target? If so, will the user have an opportunity to do this once they've been saved?
Yea, I think we will probably want to have some kind of UX for managing credentials, but I don't really know how to go about it. Right now the Delete button is only potentially useful if you first enter some invalid credentials and are re-prompted for credentials, and then decide you don't want the web-client to store those credentials after all. Pretty narrow and meaningless use-case currently. Once you've entered valid credentials for a target then there is no way to delete those credentials - although they only persist so long as the browser tab/window is open anyway, and are cleared as soon as you close/reopen or refresh the page. I could just remove the Delete button for now since it doesn't really provide any meaningful functionality. |
I'm fine with leaving it and changing it later. Up to you |
Fixes #117
Based on top of #110