Skip to content

refactor: simplify OAuth2 authorization flow and use 302 redirects #18923

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

Conversation

ThomasK33
Copy link
Member

@ThomasK33 ThomasK33 commented Jul 20, 2025

Refactor OAuth2 Provider Authorization Flow

This PR refactors the OAuth2 provider authorization flow by:

  1. Removing the authorizeMW middleware and directly implementing its functionality in the ShowAuthorizePage handler
  2. Simplifying function signatures by removing unnecessary parameters:
    • Removed db parameter from ShowAuthorizePage
    • Removed accessURL parameter from ProcessAuthorize
  3. Changing the redirect status code in ProcessAuthorize from 307 (Temporary Redirect) to 302 (Found) to improve compatibility with external OAuth2 apps and browsers. (Technical explanation: we replied with a 307 to a POST request, thus the browser performs a redirect to that URL as a POST request, but we need it to be a GET request to be compatible. Thus, we use the 302 redirect so that browsers turn it into a GET request when redirecting back to the redirect_uri.)

The changes maintain the same functionality while simplifying the code and improving compatibility with external systems.

@ThomasK33 ThomasK33 marked this pull request as ready for review July 20, 2025 08:46
Copy link
Member Author

ThomasK33 commented Jul 20, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ThomasK33 ThomasK33 force-pushed the thomask33/07-20-refactor_oauth2_simplify_authorization_flow_by_removing_middleware_layer branch from ee2ff90 to 6f159c6 Compare July 20, 2025 08:47
@ThomasK33 ThomasK33 requested a review from kylecarbs July 20, 2025 08:48
…layer

Change-Id: Ieff16b08aeb2cf2357ada11d83fd408cc66c6c5a
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33 ThomasK33 force-pushed the thomask33/07-20-refactor_oauth2_simplify_authorization_flow_by_removing_middleware_layer branch from 6f159c6 to 3f20259 Compare July 20, 2025 14:12
@ThomasK33 ThomasK33 merged commit 7b06fc7 into main Jul 20, 2025
33 checks passed
@ThomasK33 ThomasK33 deleted the thomask33/07-20-refactor_oauth2_simplify_authorization_flow_by_removing_middleware_layer branch July 20, 2025 14:22
@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants