-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat(auth): Redirect to OpenShift login on empty auth request #748
Conversation
233d075
to
052005e
Compare
src/main/java/io/cryostat/net/web/http/api/v2/AbstractV2RequestHandler.java
Outdated
Show resolved
Hide resolved
052005e
to
6a705f6
Compare
6a705f6
to
0c58bcd
Compare
28b2899
to
b5ac86d
Compare
0c1d876
to
b9bace3
Compare
src/main/java/io/cryostat/net/web/http/api/v2/AuthPostHandler.java
Outdated
Show resolved
Hide resolved
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.
Despite my nitpicking of fine internal details, this looks excellent and I'm really excited to have it in a future release. It will really smooth out the user experience and makes it feel much more polished and well-integrated into the deployment platform.
91a4a13
to
a621150
Compare
0db3f32
to
5034298
Compare
🎉 Great news! Looks like all the dependencies have been resolved:
💡 To add or remove a dependency please update this issue/PR description. Brought to you by Dependent Issues (:robot: ). Happy coding! |
Add unit tests Cache authorization url
0d4a129
to
1d35cd0
Compare
Awesome, seems to be working great. One question I have, which is definitely out of the scope of this specific PR, is if we can somehow get the OAuth redirected login page to remember our credentials so that they don't need to be provided every time. Is there some cookie we need to set and send along with the request when we redirect the user to OAuth perhaps? |
Here's what happens if I login a second time. Do you mean to make a different request to the OAuth server if we already have a token to prevent another redirection? Screencast.from.2021-12-10.11.57.48.AM.mp4
|
Hmm. Let me try that again with Chrome. I'm in Firefox and when I tried closing and re-opening the Cryostat tab, or even just F5 refreshing it, I was redirected back to the platform OAuth login page and had to re-enter my credentials. Maybe it's a browser-specific thing, or maybe it's one of my installed add-ons. |
Nope, doesn't seem to be browser-specific. Same behaviour in Firefox, Chrome, and ungoogled-chromium. Maybe this is a CRC thing? |
oauth-redirect-crc-2021-12-10_12.34.22.mp4Here's what I see deployed in CRC and using Chrome in this video. The last OAuth login page at the end is from me pressing |
Possibly? Firefox and Chrome show the same behavior as what I posted above. Refreshing keeps you logged in. |
Related #717
Depends on cryostatio/cryostat-operator#292
Depends on cryostatio/cryostat-web#350
Edit: changed Fixes to Related as we will need a follow-up PR to add the logout feature
Screencast.from.2021-12-01.03.10.16.PM.mp4
As of now, I've also set the
grantMethod
toprompt
. If a user logs into Cryostat via the OCP login page for the first time, they will see this page before they are redirected to the dashboard. Alternatively, I can setgrantMethod: auto
to hide this page. (OAuth Grant Options docs)