diff --git a/docs/services/oauth-server.md b/docs/services/oauth-server.md index efac69bd..a2a75d6c 100644 --- a/docs/services/oauth-server.md +++ b/docs/services/oauth-server.md @@ -39,7 +39,7 @@ func newServer() *wristband.Server { > [!WARNING] > Always set `wristband.Options.Resource` for your deployment. `wristband.DefaultOptions` leaves it empty, and while it is empty every authorization request that sends a `resource` parameter is refused with `invalid_target`. -The defaults cover the rest: pending requests and codes live 10 minutes, access tokens 1 hour and refresh tokens 30 days, at most 200 unrevoked clients may register, and registration bodies are capped at 64 KiB. `wristband.Options.ScopesSupported` is both the advertised list and the authorize allow-list, one resolved slice. A non-empty list is used as given: the example above drops `ai`, so a request for it is `invalid_scope`, and `wristband.DefaultOptions` (`read`, `write`, `ai`, `offline_access`) still refuses `backend`. An empty or nil list on the MCP flavour resolves to `read`, `write`, `ai`, `offline_access` and `backend`. OpenID Connect discovery stays `openid`, `profile` and `email`. +The defaults cover the rest: pending requests and codes live 10 minutes, access tokens 1 hour and refresh tokens 30 days, at most 200 unrevoked clients may register, and registration bodies are capped at 64 KiB. `wristband.Options.ScopesSupported` is both the advertised list and the authorize allow-list, one resolved slice. A non-empty list is used as given: the example above drops `ai`, so a request for it is `invalid_scope`, and `wristband.DefaultOptions` (`read`, `write`, `ai`, `offline_access`) still refuses `backend`. The OpenID Connect identity scopes `openid`, `profile` and `email` are dropped from the request before the allow-list check (RFC 6749 section 3.3 lets a server grant fewer scopes than requested), so a client that also reads the issuer's OpenID Connect discovery document still connects; a request with nothing else left gets the default `read`, and the three are never advertised. An empty or nil list on the MCP flavour resolves to `read`, `write`, `ai`, `offline_access` and `backend`. OpenID Connect discovery stays `openid`, `profile` and `email`. The MCP advertised set is that resolved list. The fixed names `read`, `write`, `ai`, `offline_access` and `backend` are always on it. Declared member scopes (`pact.FrontendTokenScope`, including `pact.FrontendTokenScope.Implies`) come from the published `pact.MemberTokenScopeCatalog` and are filtered by the OAuth server plugin's key `golem15.oauthserver.mcp_scopes`. That key is the plugin's, not a framework config key: unset means every declared member scope, a set filter means only those declared names, and an empty filter means the five fixed names only. The plugin publishes the list as `pact.MCPAdvertisedScopes`. pier copies `pact.MCPAdvertisedScopes.MCPScopesSupported` when it is non-empty and otherwise keeps `pier.PRMScopes`. The read/write ceiling is unchanged. A route the endpoint runs reads the caller with `pact.MCPCallFrom`: `pact.MCPSubject.TokenID` is a member personal-token id only when `pact.MCPSubject.ClientID` is empty, and the OAuth access-token row otherwise, and `pact.MCPSubject.Scopes` is the grant, not an implied expansion. The metadata document serves the configured values: @@ -214,7 +214,7 @@ Before either render method runs, the engine sets `X-Frame-Options: DENY`, `Cont ### Client kinds -`wristband.ClientRecord.Kind` and `wristband.OIDCClientRecord.Kind` tell the two kinds of client apart: `wristband.ClientKindMCP` for MCP clients (dynamically registered or issued by an operator command; an empty kind means the same) and `wristband.ClientKindOIDC` for clients an administrator registers. Each server accepts only its own kind at authorize, token and userinfo, and the MCP server keeps refusing the `openid` scope, so a dynamically registered client can never obtain identity scopes or an `id_token`. +`wristband.ClientRecord.Kind` and `wristband.OIDCClientRecord.Kind` tell the two kinds of client apart: `wristband.ClientKindMCP` for MCP clients (dynamically registered or issued by an operator command; an empty kind means the same) and `wristband.ClientKindOIDC` for clients an administrator registers. Each server accepts only its own kind at authorize, token and userinfo, and the MCP server drops `openid`, `profile` and `email` at authorize and never issues an `id_token`, so a dynamically registered client can never obtain identity scopes or an `id_token`. ### Routes diff --git a/modules/wristband/README.md b/modules/wristband/README.md index 1aef7562..ee3c3e5c 100644 --- a/modules/wristband/README.md +++ b/modules/wristband/README.md @@ -27,7 +27,7 @@ Use cases: - RFC 8414 metadata document (`wristband.Server.Metadata`) advertising the authorize, token and registration endpoints under the issuer, the `authorization_code` and `refresh_token` grants, S256 PKCE and the RFC 9207 `iss` response parameter. - RFC 7591 dynamic client registration (`wristband.Server.Register`): JSON only, a bounded request body, redirect URI validation (HTTPS, or loopback HTTP), public (`none`) and confidential (`client_secret_post`, `client_secret_basic`) clients, a cap on unrevoked clients and a sweep of old clients that never got consent, all in one transaction. -- Authorization endpoint (`wristband.Server.Authorize`): the client and its exact registered redirect URI are validated before any redirect is sent (an unknown client gets a local plain-text 400, never an open redirect); then S256 PKCE, the client's scope ceiling and the RFC 8707 `resource` value are checked, a pending request is stored and the browser is sent to the application's consent page at `/connect?request=`. The accepted scopes are the same resolved list `wristband.Server.Metadata` advertises. A non-empty `wristband.Options.ScopesSupported` is used as given (`read`, `write`, `ai` and `offline_access` from `wristband.DefaultOptions`, which still refuses `backend`; a host that lists an extra scope accepts it). An empty or nil list on the MCP flavour resolves to `read`, `write`, `ai`, `offline_access` and `backend`. Any other value is `invalid_scope`. OpenID Connect discovery stays `openid`, `profile` and `email` and does not use this list. The OAuth server plugin publishes that list as `pact.MCPAdvertisedScopes`; pier copies `pact.MCPAdvertisedScopes.MCPScopesSupported` when it is non-empty. A route the MCP endpoint runs reads the caller with `pact.MCPCallFrom`. +- Authorization endpoint (`wristband.Server.Authorize`): the client and its exact registered redirect URI are validated before any redirect is sent (an unknown client gets a local plain-text 400, never an open redirect); then S256 PKCE, the client's scope ceiling and the RFC 8707 `resource` value are checked, a pending request is stored and the browser is sent to the application's consent page at `/connect?request=`. The accepted scopes are the same resolved list `wristband.Server.Metadata` advertises. A non-empty `wristband.Options.ScopesSupported` is used as given (`read`, `write`, `ai` and `offline_access` from `wristband.DefaultOptions`, which still refuses `backend`; a host that lists an extra scope accepts it). An empty or nil list on the MCP flavour resolves to `read`, `write`, `ai`, `offline_access` and `backend`. The OpenID Connect identity scopes `openid`, `profile` and `email` are dropped from the request before that check (RFC 6749 section 3.3 lets a server grant fewer scopes than requested), so a client that also reads the issuer's OpenID Connect discovery document still connects; a request with nothing else left gets the default `read`, and the three are never advertised. Any other value is `invalid_scope`. OpenID Connect discovery stays `openid`, `profile` and `email` and does not use this list. The OAuth server plugin publishes that list as `pact.MCPAdvertisedScopes`; pier copies `pact.MCPAdvertisedScopes.MCPScopesSupported` when it is non-empty. A route the MCP endpoint runs reads the caller with `pact.MCPCallFrom`. - Consent operations for the application's own consent screen: `wristband.Server.PendingRequest` (what to show, including `wristband.PendingRequestView.OfflineAccess`, the pending row's flag; `offline_access` is not a scope on the view), `wristband.Server.IssueCode` (grant, returning the redirect URL with `code`, `iss` and `state`; the signature is unchanged) and `wristband.Server.DenyPending` (returning an `access_denied` redirect). Missing, foreign, used and expired requests all report the same `wristband.ErrPendingNotFound`. - 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. @@ -36,7 +36,7 @@ Use cases: - OIDC token and userinfo follow the PHP contract: client authentication by Basic credentials (urldecoded) or form fields with one uniform `invalid_client` answer and `Basic realm="oauth"`, codes burned before any other check so a replayed code revokes the tokens issued from it, `{error, error_description}` bodies with `no-store, private` and `Pragma: no-cache`, and one uniform userinfo 401 with an empty body. The `id_token` carries `iss`, `sub`, `aud` as a string, `exp`, `iat`, `nonce` when sent, `at_hash` and only allowlisted identity claims (no `auth_time`, no `azp`). - Keys and subjects: RS256 and ES256 keys (`wristband.GenerateSigningKey`), a mixed ring during a switch with discovery advertising what the ring holds, retired keys published through their grace window; public subjects by default and pairwise subjects per sector (`wristband.SubjectSector`, `wristband.ErrSectorRequired`), always random UUIDv4 values stored per user and sector. - Opt-in refresh for OIDC (`wristband.OIDCOptions.RefreshEnabled`, off by default): only a client with `wristband.OIDCClientRecord.OfflineAccess` granted `offline_access` receives a rotating `oas_rt_` refresh token; a refresh returns a fresh `id_token` without `nonce`, and replaying a rotated token revokes the lineage and its access tokens. -- 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`. +- 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 drops `openid`, `profile` and `email` at authorize and never issues an `id_token`, 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 diff --git a/modules/wristband/authorize.go b/modules/wristband/authorize.go index 2d5a4439..edbf3ea9 100644 --- a/modules/wristband/authorize.go +++ b/modules/wristband/authorize.go @@ -219,21 +219,42 @@ func (s *Server) authorizeErrorRedirect(w http.ResponseWriter, redirectURI, errC writeRedirectHTML(w, http.StatusFound, appendOrderedQuery(redirectURI, pairs)) } -// parseAuthorizeScopes ports OAuthAuthorizeController::parseScopes. An -// absent or empty scope query value defaults to ["read"]; otherwise every -// whitespace-separated token must be in allowed, the server's set -// (allowedAuthorizeScopes). +// mcpIgnoredIdentityScopes are the OpenID Connect identity scopes the MCP +// flavour drops from an authorize request before validating it. The match +// is exact and case-sensitive. +var mcpIgnoredIdentityScopes = map[string]bool{ + "openid": true, + "profile": true, + "email": true, +} + +// parseAuthorizeScopes ports OAuthAuthorizeController::parseScopes with one +// deliberate divergence. The identity scopes openid, profile and email are +// dropped first, because clients that also read the issuer's OpenID Connect +// discovery document request them on the MCP endpoint, and RFC 6749 section +// 3.3 lets the server grant fewer scopes than requested. The MCP flavour +// issues no ID tokens and never grants identity scopes (phase 19.4 D-07, +// narrowed from refusal to dropping), so the drop happens even when a host +// lists one of them in Options.ScopesSupported. An absent, empty or +// identity-only scope value falls back to ["read"]; any other token outside +// allowed, the server's set (allowedAuthorizeScopes), is refused. func parseAuthorizeScopes(raw string, allowed map[string]bool) ([]string, string) { parts := strings.Fields(strings.TrimSpace(raw)) - if len(parts) == 0 { + kept := make([]string, 0, len(parts)) + for _, p := range parts { + if !mcpIgnoredIdentityScopes[p] { + kept = append(kept, p) + } + } + if len(kept) == 0 { return []string{"read"}, "" } - for _, p := range parts { + for _, p := range kept { if !allowed[p] { return nil, "scope contains an unsupported value" } } - return parts, "" + return kept, "" } // stringSliceContains reports whether ss contains v. diff --git a/modules/wristband/authorize_test.go b/modules/wristband/authorize_test.go index ab78dca8..e226be37 100644 --- a/modules/wristband/authorize_test.go +++ b/modules/wristband/authorize_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "slices" "strings" "testing" "time" @@ -470,6 +471,109 @@ func TestAuthorizeScopeInvalidValueRedirectsInvalidScope(t *testing.T) { } } +// TestAuthorizeDropsOIDCIdentityScopes pins the MCP flavour's tolerance for +// the OpenID Connect identity scopes: openid, profile and email are dropped +// from the request before the allow-list check (RFC 6749 section 3.3 lets +// the server grant fewer scopes than requested), never persisted, and an +// identity-only request falls back to the read default. Any other unknown +// token, including a differently cased identity name, is still refused. +func TestAuthorizeDropsOIDCIdentityScopes(t *testing.T) { + cases := []struct { + name string + scope string + ceiling []string + extra map[string]string + want []string // nil: refused with invalid_scope + }{ + { + // A client that also reads the issuer's OpenID Connect discovery + // document adds openid and email to its MCP request. + name: "discovery-reading client request", + scope: "openid email read", + extra: map[string]string{"resource": "https://resource.example.test/mcp", "ui_locales": "en-US"}, + want: []string{"read"}, + }, + {name: "identity only defaults to read", scope: "openid profile", want: []string{"read"}}, + {name: "openid alone defaults to read", scope: "openid", want: []string{"read"}}, + {name: "order preserved", scope: "openid profile email read write", want: []string{"read", "write"}}, + {name: "ceiling applies after the drop", scope: "openid email read ai", ceiling: []string{"read", "write"}, want: []string{"read"}}, + {name: "unknown scope beside openid refused", scope: "openid bogus"}, + {name: "unknown scope refused", scope: "read bogus"}, + {name: "case differs refused", scope: "OPENID read"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv, backend := newAuthorizeTestServer() + insertAuthorizeTestClient(backend, "cli-identity", []string{authorizeTestRedirect}, tc.ceiling) + _, challenge := s256Pair(t) + before := pendingCount(backend) + + params := map[string]string{ + "client_id": "cli-identity", + "redirect_uri": authorizeTestRedirect, + "response_type": "code", + "code_challenge": challenge, + "code_challenge_method": "S256", + "scope": tc.scope, + "state": "xyz", + } + for k, v := range tc.extra { + params[k] = v + } + rec := httptest.NewRecorder() + srv.Authorize(rec, authorizeRequest(params)) + if rec.Code != http.StatusFound { + t.Fatalf("status = %d, want %d (body=%s)", rec.Code, http.StatusFound, rec.Body.String()) + } + loc := rec.Header().Get("Location") + q := queryOf(t, loc) + + if tc.want == nil { + if q["error"] != "invalid_scope" { + t.Fatalf("error = %q, want invalid_scope (Location=%q)", q["error"], loc) + } + if q["error_description"] != "scope contains an unsupported value" { + t.Fatalf("error_description = %q", q["error_description"]) + } + if got := pendingCount(backend); got != before { + t.Fatalf("pending rows = %d, want unchanged %d", got, before) + } + return + } + + if !strings.HasPrefix(loc, srv.opts.Issuer+"/connect?request=") { + t.Fatalf("Location = %q, want prefix %q", loc, srv.opts.Issuer+"/connect?request=") + } + if _, has := q["error"]; has { + t.Fatalf("Location %q carries an error", loc) + } + backend.mu.Lock() + defer backend.mu.Unlock() + if got := len(backend.codes); got != before+1 { + t.Fatalf("pending rows = %d, want %d", got, before+1) + } + row := backend.codes[len(backend.codes)-1] + if !slices.Equal(row.Scopes, tc.want) { + t.Fatalf("scopes = %v, want %v", row.Scopes, tc.want) + } + if row.OfflineAccess { + t.Fatal("OfflineAccess = true, want false") + } + if row.State == nil || *row.State != "xyz" { + t.Fatalf("state = %v, want xyz", row.State) + } + if res, ok := tc.extra["resource"]; ok { + if res != srv.opts.Resource { + t.Fatalf("fixture resource %q differs from server resource %q", res, srv.opts.Resource) + } + if row.Resource == nil || *row.Resource != srv.opts.Resource { + t.Fatalf("resource = %v, want %q", row.Resource, srv.opts.Resource) + } + } + }) + } +} + func TestAuthorizeNullCeilingPreservesAiScope(t *testing.T) { srv, backend := newAuthorizeTestServer() insertAuthorizeTestClient(backend, "cli-null-ceiling", []string{authorizeTestRedirect}, nil) diff --git a/modules/wristband/mcp_scopes_test.go b/modules/wristband/mcp_scopes_test.go index 212d3719..295fd948 100644 --- a/modules/wristband/mcp_scopes_test.go +++ b/modules/wristband/mcp_scopes_test.go @@ -11,8 +11,9 @@ import ( // TestMCPScopes (D-21, the framework side of the backend scope): the MCP // flavour's authorize allow-list is exactly Options.ScopesSupported. The // default server refuses backend; a server that lists it accepts "read -// backend" and advertises it in its metadata; on either, openid and a scope -// nobody lists stay refused. +// backend" and advertises it in its metadata; on either, the identity scopes +// openid, profile and email are dropped before the check and a scope nobody +// lists stays refused. func TestMCPScopes(t *testing.T) { _, challenge := s256Pair(t) authorize := func(t *testing.T, s *Server, scope string) map[string]string { @@ -68,13 +69,26 @@ func TestMCPScopes(t *testing.T) { } }) - t.Run("openid and unlisted scopes stay refused", func(t *testing.T) { + t.Run("identity scopes are dropped and unlisted scopes stay refused", func(t *testing.T) { for _, s := range []*Server{plain, withBackend} { - for _, scope := range []string{"openid backend", "read openid", "read admin"} { - if q := authorize(t, s, scope); q["error"] != "invalid_scope" { - t.Fatalf("%q = %v", scope, q) - } + if q := authorize(t, s, "read openid"); q["error"] != "" { + t.Fatalf("read openid = %v", q) } + if q := authorize(t, s, "read admin"); q["error"] != "invalid_scope" { + t.Fatalf("read admin = %v", q) + } + } + if q := authorize(t, plain, "openid backend"); q["error"] != "invalid_scope" { + t.Fatalf("default server openid backend = %v", q) + } + if q := authorize(t, withBackend, "openid backend"); q["error"] != "" { + t.Fatalf("listing server openid backend = %v", q) + } + backend.mu.Lock() + got := backend.codes[len(backend.codes)-1].Scopes + backend.mu.Unlock() + if !slices.Equal(got, []string{"backend"}) { + t.Fatalf("listing server openid backend scopes = %v, want [backend]", got) } }) } diff --git a/modules/wristband/oidc_authorize_smoke_test.go b/modules/wristband/oidc_authorize_smoke_test.go index fcd99db7..ee70caf3 100644 --- a/modules/wristband/oidc_authorize_smoke_test.go +++ b/modules/wristband/oidc_authorize_smoke_test.go @@ -227,15 +227,21 @@ func TestKindIsolationSmoke(t *testing.T) { t.Fatalf("MCP token with an oidc client = %d %s", rec.Code, rec.Body.String()) } - // MCP authorize still refuses openid (D-07). + // MCP authorize never grants openid (D-07): it is dropped, not persisted. 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" { + if _, has := queryOf(t, rec.Header().Get("Location"))["error"]; rec.Code != http.StatusFound || has { t.Fatalf("MCP authorize with openid = %d %q", rec.Code, rec.Header().Get("Location")) } + backend.mu.Lock() + stored := backend.codes[len(backend.codes)-1].Scopes + backend.mu.Unlock() + if len(stored) != 1 || stored[0] != "read" { + t.Fatalf("MCP authorize with openid stored scopes %v, want [read]", stored) + } // An mcp-kind client is unknown_client at OIDC authorize and // invalid_client at the OIDC token endpoint. diff --git a/modules/wristband/oidc_flavour_test.go b/modules/wristband/oidc_flavour_test.go index c831ce63..5fbd59a7 100644 --- a/modules/wristband/oidc_flavour_test.go +++ b/modules/wristband/oidc_flavour_test.go @@ -198,9 +198,10 @@ func TestSetOIDCBackendValidation(t *testing.T) { } } -// TestMCPRefusesOpenID pins D-07: the MCP flavour keeps refusing openid at -// authorize, and its token answer never carries an id_token, even for a code -// whose stored scopes mention openid. +// TestMCPRefusesOpenID pins D-07: the MCP flavour never grants openid. +// Authorize drops it (with profile and email) instead of persisting it, and +// the token answer never carries an id_token, even for a code whose stored +// scopes mention openid. func TestMCPRefusesOpenID(t *testing.T) { backend := newMemoryBackend() mcp := newTestServer(backend) @@ -212,8 +213,17 @@ func TestMCPRefusesOpenID(t *testing.T) { "client_id": client.ClientID, "redirect_uri": "https://client.example.test/cb", "response_type": "code", "code_challenge": challenge, "code_challenge_method": "S256", "scope": scope, "state": "s", })) - if rec.Code != http.StatusFound || queryOf(t, rec.Header().Get("Location"))["error"] != "invalid_scope" { - t.Fatalf("MCP authorize with %q = %d %q", scope, rec.Code, rec.Header().Get("Location")) + if rec.Code != http.StatusFound { + t.Fatalf("MCP authorize with %q = %d", scope, rec.Code) + } + if _, has := queryOf(t, rec.Header().Get("Location"))["error"]; has { + t.Fatalf("MCP authorize with %q = %q", scope, rec.Header().Get("Location")) + } + backend.mu.Lock() + got := backend.codes[len(backend.codes)-1].Scopes + backend.mu.Unlock() + if len(got) != 1 || got[0] != "read" { + t.Fatalf("MCP authorize with %q stored scopes %v, want [read]", scope, got) } }