Skip to content

fix(hub): preserve requested page through sign-in - #250

Merged
cmcnally merged 3 commits into
mainfrom
cmcnally-hub-sign-in-destination
Nov 13, 2026
Merged

cmcnally merged 3 commits into
mainfrom
cmcnally-hub-sign-in-destination

Conversation

@cmcnally

Copy link
Copy Markdown
Contributor

Why

Opening a Hub Create link while signed out loses the requested page. After GitHub sign-in, Hub sends the user to its default destination instead. Hub only saved canvas pairing requests, and its sign-in callback only accepted preview authorization or pairing destinations.

What changes

Hub remembers the requested internal page and query through sign-in, including Create. GitHub and local sign-in share destination validation, which rejects external URLs and unsafe paths. Only GET destinations are saved, and the destination still requires its usual access checks. Sign-ins without a saved destination keep their existing welcome or repository landing page.

Adds regression coverage for Create links, default destinations, repository access, unsafe redirects, and local sign-in. This changes Replica Hub only; it does not change the migration modal in github/prototyping#3604.

The full Create flow and normal-browser authentication remain unverified. Local checks were blocked by npm TLS connection failures that left Vite assets unavailable; local repository fixtures also reported Git errors. Browser verification requires this branch on a configured GitHub-backed Hub.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
@cmcnally
cmcnally marked this pull request as ready for review November 13, 2026 19:14
@cmcnally
cmcnally requested review from a team and nshki as code owners November 13, 2026 19:14
Copilot AI dynamic review requested due to automatic review settings November 13, 2026 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Saved sign-in destinations can overflow the session cookie or be overwritten by background requests, causing sign-in failures or redirects to raw JSON.

3 open issues
What changed in this PR

Updates Hub sign-in to return users to their requested internal page, including Create links, while preserving normal access checks and default landing pages.

Changes:

  • Saves requested GET paths and queries and shares destination validation between GitHub and local sign-in.
  • Adds regression coverage for redirects, repository access, unsafe destinations, and local sign-in.
  • Updates authentication guidance and session-revocation expectations.
File Description
hub/​test/​integration/​session_revocations_test.rb Expects sign-in to return to session revocation.
hub/​test/​integration/​local_development_test.rb Covers returning to Create after local sign-in.
hub/​test/​integration/​authentication_test.rb Covers saved destinations, defaults, access checks, and unsafe redirects.
hub/​docs/​authentication-and-repository-authorization.md Documents destination preservation and fallback behavior.
hub/​app/​controllers/​local_authentications_controller.rb Uses shared destination validation.
hub/​app/​controllers/​github_authentications_controller.rb Removes the provider-specific destination validator.
hub/​app/​controllers/​concerns/​authentication.rb Captures GET destinations and centralizes validation.

Review effort: Max (dynamic)

@nshki nshki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, @cmcnally!

Add condition to limit session return path size for GET requests.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@cmcnally
cmcnally merged commit 88b620e into main Nov 13, 2026
39 checks passed
@cmcnally
cmcnally deleted the cmcnally-hub-sign-in-destination branch November 13, 2026 20:01
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