From dfa3c3e8b4ad29fc9b0504483f438480b05a1f22 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 19 Oct 2026 18:21:11 +0200 Subject: [PATCH] feat(19.4-01): OIDC authorize, consent and switch account match the PHP provider - Full validation chain with PHP order and messages; repeated params last-wins - is_dev loopback port matching (matchOIDCRedirectURI); PHP http_build_query redirects, no iss - One-shot consent token burned under row lock; deny; SwitchAccount; JSON-mode ConsentRequest and DecideConsent - ClientRecord.Kind with ClientKindMCP/ClientKindOIDC; each flavour refuses the other kind at authorize, token and userinfo (D-08) - Smoke tests: start, resume/decide/switch, kind isolation, PHP encoding --- modules/wristband/README.md | 9 +- modules/wristband/authorize.go | 3 +- modules/wristband/oidc_authorize.go | 60 ++-- .../wristband/oidc_authorize_smoke_test.go | 326 ++++++++++++++++++ modules/wristband/oidc_consent.go | 156 ++++++++- modules/wristband/oidc_fakes_test.go | 3 +- modules/wristband/oidc_redirect.go | 53 +++ modules/wristband/oidc_token.go | 2 +- modules/wristband/oidc_userinfo.go | 2 +- modules/wristband/register.go | 1 + modules/wristband/stores.go | 16 + modules/wristband/token.go | 4 +- 12 files changed, 602 insertions(+), 33 deletions(-) create mode 100644 modules/wristband/oidc_authorize_smoke_test.go diff --git a/modules/wristband/README.md b/modules/wristband/README.md index 813b34b3..ebbcf461 100644 --- a/modules/wristband/README.md +++ b/modules/wristband/README.md @@ -29,6 +29,8 @@ Use cases: - Token endpoint (`wristband.Server.Token`): code exchange with PKCE verification, then an access token minted by the application plus a rotating refresh token. Reusing a spent refresh token revokes its whole lineage and the linked access tokens. Expired codes and refresh tokens are swept on each call. - Connected-app revocation: `wristband.Server.Revoke` kills an access token and the refresh lineage attached to it. - OpenID Connect provider flavour: `wristband.NewOIDCServer` builds a second `wristband.Server` on the same engine with discovery (`wristband.Server.Discovery`), a JWKS endpoint (`wristband.Server.JWKS`), authorization with a resumable request and consent (`wristband.Server.Authorize`, `wristband.Server.Resume`), a token endpoint that adds an `id_token` signed by the active key, and userinfo (`wristband.Server.UserInfo`). Storage arrives through `wristband.OIDCBackend` and `wristband.OIDCTx`, signing keys through `wristband.KeyStore`, and pages and the signed-in user through `wristband.OIDCHost`. The MCP flavour and its interfaces are unchanged. +- OIDC authorization follows the PHP provider's contract: validation in its order with its messages; unknown clients, unregistered redirect URIs and inactive clients rendered by the host (never redirected); error redirects encoded like PHP `http_build_query` without `iss`; an anonymous user sent to the login URL with only `redirect=`; silent resume when consent already covers the request, incremental consent listing only new scopes, a one-shot consent token bound to the user, deny, and `wristband.Server.SwitchAccount`. `wristband.Server.ConsentRequest` and `wristband.Server.DecideConsent` give a host's own consent page the same logic as JSON operations. Dev clients may vary the port of a loopback redirect URI. +- Client kinds: `wristband.ClientRecord.Kind` is `wristband.ClientKindMCP` (or empty) for MCP clients and `wristband.ClientKindOIDC` for administrator-registered OIDC clients. Each flavour accepts only its own kind at authorize, token and userinfo, and the MCP flavour keeps refusing the `openid` scope, so a dynamically registered client never receives identity scopes or an `id_token`. - Secret handling: client secrets, codes and refresh tokens are random base64url strings, persisted only as SHA-256 hashes and compared in constant time. `wristband.IssueClientCredentials` and `wristband.RejectRedirectURI` expose the same issuing and validation rules to operator tooling that creates clients outside registration. ## Usage @@ -95,7 +97,7 @@ func Approve(ctx context.Context, w http.ResponseWriter, srv *wristband.Server, | `wristband.DefaultOptions` | Defaults for every option except `wristband.Options.Issuer`. | | `wristband.Backend` | Runs a function inside one transaction with a `wristband.Tx`. | | `wristband.Tx` | Transaction-scoped bundle of the stores and the token issuer. | -| `wristband.ClientStore` | Persists `wristband.ClientRecord` rows: lookup, capped create, sweep, consent stamp. | +| `wristband.ClientStore` | Persists `wristband.ClientRecord` rows: lookup, capped create, sweep, consent stamp. `wristband.ClientRecord.Kind` tells MCP and OIDC clients apart. | | `wristband.AuthCodeStore` | Persists `wristband.AuthCodeRecord` rows: pending requests and issued codes. | | `wristband.RefreshTokenStore` | Persists `wristband.RefreshTokenRecord` lineage rows, including rotation and lineage revocation. | | `wristband.AccessTokenIssuer` | Mints and revokes the application's access tokens, returning a `wristband.IssuedToken`. | @@ -110,6 +112,11 @@ func Approve(ctx context.Context, w http.ResponseWriter, srv *wristband.Server, | `wristband.Server.JWKS` | Handler for `GET /oauth/jwks`: the active key plus retired keys inside their grace window. | | `wristband.Server.Resume` | Handler for `GET /oauth/authorize/resume/{request}`: silent code redirect when consent covers the request, otherwise the host's consent page. | | `wristband.Server.UserInfo` | Handler for `GET /oauth/userinfo`. | +| `wristband.Server.SwitchAccount` | Handler for `POST /oauth/authorize/switch-account`: burns the consent token, ends the session through the host and returns to the resume path. | +| `wristband.Server.ConsentRequest` | JSON-mode consent: mints a consent token and returns the `wristband.ConsentView` for a signed-in user. | +| `wristband.Server.DecideConsent` | JSON-mode decision: verifies the consent token and returns the code or `access_denied` redirect URL. | +| `wristband.ClientKindMCP` | Kind of an MCP client (`mcp`; an empty `wristband.ClientRecord.Kind` means the same). | +| `wristband.ClientKindOIDC` | Kind of an administrator-registered OIDC client (`oidc`). | | `wristband.OIDCOptions` | Login URL, lifetimes, refresh switch, advertised scopes and claims, the identity-claim allowlist, subject types and the `oas_` prefixes. | | `wristband.DefaultOIDCOptions` | The PHP-compatible defaults for `wristband.OIDCOptions`. | | `wristband.OIDCHost` | The host seam: `wristband.OIDCHost.SignedInUser`, `wristband.OIDCHost.RenderAuthorizeError`, `wristband.OIDCHost.RenderConsent`, `wristband.OIDCHost.EndSession`. | diff --git a/modules/wristband/authorize.go b/modules/wristband/authorize.go index 6e6f4c0f..5ad19c07 100644 --- a/modules/wristband/authorize.go +++ b/modules/wristband/authorize.go @@ -60,7 +60,8 @@ func (s *Server) Authorize(w http.ResponseWriter, r *http.Request) { return } } - if client == nil || client.RevokedAt != nil { + // D-08: an OpenID Connect client is unknown to the MCP flavour. + if client == nil || client.RevokedAt != nil || client.Kind == ClientKindOIDC { writeAuthorizeLocalError(w, "Unknown client.") return } diff --git a/modules/wristband/oidc_authorize.go b/modules/wristband/oidc_authorize.go index fb7a9424..77934076 100644 --- a/modules/wristband/oidc_authorize.go +++ b/modules/wristband/oidc_authorize.go @@ -57,7 +57,8 @@ type oidcAuthorizeData struct { // oidcStart handles GET /oauth/authorize in the PHP order: client, redirect // URI, active flag (all rendered), then the request parameters (redirected -// errors), then the parked request and the handoff. +// errors), then the parked request and the handoff. resource, prompt, +// max_age and login_hint are ignored, as in the PHP provider. func (s *Server) oidcStart(w http.ResponseWriter, r *http.Request) { ctx := r.Context() q := r.URL.Query() @@ -76,11 +77,11 @@ func (s *Server) oidcStart(w http.ResponseWriter, r *http.Request) { return } } - if client == nil { + if client == nil || client.Kind != ClientKindOIDC { s.renderAuthorizeError(w, r, AuthorizeErrorUnknownClient) return } - if !s.redirectMatches(client, redirectURI) { + if !matchOIDCRedirectURI(client.RedirectURIs, redirectURI, client.IsDev) { s.renderAuthorizeError(w, r, AuthorizeErrorRedirectMismatch) return } @@ -135,6 +136,12 @@ func (s *Server) validateOIDCAuthorize(q url.Values, client *OIDCClientRecord) ( if lastValue(q, "code_challenge_method") != "S256" { return oidcAuthorizeData{}, "invalid_request", "The code challenge method is required and must be S256." } + state := lastValuePtr(q, "state") + nonce := lastValuePtr(q, "nonce") + if (state != nil && utf8.RuneCountInString(*state) > maxOpaqueLength) || + (nonce != nil && utf8.RuneCountInString(*nonce) > maxOpaqueLength) { + return oidcAuthorizeData{}, "invalid_request", "The state and nonce parameters must be strings of at most 255 characters." + } scopeRaw := lastValue(q, "scope") if strings.TrimSpace(scopeRaw) == "" || utf8.RuneCountInString(scopeRaw) > maxOpaqueLength { return oidcAuthorizeData{}, "invalid_scope", "A scope parameter is required." @@ -143,14 +150,39 @@ func (s *Server) validateOIDCAuthorize(q url.Values, client *OIDCClientRecord) ( if !stringSliceContains(requested, "openid") { return oidcAuthorizeData{}, "invalid_scope", "The scope parameter must include openid." } + supported := s.oidcSupportedScopes() + for _, sc := range requested { + if !stringSliceContains(supported, sc) { + return oidcAuthorizeData{}, "invalid_scope", "One or more requested scopes are not supported." + } + } + allowed := clientAllowedScopes(client) + for _, sc := range requested { + if !stringSliceContains(allowed, sc) { + return oidcAuthorizeData{}, "invalid_scope", "One or more requested scopes are not allowed for this client." + } + } return oidcAuthorizeData{ - Scopes: intersectScopes(requested, client.Scopes, s.oidcSupportedScopes()), - State: lastValuePtr(q, "state"), - Nonce: lastValuePtr(q, "nonce"), + Scopes: intersectScopes(requested, allowed, supported), + State: state, + Nonce: nonce, Challenge: challenge, }, "", "" } +// clientAllowedScopes is the client's non-empty scopes; offline_access +// additionally needs OfflineAccess on the client (D-17). +func clientAllowedScopes(client *OIDCClientRecord) []string { + out := []string{} + for _, sc := range client.Scopes { + if sc == "" || (sc == "offline_access" && !client.OfflineAccess) { + continue + } + out = append(out, sc) + } + return out +} + // Resume handles GET /oauth/authorize/resume/{request} on an OIDC server // (404 on an MCP server). The request ID comes from r.PathValue("request"). // A missing, expired or consumed request is rendered as expired_request; an @@ -275,7 +307,7 @@ func (s *Server) usableClient(ctx context.Context, tx OIDCTx, clientID string) ( if err != nil { return nil, "", err } - if client == nil { + if client == nil || client.Kind != ClientKindOIDC { return nil, AuthorizeErrorUnknownClient, nil } if !client.IsActive { @@ -314,20 +346,6 @@ func (s *Server) signedInUser(r *http.Request) (uint, bool) { return id, true } -// redirectMatches reports whether the presented redirect URI is registered -// for the client byte for byte. -func (s *Server) redirectMatches(client *OIDCClientRecord, presented string) bool { - if presented == "" { - return false - } - for _, uri := range client.RedirectURIs { - if uri != "" && constantEqual(uri, presented) { - return true - } - } - return false -} - // loginHandoffURL is rtrim(LoginURL, "/") plus a single redirect parameter // carrying the resume path (D-04): the OIDC query never travels through the // login page. diff --git a/modules/wristband/oidc_authorize_smoke_test.go b/modules/wristband/oidc_authorize_smoke_test.go new file mode 100644 index 00000000..fcd99db7 --- /dev/null +++ b/modules/wristband/oidc_authorize_smoke_test.go @@ -0,0 +1,326 @@ +package wristband + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// oidcAuthorizeParams is a valid authorization request for the fixture's +// client; mutate adjusts it per case. +func oidcAuthorizeParams(f *oidcFixture, mutate func(url.Values)) url.Values { + q := url.Values{ + "response_type": {"code"}, + "client_id": {f.client.ClientID}, + "redirect_uri": {testOIDCRedirect}, + "scope": {"openid profile"}, + "state": {"st-1"}, + "nonce": {"n-1"}, + "code_challenge": {s256Challenge(oidcTestVerifier)}, + "code_challenge_method": {"S256"}, + } + if mutate != nil { + mutate(q) + } + return q +} + +// oidcGet serves one GET through the fixture's routes. +func (f *oidcFixture) get(target string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + f.mux().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, target, nil)) + return rec +} + +// post serves one urlencoded POST through the fixture's routes. +func (f *oidcFixture) post(target string, form url.Values) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodPost, target, strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + f.mux().ServeHTTP(rec, req) + return rec +} + +// startAndResume starts an authorization request as the signed-in user and +// follows the resume redirect. +func (f *oidcFixture) startAndResume(t *testing.T, q url.Values) *httptest.ResponseRecorder { + t.Helper() + start := f.get("/oauth/authorize?" + q.Encode()) + loc := start.Header().Get("Location") + if start.Code != http.StatusFound || !strings.HasPrefix(loc, resumePathPrefix) { + t.Fatalf("start = %d %q %s", start.Code, loc, start.Body.String()) + } + return f.get(loc) +} + +func assertRendered(t *testing.T, rec *httptest.ResponseRecorder, code string) { + t.Helper() + if rec.Code != http.StatusBadRequest || rec.Body.String() != code { + t.Fatalf("want rendered %s, got %d %q", code, rec.Code, rec.Body.String()) + } + if rec.Header().Get("Location") != "" { + t.Fatalf("rendered error must not redirect, Location = %q", rec.Header().Get("Location")) + } + for k, v := range map[string]string{ + "X-Frame-Options": "DENY", + "Content-Security-Policy": "frame-ancestors 'none'", + "Cache-Control": "no-store, private", + "Referrer-Policy": "no-referrer", + } { + if got := rec.Header().Get(k); got != v { + t.Fatalf("%s = %q, want %q", k, got, v) + } + } +} + +func TestOIDCAuthorizeStartSmoke(t *testing.T) { + f := newOIDCFixture(t, testOIDCIssuer, nil) + + // Unknown client and redirect mismatch are rendered, never redirected. + assertRendered(t, f.get("/oauth/authorize?"+oidcAuthorizeParams(f, func(q url.Values) { q.Set("client_id", "oas_nope") }).Encode()), AuthorizeErrorUnknownClient) + assertRendered(t, f.get("/oauth/authorize?"+oidcAuthorizeParams(f, func(q url.Values) { q.Set("redirect_uri", testOIDCRedirect+"/") }).Encode()), AuthorizeErrorRedirectMismatch) + + // Missing PKCE redirects with invalid_request and the state. + rec := f.get("/oauth/authorize?" + oidcAuthorizeParams(f, func(q url.Values) { q.Del("code_challenge") }).Encode()) + want := testOIDCRedirect + "?error=invalid_request&error_description=A+PKCE+code+challenge+is+required+and+must+be+a+base64url-encoded+SHA-256+digest.&state=st-1" + if rec.Code != http.StatusFound || rec.Header().Get("Location") != want { + t.Fatalf("missing PKCE = %d %q", rec.Code, rec.Header().Get("Location")) + } + if rec.Header().Get("Cache-Control") != "no-store, private" { + t.Fatalf("Cache-Control = %q", rec.Header().Get("Cache-Control")) + } + + // Scope rules in PHP order. + for _, tc := range []struct { + scope, desc string + }{ + {"profile", "The+scope+parameter+must+include+openid."}, + {"openid phone", "One+or+more+requested+scopes+are+not+supported."}, + {"openid offline_access", "One+or+more+requested+scopes+are+not+supported."}, + } { + rec := f.get("/oauth/authorize?" + oidcAuthorizeParams(f, func(q url.Values) { q.Set("scope", tc.scope) }).Encode()) + if !strings.Contains(rec.Header().Get("Location"), "error=invalid_scope&error_description="+tc.desc) { + t.Fatalf("scope %q: Location = %q", tc.scope, rec.Header().Get("Location")) + } + } + + // Anonymous user goes to the login URL carrying only redirect=. + rec = f.get("/oauth/authorize?" + oidcAuthorizeParams(f, nil).Encode()) + loc := rec.Header().Get("Location") + if rec.Code != http.StatusFound || !strings.HasPrefix(loc, "/login?redirect=%2Foauth%2Fauthorize%2Fresume%2F") { + t.Fatalf("anonymous = %d %q", rec.Code, loc) + } + u, _ := url.Parse(loc) + if len(u.Query()) != 1 { + t.Fatalf("login URL must carry only redirect=, got %q", loc) + } + + // Last value of a repeated parameter wins (PHP parse_str). + q := oidcAuthorizeParams(f, nil) + q["state"] = []string{"first", "last"} + q["code_challenge"] = []string{"short", s256Challenge(oidcTestVerifier)} + f.host.signIn(7) + if rec := f.startAndResume(t, q); rec.Code != http.StatusOK { + t.Fatalf("repeated params = %d", rec.Code) + } +} + +func TestOIDCResumeDecideSwitchSmoke(t *testing.T) { + f := newOIDCFixture(t, testOIDCIssuer, nil) + f.host.signIn(7) + + // First run: consent for openid profile. + if rec := f.startAndResume(t, oidcAuthorizeParams(f, nil)); rec.Code != http.StatusOK { + t.Fatalf("first resume = %d", rec.Code) + } + view := f.host.lastView(t) + if strings.Join(view.Scopes, " ") != "profile" || view.Incremental { + t.Fatalf("first view = %+v", view) + } + allow := url.Values{"request_id": {view.RequestID}, "consent_token": {view.ConsentToken}, "decision": {"allow"}} + rec := f.post("/oauth/authorize", allow) + if rec.Code != http.StatusFound || !strings.Contains(rec.Header().Get("Location"), "?code=oas_ac_") { + t.Fatalf("allow = %d %q", rec.Code, rec.Header().Get("Location")) + } + + // The consent token was one-shot. + assertRendered(t, f.post("/oauth/authorize", allow), AuthorizeErrorExpiredRequest) + + // Second run with the same scopes is silent: resume redirects with a code. + views := len(f.host.views) + rec = f.startAndResume(t, oidcAuthorizeParams(f, nil)) + if rec.Code != http.StatusFound || !strings.HasPrefix(rec.Header().Get("Location"), testOIDCRedirect+"?code=oas_ac_") || len(f.host.views) != views { + t.Fatalf("silent resume = %d %q", rec.Code, rec.Header().Get("Location")) + } + + // Incremental consent lists only the added scope. + if rec := f.startAndResume(t, oidcAuthorizeParams(f, func(q url.Values) { q.Set("scope", "openid profile email") })); rec.Code != http.StatusOK { + t.Fatalf("incremental resume = %d", rec.Code) + } + view = f.host.lastView(t) + if strings.Join(view.Scopes, " ") != "email" || !view.Incremental { + t.Fatalf("incremental view = %+v", view) + } + + // A token minted for user 7 does not work for user 8. + f.host.signIn(8) + assertRendered(t, f.post("/oauth/authorize", url.Values{"request_id": {view.RequestID}, "consent_token": {view.ConsentToken}, "decision": {"allow"}}), AuthorizeErrorExpiredRequest) + + // Switch account: user 8 gets a consent page, switches, and the request + // is still answerable for the next user. + if rec := f.get(resumePathPrefix + view.RequestID); rec.Code != http.StatusOK { + t.Fatalf("resume as user 8 = %d", rec.Code) + } + view = f.host.lastView(t) + rec = f.post("/oauth/authorize/switch-account", url.Values{"request_id": {view.RequestID}, "consent_token": {view.ConsentToken}}) + if rec.Code != http.StatusFound || rec.Header().Get("Location") != resumePathPrefix+view.RequestID || f.host.endSessions != 1 { + t.Fatalf("switch = %d %q, endSessions %d", rec.Code, rec.Header().Get("Location"), f.host.endSessions) + } + if rec := f.get(resumePathPrefix + view.RequestID); rec.Code != http.StatusFound || !strings.HasPrefix(rec.Header().Get("Location"), "/login?redirect=") { + t.Fatalf("resume after switch = %d %q", rec.Code, rec.Header().Get("Location")) + } + f.host.signIn(9) + if rec := f.get(resumePathPrefix + view.RequestID); rec.Code != http.StatusOK { + t.Fatalf("resume as user 9 = %d", rec.Code) + } + + // Deny consumes the request and redirects with access_denied only. + view = f.host.lastView(t) + rec = f.post("/oauth/authorize", url.Values{"request_id": {view.RequestID}, "consent_token": {view.ConsentToken}, "decision": {"no"}}) + if rec.Header().Get("Location") != testOIDCRedirect+"?error=access_denied&state=st-1" { + t.Fatalf("deny = %q", rec.Header().Get("Location")) + } + assertRendered(t, f.get(resumePathPrefix+view.RequestID), AuthorizeErrorExpiredRequest) +} + +func TestKindIsolationSmoke(t *testing.T) { + // An oidc-kind client is unknown at MCP authorize and invalid_client at + // MCP token. + backend := newMemoryBackend() + opts := DefaultOptions() + opts.Issuer = testOIDCIssuer + mcp := NewServer(opts) + mcp.SetBackend(backend) + secret := "mcp-secret" + h := sha256Hex(secret) + oidcClient := insertTokenTestClient(backend, "oidc-kind-client", "client_secret_post", &h, nil) + oidcClient.Kind = ClientKindOIDC + mcpClient := insertAuthorizeTestClient(backend, "mcp-kind-client", []string{"https://client.example.test/cb"}, nil) + _, challenge := s256Pair(t) + + rec := httptest.NewRecorder() + mcp.Authorize(rec, authorizeRequest(map[string]string{ + "client_id": oidcClient.ClientID, "redirect_uri": tokenTestRedirect, "response_type": "code", + "code_challenge": challenge, "code_challenge_method": "S256", + })) + if rec.Code != http.StatusBadRequest || rec.Body.String() != "Unknown client." { + t.Fatalf("MCP authorize with an oidc client = %d %q", rec.Code, rec.Body.String()) + } + rec = httptest.NewRecorder() + mcp.Token(rec, tokenRequest(url.Values{ + "grant_type": {"authorization_code"}, "client_id": {oidcClient.ClientID}, "client_secret": {secret}, + "code": {"x"}, "redirect_uri": {tokenTestRedirect}, "code_verifier": {"y"}, + }, "")) + if rec.Code != http.StatusUnauthorized || !strings.Contains(rec.Body.String(), "invalid_client") { + t.Fatalf("MCP token with an oidc client = %d %s", rec.Code, rec.Body.String()) + } + + // MCP authorize still refuses openid (D-07). + rec = httptest.NewRecorder() + mcp.Authorize(rec, authorizeRequest(map[string]string{ + "client_id": mcpClient.ClientID, "redirect_uri": "https://client.example.test/cb", "response_type": "code", + "code_challenge": challenge, "code_challenge_method": "S256", "scope": "openid read", + })) + if rec.Code != http.StatusFound || queryOf(t, rec.Header().Get("Location"))["error"] != "invalid_scope" { + t.Fatalf("MCP authorize with openid = %d %q", rec.Code, rec.Header().Get("Location")) + } + + // An mcp-kind client is unknown_client at OIDC authorize and + // invalid_client at the OIDC token endpoint. + f := newOIDCFixture(t, testOIDCIssuer, nil) + mcpInOIDC := f.backend.addClient(OIDCClientRecord{ + ClientID: "oas_mcpkind", Kind: ClientKindMCP, RedirectURIs: []string{testOIDCRedirect}, + Scopes: []string{"openid"}, IsActive: true, + }, testOIDCSecret) + assertRendered(t, f.get("/oauth/authorize?"+oidcAuthorizeParams(f, func(q url.Values) { q.Set("client_id", mcpInOIDC.ClientID) }).Encode()), AuthorizeErrorUnknownClient) + rec = f.post("/oauth/token", url.Values{ + "grant_type": {"authorization_code"}, "client_id": {mcpInOIDC.ClientID}, "client_secret": {testOIDCSecret}, + "code": {"oas_ac_x"}, "redirect_uri": {testOIDCRedirect}, "code_verifier": {oidcTestVerifier}, + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("OIDC token with an mcp client = %d %s", rec.Code, rec.Body.String()) + } + + // Each flavour's own handlers answer 404 on the other flavour. + for name, h := range map[string]http.HandlerFunc{"Metadata": f.srv.Metadata, "Register": f.srv.Register} { + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusNotFound { + t.Fatalf("%s on OIDC = %d", name, rec.Code) + } + } + for name, h := range map[string]http.HandlerFunc{"Discovery": mcp.Discovery, "JWKS": mcp.JWKS, "UserInfo": mcp.UserInfo, "Resume": mcp.Resume, "SwitchAccount": mcp.SwitchAccount} { + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusNotFound { + t.Fatalf("%s on MCP = %d", name, rec.Code) + } + } + if err := mcp.SetOIDCBackend(f.backend, f.keys, f.host); err == nil { + t.Fatal("SetOIDCBackend on an MCP server must fail") + } + if err := f.srv.SetOIDCBackend(nil, f.keys, f.host); err == nil { + t.Fatal("SetOIDCBackend with a nil backend must fail") + } +} + +func TestPHPRedirectEncodingSmoke(t *testing.T) { + f := newOIDCFixture(t, testOIDCIssuer, nil) + state := "a b~c+d/é" + rec := f.get("/oauth/authorize?" + oidcAuthorizeParams(f, func(q url.Values) { + q.Set("state", state) + q.Set("response_type", "token") + }).Encode()) + loc := rec.Header().Get("Location") + if !strings.HasSuffix(loc, "&state=a+b%7Ec%2Bd%2F%C3%A9") { + t.Fatalf("Location = %q", loc) + } + if strings.Contains(loc, "iss=") { + t.Fatalf("OIDC redirects carry no iss: %q", loc) + } + if got := phpURLEncode("A-z_0.9 ~*'()!"); got != "A-z_0.9+%7E%2A%27%28%29%21" { + t.Fatalf("phpURLEncode = %q", got) + } + // An empty state is dropped, an empty description is dropped. + if got := errorRedirectURL("https://rp.example.test/cb?x=1", "access_denied", "", ""); got != "https://rp.example.test/cb?x=1&error=access_denied" { + t.Fatalf("errorRedirectURL = %q", got) + } + + // is_dev loopback port variants only. + reg := []string{"http://127.0.0.1:8080/cb", "http://localhost/cb?x=1", "https://rp.example.test/cb"} + for _, tc := range []struct { + uri string + isDev bool + want bool + }{ + {"http://127.0.0.1:9999/cb", true, true}, + {"http://127.0.0.1:9999/cb", false, false}, + {"http://localhost:3000/cb?x=1", true, true}, + {"http://localhost:3000/cb?x=2", true, false}, + {"http://localhost:3000/cb/", true, false}, + {"https://127.0.0.1:9999/cb", true, false}, + {"//127.0.0.1:9999/cb", true, false}, + {"http://127.0.0.2:8080/cb", true, false}, + {"https://rp.example.test/cb", false, true}, + {"https://rp.example.test.evil/cb", true, false}, + } { + if got := matchOIDCRedirectURI(reg, tc.uri, tc.isDev); got != tc.want { + t.Fatalf("match(%q, dev=%v) = %v", tc.uri, tc.isDev, got) + } + } + if matchOIDCRedirectURI(nil, "https://rp.example.test/cb", true) { + t.Fatal("no registered URIs must match nothing") + } +} diff --git a/modules/wristband/oidc_consent.go b/modules/wristband/oidc_consent.go index ab749dd5..12217710 100644 --- a/modules/wristband/oidc_consent.go +++ b/modules/wristband/oidc_consent.go @@ -5,6 +5,8 @@ package wristband import ( "context" + "errors" + "net/http" "strconv" ) @@ -70,14 +72,14 @@ func (s *Server) decideConsent(ctx context.Context, requestID string, userID uin failure = AuthorizeErrorExpiredRequest return nil } - if req.ConsentTokenHash == nil || *req.ConsentTokenHash == "" || token == "" || - !constantEqual(*req.ConsentTokenHash, consentDigest(token, userID)) { + ok, err := burnConsentToken(ctx, tx, req, userID, token) + if err != nil { + return err + } + if !ok { failure = AuthorizeErrorExpiredRequest return nil } - if err := tx.SetConsentTokenHash(ctx, req.ID, nil); err != nil { - return err - } client, code, err := s.usableClient(ctx, tx, req.ClientID) if err != nil { return err @@ -98,7 +100,7 @@ func (s *Server) decideConsent(ctx context.Context, requestID string, userID uin target = errorRedirectURL(req.RedirectURI, "access_denied", "", state) return nil } - granted := intersectScopes(req.Scopes, client.Scopes, s.oidcSupportedScopes()) + granted := intersectScopes(req.Scopes, clientAllowedScopes(client), s.oidcSupportedScopes()) consumed, err := tx.ConsumeAuthRequest(ctx, req.ID, now) if err != nil { return err @@ -125,3 +127,145 @@ func (s *Server) decideConsent(ctx context.Context, requestID string, userID uin } return target, nil } + +// burnConsentToken verifies the presented consent token against the +// request's digest in constant time and clears the digest on success, so a +// token works once (PHP ConsentCsrf::verify). The caller read req under a +// row lock. +func burnConsentToken(ctx context.Context, tx OIDCTx, req *AuthRequestRecord, userID uint, token string) (bool, error) { + if req.ConsentTokenHash == nil || *req.ConsentTokenHash == "" || token == "" { + return false, nil + } + if !constantEqual(*req.ConsentTokenHash, consentDigest(token, userID)) { + return false, nil + } + if err := tx.SetConsentTokenHash(ctx, req.ID, nil); err != nil { + return false, err + } + return true, nil +} + +// ConsentRequest is the JSON-mode counterpart of Resume for a host whose +// consent page lives in its own frontend (D-03): for the signed-in userID +// it allocates the subject, mints a fresh one-shot consent token and +// returns the view listing the scopes not consented to yet. It never issues +// a code silently; Resume does that before the host's page is reached. A +// missing, expired or consumed request, or a gone or inactive client, is +// ErrAuthRequestExpired. +func (s *Server) ConsentRequest(ctx context.Context, requestID string, userID uint) (ConsentView, error) { + if !s.oidcReady() { + return ConsentView{}, errors.New("wristband: OIDC backend is not configured") + } + req, err := s.loadAuthRequest(ctx, requestID) + if err != nil { + return ConsentView{}, err + } + if !s.authRequestUsable(req) { + return ConsentView{}, ErrAuthRequestExpired + } + var view ConsentView + var failed bool + err = s.oidcBackend.WithinOIDCTx(ctx, func(tx OIDCTx) error { + client, code, err := s.usableClient(ctx, tx, req.ClientID) + if err != nil { + return err + } + if code != "" { + failed = true + return nil + } + if _, err := subjectFor(ctx, tx, client, userID); err != nil { + return err + } + consent, err := tx.ConsentFor(ctx, userID, req.ClientID) + if err != nil { + return err + } + view, err = s.prepareConsent(ctx, tx, req, client, userID, consent) + return err + }) + if err != nil { + return ConsentView{}, err + } + if failed { + return ConsentView{}, ErrAuthRequestExpired + } + return view, nil +} + +// DecideConsent is the JSON-mode counterpart of the consent form POST: it +// verifies and burns consentToken for userID, then denies (the +// access_denied redirect URL) or allows (consent merged, code minted, the +// code redirect URL). Every request or token failure is +// ErrAuthRequestExpired (T-19.4-04). +func (s *Server) DecideConsent(ctx context.Context, requestID string, userID uint, consentToken string, allow bool) (string, error) { + if !s.oidcReady() { + return "", errors.New("wristband: OIDC backend is not configured") + } + target, err := s.decideConsent(ctx, requestID, userID, consentToken, allow) + var failure errAuthorizeRender + if errors.As(err, &failure) { + return "", ErrAuthRequestExpired + } + return target, err +} + +// SwitchAccount handles POST /oauth/authorize/switch-account on an OIDC +// server (404 on an MCP server): request_id and consent_token from the +// form. It verifies and burns the consent token for the signed-in user +// (any failure renders expired_request), asks the host to end the session +// and sends the browser back to the resume path, where the next user signs +// in. The request is not consumed. Without a signed-in user it goes +// straight back to the resume path. +func (s *Server) SwitchAccount(w http.ResponseWriter, r *http.Request) { + if s.flavour != flavourOIDC { + w.WriteHeader(http.StatusNotFound) + return + } + if !s.oidcReady() { + w.WriteHeader(http.StatusInternalServerError) + return + } + _ = r.ParseForm() + ctx := r.Context() + req, err := s.loadAuthRequest(ctx, inputValue(r, "request_id")) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + if !s.authRequestUsable(req) { + s.renderAuthorizeError(w, r, AuthorizeErrorExpiredRequest) + return + } + resume := resumePathPrefix + req.RequestID + userID, ok := s.signedInUser(r) + if !ok { + s.redirectNoStore(w, resume) + return + } + token := inputValue(r, "consent_token") + var verified bool + err = s.oidcBackend.WithinOIDCTx(ctx, func(tx OIDCTx) error { + locked, err := tx.AuthRequestByRequestIDForUpdate(ctx, req.RequestID) + if err != nil { + return err + } + if !s.authRequestUsable(locked) { + return nil + } + verified, err = burnConsentToken(ctx, tx, locked, userID, token) + return err + }) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + if !verified { + s.renderAuthorizeError(w, r, AuthorizeErrorExpiredRequest) + return + } + // Ending the session is best effort, as in the PHP provider: the + // browser returns to the resume path either way. + _ = s.host.EndSession(w, r) + s.redirectNoStore(w, resume) +} diff --git a/modules/wristband/oidc_fakes_test.go b/modules/wristband/oidc_fakes_test.go index aa4ef605..8e432385 100644 --- a/modules/wristband/oidc_fakes_test.go +++ b/modules/wristband/oidc_fakes_test.go @@ -554,7 +554,7 @@ func newOIDCFixture(t *testing.T, issuer string, mutate func(*OIDCOptions)) *oid f.client = f.backend.addClient(OIDCClientRecord{ ClientID: "oas_0123456789abcdef01234567", Name: "Example RP", - Kind: "oidc", + Kind: ClientKindOIDC, RedirectURIs: []string{testOIDCRedirect}, Scopes: []string{"openid", "profile", "email"}, IsActive: true, @@ -576,6 +576,7 @@ func (f *oidcFixture) mux() *http.ServeMux { mux.HandleFunc("GET /oauth/authorize", f.srv.Authorize) mux.HandleFunc("POST /oauth/authorize", f.srv.Authorize) mux.HandleFunc("GET /oauth/authorize/resume/{request}", f.srv.Resume) + mux.HandleFunc("POST /oauth/authorize/switch-account", f.srv.SwitchAccount) mux.HandleFunc("POST /oauth/token", f.srv.Token) mux.HandleFunc("GET /oauth/userinfo", f.srv.UserInfo) return mux diff --git a/modules/wristband/oidc_redirect.go b/modules/wristband/oidc_redirect.go index 0dd66ee9..4a833a3b 100644 --- a/modules/wristband/oidc_redirect.go +++ b/modules/wristband/oidc_redirect.go @@ -2,6 +2,7 @@ package wristband import ( "fmt" + "net/url" "strings" ) @@ -77,3 +78,55 @@ func unhex(c byte) byte { return c - 'A' + 10 } } + +// loopbackHosts are the hosts an is_dev client may vary the port on. +var loopbackHosts = map[string]bool{"localhost": true, "127.0.0.1": true, "::1": true} + +// matchOIDCRedirectURI ports RedirectUriMatcher::matches (T-19.4-01): the +// presented URI matches a registered one byte for byte. Only for an is_dev +// client, an http URI on localhost, 127.0.0.1 or [::1] also matches a +// registered URI that differs only in the port (scheme, user info, host, +// path, query and fragment identical). No registered URIs match nothing; +// scheme-relative, https-versus-http, trailing-slash and lookalike-host +// variants never match. +func matchOIDCRedirectURI(registered []string, presented string, isDev bool) bool { + if presented == "" { + return false + } + for _, uri := range registered { + if uri != "" && constantEqual(uri, presented) { + return true + } + } + if !isDev { + return false + } + for _, uri := range registered { + if uri != "" && isLoopbackPortVariant(uri, presented) { + return true + } + } + return false +} + +func isLoopbackPortVariant(registered, presented string) bool { + a, err := url.Parse(registered) + if err != nil { + return false + } + b, err := url.Parse(presented) + if err != nil { + return false + } + if a.Scheme != "http" || b.Scheme != "http" || a.Opaque != "" || b.Opaque != "" { + return false + } + host := a.Hostname() + if host == "" || host != b.Hostname() || !loopbackHosts[host] { + return false + } + return a.User.String() == b.User.String() && + a.EscapedPath() == b.EscapedPath() && + a.RawQuery == b.RawQuery && a.ForceQuery == b.ForceQuery && + a.EscapedFragment() == b.EscapedFragment() +} diff --git a/modules/wristband/oidc_token.go b/modules/wristband/oidc_token.go index 5573cb61..83283775 100644 --- a/modules/wristband/oidc_token.go +++ b/modules/wristband/oidc_token.go @@ -153,7 +153,7 @@ func (s *Server) authenticateOIDCClient(ctx context.Context, clientID, secret st constantEqual(dummySecretHash, sha256Hex(secret)) return nil, nil } - if !client.IsActive || !s.verifyClientSecret(client, secret) { + if client.Kind != ClientKindOIDC || !client.IsActive || !s.verifyClientSecret(client, secret) { return nil, nil } return client, nil diff --git a/modules/wristband/oidc_userinfo.go b/modules/wristband/oidc_userinfo.go index 55201654..29fab2db 100644 --- a/modules/wristband/oidc_userinfo.go +++ b/modules/wristband/oidc_userinfo.go @@ -44,7 +44,7 @@ func (s *Server) UserInfo(w http.ResponseWriter, r *http.Request) { if err != nil { return err } - if client == nil || !client.IsActive { + if client == nil || client.Kind != ClientKindOIDC || !client.IsActive { return nil } identity, ok, err := tx.IdentityClaims(ctx, tok.UserID, tok.Scopes) diff --git a/modules/wristband/register.go b/modules/wristband/register.go index 2fdafaba..761179fa 100644 --- a/modules/wristband/register.go +++ b/modules/wristband/register.go @@ -140,6 +140,7 @@ func (s *Server) Register(w http.ResponseWriter, r *http.Request) { GrantTypes: grants, TokenEndpointAuthMethod: authMethod, RegistrationIP: &ip, + Kind: ClientKindMCP, } ctx := r.Context() diff --git a/modules/wristband/stores.go b/modules/wristband/stores.go index f737eed6..6150ac35 100644 --- a/modules/wristband/stores.go +++ b/modules/wristband/stores.go @@ -28,8 +28,24 @@ type ClientRecord struct { RevokedAt *time.Time ScopeCeiling []string // nil/empty means no ceiling CreatedAt time.Time + // Kind is "" or ClientKindMCP for an MCP client (dynamically registered + // or issued by an operator command) and ClientKindOIDC for an + // administrator-registered OpenID Connect client. The MCP flavour + // treats an OIDC client as unknown (D-08, T-19.4-05). + Kind string } +// The client kinds (D-08). Each Server flavour accepts only its own kind at +// authorize, token and userinfo, so dynamic registration can never produce +// a client that receives OpenID Connect identity scopes. +const ( + // ClientKindMCP marks an MCP client. An empty Kind means the same. + ClientKindMCP = "mcp" + // ClientKindOIDC marks an administrator-registered OpenID Connect + // client. + ClientKindOIDC = "oidc" +) + // AuthCodeRecord is the app-agnostic persisted shape of a pending or issued // authorization row. Its full lifecycle (issue, exchange, replay) belongs to // a later Phase 8 plan (08-03/08-04); this plan only needs the shape and the diff --git a/modules/wristband/token.go b/modules/wristband/token.go index b498a29b..2e597c84 100644 --- a/modules/wristband/token.go +++ b/modules/wristband/token.go @@ -192,7 +192,9 @@ func (s *Server) authenticateClient(ctx context.Context, r *http.Request) (*Clie if err != nil { return nil, err } - if client == nil || client.RevokedAt != nil { + // D-08: an OpenID Connect client never authenticates at the MCP token + // endpoint. + if client == nil || client.RevokedAt != nil || client.Kind == ClientKindOIDC { return nil, errInvalidClient } if client.TokenEndpointAuthMethod == "none" {