Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
838f56a
feat(api): require step-up consent for connected apps and run the OAu…
mforce Oct 9, 2026
6f24269
fix(api): supply OAuth:Issuer to every Production serving boot in CI …
mforce Oct 9, 2026
2624649
test(api): keep OAuth rate-limit tests inside one clock-aligned window
mforce Oct 9, 2026
b0fceb9
fix(api): ask for the password on every OAuth connection, a reconnect…
mforce Oct 9, 2026
dd48441
test(api): trim two consent test comments
mforce Oct 9, 2026
8c49881
chore(web): raise the precache ceiling to 1,920 KiB for the OAuth mil…
mforce Oct 9, 2026
8a594d5
feat(api): name the connected app on the sign-in screen before sign-in
mforce Oct 9, 2026
6ccbe3e
feat(web): consent screen for connected apps, login B, and one shared…
mforce Oct 9, 2026
1a81d73
feat(web): neutral write-permission copy, the 1,950 KiB precache ceil…
mforce Oct 9, 2026
c2c7c82
refactor(web): read the consent page's role once
mforce Oct 9, 2026
57c5f38
fix(web): keep Allow enabled on the consent card, as drawn
mforce Oct 9, 2026
b5524de
docs(design): count visible lab text without a tag-stripping regex
mforce Oct 9, 2026
6368134
test(api): follow two mutation anchors to the restructured anonymous …
mforce Oct 9, 2026
f044a04
Merge origin/main (#800) into feat/798-oauth-consent
mforce Oct 9, 2026
d4cc60d
fix(web): state an app's real reach on consent, and start focus at th…
mforce Oct 9, 2026
987dc79
docs: record what the consent payload's assignedFlocks states
mforce Oct 9, 2026
46e7ee0
test(web): drop a comment that repeats the page's own
mforce Oct 9, 2026
887a7b2
test(api): regenerate the reconnect mutant for the payload's new field
mforce Oct 9, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ jobs:
-e RateLimiting__AllowNoTrustedProxies=true \
-e "Jwt__PrivateKeyPem=$JWT_PRIV" -e "Jwt__PublicKeyPem=$JWT_PUB" \
-e "DataProtection__CertificatePem=$DP_CERT" -e "DataProtection__PrivateKeyPem=$DP_KEY" \
-e OAuth__Issuer=https://cluckwork-ci.local/ \
-e Jwt__Issuer=cluckwork-ci -e Jwt__Audience=cluckwork-api-ci \
cluckwork-api:ci

Expand Down
5 changes: 5 additions & 0 deletions deploy/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ Jwt__PrivateKeyPem="-----BEGIN PRIVATE KEY-----\nreplace-me\n-----END PRIVATE KE
# docs/runbooks/data-protection-key-ring.md. One-shot verbs never need it.
DataProtection__CertificatePem="-----BEGIN CERTIFICATE-----\nreplace-me\n-----END CERTIFICATE-----"
DataProtection__PrivateKeyPem="-----BEGIN PRIVATE KEY-----\nreplace-me\n-----END PRIVATE KEY-----"
# The OAuth authorization server for connected apps (#798) names itself with this URL, and
# its discovery document points apps at endpoints under it. Set it to the public https URL
# people use to reach Cluckwork, with a trailing slash. The SERVING process refuses to start
# in Production without it, or when it is not an absolute https URL. One-shot verbs never need it.
OAuth__Issuer=https://cluckwork.local/

# Base provisioning (#283): the default account, the four assignable roles,
# and the default egg grades ship AS PART OF the EF migrations (static
Expand Down
2 changes: 1 addition & 1 deletion docs/decisions/795-openiddict-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
> insufficient, what not to break). The design it implements is in
> [`docs/plans/788-mcp-oauth/`](../plans/788-mcp-oauth/00-README.md).

**Status:** accepted
**Status:** accepted; the Production gate is superseded by [`798-oauth-consent.md`](798-oauth-consent.md)
**Date:** 2026-10-08

## What happened
Expand Down
134 changes: 134 additions & 0 deletions docs/decisions/798-oauth-consent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# Consent with step-up, and the OAuth server in Production (#798)

> **Rule** — the one-paragraph version lives in [`src/AGENTS.md`](../../src/AGENTS.md);
> this file is the rationale. The design it implements is in
> [`docs/plans/788-mcp-oauth/`](../plans/788-mcp-oauth/02-design.md) and the
> "Consent screen: decided" comment on #788.

**Status:** accepted
**Date:** 2026-10-09

## What happened

No incident. #795 shipped a stub: a caller signed in with a session bearer approved its
own authorization request, and Production ran no authorization server. This slice
replaces the stub with consent and turns the server on in Production.

## The flow

The SPA keeps its access token in tab memory, so a browser navigation to the authorize
endpoint carries no bearer, and `fetch` cannot read a cross-origin redirect. The
endpoint therefore answers three kinds of caller:

1. **A navigation** (no bearer). OpenIddict has already validated the client, the
redirect URI, PKCE and the scopes. The endpoint redirects to the SPA's `/connect`
route with the same query. The path is relative, so it cannot leave this origin.
2. **The SPA asking** (session bearer). The endpoint returns what the consent screen
shows, `{ clientId, clientName, redirectHost, scopes, alreadyAllowed, alreadyApproved, assignedFlocks }`,
or, when it issues a code or an error, `{ redirectUri }` for the SPA to navigate to. An inline
handler on `ApplyAuthorizationResponseContext` writes that JSON for any request that
carries a bearer. Response modes other than `query` are removed, so the redirect is
always one URL.
3. **An expired bearer.** 401, so the SPA refreshes and asks again. A redirect to
`/connect` here would hand `fetch` the SPA's HTML.

The SPA approves by repeating its request with `X-Cluckwork-Step-Up` and cancels with
`X-Cluckwork-Consent: deny`, which sends `access_denied` to the client. Approval and
cancel stay on the GET authorize endpoint because #796 refuses any other method there.

## Consent

- **All or nothing.** The token gets every requested scope or none. An empty `scope`
defaults to `farm:read`, the narrower of the two; the screen shows what it gets.
- **Step-up.** `IAccessModule.ConsumeStepUpGrantAsync` validates and spends a #308 grant,
exactly as the user-administration handlers do. `IStepUpGrantService` stays internal to
Access (#857).
- **Every code costs the password.** No path issues a code without a spent step-up
grant, a reconnect included. Approval creates a *permanent* authorization holding the
requested scopes. When one valid permanent authorization of this user for this app
already holds every requested scope, the payload says `alreadyApproved`, the SPA asks
for the password only, and the approval reuses that authorization. A request for more
sets `alreadyApproved` to false and shows the permissions again, with `alreadyAllowed`
naming what an earlier approval covered.
- **What the app would reach.** `assignedFlocks` is the flock scope this very request
resolved (#388, #612): null when the user reaches every flock, which includes a Worker
with no assignments, else the assigned flocks' names, read through `IAccessModule`.
The screen states the requested scopes within that scope and the role, never what the
role name alone suggests.
- **The redirect host** comes from the redirect URI OpenIddict validated, not the
request's `redirect_uri`. A client with one registered URI may omit the parameter, and
OpenIddict then keeps the registered one on its validation context.
- **Disconnect.** Revoking the authorization revokes every token it issued (#796), and a
revoked authorization never skips consent. Two approvals racing can create two
authorizations, so #799's Disconnect must revoke every valid authorization of the user
for that app, not one.

## A reconnect skips the permissions, never the password

#788 decided to skip re-approval when an app asks for nothing new. The first version of
this slice skipped the password too, and review found what that cost. Someone holding
only a stolen session access token (the #308 threat) could ask the authorize endpoint, as
the SPA does, for an app the user had approved, with their own PKCE challenge. The JSON
response handed them a code, which redeemed for an access token that lives until revoked,
far beyond the 15-minute session token. #308 exists to stop that kind of escalation.

The maintainer decided on 2026-10-09: a reconnect skips the permission explanation but
still asks for the password. App tokens never expire, so a person reconnects an app
rarely, and the password costs little each time. What #788 wanted to avoid was people
clicking through the same permission list; the list is still skipped.

Rejected: validating the refresh cookie as an independent browser proof. It keeps a
reconnect silent, but needs a new proof endpoint under the cookie's path and its own
replay and substitution tests, and it still falls to full same-origin script execution.

## Production

`OAuthIssuer()` no longer exempts Production. A Production serving process requires
`OAuth:Issuer` to be an absolute https URL with no query or fragment, and refuses to
start otherwise. The guard is serving-only (#347), so one-shot verbs run without it.
Outside Production the issuer stays optional, as #795 chose. OpenIddict's HTTPS
requirement stays on outside Development, so a Production deployment must present
https to the API, directly or through a trusted proxy (#260).

**Discovery names endpoints under the issuer**, not under the request's Host. #795's
discovery followed the Host the client called; behind a proxy that can be an internal
name. The issuer is the one URL the deployment declares as public, so the
authorization, token and registration endpoints are rebased onto it.

The sim harness supplies `https://cluckwork-sim.local/`. That stack is plain http, so
OpenIddict refuses its protocol requests; only the boot needs the value. The AppHost runs
Development, where `appsettings.Development.json` already sets an issuer.

## The return path after sign-in

Sign-in returns to the route the user came from, carried in router state, never in a
query parameter. `returnPath` still resolves it against the page's origin and goes home
for anything that lands elsewhere: an absolute URL, `//host` and `/\host`. `/connect`
keeps its query, which is the app's request; every other route returns without one, as
before.

## What this does NOT cover

- **The consent and login screens.** The SPA route, the login notice, the shared step-up
password component, translations, Help and the glossary follow in this PR once the
maintainer picks a direction.
- **`prompt` and `max_age`.** Ignored, so `prompt=consent` cannot force the screen.
- **Audit events for connect and cancel.** Not decided (#788's mockup question 4).

## How it is enforced

- `OAuthConsentTests` drives each rule: the consent payload and no row before approval,
a refused and a replayed grant, the JSON redirect with `state` and `iss`, a reconnect
that gets no code from the bearer alone and spends its grant, a wider request, another
user's approval, a revoked approval, cancel, a signed session expired past the clock
skew, a malformed bearer, an omitted `redirect_uri`, the read-only default, an unknown
scope, and discovery from another host.
- `OAuthServerTests.AuthorizationRequest_WithoutASignedInUser_GoesToTheConsentRoute`
pins the navigation hand-off.
- `OAuthServerProductionTests` runs the whole flow in a Production host and refuses
three bad issuers.
- `ProcessRoleGuardTests` has a row per issuer violation; `ServingGuardCoverageTests`
maps `EnsureOAuthIssuer` to both.
- `web/src/auth/returnPath.test.ts` and `Login.test.tsx` cover the return path.
- `tools/oauth/mutation-check.sh` applies one mutation per claim and requires the named
test to fail.
1 change: 1 addition & 0 deletions docs/decisions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Starting a new record: copy [`TEMPLATE.md`](TEMPLATE.md).
| [OpenIddict outside Production only, on the shared Data Protection ring (#795)](795-openiddict-server.md) | src/AGENTS · Auth and credentials |
| [OAuth tokens run the session chain, on opted-in endpoints only (#796)](796-oauth-fail-closed.md) | src/AGENTS · Auth and credentials |
| [OAuth client self-registration and the OAuth purge sweep (#797)](797-oauth-client-registration.md) | src/AGENTS · Auth and credentials |
| [Consent with step-up, and the OAuth server in Production (#798)](798-oauth-consent.md) | src/AGENTS · Auth and credentials |
| [Nothing writes an audit event without an actor (#500)](500-audit-actor.md) | AGENTS · Conventions |
| [Break-glass recovery: `recover-admin` (#265)](265-break-glass-recovery.md) | AGENTS · Conventions · and the [runbook](../runbooks/break-glass-account-recovery.md) |
| [Farm timezone, and the tzdata/ICU constraint (#264)](264-farm-timezone.md) | AGENTS · Conventions |
Expand Down
Loading
Loading