mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-22 18:43:45 +00:00
xarmian
7d0de978f7
* feat(oauth): consent UI with workspace allow-list + capability tier (TASK-952)
Replace the inline-HTML stub from sub-PR C (TASK-1025) with the real
consent page described in PLAN-943: server-rendered HTML with
workspace multi-select, "any workspace" wildcard, and a capability-
tier radio (read / write / admin).
## What the page does
- Lists every workspace the user is a member of, with their role
shown next to each row (informational — TASK-953 does live role
resolution at MCP-call time).
- Wildcard checkbox grants "any workspace I currently or later have
access to," with a clear warning when checked. Mutually exclusive
with per-workspace boxes (vanilla JS for UX, server-side rejection
as the security gate).
- Capability tier radio is constrained to the intersection of
{pad:read, pad:write, pad:admin} and the client's requested
scopes — fosite's grant-time subset check (RFC 6749 §3.3) rejects
scopes outside the request, so the UI must never offer them. Default
selects the highest tier the client requested.
- Allow button stays disabled until ≥1 workspace (or wildcard) is
selected. Server-side validation enforces the same rule regardless
of JS state.
## Selective consent
This is the central security property. The decide handler now grants
*exactly* the chosen tier scope, NOT every requested scope. If the
client requests `pad:read pad:write` and the user picks "read", the
issued token has `scope=pad:read` only.
Bonus fix: removed redundant scope re-grant loop in handleOAuthToken
that would have expanded granted scopes back to the full requested
set on every /token exchange — a real security bug that the
auto-approve stub from sub-PR C masked because granted == requested
for that flow. fosite's flow_authorize_code_token.go:134-138 +
flow_refresh.go:91-103 copy GrantedScope/Audience automatically;
our loop was undoing selective consent.
## Workspace allow-list storage
The user's workspace selections live in `session.Extra["allowed_workspaces"]`
(round-trips via storage.go's existing JSON marshal). Either
`["*"]` for wildcard or a list of slugs. fosite's
WriteIntrospectionResponse serializes Extra into the introspection
response as top-level fields, so TASK-953's enforcement layer reads
them off `/oauth/introspect` (or in-process via fosite.IntrospectToken).
This sidesteps fosite's strict "granted ⊆ requested ⊆ client.Scopes"
check — clients don't request `pad:workspaces:foo`, but the consent
UI lets the user pick from their workspaces regardless. TASK-953
implements the live role resolution + workspace gate.
## Defense in depth
- Server validates `capability_tier ∈ {read, write, admin}` AND that
the chosen tier is among the client's requested scopes — fosite
would reject otherwise with a less-readable error.
- Server validates every non-wildcard slug is in the user's current
membership table. A tampered form sending other slugs gets 400.
- Wildcard wins: if a tampered POST sends both `*` and specific
slugs, the result is `["*"]` only — never partial allow-list.
## Tests
- TestConsent_RendersUserWorkspaces — multi-workspace list with role
labels.
- TestConsent_NoWorkspaces_ShowsEmptyState — clean empty state.
- TestConsent_TierRadios_OnlyRequestedScopes — UI hides tiers the
client didn't request.
- TestConsent_ApproveWithSpecificWorkspaces — happy path, asserts
introspection returns `allowed_workspaces=[alpha, beta]`.
- TestConsent_ApproveWithWildcard — wildcard yields `["*"]`.
- TestConsent_ApproveWithoutWorkspaceSelection_Rejected — 400 on
empty allow-list.
- TestConsent_ApproveWithUntrustedSlug_Rejected — defense in depth.
- TestConsent_ApproveWithUnrequestedTier_Rejected — server tier
validation matches UI's tier-radio constraint.
- TestConsent_TokenScopeMatchesTierChoice_Read — selective consent:
user picks read-only despite client requesting both, token has
exactly `pad:read`.
Existing tests + helpers updated to include the new consent fields
(`capability_tier`, `allowed_workspaces`).
* fix(oauth): prevent URL parameter pollution attack on consent UI (round 1)
Codex review #376 round 1 caught a P1 security bug in the consent
UI. The hidden-input round-trip used the full r.URL.Query() with
only `csrf_token` stripped, so a malicious OAuth client could craft
/oauth/authorize?...&capability_tier=admin&allowed_workspaces=*
and the consent form would render those as hidden inputs BEFORE the
user-controlled radios + checkboxes. On submit, the hidden values
precede the user's selection in the form encoding, so:
- r.FormValue("capability_tier") returns "admin" (first value
matches the attacker's, not the user's)
- r.PostForm["allowed_workspaces"] sees "*" first, the wildcard
scan matches, the result is ["*"] regardless of which boxes
the user actually checked
Net effect: a user clicking through the consent UI for "read-only,
just my docapp workspace" would silently authorize "admin, all
workspaces" — without any visible cue that the values were wrong.
Fix: build hidden inputs from an explicit allowlist of OAuth-standard
authorize-request parameters (response_type, client_id, redirect_uri,
scope, state, audience, resource, code_challenge, code_challenge_method,
nonce). Anything outside the allowlist is silently dropped. This is
strictly stronger than blocklisting consent-control names, because
it also defends against future OAuth extensions adding new attacker-
controllable params we haven't enumerated.
Test: TestConsent_URLPollution_DoesNotOverrideUserSelection simulates
the attack — GET /authorize with attacker params, asserts the rendered
HTML contains zero `<input type="hidden" name="<attacker_name>">`,
then completes the flow with the user's actual selection and
confirms the issued token's scope matches the user's choice
(pad:read), not the attacker's URL injection (pad:admin).