Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because there is no eligible user to bill.
An enterprise owner can allow the organization to be billed for reviews like this by enabling Copilot code review for users without a license.
aa35eb3 to
b4b9597
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical defects can prevent API startup, break existing database deployments, and allow the legacy admission policy to admit any GitHub account through a public installed repository.
9 open issues
Add a new migration for session generation columns · New Validate configuration before advancing the provider revision · New Allocate provider revisions when existing providers are disabled · New Require explicit collaboration for public repository admission · New Avoid unsupported TypeScript parameter properties in API startup · New Display authorization errors reported by failed callbacks · New Refresh session state and CSRF tokens before mutations · New Bound repository admission checks during denied sign-ins · New Cover successful callback cookie, CSRF, and redirect protections · New
Review effort: Max
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
b4b9597 to
8aa3ea2
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
|
||
| The callback consumes an attempt once, checks the same-browser binding and current provider and revocation state, exchanges the code with PKCE, and reads the authenticated GitHub profile. Hub maps GitHub's immutable numeric account ID to `ExternalIdentity.subject`. The user access token exists only while requesting that profile; Hub never stores it. | ||
|
|
||
| Hub sessions use a random opaque cookie and store only its digest. Authentication and session cookies are `HttpOnly`, `SameSite=Lax`, path-scoped, and `Secure` on HTTPS. State-changing routes require the configured origin and a matching CSRF cookie and header. Authentication routes omit request payloads from logs. |
There was a problem hiding this comment.
If you could run documentation through a doc convention pass, that would be great. Lots of AI language here that makes it hard to read.
There was a problem hiding this comment.
The directory name and function name don't line up. Let's make sure they align here and across this PR.
On an unrelated note, factory functions can get confusing to reason about, so I'd prefer simplifying by just calling these bound functions at their respective call sites. The test files for this function have also ballooned unnecessarily. I'd argue that this factory function (and others) aren't needed unless there's a clear reason we should include them.
| import { readTrustedProxyHops } from "#server/integrations/express/read-trusted-proxy-hops" | ||
| import { prisma } from "#server/integrations/prisma" | ||
|
|
||
| function readOrigin(environment: NodeJS.ProcessEnv): URL { |
There was a problem hiding this comment.
Let's extract nested helpers into their own function directories. They don't need their own corresponding test files if the parent function covers important cases.
This applies across this PR.
There was a problem hiding this comment.
This could arguably live at domains/authentication/constants.ts instead of a nested directory with nothing else in it.


Why
Hub needs a production sign-in path that preserves the current GitHub-based experience without treating GitHub identity as permission to use Hub. GitHub proves who the user is; Hub applies a separate admission policy, and repository authorization remains a separate product boundary.
What changes
Hub now supports GitHub user authorization with OAuth state, a browser binding, and PKCE. Successful authorization creates a long-lived provider-neutral Hub session, while provider credentials and source-integration credentials remain independent.
The API sets protected session and CSRF cookies, checks the Hub origin for state-changing requests, omits authentication payloads from logs, and rate-limits authentication endpoints using the configured trusted-proxy boundary. Users can sign out, require fresh verification for sensitive actions, or revoke every session. Fresh verification expires after five minutes.
Admission is an explicit policy hook. The compatibility policy can admit a GitHub user under the configured rule, but authentication does not grant repository access and does not make repository permission checks the default admission mechanism.
Storybook screenshots
Hub/Pages/Identity/Sign in / Signed out — dark theme, 1280 × 720
Hub/Pages/Identity/Sign in / Signed in — dark theme, 1280 × 720
Hub/Pages/Identity/Sign in / Unconfigured — dark theme, 1280 × 720
Hub/Pages/Identity/Sign in / Authorization failed — dark theme, 1280 × 720