Skip to content

feat(github): support redirect flow for github link#2291

Merged
whutchinson98 merged 2 commits intomainfrom
hutch/support-redirect-flow-for-github-link
Mar 31, 2026
Merged

feat(github): support redirect flow for github link#2291
whutchinson98 merged 2 commits intomainfrom
hutch/support-redirect-flow-for-github-link

Conversation

@whutchinson98
Copy link
Copy Markdown
Member

No description provided.

@whutchinson98 whutchinson98 requested a review from a team as a code owner March 31, 2026 14:17
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • GitHub account linking accepts an optional original_url so users can be redirected back to their original page after linking (instead of closing the popup).
  • Documentation
    • API documentation updated to expose the new optional original_url query parameter for the GitHub link endpoint.

Walkthrough

Added an optional original_url query parameter to the GitHub account-linking flow; the init handler now accepts and stores it in OAuth state, and the OAuth completion handler may redirect to that URL after a successful link.

Changes

Cohort / File(s) Summary
Init handler and query type
rust/cloud-storage/authentication_service/src/api/link/github.rs
Added InitGithubLinkQueryParams with original_url: Option<UrlEncoded<Url>>; updated init_github_link_handler signature to accept Query<InitGithubLinkQueryParams> and pass original_url into OAuthState. Updated utoipa metadata for the new query parameter.
OAuth completion flow
rust/cloud-storage/authentication_service/src/api/oauth2/github.rs
Imported Redirect and extended the account-linking branch: after successful link, if state.original_url is present, return a temporary redirect to that URL instead of always returning popup-closing HTML.
API contract
js/app/packages/service-clients/service-auth/openapi.json
Added an optional original_url query parameter (string) to the POST /link/github (init_github_link) OpenAPI operation.

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author. Add a description explaining the purpose, implementation details, and rationale for supporting the redirect flow in the GitHub link authentication handler.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commits format with 'feat:' prefix and is 51 characters, well under the 72-character limit, accurately describing the main change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rust/cloud-storage/authentication_service/src/api/oauth2/github.rs`:
- Around line 105-108: The callback handler uses state.original_url directly in
Redirect::temporary (returning GithubOAuthSuccess::Login) which opens an
unvalidated open-redirect; fix by validating state.original_url against an
allowlist or by parsing and ensuring it is a same-origin or permitted absolute
path before using it, and if validation fails fall back to a safe internal URL
(e.g., homepage or dashboard) and log the rejection; update the code paths that
construct Redirect::temporary(original_url) to perform this validation
(referencing state.original_url, Redirect::temporary, and
GithubOAuthSuccess::Login) and return the safe fallback redirect when the check
does not pass.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 533cbe17-5295-4296-bfec-f8191d510ce2

📥 Commits

Reviewing files that changed from the base of the PR and between f17bfac and 33a3fad.

📒 Files selected for processing (2)
  • rust/cloud-storage/authentication_service/src/api/link/github.rs
  • rust/cloud-storage/authentication_service/src/api/oauth2/github.rs

@whutchinson98 whutchinson98 requested a review from a team as a code owner March 31, 2026 14:31
@github-actions
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/service-clients/service-auth/openapi.json`:
- Around line 373-379: The OpenAPI parameter "original_url" is marked required
but the description and generated TypeScript type should be optional; update the
OpenAPI parameter entry in openapi.json (the parameter named "original_url"
under the relevant operation) to remove or set "required": false (and/or adjust
"nullable" if intended) so the contract reflects optionality, then regenerate
the client so the generated type InitGithubLinkParams
(generated/schemas/initGithubLinkParams.ts) uses original_url?: string instead
of original_url: string.
- Around line 375-381: The OpenAPI schema exposes the query parameter
"original_url" as an unconstrained string and the backend consumes a Url type
then calls Redirect::temporary(original_url), enabling open-redirects and
custom-scheme redirects; fix by constraining "original_url" to same-origin
relative paths or an explicit allowlist: update the OpenAPI schema for parameter
original_url to require a path-only pattern (e.g. begins with "/") or enum of
allowed schemes/hosts, and add server-side validation where Url is parsed (the
code calling Redirect::temporary) to reject absolute URLs or validate against a
whitelist of allowed origins/schemes (including any mobile custom-schemes)
before performing Redirect::temporary.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 906bcb6d-55de-4125-9ac3-cb62aa6db607

📥 Commits

Reviewing files that changed from the base of the PR and between 33a3fad and 66db9da.

⛔ Files ignored due to path filters (3)
  • js/app/packages/service-clients/service-auth/generated/client.ts is excluded by !**/generated/**
  • js/app/packages/service-clients/service-auth/generated/schemas/index.ts is excluded by !**/generated/**
  • js/app/packages/service-clients/service-auth/generated/schemas/initGithubLinkParams.ts is excluded by !**/generated/**
📒 Files selected for processing (1)
  • js/app/packages/service-clients/service-auth/openapi.json

@whutchinson98 whutchinson98 merged commit ffe768b into main Mar 31, 2026
39 checks passed
@whutchinson98 whutchinson98 deleted the hutch/support-redirect-flow-for-github-link branch March 31, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant