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

Optimize OIDC code flow to do one less redirect when the original path has to be restored #12857

Merged

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Oct 21, 2020

OIDC CodeAuthenticationMechanism can restore the original request path, for example:

  1. User does GET /web-app/service
  2. Redirect path is configured as /web-app
  3. The user, after authenticating, is redirected back to Quarkus to /web-app
  4. Now, before proceeding with the code to token exchange, the user is redirected to /web-app/service thus restoring the original request URI; to implement it, a custom pathChecked query is added, so it is not ideal
  5. Finally, after the code has been exchanged, another redirect follows, dropping code and state and other non-custom query parameters

So, step 4 is redundant, it adds to the cost of processing and also returns code and state back to the browser because the code flow has not completed yet.

As such this PR drops step 4 and the original path, if needed, will be restored as part of the last step

I had to fix a few tests because they were written around the flow involving step4.

@sberyozkin sberyozkin force-pushed the oidc_code_flow_custom_code_query branch from 4206a76 to 870f460 Compare October 21, 2020 15:04
@sberyozkin sberyozkin changed the title Oidc code flow custom code query Support OIDC code flow custom 'code' query parameter Oct 21, 2020
@sberyozkin sberyozkin requested a review from pedroigor October 21, 2020 15:06
@sberyozkin sberyozkin added this to the 1.10 - master milestone Oct 21, 2020
@sberyozkin sberyozkin force-pushed the oidc_code_flow_custom_code_query branch 3 times, most recently from 2cacc58 to 7c6de99 Compare November 3, 2020 11:12
@sberyozkin
Copy link
Member Author

sberyozkin commented Nov 6, 2020

@pedroigor Hey, I've spent a lot of time trying to make it work and came to the conclusion it is that is impossible to avoid some hacks :-) and as such I'm going to close this PR, but the good news is, while refactoring the code to make it work I've managed to get rid of one local redirect where the original path is restored :-). so I'll be working on re-focusing this PR going forward

@sberyozkin sberyozkin changed the title Support OIDC code flow custom 'code' query parameter WIP: Support OIDC code flow custom 'code' query parameter Nov 6, 2020
@sberyozkin sberyozkin force-pushed the oidc_code_flow_custom_code_query branch 3 times, most recently from 8657b02 to fa9d71d Compare November 8, 2020 16:10
@sberyozkin sberyozkin changed the title WIP: Support OIDC code flow custom 'code' query parameter Optimize OIDC code flow to do a one less redirect when the original path has to be restored Nov 9, 2020
@sberyozkin sberyozkin force-pushed the oidc_code_flow_custom_code_query branch from fa9d71d to 290520e Compare November 9, 2020 11:18
@sberyozkin
Copy link
Member Author

@pedroigor Hey Pedro
I've completely re-focused the PR on optimizing the redirects, please see the description, it drops one redundant redirect which can only a good thing for the performance and the security of the flow too :-)

@sberyozkin sberyozkin changed the title Optimize OIDC code flow to do a one less redirect when the original path has to be restored Optimize OIDC code flow to do one less redirect when the original path has to be restored Nov 9, 2020
@gsmet gsmet merged commit 67fd251 into quarkusio:master Nov 9, 2020
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.

3 participants