fix(oidc): server-side intersect consent scopes; refuse to revive revoked entitlement rows #292
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/bunyip!292
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/BUNYIP-261-consent-scope-smuggling"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
BUNYIP-261: two of the consent-flow holes the 2026-06-30 audit named.
Scope smuggling.
POST /v1/users/me/consentspreviously wrotebody.scopesdirectly intouser_application_access.granted_scopeswithout checking the request against the client's registeredallowed_scopes. The consent screen took themissingset from the URL query, the user clicked Allow, and any scope name in that body landed forever. Combined with bunyip-web's consent.rs echoing the URLmissingparam verbatim into the hidden form field, a phishing link with?missing=admin.godcould permanently grant a fake scope for any client. The audit called this high-severity.Now: the handler loads the client and intersects body.scopes with
client.allowed_scopesbefore callingadd_scopes_to_grant. A request whose scopes ALL fall outside the allowlist returns 400 ("no requested scope is in this client's allowed_scopes") so the caller's intent isn't silently downgraded to a no-op. Unknown or disabled client_id falls through to 400. The structured log line on the audit / debug path now records bothrequestedandgrantedso the BUNYIP-234 consent-loop investigation can tell when an upstream caller asked for more than they got.The audit's larger fix (a signed consent_ticket that closes the URL-param trust chain entirely) is deferred to a follow-up; rewriting bunyip-web's consent UI + adding the ticket table is heavier than this PR's scope. The intersection check closes the immediate exploit even without the ticket rework.
Refuse to revive revoked rows.
grant_entitlementandadd_scopes_to_grantboth clearedrevoked_atviaON CONFLICT DO UPDATE. That made the JIT baseline path at the next /authorize silently re-grant any entitlement an admin had explicitly revoked, and any subsequent user-driven "Allow" click would undo an admin's revocation. AddWHERE user_application_access.revoked_at IS NULLto both UPDATE branches so a revoked row stays revoked until an explicit admin re-grant. New INSERT rows are unaffected because the WHERE clause only gates the ON CONFLICT branch.#BUNYIP-261
BUNYIP-261: two of the consent-flow holes the 2026-06-30 audit named. Scope smuggling. `POST /v1/users/me/consents` previously wrote `body.scopes` directly into `user_application_access.granted_scopes` without checking the request against the client's registered `allowed_scopes`. The consent screen took the `missing` set from the URL query, the user clicked Allow, and any scope name in that body landed forever. Combined with bunyip-web's consent.rs echoing the URL `missing` param verbatim into the hidden form field, a phishing link with `?missing=admin.god` could permanently grant a fake scope for any client. The audit called this high-severity. Now: the handler loads the client and intersects body.scopes with `client.allowed_scopes` before calling `add_scopes_to_grant`. A request whose scopes ALL fall outside the allowlist returns 400 ("no requested scope is in this client's allowed_scopes") so the caller's intent isn't silently downgraded to a no-op. Unknown or disabled client_id falls through to 400. The structured log line on the audit / debug path now records both `requested` and `granted` so the BUNYIP-234 consent-loop investigation can tell when an upstream caller asked for more than they got. The audit's larger fix (a signed consent_ticket that closes the URL-param trust chain entirely) is deferred to a follow-up; rewriting bunyip-web's consent UI + adding the ticket table is heavier than this PR's scope. The intersection check closes the immediate exploit even without the ticket rework. Refuse to revive revoked rows. `grant_entitlement` and `add_scopes_to_grant` both cleared `revoked_at` via `ON CONFLICT DO UPDATE`. That made the JIT baseline path at the next /authorize silently re-grant any entitlement an admin had explicitly revoked, and any subsequent user-driven "Allow" click would undo an admin's revocation. Add `WHERE user_application_access.revoked_at IS NULL` to both UPDATE branches so a revoked row stays revoked until an explicit admin re-grant. New INSERT rows are unaffected because the WHERE clause only gates the ON CONFLICT branch. #BUNYIP-261