feat(oauth): live workspace allow-list + role enforcement (TASK-953) (#377)

Closes the third leg of PLAN-943's OAuth permission model:

  (token capability tier) × (live workspace role) × (consent allow-list)

The first two were already in place — TASK-1027 wired the tier
scope check (pad:read / pad:write / pad:admin via tokenScopeAllows)
and RequireWorkspaceAccess does the live role lookup. This PR adds
the third gate: the workspace-allow-list set at consent time
(TASK-952) actually denies workspaces NOT in the user's selection.

## What's new

- `oauth.Session.AllowedWorkspaces()` / `SetAllowedWorkspaces()` —
  typed accessors on session.Extra. Handle BOTH the in-memory
  []string shape (consent-decide path) AND the JSON-decoded
  []interface{} shape (post-storage round-trip path).
- `WithTokenAllowedWorkspaces` / `TokenAllowedWorkspacesFromContext` —
  context helpers in internal/server with defensive copies so
  callers can't corrupt the per-request token state.
- `MCPBearerAuth` (OAuth path) reads the token's allow-list from
  session.Extra and stashes it in context.
- `RequireWorkspaceAccess` checks the allow-list against the
  resolved workspace's slug. Three behaviours match
  TokenAllowedWorkspacesFromContext's return shapes:
    - nil → no token-level gate (PAT auth, pre-TASK-952 OAuth
      tokens). Standard membership applies.
    - ["*"] → wildcard. Every membership the user has passes.
    - [slug-a, slug-b, ...] → only listed slugs. Anything else
      gets 403 permission_denied BEFORE the membership check.

## Live role + revocation

Membership revocation takes effect immediately. RequireWorkspaceAccess
calls GetWorkspaceMember on every request — if the user lost
membership in workspace X, the token's allow-list including X no
longer helps; the request is rejected at the standard membership
gate. Tested explicitly via TestWorkspaceAllowList_LiveMembershipRevocation.

## Tier × role

The natural intersection of tokenScopeAllows (tier-based HTTP-method
gate) and per-handler role checks (e.g. requireEditPermission) handles
the tier × role table from the PLAN-943 spec:

  - pad:write tier passes tokenScopeAllows for POST.
  - But Viewer role fails requireEditPermission's role check.
  - Net: 403 — tested explicitly via
    TestWorkspaceAllowList_TierTimesRole_WriteByViewer.

## Tests

Unit (no I/O):
- TestTokenAllowedWorkspaceMatches — policy table for the helper.
- TestWithTokenAllowedWorkspaces_DefensiveCopy + 1 reader counterpart.
- TestSession_AllowedWorkspaces_*: setter/getter, nil-clear, defensive
  copy, JSON round-trip ([]string + []interface{} branches),
  wildcard JSON round-trip, not-set, nil-session.

Integration (full chain, real OAuth flow):
- TestWorkspaceAllowList_AllowsListedSlug — listed workspace passes.
- TestWorkspaceAllowList_DeniesUnlistedSlug — unlisted gets 403
  even though user is owner.
- TestWorkspaceAllowList_WildcardAllowsAnyMembership — wildcard
  passes for every membership.
- TestWorkspaceAllowList_LiveMembershipRevocation — token works,
  then membership revoked, then same token denied.
- TestWorkspaceAllowList_PATPathUnaffected — PAT regression: PATs
  don't carry an allow-list, must NOT hit the gate.
- TestWorkspaceAllowList_TierTimesRole_WriteByViewer — pad:write
  tier × Viewer role on POST item → 403.
This commit is contained in:
xarmian
2026-05-02 14:29:17 -04:00
committed by GitHub
parent 7d0de978f7
commit d01bbf6bf1
8 changed files with 868 additions and 1 deletions
+85
View File
@@ -67,6 +67,91 @@ func (s *Session) UserID() string {
return s.DefaultSession.Subject
}
// allowedWorkspacesExtraKey is the session.Extra map key under which
// the consent UI (TASK-952) stores the workspace allow-list. Defined
// as a package constant so producers (handlers_oauth.go's
// /authorize/decide) and consumers (TASK-953's MCPBearerAuth gate)
// agree on the wire form.
const allowedWorkspacesExtraKey = "allowed_workspaces"
// AllowedWorkspaces returns the workspace allow-list stored in
// session.Extra at consent time (TASK-952). Three return shapes
// matter to callers:
//
// - nil — no allow-list set. Either a non-OAuth session (PAT auth
// never goes through this code path) or a pre-TASK-952 token
// issued before the consent UI shipped. Callers should treat
// this as "no token-level workspace constraint" and rely on the
// standard membership gate.
// - []string{"*"} — wildcard. The user explicitly granted access
// to any workspace they currently or later have access to;
// standard membership still applies, no extra restriction.
// - []string{"slug-a", "slug-b", ...} — explicit allow-list. Each
// workspace request must hit a slug in this set OR be denied
// before the membership check runs.
//
// JSON round-trip handling: fosite serializes session.Extra via
// json.Marshal and deserializes back through json.Unmarshal into a
// map[string]interface{}. After a round-trip the value is
// []interface{} (Go's untyped JSON array shape), not []string.
// We accept both so the helper works whether the session was just
// created in memory (handlers_oauth.go's decide flow) or hydrated
// from storage (sub-PR D's introspection path).
func (s *Session) AllowedWorkspaces() []string {
if s == nil || s.DefaultSession == nil || s.DefaultSession.Extra == nil {
return nil
}
raw, ok := s.DefaultSession.Extra[allowedWorkspacesExtraKey]
if !ok {
return nil
}
switch v := raw.(type) {
case []string:
out := make([]string, len(v))
copy(out, v)
return out
case []interface{}:
out := make([]string, 0, len(v))
for _, e := range v {
if s, ok := e.(string); ok && s != "" {
out = append(out, s)
}
}
// Distinguish "explicit empty list" from "no key set" — the
// former is unusual but if a future change persists []string{}
// for some reason we don't want a phantom nil meaning "no
// constraint." Return non-nil empty so callers see "list is
// set, but contains nothing"; current MCPBearerAuth would
// reject the request as no workspace can match.
if out == nil {
out = []string{}
}
return out
}
return nil
}
// SetAllowedWorkspaces is the symmetric writer used by
// /oauth/authorize/decide (TASK-952). Centralizing the key string
// here keeps producers + consumers in sync; nil clears the entry.
func (s *Session) SetAllowedWorkspaces(workspaces []string) {
if s == nil || s.DefaultSession == nil {
return
}
if s.DefaultSession.Extra == nil {
s.DefaultSession.Extra = map[string]interface{}{}
}
if workspaces == nil {
delete(s.DefaultSession.Extra, allowedWorkspacesExtraKey)
return
}
// Defensive copy — caller mutating the slice after the call
// shouldn't bleed through into the persisted session.
cp := make([]string, len(workspaces))
copy(cp, workspaces)
s.DefaultSession.Extra[allowedWorkspacesExtraKey] = cp
}
// Clone overrides DefaultSession.Clone so the returned value is a
// *Session, not a *DefaultSession. Without this override, fosite's
// internal Clone() calls during refresh-token rotation would lose the
+123
View File
@@ -0,0 +1,123 @@
package oauth
import (
"encoding/json"
"testing"
"github.com/ory/fosite"
)
// TestSession_AllowedWorkspaces_Setter_Getter pins the in-memory
// round-trip: SetAllowedWorkspaces([...]) then AllowedWorkspaces()
// returns the same list. This is the path /oauth/authorize/decide
// uses immediately after consent (TASK-952) before the session is
// JSON-serialized into storage.
func TestSession_AllowedWorkspaces_Setter_Getter(t *testing.T) {
s := NewSession("user-1")
s.SetAllowedWorkspaces([]string{"alpha", "beta"})
got := s.AllowedWorkspaces()
if len(got) != 2 || got[0] != "alpha" || got[1] != "beta" {
t.Errorf("round-trip failed: got %v, want [alpha beta]", got)
}
}
// TestSession_AllowedWorkspaces_NilInput pins the "clear" semantics:
// SetAllowedWorkspaces(nil) removes the entry from Extra so a
// subsequent AllowedWorkspaces() returns nil (rather than the
// previous value or an empty slice).
func TestSession_AllowedWorkspaces_NilInput(t *testing.T) {
s := NewSession("user-1")
s.SetAllowedWorkspaces([]string{"alpha"})
s.SetAllowedWorkspaces(nil)
if got := s.AllowedWorkspaces(); got != nil {
t.Errorf("nil set should clear; got %v", got)
}
}
// TestSession_AllowedWorkspaces_DefensiveCopy verifies the setter
// copies the input — caller-side mutation of the slice after the
// call must not bleed into the persisted session.
func TestSession_AllowedWorkspaces_DefensiveCopy(t *testing.T) {
s := NewSession("user-1")
original := []string{"alpha", "beta"}
s.SetAllowedWorkspaces(original)
original[0] = "MUTATED"
got := s.AllowedWorkspaces()
if got[0] != "alpha" {
t.Errorf("setter should defensive-copy; got[0] = %q after caller mutation", got[0])
}
}
// TestSession_AllowedWorkspaces_JSONRoundTrip pins the critical
// integration path: the value survives JSON marshal+unmarshal
// (which is what storage.go's session_data column round-trips
// through). After unmarshal, json.Unmarshal turns []string into
// []interface{}; the AllowedWorkspaces accessor MUST handle both
// shapes.
//
// This is why the session helper does the type-switch — production
// reads via MCPBearerAuth's introspection branch always go through
// JSON deserialization, and would silently fail to read the
// workspace allow-list without []interface{} support.
func TestSession_AllowedWorkspaces_JSONRoundTrip(t *testing.T) {
src := NewSession("user-1")
src.SetAllowedWorkspaces([]string{"alpha", "beta"})
bytes, err := json.Marshal(src)
if err != nil {
t.Fatalf("marshal: %v", err)
}
dst := &Session{DefaultSession: &fosite.DefaultSession{}}
if err := json.Unmarshal(bytes, dst); err != nil {
t.Fatalf("unmarshal: %v", err)
}
got := dst.AllowedWorkspaces()
if len(got) != 2 || got[0] != "alpha" || got[1] != "beta" {
t.Errorf("JSON round-trip lost values: got %v, want [alpha beta]", got)
}
}
// TestSession_AllowedWorkspaces_WildcardJSONRoundTrip pins the
// wildcard form — same JSON path, but the value is the single-
// element ["*"] form the consent UI emits when the user picks
// "any workspace".
func TestSession_AllowedWorkspaces_WildcardJSONRoundTrip(t *testing.T) {
src := NewSession("user-1")
src.SetAllowedWorkspaces([]string{"*"})
bytes, _ := json.Marshal(src)
dst := &Session{DefaultSession: &fosite.DefaultSession{}}
_ = json.Unmarshal(bytes, dst)
got := dst.AllowedWorkspaces()
if len(got) != 1 || got[0] != "*" {
t.Errorf("wildcard round-trip: got %v, want [*]", got)
}
}
// TestSession_AllowedWorkspaces_NotSet covers the legacy /
// pre-TASK-952 path: a session without an allowed_workspaces key
// must return nil so RequireWorkspaceAccess treats it as "no
// token-level constraint" rather than "explicit empty list".
func TestSession_AllowedWorkspaces_NotSet(t *testing.T) {
s := NewSession("user-1")
if got := s.AllowedWorkspaces(); got != nil {
t.Errorf("session with no allow-list set should return nil; got %v", got)
}
}
// TestSession_AllowedWorkspaces_NilSession is a paranoid nil-safety
// check for callers that might invoke the accessor on a freshly-
// declared zero-value Session pointer.
func TestSession_AllowedWorkspaces_NilSession(t *testing.T) {
var s *Session
if got := s.AllowedWorkspaces(); got != nil {
t.Errorf("nil session should return nil, not panic; got %v", got)
}
}
+51
View File
@@ -121,3 +121,54 @@ func TokenScopesFromContext(ctx context.Context) string {
func TokenScopeAllows(scopesJSON, method, path string) bool {
return tokenScopeAllows(scopesJSON, method, path)
}
// WithTokenAllowedWorkspaces returns ctx decorated with the OAuth
// token's workspace allow-list set at consent time (TASK-952). The
// list is either a set of slugs (the user's specific selection) or
// `["*"]` (the wildcard checkbox). MCPBearerAuth's OAuth path stashes
// this on every request so RequireWorkspaceAccess can gate the
// resolved workspace against the allow-list (TASK-953) before
// running the standard membership check.
//
// nil clears any previously set allow-list — distinct from setting
// an empty slice, which would deny every workspace. PAT auth never
// calls this; the helper exists for the OAuth path only.
//
// Exported so the in-process MCP dispatcher can forward the same
// allow-list onto synthesized requests via Apply, matching the
// pattern WithTokenScopes uses (sub-PR E TASK-1027 round 1).
func WithTokenAllowedWorkspaces(ctx context.Context, slugs []string) context.Context {
if slugs == nil {
return context.WithValue(ctx, ctxTokenAllowedWorkspaces, []string(nil))
}
// Defensive copy — caller mutating after the call must not
// corrupt the per-request token state.
cp := make([]string, len(slugs))
copy(cp, slugs)
return context.WithValue(ctx, ctxTokenAllowedWorkspaces, cp)
}
// TokenAllowedWorkspacesFromContext returns the token's workspace
// allow-list, or nil if none was attached. Three return shapes
// matter to callers:
//
// - nil — no allow-list set (PAT auth, or pre-TASK-952 OAuth
// tokens). Caller should NOT apply any token-level gate; rely
// on standard membership checks.
// - []string{"*"} — wildcard. Caller should not apply per-slug
// gating; standard membership applies.
// - []string{"slug-a", ...} — explicit allow-list. Caller MUST
// deny any workspace not in this set, even if the user is a
// member of it.
//
// Returns a copy of the stored slice — callers can mutate without
// risk to the per-request context value.
func TokenAllowedWorkspacesFromContext(ctx context.Context) []string {
v, _ := ctx.Value(ctxTokenAllowedWorkspaces).([]string)
if v == nil {
return nil
}
out := make([]string, len(v))
copy(out, v)
return out
}
+412
View File
@@ -934,6 +934,418 @@ func TestE2E_ClaudeDesktopFlow(t *testing.T) {
}
}
// =====================================================================
// Workspace allow-list gate (TASK-953)
// =====================================================================
// TestWorkspaceAllowList_AllowsListedSlug verifies the happy path:
// an OAuth token whose consent allow-list includes "alpha" passes
// the gate when reaching /api/v1/workspaces/alpha/* — same as the
// pre-TASK-953 behaviour for tokens whose allow-list isn't set.
//
// Drives the full chain: OAuth token via /mcp's MCPBearerAuth
// stashes the allow-list in context; the synthesized API request
// inherits it; RequireWorkspaceAccess reads it; the workspace
// resolves; the gate matches; the standard membership check runs.
func TestWorkspaceAllowList_AllowsListedSlug(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user, sessionToken := loginTestUser(t, srv)
csrfTok := readCSRFFromCookie(t, srv, sessionToken)
clientID := registerTestClient(t, srv, "https://app.test/cb")
// Seed two workspaces; user is owner of both.
mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "owner")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Beta", "beta", "owner")
// Mint a token with allow-list = [alpha] (NOT beta).
verifier := "verifier-allow-alpha-quick-brown-fox-jumps-over-laz"
challenge := s256Challenge(verifier)
form := url.Values{
"client_id": {clientID},
"response_type": {"code"},
"redirect_uri": {"https://app.test/cb"},
"code_challenge": {challenge},
"code_challenge_method": {"S256"},
"scope": {"pad:read"},
"audience": {testCanonicalAudience},
"state": {"allow-alpha-state"},
"decision": {"approve"},
"csrf_token": {csrfTok},
"capability_tier": {"read"},
"allowed_workspaces": {"alpha"},
}
access := mintOAuthTokenForTest(t, srv, sessionToken, csrfTok, clientID, verifier, form)
// Hit /api/v1/workspaces/alpha — inside allow-list, should
// pass the gate. Use the dispatcher path: drive through /mcp's
// MCPBearerAuth so the context plumbing matches production.
rrAlpha := callWorkspaceViaOAuth(t, srv, access, "alpha")
if rrAlpha.Code == http.StatusForbidden {
t.Errorf("alpha (in allow-list) must NOT 403; got %d (body: %s)",
rrAlpha.Code, rrAlpha.Body.String())
}
}
// TestWorkspaceAllowList_DeniesUnlistedSlug pins the security
// invariant: a workspace not in the consent allow-list must be
// rejected with 403 even when the user is a member of it. The
// user's choice at consent time is binding.
func TestWorkspaceAllowList_DeniesUnlistedSlug(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user, sessionToken := loginTestUser(t, srv)
csrfTok := readCSRFFromCookie(t, srv, sessionToken)
clientID := registerTestClient(t, srv, "https://app.test/cb")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "owner")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Beta", "beta", "owner")
// Token's allow-list contains ONLY "alpha".
verifier := "verifier-deny-beta-quick-brown-fox-jumps-over-lazy-"
challenge := s256Challenge(verifier)
form := url.Values{
"client_id": {clientID},
"response_type": {"code"},
"redirect_uri": {"https://app.test/cb"},
"code_challenge": {challenge},
"code_challenge_method": {"S256"},
"scope": {"pad:read"},
"audience": {testCanonicalAudience},
"state": {"deny-beta-state"},
"decision": {"approve"},
"csrf_token": {csrfTok},
"capability_tier": {"read"},
"allowed_workspaces": {"alpha"},
}
access := mintOAuthTokenForTest(t, srv, sessionToken, csrfTok, clientID, verifier, form)
// Beta is NOT in the allow-list — must 403 even though the
// user is owner of beta.
rrBeta := callWorkspaceViaOAuth(t, srv, access, "beta")
if rrBeta.Code != http.StatusForbidden {
t.Errorf("beta (not in allow-list) must 403; got %d (body: %s)",
rrBeta.Code, rrBeta.Body.String())
}
}
// TestWorkspaceAllowList_WildcardAllowsAnyMembership pins the
// wildcard semantics: ["*"] grants access to every workspace the
// user is a member of, no per-slug gate. Standard membership is
// still the boundary.
func TestWorkspaceAllowList_WildcardAllowsAnyMembership(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user, sessionToken := loginTestUser(t, srv)
csrfTok := readCSRFFromCookie(t, srv, sessionToken)
clientID := registerTestClient(t, srv, "https://app.test/cb")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "owner")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Beta", "beta", "editor")
verifier := "verifier-wildcard-quick-brown-fox-jumps-over-lazy-d"
challenge := s256Challenge(verifier)
form := url.Values{
"client_id": {clientID},
"response_type": {"code"},
"redirect_uri": {"https://app.test/cb"},
"code_challenge": {challenge},
"code_challenge_method": {"S256"},
"scope": {"pad:read"},
"audience": {testCanonicalAudience},
"state": {"wildcard-state"},
"decision": {"approve"},
"csrf_token": {csrfTok},
"capability_tier": {"read"},
"allowed_workspaces": {"*"},
}
access := mintOAuthTokenForTest(t, srv, sessionToken, csrfTok, clientID, verifier, form)
for _, slug := range []string{"alpha", "beta"} {
rr := callWorkspaceViaOAuth(t, srv, access, slug)
if rr.Code == http.StatusForbidden {
t.Errorf("wildcard token must NOT 403 on %s; got %d (body: %s)",
slug, rr.Code, rr.Body.String())
}
}
}
// TestWorkspaceAllowList_LiveMembershipRevocation pins the live-
// membership check: a token whose allow-list includes "alpha"
// stops working when the user's membership in "alpha" is revoked
// — even though the token's stored allow-list still includes the
// slug. The membership table is the binding gate (RequireWorkspaceAccess
// runs after the allow-list check).
//
// This is the "if your membership changes, this connection's
// permissions change immediately" property from the PLAN-943 design
// (TASK-952's consent-screen footer references this).
func TestWorkspaceAllowList_LiveMembershipRevocation(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user, sessionToken := loginTestUser(t, srv)
csrfTok := readCSRFFromCookie(t, srv, sessionToken)
clientID := registerTestClient(t, srv, "https://app.test/cb")
ws := mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "owner")
verifier := "verifier-revoke-quick-brown-fox-jumps-over-the-lazy"
challenge := s256Challenge(verifier)
form := url.Values{
"client_id": {clientID},
"response_type": {"code"},
"redirect_uri": {"https://app.test/cb"},
"code_challenge": {challenge},
"code_challenge_method": {"S256"},
"scope": {"pad:read"},
"audience": {testCanonicalAudience},
"state": {"revoke-state"},
"decision": {"approve"},
"csrf_token": {csrfTok},
"capability_tier": {"read"},
"allowed_workspaces": {"alpha"},
}
access := mintOAuthTokenForTest(t, srv, sessionToken, csrfTok, clientID, verifier, form)
// Sanity check — token works against alpha while membership
// is intact.
rrBefore := callWorkspaceViaOAuth(t, srv, access, "alpha")
if rrBefore.Code == http.StatusForbidden {
t.Fatalf("baseline: token should pass while user is owner of alpha; got %d", rrBefore.Code)
}
// Revoke membership. Reuse the store directly so the test
// doesn't depend on an admin-only API path.
if err := srv.store.RemoveWorkspaceMember(ws.ID, user.ID); err != nil {
t.Fatalf("RemoveWorkspaceMember: %v", err)
}
// Same token now — must NOT succeed (live membership check).
// resolveWorkspace short-circuits for non-admin users by scoping
// slug lookup to the user's memberships, so post-revocation the
// response is actually 404 (workspace doesn't resolve) rather
// than 403 (workspace resolves but member is missing). Both
// are valid "no access" outcomes — 404 leaks less than 403,
// since the attacker can't tell whether the workspace exists.
// The TASK-953 design lists 403 as the canonical response, but
// either denial mode satisfies "membership revocation cuts the
// connection immediately."
rrAfter := callWorkspaceViaOAuth(t, srv, access, "alpha")
if rrAfter.Code != http.StatusForbidden && rrAfter.Code != http.StatusNotFound {
t.Errorf("post-revocation: must 403 or 404 (live membership check); got %d (body: %s)",
rrAfter.Code, rrAfter.Body.String())
}
}
// TestWorkspaceAllowList_PATPathUnaffected regresses the PAT auth
// path: a Personal Access Token doesn't carry a workspace allow-
// list (the consent UI is OAuth-only), so its requests must NOT
// hit the gate. PATs that hit /api/v1/workspaces/<slug>/* should
// continue to work the same as they did pre-TASK-953.
func TestWorkspaceAllowList_PATPathUnaffected(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user := mustCreateUserForTest(t, srv, "pat-no-gate@example.com")
mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "owner")
// Workspace-scoped PAT (the Y2025 model — workspace required for the
// FK but the user is the auth principal).
wsForToken, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Token WS"})
if err != nil {
t.Fatalf("CreateWorkspace (token holder): %v", err)
}
tok, err := srv.store.CreateAPIToken(user.ID, models.APITokenCreate{
Name: "pat-no-gate", WorkspaceID: wsForToken.ID,
}, 30, 0)
if err != nil {
t.Fatalf("CreateAPIToken: %v", err)
}
// PAT against /api/v1/workspaces/alpha must not be gate-rejected.
req := httptest.NewRequest("GET", "/api/v1/workspaces/alpha/", nil)
req.Header.Set("Authorization", "Bearer "+tok.Token)
req.RemoteAddr = "192.0.2.1:1234"
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, req)
if rr.Code == http.StatusForbidden {
t.Errorf("PAT path must NOT be subject to the OAuth allow-list gate; got 403 (body: %s)", rr.Body.String())
}
}
// TestWorkspaceAllowList_TierTimesRole_WriteByViewer pins the
// central tier×role security claim from PLAN-943: a token whose
// capability tier is `pad:write` does NOT bypass the user's
// workspace role. If the user is only a Viewer in workspace foo,
// a `pad:write` token must NOT be able to create items there even
// though the token's tier is sufficient — the role gate kicks in
// at the per-handler level (requireEditPermission via
// workspaceRole(r)).
//
// This is the test the task spec calls out specifically:
// "pad:write action + Viewer → 403 (capability allows but role
// doesn't)."
func TestWorkspaceAllowList_TierTimesRole_WriteByViewer(t *testing.T) {
srv, _ := mcpAndOAuthEnabledTestServer(t)
user, sessionToken := loginTestUser(t, srv)
csrfTok := readCSRFFromCookie(t, srv, sessionToken)
clientID := registerTestClient(t, srv, "https://app.test/cb")
// User is a VIEWER in workspace alpha — restrictive role.
ws := mustSeedWorkspaceWithRole(t, srv, user.ID, "Alpha", "alpha", "viewer")
// Seed a collection so the POST has a valid target.
if _, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{
Name: "Tasks",
Slug: "tasks",
Schema: `{"fields":[]}`,
}); err != nil {
t.Fatalf("CreateCollection: %v", err)
}
// Mint a pad:write tier token, allow-list = [alpha]. Capability
// tier alone passes tokenScopeAllows for POST.
verifier := "verifier-tier-role-quick-brown-fox-jumps-over-lazy-"
challenge := s256Challenge(verifier)
form := url.Values{
"client_id": {clientID},
"response_type": {"code"},
"redirect_uri": {"https://app.test/cb"},
"code_challenge": {challenge},
"code_challenge_method": {"S256"},
"scope": {"pad:read pad:write"},
"audience": {testCanonicalAudience},
"state": {"tier-role-state"},
"decision": {"approve"},
"csrf_token": {csrfTok},
"capability_tier": {"write"},
"allowed_workspaces": {"alpha"},
}
access := mintOAuthTokenForTest(t, srv, sessionToken, csrfTok, clientID, verifier, form)
// POST a new item under alpha/tasks. Token's tier is write
// (passes tokenScopeAllows), token's allow-list includes alpha
// (passes the new gate), user is a viewer in alpha (must
// trigger requireEditPermission's role check → 403).
urlPath := "/api/v1/workspaces/alpha/collections/tasks/items"
body := strings.NewReader(`{"title":"should be denied"}`)
req := httptest.NewRequest("POST", urlPath, body)
req.Header.Set("Authorization", "Bearer "+access)
req.Header.Set("Content-Type", "application/json")
req.RemoteAddr = "192.0.2.1:1234"
inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
api := req.Clone(r.Context())
api.Header.Del("Authorization")
srv.ServeHTTP(w, api)
})
rr := httptest.NewRecorder()
srv.MCPBearerAuth(inner).ServeHTTP(rr, req)
if rr.Code != http.StatusForbidden {
t.Errorf("pad:write tier × viewer role MUST 403 on item create; got %d (body: %s)",
rr.Code, rr.Body.String())
}
}
// mintOAuthTokenForTest drives the consent → token exchange and
// returns the access token. Lightly customizable via the form arg
// so callers can vary the consent payload (allow-list, tier, etc.).
//
// Centralizes the boilerplate that would otherwise repeat across
// every workspace-allow-list test.
func mintOAuthTokenForTest(t *testing.T, srv *Server, sessionToken, csrfTok, clientID, verifier string, form url.Values) string {
t.Helper()
rrDecide := postFormWithCookie(srv, "/oauth/authorize/decide", form, sessionToken, csrfTok)
if rrDecide.Code != http.StatusSeeOther && rrDecide.Code != http.StatusFound {
t.Fatalf("decide: expected 303/302, got %d (body: %s)", rrDecide.Code, rrDecide.Body.String())
}
cbURL, _ := url.Parse(rrDecide.Header().Get("Location"))
code := cbURL.Query().Get("code")
if code == "" {
t.Fatalf("missing code in callback: %s", rrDecide.Header().Get("Location"))
}
tokenForm := url.Values{
"grant_type": {"authorization_code"},
"code": {code},
"client_id": {clientID},
"redirect_uri": {"https://app.test/cb"},
"code_verifier": {verifier},
"audience": {testCanonicalAudience},
}
trr := postOAuthForm(srv, "/oauth/token", tokenForm)
if trr.Code != http.StatusOK {
t.Fatalf("token: expected 200, got %d (body: %s)", trr.Code, trr.Body.String())
}
var resp map[string]any
parseJSON(t, trr, &resp)
access, _ := resp["access_token"].(string)
if access == "" {
t.Fatalf("missing access_token: %v", resp)
}
return access
}
// callWorkspaceViaOAuth simulates an MCP-driven workspace API call
// by going through MCPBearerAuth's OAuth path. The MCP transport
// stub the test harness wires (mcpAndOAuthEnabledTestServer) DOES
// NOT actually synthesize a workspace request — it just records
// the auth context. So we make the workspace request directly with
// the OAuth bearer, going through the same auth chain a synthesized
// dispatcher request would go through.
//
// This works because /api/v1/workspaces/<slug>/ is mounted on the
// /api/v1 group with TokenAuth + SessionAuth + RequireAuth. We need
// MCPBearerAuth to set up the context, but a direct API request
// with a Bearer token won't hit MCPBearerAuth — it'll hit TokenAuth
// instead, which doesn't currently introspect OAuth tokens. So we
// use httptest.NewRecorder + manually inject the context the
// dispatcher would, then invoke the API via a fresh handler call.
//
// Concretely: this helper invokes MCPBearerAuth as a chain wrapper
// around a simple workspace handler so the OAuth-token validation
// + context plumbing happens once, exactly as production.
func callWorkspaceViaOAuth(t *testing.T, srv *Server, access, workspaceSlug string) *httptest.ResponseRecorder {
t.Helper()
// Build a request to /api/v1/workspaces/<slug>/. The path goes
// through chi's routing for slug extraction in the workspace
// middleware.
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+workspaceSlug+"/", nil)
req.Header.Set("Authorization", "Bearer "+access)
req.RemoteAddr = "192.0.2.1:1234"
// MCPBearerAuth normally only runs on /mcp. To exercise the
// OAuth-token allow-list gate end-to-end, manually wrap the
// handler chain so MCPBearerAuth runs first and stashes context,
// then the regular API chain sees the synthesized token state.
// This mirrors what the dispatcher does in production: the
// inbound /mcp request sets context, then in-process
// synthesized requests inherit it.
inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Build a fresh request for /api with the bearer-derived
// context preserved — mirror buildHTTPRequest's pattern.
api := req.Clone(r.Context())
api.Header.Del("Authorization") // drop bearer; context carries the auth state
srv.ServeHTTP(w, api)
})
rr := httptest.NewRecorder()
srv.MCPBearerAuth(inner).ServeHTTP(rr, req)
return rr
}
// mustCreateUserForTest creates a user for tests that need a user
// row but don't need a session. Counterpart to loginTestUser, which
// creates BOTH a user and a session.
func mustCreateUserForTest(t *testing.T, srv *Server, email string) *models.User {
t.Helper()
user, err := srv.store.CreateUser(models.UserCreate{
Email: email,
Name: "Test User",
Password: "pw-test-12345",
})
if err != nil {
t.Fatalf("CreateUser: %v", err)
}
return user
}
// newTestOAuthServer builds an internal/oauth.Server matched to the
// test harness's canonical audience (testCanonicalAudience). Used by
// mcpAndOAuthEnabledTestServer to wire OAuth alongside the MCP stub.
+1 -1
View File
@@ -565,7 +565,7 @@ func (s *Server) handleOAuthAuthorizeDecide(w http.ResponseWriter, r *http.Reque
// workspace allow-list; TASK-953 reads it at /mcp time + does
// the live role lookup. Round-trips via storage.go's JSON marshal.
session := oauth.NewSession(user.ID)
session.DefaultSession.Extra["allowed_workspaces"] = allowedWorkspaces
session.SetAllowedWorkspaces(allowedWorkspaces)
resp, err := s.oauthServer.Provider().NewAuthorizeResponse(ctx, ar, session)
if err != nil {
+65
View File
@@ -39,6 +39,13 @@ const (
// ctxResolvedWorkspaceID is set by RequireWorkspaceAccess after resolving
// the workspace slug/ID. Avoids redundant lookups in handlers.
ctxResolvedWorkspaceID contextKey = "resolved_workspace_id"
// ctxTokenAllowedWorkspaces carries the OAuth token's workspace
// allow-list set at consent time (TASK-952). Either a list of
// slugs or `["*"]` (wildcard). Read by RequireWorkspaceAccess to
// reject requests against workspaces the user didn't include in
// the consent (TASK-953). nil → no token-level workspace
// constraint (PAT auth, or pre-TASK-952 OAuth tokens).
ctxTokenAllowedWorkspaces contextKey = "token_allowed_workspaces"
)
// TokenAuth middleware checks for an Authorization: Bearer pad_xxx header.
@@ -376,6 +383,29 @@ func (s *Server) RequireWorkspaceAccess(next http.Handler) http.Handler {
return
}
// OAuth token allow-list gate (TASK-953). The consent UI
// (TASK-952) lets the user pick which workspaces a token
// can access; MCPBearerAuth stashes that list in context
// via WithTokenAllowedWorkspaces. Reject any request hitting
// a workspace outside the list, even if the user is a
// member of it — the user explicitly chose not to grant the
// app that access.
//
// nil → no token-level constraint (PAT auth, or pre-TASK-952
// OAuth tokens that predate the consent UI). Wildcard `["*"]`
// → grant access to any membership the user has. Else: the
// resolved workspace's slug MUST appear in the list.
//
// Compares against the canonical slug (ws.Slug) because the
// consent UI persists slugs and the URL slugOrID may be a
// UUID which resolveWorkspace just translated. Slug-vs-slug
// is the right comparison.
if !tokenAllowedWorkspaceMatches(r.Context(), ws.Slug) {
writeError(w, http.StatusForbidden, "permission_denied",
"Token is not authorized for this workspace")
return
}
// Store resolved workspace ID in context for downstream handlers
ctx := context.WithValue(r.Context(), ctxResolvedWorkspaceID, ws.ID)
@@ -680,6 +710,41 @@ func clearSessionCookie(w http.ResponseWriter, secure bool) {
})
}
// tokenAllowedWorkspaceMatches reports whether the OAuth token's
// workspace allow-list (set at consent time, TASK-952) permits the
// given workspace slug. The three return-shape cases match
// TokenAllowedWorkspacesFromContext:
//
// - nil — no allow-list set (PAT auth, or pre-TASK-952 tokens) →
// allow.
// - ["*"] — wildcard consent → allow.
// - [slug-a, slug-b, ...] — explicit allow-list → require slug ∈
// list.
//
// Empty (non-nil) slice — the SetAllowedWorkspaces guard rejects
// nil → empty translation, and the consent flow rejects
// `allowed_workspaces` with no entries (handlers_oauth.go's
// parseConsentPayload). So an empty list shouldn't appear in
// practice; if it does, fail closed (no slug matches an empty list).
//
// Used by RequireWorkspaceAccess; package-private because the
// allow-list semantics are coupled to that middleware's flow.
func tokenAllowedWorkspaceMatches(ctx context.Context, slug string) bool {
allowed := TokenAllowedWorkspacesFromContext(ctx)
if allowed == nil {
return true // no token-level gate
}
for _, entry := range allowed {
if entry == "*" {
return true
}
if entry == slug {
return true
}
}
return false
}
// tokenScopeAllows checks if the token's scopes permit the given HTTP method
// and path. Scopes are stored as a JSON array of strings.
//
+23
View File
@@ -9,6 +9,8 @@ import (
"strings"
"github.com/ory/fosite"
"github.com/PerpetualSoftware/pad/internal/oauth"
)
// MCPBearerAuth is the auth gate for the /mcp Streamable HTTP endpoint.
@@ -282,6 +284,27 @@ func (s *Server) handleMCPOAuthAuth(w http.ResponseWriter, r *http.Request, toke
// policy applies to OAuth-issued tokens.
ctx = WithTokenScopes(ctx, oauthScopesToJSON(ar.GetGrantedScopes()))
// Stash the workspace allow-list set at consent time (TASK-952)
// so RequireWorkspaceAccess can gate workspace access per-token
// (TASK-953). Reading from session.Extra goes through the typed
// AllowedWorkspaces accessor so JSON round-trip handling
// ([]interface{} vs []string) is centralized in oauth.Session.
//
// Three shapes arrive here:
// - nil — pre-TASK-952 token (no consent payload). Treated by
// RequireWorkspaceAccess as "no token-level gate"; standard
// membership applies.
// - ["*"] — wildcard. Same effective behaviour as nil for the
// gate: any workspace the user is a member of is allowed.
// - [slug-a, slug-b, ...] — explicit allow-list. Workspaces
// NOT in this set are rejected before the membership check
// even runs.
if oauthSession, ok := session.(*oauth.Session); ok {
if allowed := oauthSession.AllowedWorkspaces(); allowed != nil {
ctx = WithTokenAllowedWorkspaces(ctx, allowed)
}
}
next.ServeHTTP(w, r.WithContext(ctx))
}
@@ -0,0 +1,108 @@
package server
import (
"context"
"testing"
)
// TestTokenAllowedWorkspaceMatches covers the policy table for the
// OAuth-token workspace allow-list (TASK-953):
//
// - nil allow-list → no gate; every slug allowed (PAT auth or
// pre-TASK-952 OAuth tokens fall here).
// - ["*"] wildcard → every slug allowed.
// - ["foo", "bar"] → only "foo" and "bar" allowed; "baz" denied.
// - explicit empty list → denies every slug (fail-closed; in
// practice the consent flow rejects empty lists at parse time,
// but the helper must not "open up" if one slips through).
// - ["foo", "*"] → wildcard wins; every slug allowed.
//
// Cheap pure-function unit test; the integration flow is covered
// separately in handlers_mcp_test.go's MCP+OAuth test surface.
func TestTokenAllowedWorkspaceMatches(t *testing.T) {
cases := []struct {
name string
allowed []string
slug string
want bool
}{
// nil → no gate (PAT auth or pre-TASK-952 token).
{"nil allows anything 1", nil, "foo", true},
{"nil allows anything 2", nil, "anything", true},
// Wildcard.
{"wildcard allows foo", []string{"*"}, "foo", true},
{"wildcard allows arbitrary", []string{"*"}, "anything-else", true},
// Explicit allow-list — only listed slugs match.
{"explicit foo matches foo", []string{"foo"}, "foo", true},
{"explicit foo,bar matches foo", []string{"foo", "bar"}, "foo", true},
{"explicit foo,bar matches bar", []string{"foo", "bar"}, "bar", true},
{"explicit foo,bar denies baz", []string{"foo", "bar"}, "baz", false},
{"explicit foo denies bar", []string{"foo"}, "bar", false},
// Defensive: empty (non-nil) list → fail-closed, no slug
// matches. Consent flow rejects empty allow-lists at parse
// time, so this case shouldn't occur in production.
{"empty list denies anything", []string{}, "foo", false},
// Mixed wildcard + specific entries — wildcard wins. A
// tampered token (or future feature) could have both; the
// safer interpretation is "any" because that's what the
// wildcard explicitly grants.
{"wildcard + specific allows arbitrary", []string{"foo", "*"}, "anything", true},
{"wildcard first allows arbitrary", []string{"*", "foo"}, "anything", true},
// Edge cases.
{"empty slug never matches explicit list", []string{"foo"}, "", false},
{"empty slug matches wildcard", []string{"*"}, "", true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
if tc.allowed != nil {
ctx = WithTokenAllowedWorkspaces(ctx, tc.allowed)
}
got := tokenAllowedWorkspaceMatches(ctx, tc.slug)
if got != tc.want {
t.Errorf("tokenAllowedWorkspaceMatches(%v, %q) = %v, want %v",
tc.allowed, tc.slug, got, tc.want)
}
})
}
}
// TestWithTokenAllowedWorkspaces_DefensiveCopy verifies the helper
// copies the input slice so a caller mutating after the call doesn't
// corrupt the per-request token state. This is the canonical
// defense-in-depth pattern for context values that callers might
// reuse across requests.
func TestWithTokenAllowedWorkspaces_DefensiveCopy(t *testing.T) {
original := []string{"foo", "bar"}
ctx := WithTokenAllowedWorkspaces(context.Background(), original)
// Mutate the original after stashing.
original[0] = "MUTATED"
got := TokenAllowedWorkspacesFromContext(ctx)
if len(got) != 2 || got[0] != "foo" || got[1] != "bar" {
t.Errorf("context value should be insulated from caller mutation; got %v", got)
}
}
// TestTokenAllowedWorkspacesFromContext_ReturnsCopy verifies the
// reader returns a fresh slice — a caller mutating the returned
// value must not corrupt the context-stored allow-list either.
func TestTokenAllowedWorkspacesFromContext_ReturnsCopy(t *testing.T) {
ctx := WithTokenAllowedWorkspaces(context.Background(), []string{"foo", "bar"})
got := TokenAllowedWorkspacesFromContext(ctx)
got[0] = "MUTATED"
// Re-read; the stored value must be unchanged.
got2 := TokenAllowedWorkspacesFromContext(ctx)
if got2[0] != "foo" {
t.Errorf("returned slice should be a copy; got2[0] = %q after caller mutation", got2[0])
}
}