feat(registry): sign-in through GitHub, GitLab and Codeberg with OAuth 2.0 and PKCE #257
argus
Loading…
Reference in a new issue
No description provided.
Delete branch "loop/224-registry-oauth"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #224
Sign-in to the registry's account page through GitHub, GitLab and Codeberg (a configurable Forgejo/Gitea), slice 2 of #16.
src/signin.trb: authorization request, code exchange (client_secret_post, PKCES256), user call; numeric id as subject, limits and deadlines on every provider answer.src/registry.trb:startSignIn,finishSignIn,pendingSignUp,signUp. The flow is consumed before any check (single-use state bound to the browser by__Host-sign-in), the redirect URI is fixed per provider, the sign-up makes account, person owner and identity in one transaction under the name and similarity rules. Linking needs a session from the last ten minutes; an identity linked elsewhere is a409; a provider moved to another instance is refused.src/account.trb: routes/account/sign-in,/account/sign-in/<provider>/callback,/account/sign-up,/account/identities/link;form-actionallows the configured providers' origins.src/config.trb:REGISTRY_PUBLIC_URL,REGISTRY_{GITHUB,GITLAB,CODEBERG}_CLIENT_ID/_SECRET,REGISTRY_GITLAB_URL,REGISTRY_CODEBERG_URL; a provider without both credentials is not offered.Test
torb test tools/registry/tests: the newtests/sign-in.test.trbruns the registry against a loopback stand-in for GitHub, GitLab and Codeberg with recorded token and user answers (no network, no credentials). Each step is tested once working and once refused: wrong state, reused state, missing flow cookie, provider mismatch, wrong PKCE verifier (GitHub-style200error and400error), provider error, broken provider, user without id, unknown provider, taken / confusing / reserved / malformed name, wrong form secret, cross-site posts, identity linked to another account, link callback without or with another session, an old session linking, an expired flow, and a moved provider instance. Includes the RFC 7636 appendix B vector.Review
The diff was security-reviewed before opening, against RFC 6749/7636/9700, injection, CSRF, mix-up and resource limits. The findings were fixed: provider instance binding, one pending sign-up per identity, recent session for linking, reserved names by skeleton, a
409on a link race, a capped login, and clearer token errors.Authorization code flow with PKCE S256 (RFC 6749, RFC 7636, RFC 9700): single-use state bound to the browser by a flow cookie, a fixed redirect URI per provider, the provider's numeric id as the identity, sign-up that chooses the owner name and makes account, owner and identity in one transaction, and linking of further providers from a recent session. Providers come from REGISTRY_{GITHUB,GITLAB,CODEBERG}_CLIENT_ID/_SECRET; Codeberg is a configurable Forgejo.181e2cc4dacd2c7f21eeCI red on
cd2c7f2:tools/release-sync and tools/registry (linux-x64),tier A (linux-x64)(andgatesas the aggregate).tools/registry/tests/sign-in.test.trbdoes not parse. Lines 65, 70, 75, 80, 85, 90: the JSON fixture bodies start with{and need the literal brace written\{. Line 574:assert !signInPage.body.contains("sourcehut")fails with "found!" (and "This value is not used").format --checkreportssign-in.test.trb: it does not parseandtools/registry/src/account.trbwould change (needstorb format).My review request stays pending; the next CI run on a new head brings the review back.
Approved at
dd70a27. CI green on this head (all required checks, incl.tools/release-sync and tools/registryandtier A); the earlier red oncd2c7f2(test parse errors, formatting) is fixed.sign-in.test.trbruns the real registry against loopback stand-ins for the three providers and covers each step working and refused (state reuse/mismatch, missing flow cookie, provider mismatch, PKCE verifier incl. RFC 7636 appendix B, provider errors/502, moved instance, taken/confusing/reserved names, CSRF, link without/with other/old session, link race).__Host-sign-in(Secure, HttpOnly, Lax); S256 PKCE only; redirect URI from config, never from the request; subject is the numeric provider id; access token used once and not stored; answers limited (256 KiB) and time-bounded (15 s); onlyhttps://provider/public URLs accepted; the sign-up needs its own cookie plus a form secret; all dynamic text escaped; CSPform-actionwidened only to the configured authorization origins. Account, owner, membership and identity are one transaction; unique-constraint races map to 409.CREATE TABLE IF NOT EXISTSonly.Non-blocking (follow-up if wanted):
tools/deploy/compose.example.ymlis described in the README as the place that sets the environment; it does not listREGISTRY_PUBLIC_URLor the provider variables. Worth adding before the first deploy that enables a provider.POST /account/sign-inhas no CSRF secret (login CSRF); the flow-cookie binding makes this harmless today, just keep it in mind if the callback ever starts to act on a pre-existing session.