diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea60741d1..f2518849a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/deploy/.env.example b/deploy/.env.example index 17ece4ce3..6c1404fa7 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -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 diff --git a/docs/decisions/795-openiddict-server.md b/docs/decisions/795-openiddict-server.md index ae918712c..f5c5d7536 100644 --- a/docs/decisions/795-openiddict-server.md +++ b/docs/decisions/795-openiddict-server.md @@ -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 diff --git a/docs/decisions/798-oauth-consent.md b/docs/decisions/798-oauth-consent.md new file mode 100644 index 000000000..6424feb0d --- /dev/null +++ b/docs/decisions/798-oauth-consent.md @@ -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. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index bfd5b6bfb..2cf17b8ac 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -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 | diff --git a/docs/designs/788-oauth-screens/README.md b/docs/designs/788-oauth-screens/README.md new file mode 100644 index 000000000..63fa17c10 --- /dev/null +++ b/docs/designs/788-oauth-screens/README.md @@ -0,0 +1,101 @@ +# #798, #799, #800: OAuth screen directions + +Mockups for the OAuth milestone's screens, for the maintainer to choose from before the slices build them. Open `oauth-direction-lab.html` in a browser. It has switches for screen, direction, state, signed-in role (consent only), width (1280 or 390) and night mode. It is a prototype: English source copy, sample data, no API calls. Nothing is sent, saved or connected. + +The lab uses the live app's tokens, type and controls, copied from `web/src/styles.css` and `web/src/theme/FarmThemeProvider.tsx`: the aubergine palette, Inter with Georgia titles, 4px controls, 8px panels, 12px dialogs, the lavender rail, status words with an 8px dot. Nothing new is invented. Account and Audit sit inside the maintainer's chosen C "Focus panels" compositions (`../674-tail-redesign/SELECTION.md`), and the login screens use the chosen Login B card (`AuthShell.tsx`). + +## Naming + +The maintainer decided the user-facing term is **Connected apps**. OAuth calls these programs "clients", and the code and issues may keep that word. Every label, heading and line of copy in the UI says "app" or "Connected apps", never "assistant" or "AI". The action is **Disconnect** everywhere, for a person's own apps and on the Owner's view. Consent still names the specific app ("Allow Claude Desktop to act as you?"). + +`node shoot.mjs` regenerates the 207 renders into `renders/` at 1:1 and prints each consent direction's word count. Names follow `---.png`. A `-viewport` suffix means one 1280x800 or 390x844 screen scrolled to the new part. Without it the frame is the whole page. Extra frames are `consent---role-` (Worker is the default state) and a `-dark` frame for each screen's recommended direction. The renders are not committed; they attach to the issues, following `941-expand-chart`. + +## Consent (#798) + +Every direction carries what #788 requires: the app's name, the two permissions, that it acts as you and can never do more than you can, where to undo it (Account, then Connected apps), the password step, and Allow or Cancel. It also shows two safety facts: the app chose its own name and Cluckwork does not check it, and where the browser returns afterwards. + +### The compact rework: D, E and F + +The maintainer found A to C far too wordy (145 to 161 words before the password field), so D to F say the same things in under about 40 words. They share these parts: + +- One headline question, "Let Claude Desktop act as you?", with an **Unverified app** badge beside it. +- Two permission rows with Lucide icons: "Read farm data" and "Record daily entries", each with at most one short line. +- One "acts as you" line, "Acts as you, with only your access", with a chip carrying the user's role. The chip reads "Your role: Worker" to a screen reader. +- "Returns to this computer". The address, 127.0.0.1, is under Details. +- The password field, then Allow and Cancel. +- Under the buttons, "Disconnect it anytime in Account › Connected apps." and a closed **Details** disclosure with the full explanations: the role sentence, the unverified name, the return address, what reading covers, and "Signed in as … Not you? Sign out". + +Focus order is headline, permissions, password, Allow, Cancel, then Details. The headline and rows are text, so a screen reader reaches them in that order. + +| | Direction | Words before the password | What it does | +|---|---|---|---| +| D | Compact card | 39 | A narrow card on the canvas. The farm and signed-in email sit above it, and the shared parts stack inside it. | +| **E** | **Login card** | **40** | The Login B card the user just signed in through. The brand panel holds Cluckwork, the farm and the signed-in email; the form side carries the request. On the phone the brand panel shrinks to one band. | +| F | Ledger | 29 | A labelled ruled list: Can, Acts as, Returns to. One value per row and no description lines, so the scope detail moves into Details. | + +States for D to F: default, Details open, wrong password ("Wrong password. Nothing was connected." in a live region), asks for more (the headline becomes "Let Claude Desktop do more?", the new permission comes first with a tint and a "New" marker, and the old one drops to a muted "Already allowed" line), refused ("Nothing to approve", one line, one button), long strings, and the four roles. For Read-only, the "Record daily entries" row says "Not allowed for your role" in place of its description, because the app cannot do what the role cannot. + +**Recommendation: E.** The maintainer chose Login B, so a user who has to sign in sees the request in the same card they just used, and one who is already signed in sees a card they recognise. The brand panel also answers "whose account is this?" without spending words in the form. F is the tightest; if E still reads as too much, its ledger rows could replace E's permission rows. + +### A, B and C (history) + +The first three directions stay in the lab, marked as history: A, the sign-in panel; B, a plain statement; and C, an "asks vs gets" table. They carry the full explanations in prose. C's per-role table is still the clearest way to show what a Read-only user's app gets, and it could return inside Details if wanted. + +### Login, continuing to consent + +**Chosen: B.** At 1280 the brand panel's tagline gives way to a box reading "Next: approve Claude Desktop. After you sign in, you see what it asks for and choose whether to allow it." The form is unchanged. At 390 the brand panel is a short band with no room, so the same box sits above the form instead (`login-b-390-*.png`). + +A (a notice above the form) and C (a two-step header) stay in the lab for comparison. + + +## Connected apps on Account (#799, the user's own) + +A third focus panel after Change password, titled "Connected apps". It lists name, permissions in the consent screen's words, connected date, last used, and Disconnect. States: two connected apps, the Disconnect confirmation, after disconnecting ("It stops working on its next request"), empty, and long strings. + +| | Direction | What it does | +|---|---|---| +| **A** | **Ruled list** | DIRECTION.md's row rule: a text action at 1280 and a full-width 44px button at 390. | +| B | One card each | An outlined card per app with an "In use" or "Not used for 47 days" status word. | +| C | Sentence and disclosure | "2 connected apps can act as you", a nudge to disconnect the idle one, and each app as a disclosure row. | + +**Recommendation: A.** It reuses the row pattern the ledgers already use and stays short when a person has one or two apps. B's idle status word is worth carrying into A's Last used column. + +## The Owner's farm-wide view (#799) + +The directions differ by placement, which is the open decision. Each shows every connection with person, role, app, permissions, dates and Disconnect, plus the line explaining why an Owner may do this ("does less than disabling the person"). + +| | Placement | Trade-off | +|---|---|---| +| A | A "Connected apps on this farm" section under the Users ledger | It sits next to the person it belongs to. The Users page gets longer, and on a phone the section is below every user. | +| B | A "Connected apps on this farm" panel in Farm settings, grouped by person | Farm settings is already Owner-only (#729). But settings are about the farm's configuration, and connections are about people. | +| **C** | **Its own Setup page, "Connected apps"** | It has room for a person filter and grows when apps do. It costs one more nav entry; on the phone it sits under More. | + +**Recommendation: C.** It answers the question #799 gives this view ("is a departed employee's app still connected?") in one place, with a filter, and does not lengthen Users or Settings. + +## Audit provenance (#800) + +The actor stays the human (#500). The application is added beside it. Sample rows use the existing `Daily entry created` and `Daily entry draft edited` actions, which are what the phase-1 write tool produces. + +| | Direction | Row | Filter | +|---|---|---|---| +| A | Tag in the summary | A "via Claude Desktop" status word at the end of the summary line | A fifth field, "Made through": Everything, The Cluckwork app, Any connected app, or one app | +| B | Who block | A plug icon in the summary; Who and Through rows in the body | A segmented Everyone, People, Connected apps control, and an App field once Connected apps is chosen | +| **C** | **Actor line on every row** | Every summary gets a second line with the actor's email, plus "via app" when one acted | A checkbox, "Only actions through connected apps", and a "Show only Claude Desktop" button inside an event | + +**Recommendation: C.** Today a reader has to open each event to see who acted. C shows the actor on every row, so "Ana, or an app acting as Ana?" is answered without expanding anything, which is the gap #800 names. Its pivot button gives the per-app filter without a fifth field. A's fifth field is the fallback if the maintainer wants app filtering in the filter grid. + +## Accessibility in the mockups + +- Every field has a visible label tied to its input. The password errors are in `aria-live` regions with `role="alert"`, and the field gets `aria-invalid`. +- The focus ring is the app's 2px `--focus` outline. Disconnect buttons in a list carry the app's name in their accessible name ("Disconnect Claude Desktop"). +- Focus order follows reading order. On consent D to F that is: headline, permissions, password, Allow, Cancel, Details. The confirmation dialog takes focus on its confirm button, matching `useConfirm`. +- Long strings add 40% to every visible string. The long app name tests wrapping in titles, table cells and dialog titles. + +## Open questions for the maintainer + +1. **Where does the Owner's farm-wide view go?** The recommendation is C, its own Setup page. +2. **App names are self-chosen.** Dynamic registration lets any app call itself "Claude Desktop". Every consent direction says so and shows where the answer returns to (127.0.0.1 for a desktop client). Keep this line, soften it, or add a verified-app marker later? +3. **"A person still submits them."** That is true for phase 1, where the write tool records but does not submit. The copy needs to change if a later phase adds submitting. +4. **Are connect and disconnect themselves audit events?** None of the issues say. If they are, the audit filter needs no change, but the action list gains two entries. +5. **The role label says "Admin".** The issues say "Owner"; the app shows `Admin`. The mockups follow the app. +6. **"The Cluckwork app" beside "connected apps".** Audit A's filter offers "The Cluckwork app" for changes a person made in Cluckwork itself, next to "Any connected app". Both use the word app. If that reads as ambiguous, the first option could say "People in Cluckwork". diff --git a/docs/designs/788-oauth-screens/SELECTION.md b/docs/designs/788-oauth-screens/SELECTION.md new file mode 100644 index 000000000..aa4d222d7 --- /dev/null +++ b/docs/designs/788-oauth-screens/SELECTION.md @@ -0,0 +1,32 @@ +# OAuth screens: selected directions + +The maintainer's picks from `oauth-direction-lab.html`, as of 2026-10-09: + +| Screen | Issue | Direction | Status | +| --- | --- | --- | --- | +| Login, continuing to consent | #798 | B, the "Next: approve " box | Built in #798 | +| Consent | #798 | D, Compact card, with a prominent email | Built in #798 | +| Reconnect (an app already approved for everything it asks) | #798 | A slim password prompt in D's card | Built in #798 | +| Audit provenance | #800 | C, the actor line, with B's plug icon | Built in #800 (PR #1138) | +| Connected apps on Account | #799 | Pending | | +| The Owner's farm-wide view | #799 | Pending | | + +## Terms + +The UI says **Connected apps** for what OAuth calls clients, and **Disconnect** for revoking one, everywhere. + +## Login B + +At 1280 the box sits in the brand panel in place of the tagline. At 390 the same box sits above the form. The app name comes from the server before sign-in, and it is the app's own choice. + +## Consent D, with the maintainer's change + +D as drawn: the headline question, an **Unverified app** marker, two permission rows with icons, the return line, "Acts as you, with only your access" with a role chip, the password, Allow and Cancel, the undo line under the buttons, and a closed **Details** section with the longer explanations. + +The maintainer's change: the signed-in account's email sits near the top in normal text colour and semibold weight, so it reads as which account is approving, not as a caption. **Not you? Sign out** is in Details, which keeps the visible words under budget. + +The write permission reads "Record daily entries" with no line about drafts or submitting. #809 adds that line once the write tool's behaviour is settled. + +## Reconnect + +When the payload says `alreadyApproved`, the card asks only for the password: the headline "Reconnect ?", the Unverified marker, one line, the password, Allow and Cancel, and the undo line. It shows no permission rows and no Details. Every connection still spends a step-up grant (`docs/decisions/798-oauth-consent.md`). diff --git a/docs/designs/788-oauth-screens/oauth-direction-lab.html b/docs/designs/788-oauth-screens/oauth-direction-lab.html new file mode 100644 index 000000000..37616bd47 --- /dev/null +++ b/docs/designs/788-oauth-screens/oauth-direction-lab.html @@ -0,0 +1,1065 @@ + + +Cluckwork · OAuth screens · direction lab (#798, #799, #800) + + + +
+ OAuth screens · #798 #799 #800 + + + + + + +
+

Prototype. English source copy and sample data. Nothing is sent, saved or connected. Tokens, type and controls are the live app's (aubergine palette); Account and Audit sit inside the chosen C "Focus panels" compositions.

+
+ + + diff --git a/docs/designs/788-oauth-screens/shoot.mjs b/docs/designs/788-oauth-screens/shoot.mjs new file mode 100644 index 000000000..9a1e28f02 --- /dev/null +++ b/docs/designs/788-oauth-screens/shoot.mjs @@ -0,0 +1,69 @@ +// Renders every screen x direction x width of oauth-direction-lab.html at 1:1. +// Usage: node shoot.mjs [outDir] (default: ./renders beside this file) +// Playwright and the Inter font come from the main checkout's installed +// node_modules; override with PLAYWRIGHT_DIR / INTER_WOFF2. +import { mkdirSync, readdirSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PW = process.env.PLAYWRIGHT_DIR ?? '/home/mforce/dev/cluckwork/tools/simulation/ui/node_modules/playwright'; +const INTER = process.env.INTER_WOFF2 ?? '/home/mforce/dev/cluckwork/web/node_modules/@fontsource-variable/inter/files/inter-latin-opsz-normal.woff2'; +const OUT = process.argv[2] ?? join(HERE, 'renders'); +const { chromium } = await import(pathToFileURL(join(PW, 'index.mjs')).href); +mkdirSync(OUT, { recursive: true }); + +const DIRS = ['a', 'b', 'c']; +const VPS = [1280, 390]; +// full: the whole page, top to bottom. fixed: one 1280x800 / 390x844 viewport, +// scrolled to the part the screen adds (needed for dialogs and live errors). +const PLAN = { + consent: { full: ['normal', 'details', 'wrong', 'widened', 'refused', 'long'], fixed: [] }, + login: { full: ['normal', 'wrong', 'long'], fixed: [] }, + account: { full: ['normal', 'long'], fixed: ['normal', 'confirm', 'after', 'empty'] }, + owner: { full: ['normal', 'long'], fixed: ['normal', 'confirm', 'after', 'empty'] }, + audit: { full: ['normal', 'filtered', 'long'], fixed: [] }, +}; +const RECOMMENDED = { consent: 'e', login: 'b', account: 'a', owner: 'c', audit: 'c' }; +// Consent A-C are kept as history; D-F are the compact rework. 'details' only exists in D-F. +const dirsFor = (screen) => screen === 'consent' ? ['a', 'b', 'c', 'd', 'e', 'f'] : DIRS; +const skip = (screen, dir, state) => screen === 'consent' && state === 'details' && 'abc'.includes(dir); + +const SHOTS = []; +for (const [screen, { full, fixed }] of Object.entries(PLAN)) + for (const dir of dirsFor(screen)) for (const vp of VPS) { + for (const state of full) if (!skip(screen, dir, state)) SHOTS.push([`${screen}-${dir}-${vp}-${state}`, { screen, dir, vp, state, full: true }]); + for (const state of fixed) SHOTS.push([`${screen}-${dir}-${vp}-${state}-viewport`, { screen, dir, vp, state, full: false }]); + } +for (const dir of dirsFor('consent')) for (const role of ['Admin', 'Manager', 'ReadOnly']) + SHOTS.push([`consent-${dir}-1280-role-${role}`, { screen: 'consent', dir, vp: 1280, state: 'normal', role, full: true }]); +// The compact directions get the role variants at 390 too. +for (const dir of ['d', 'e', 'f']) for (const role of ['Admin', 'Manager', 'ReadOnly']) + SHOTS.push([`consent-${dir}-390-role-${role}`, { screen: 'consent', dir, vp: 390, state: 'normal', role, full: true }]); +SHOTS.push(['consent-e-390-dark', { screen: 'consent', dir: 'e', vp: 390, state: 'normal', theme: 'dark', full: true }]); +for (const [screen, dir] of Object.entries(RECOMMENDED)) + SHOTS.push([`${screen}-${dir}-1280-dark`, { screen, dir, vp: 1280, state: 'normal', theme: 'dark', full: true }]); + +// The installed Playwright may expect a browser build other than the one +// downloaded; fall back to whatever headless shell the cache holds. +const cache = join(homedir(), '.cache/ms-playwright'); +const shell = readdirSync(cache).filter((d) => d.startsWith('chromium_headless_shell-')).sort().pop(); +const browser = await chromium.launch().catch(() => + chromium.launch({ executablePath: process.env.CHROMIUM_PATH ?? join(cache, shell, 'chrome-headless-shell-linux64/chrome-headless-shell') })); +const errors = [], counts = []; +const page = await browser.newPage({ viewport: { width: 1320, height: 900 }, deviceScaleFactor: 1 }); +page.on('pageerror', (e) => errors.push(e.message)); +await page.goto(pathToFileURL(join(HERE, 'oauth-direction-lab.html')).href); +await page.addStyleTag({ content: `@font-face{font-family:"Inter Variable";src:url("${pathToFileURL(INTER).href}") format("woff2");font-weight:100 900}.lab,.labnote{display:none!important}.stage{padding:0!important}` }); +await page.evaluate(() => document.fonts.ready); +for (const [name, cfg] of SHOTS) { + await page.evaluate((c) => window.configure({ role: 'Worker', theme: 'light', ...c }), cfg); + await page.waitForTimeout(60); + await page.locator('#frame').screenshot({ path: join(OUT, name + '.png') }); + if (cfg.screen === 'consent' && cfg.state === 'normal' && !cfg.role && !cfg.theme) + counts.push(`${name}: ${await page.evaluate(() => window.wordsBeforePassword())} words before the password field`); +} +console.log(counts.join('\n')); +await browser.close(); +console.log(errors.length ? 'PAGE ERRORS:\n' + errors.join('\n') : `captured ${SHOTS.length} frames into ${OUT}, no page errors`); diff --git a/docs/designs/822-mui-revamp.md b/docs/designs/822-mui-revamp.md index 32e246dac..5065df345 100644 --- a/docs/designs/822-mui-revamp.md +++ b/docs/designs/822-mui-revamp.md @@ -388,6 +388,8 @@ Ceiling **1,900 KiB**, enforced in `scripts/verify-sw.mjs` (it sums the precache **Amended 2026-10-09: the ceiling is 1,920 KiB.** The maintainer approved one raise for the OAuth milestone (#800, #798, #799), after #800 measured 1,903.38 KiB against `main`'s 1,897.12. The rest of the headroom is reserved for #798 and #799; the next raise needs a new maintainer decision. `verify-sw.mjs` carries the same record. +**Amended again 2026-10-09: the ceiling is 1,950 KiB.** The maintainer approved a second raise for the milestone. #800 measures 1,903.38 KiB and #798 adds 19.15 KiB on the same base, about 1,922.5 KiB together; the rest is reserved for #799. Not precaching the languages a device does not use (es/tl) is the planned way to win the space back, tracked separately. Any further raise needs a new maintainer decision. + **Script-duration ceiling: also re-enforced by #825, as a wide regression tripwire rather than a tight budget**, for a reason the precache ceiling does not share: byte counts are environment-independent, but a CI runner's CPU is shared and variable run to run, so a tight number here would be exactly the "wrong guard that reads as safety" the owner's own 2026-09-16 comment warned against for precache. `tools/simulation/ui/specs/dashboard-script-duration.spec.ts` measures Dashboard's script execution time via CDP `Performance.getMetrics`, median of 5 runs against the real seeded stack, and fails only past a 600 ms tripwire sized generously above ordinary noise — printed every run, gating only a multi-fold regression (the record's own named unmeasured cases, `Autocomplete` and a data grid). ### D10. Coverage: the constraint nobody filed diff --git a/docs/plans/788-mcp-oauth/00-README.md b/docs/plans/788-mcp-oauth/00-README.md index 2f4cd3f1e..7f8c58e6d 100644 --- a/docs/plans/788-mcp-oauth/00-README.md +++ b/docs/plans/788-mcp-oauth/00-README.md @@ -1,6 +1,6 @@ # MCP authentication — OAuth 2.1 via OpenIddict (#788) -**Status: design. Slice #795 (the OpenIddict server, outside Production only) is implemented; see [`795-openiddict-server.md`](../../decisions/795-openiddict-server.md).** +**Status: design. Slices #795 to #798 are implemented; see [`795-openiddict-server.md`](../../decisions/795-openiddict-server.md) and [`798-oauth-consent.md`](../../decisions/798-oauth-consent.md).** #788 began as a question: MCP clients expect OAuth discovery and a refreshable token, and Cluckwork's refresh is an HttpOnly cookie, so nobody could actually connect an assistant. This directory is the diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index a9926431e..e36103b6d 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -1419,6 +1419,20 @@ Trusted offline provisioning, seeding, and `recover-admin` commands use their separate one-shot trust boundary and never select a browser-visible bypass. TOTP/WebAuthn step-up is a deferred follow-up (#320). +**Connecting an app (#798)** — approving an outside app, such as an AI assistant, +to act as you through OAuth. The app sends you to Cluckwork's consent screen +(`/connect`), signing in first if needed; the sign-in screen names the app it +continues to. The screen shows the app's self-chosen name with an **Unverified +app** marker, what it asks for (**Read farm data**, **Record daily entries**), +that it acts as you with only your access, and where the browser returns. It is +all or nothing: **Allow** after re-entering **Your current password** (a +**step-up grant**, see above), or **Cancel**. Every approval spends a step-up +grant. When an app you already approved asks for nothing new, the screen skips +the permissions and asks only for the password; when it asks for more, the new +permission is marked **New** and the earlier one **Already allowed**. +**Details** holds the longer explanations and **Not you? Sign out**. The app's +access never expires on its own; disconnecting it is #799's job. + **First-run admin provisioning (#283)** — how a fresh deploy gets its first Owner without ever shipping a repo-known credential. The default account, the four assignable roles, and the default egg grades are **static reference diff --git a/src/AGENTS.md b/src/AGENTS.md index f84655ed0..91639c5a6 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -52,9 +52,10 @@ record you must read before changing the rule. Persistence edits also read - **Auth:** asymmetric JWTs with rotating refresh tokens. Config stores PEM keys with escaped `\n`; call `PemKey.Normalize` before `ImportFromPem`. Integration tests generate an ephemeral RSA pair at process startup (`TestJwtKeys` in `CluckworkWebApplicationFactory.cs`). Never commit real key material. - **Both JWT keys are checked at boot, and the check is serving-only (#510/#347).** `AddCluckworkIdentity` requires both keys non-blank **and importable**; use `IsNullOrWhiteSpace`, never `??` (the shipped `appsettings.json` carries `""`, which `??` does not catch), and import at boot rather than inside the `AddJwtBearer` delegate (which makes a corrupt key a per-request 500 behind a green health check). → [`510-jwt-key-boot-check.md`](../docs/decisions/510-jwt-key-boot-check.md) - **Credential epoch revocation (#364).** Every access/refresh token is bound to the user's `CredentialEpoch` and every password-reset path bumps it. Epoch `0` is permanently retired, a missing or malformed claim is **always** a mismatch, and `CredentialEpochMiddleware` does a fresh DB read per authenticated request through `ICredentialEpochVerifier` (#857) — the round trip *is* the fail-closed guarantee, so do not cache it. The mismatch holds whatever the database holds (#1031): `CK_AspNetUsers_CredentialEpoch` makes a stored epoch below 1 unwritable, and login, the verifier and refresh each refuse one anyway. Never repair a bad row by setting it to 1; that revives outstanding epoch-1 credentials. **Break it and a revoked credential keeps working.** → [`364-credential-epoch-revocation.md`](../docs/decisions/364-credential-epoch-revocation.md) -- **The OAuth server runs outside Production only (#795).** `AddCluckworkIdentity` registers OpenIddict only for a serving process outside Production with `OAuth:Issuer` set; Production registers no OpenIddict service and maps no OAuth endpoint, whatever its configuration, until consent with step-up (#798) exists. The issuer comes from configuration, never the request Host (#538). Codes and reference access tokens use the Data Protection format, so the shared ring (#794) is what lets one replica redeem another's code. OpenIddict demands a signing and an encryption key, but with that format only identity tokens use them, so they are ephemeral, `openid` is removed from the registered scopes (OpenIddict otherwise grants it implicitly) and the key-set endpoint is unmapped; never grant `openid` without a persistent key. A code filter drops every `OpenIddict*` event below Warning, because its Information dumps carry the PKCE verifier; an override cannot replace it, since a more specific child override wins. `tools/oauth/mutation-check.sh` proves each claim's test can fail. → [`795-openiddict-server.md`](../docs/decisions/795-openiddict-server.md) +- **The OAuth server runs on its configured issuer (#795/#798).** `AddCluckworkIdentity` registers OpenIddict for every Production serving process, which refuses to start unless `OAuth:Issuer` is an absolute https URL with no query or fragment (serving-only, #347); elsewhere a serving process runs it only when `OAuth:Issuer` is set. The issuer comes from configuration, never the request Host (#538), and discovery names every endpoint under it. Codes and reference access tokens use the Data Protection format, so the shared ring (#794) is what lets one replica redeem another's code. OpenIddict demands a signing and an encryption key, but with that format only identity tokens use them, so they are ephemeral, `openid` is removed from the registered scopes (OpenIddict otherwise grants it implicitly) and the key-set endpoint is unmapped; never grant `openid` without a persistent key. A code filter drops every `OpenIddict*` event below Warning, because its Information dumps carry the PKCE verifier; an override cannot replace it, since a more specific child override wins. `tools/oauth/mutation-check.sh` proves each claim's test can fail. → [`795-openiddict-server.md`](../docs/decisions/795-openiddict-server.md) - **An OAuth token runs the session chain, and only where an endpoint opts in (#796).** The authorize endpoint copies the session principal's `sub`, `email`, `account_id`, `credential_epoch`, `role` and `must_change_password` into the token, and OpenIddict validation authenticates it during `UseAuthentication`, so tenant, flock scope, epoch (disabled, suspended, role change) and must-change-password apply unchanged; never authenticate OAuth at authorization time, which skips all four. The default scheme routes by endpoint metadata, never token shape: `AcceptOAuthTokens(scopes)` endpoints accept OAuth tokens only, every other endpoint session JWTs only. That extension is the only way in (its marker is private): it attaches the `oauth-api` rate limit (shared counter, keyed per bearer hash) and a scope gate beside the role policy, so permission is role ∩ scope. No business endpoint opts in yet. Role freshness is the epoch: a demotion disconnects the assistant. Disconnect revokes the authorization; validation checks it on every request and refuses a token naming none, and there is no refresh grant. → [`796-oauth-fail-closed.md`](../docs/decisions/796-oauth-fail-closed.md) -- **OAuth clients register themselves, and dead OAuth rows are swept (#797).** `POST /api/v1/oauth/register` (RFC 7591) is anonymous on purpose: a registered client gets nothing until a user approves it (#798), so the risk is rows, and keep that reasoning beside the endpoint. It registers public clients with the authorization-code grant only; `refresh_token` is accepted and dropped, every other grant, response type and client authentication method is refused, and redirect URIs must be `https` or `http` on `localhost`, `127.0.0.1` or `[::1]`, with no fragment or user info; custom schemes are refused. A client whose redirect URIs are all `http` loopback is registered as native with port-less URIs, so it may authorize on any port (RFC 8252 §7.3) while scheme, host, path and query still match. The response returns the stored strings, and an OpenIddict validation refusal is `invalid_redirect_uri`, never a 500. The client's name is untrusted: controls and format characters, bidi among them, become spaces, and it is capped at 100 UTF-16 units on a grapheme boundary; no other client metadata is stored. The `oauth-register` policy limits it per client IP on the shared `IFixedWindowCounter`, never a process-local limiter. `OAuthPurgeSweep` runs under `DurableJobWorker`'s leader gate, never OpenIddict's Quartz job: it prunes dead tokens and authorizations older than 14 days with OpenIddict's `PruneAsync`, then deletes applications older than a day with no authorization and no token, in that order because the foreign keys do not cascade. Approval is an authorization row, and a live connection holds a valid, non-expiring access token, so it survives. The application's `CreatedAtUtc` is a shadow column OpenIddict lacks, stamped by #819's `StampCreatedBusinessRecord` trigger and configured with `BusinessRecordModel.ConfigureCreatedTimestamp`, and the window is measured with the database's `now()`. Both sit behind #795's Production gate, and `IOAuthPurge` is registered only with the server. Client ID Metadata Documents are deferred. → [`797-oauth-client-registration.md`](../docs/decisions/797-oauth-client-registration.md) +- **OAuth clients register themselves, and dead OAuth rows are swept (#797).** `POST /api/v1/oauth/register` (RFC 7591) is anonymous on purpose: a registered client gets nothing until a user approves it (#798), so the risk is rows, and keep that reasoning beside the endpoint. It registers public clients with the authorization-code grant only; `refresh_token` is accepted and dropped, every other grant, response type and client authentication method is refused, and redirect URIs must be `https` or `http` on `localhost`, `127.0.0.1` or `[::1]`, with no fragment or user info; custom schemes are refused. A client whose redirect URIs are all `http` loopback is registered as native with port-less URIs, so it may authorize on any port (RFC 8252 §7.3) while scheme, host, path and query still match. The response returns the stored strings, and an OpenIddict validation refusal is `invalid_redirect_uri`, never a 500. The client's name is untrusted: controls and format characters, bidi among them, become spaces, and it is capped at 100 UTF-16 units on a grapheme boundary; no other client metadata is stored. The `oauth-register` policy limits it per client IP on the shared `IFixedWindowCounter`, never a process-local limiter. `OAuthPurgeSweep` runs under `DurableJobWorker`'s leader gate, never OpenIddict's Quartz job: it prunes dead tokens and authorizations older than 14 days with OpenIddict's `PruneAsync`, then deletes applications older than a day with no authorization and no token, in that order because the foreign keys do not cascade. Approval is an authorization row, and a live connection holds a valid, non-expiring access token, so it survives. The application's `CreatedAtUtc` is a shadow column OpenIddict lacks, stamped by #819's `StampCreatedBusinessRecord` trigger and configured with `BusinessRecordModel.ConfigureCreatedTimestamp`, and the window is measured with the database's `now()`. `IOAuthPurge` is registered only with the server. Client ID Metadata Documents are deferred. → [`797-oauth-client-registration.md`](../docs/decisions/797-oauth-client-registration.md) +- **Consent is all or nothing, and every code costs the password (#798).** A browser navigation to `GET /api/v1/oauth/authorize` carries no bearer and is redirected, with its query, to the SPA's `/connect` route; the SPA asks the same endpoint with its session bearer and gets either the consent payload or `{ redirectUri }` as JSON, because `fetch` cannot read a cross-origin redirect; an expired bearer gets 401, never the redirect. No path issues a code without a step-up grant spent through `IAccessModule.ConsumeStepUpGrantAsync`, because the code becomes a token that never expires. 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 sets `alreadyApproved` and the SPA asks for the password only; a wider request shows the permissions again. The redirect host comes from the redirect URI OpenIddict validated, never the raw `redirect_uri`, which a single-URI client may omit. `X-Cluckwork-Consent: deny` sends `access_denied`. The scopes are `farm:read` and `daily-entries:write`, and an empty request defaults to `farm:read`. Sign-in returns only to a same-origin path (`returnPath`), and only `/connect` keeps its query. → [`798-oauth-consent.md`](../docs/decisions/798-oauth-consent.md) - **First-run admin: `bootstrap-admin` (#283).** Creates an Owner with a generated password (stdout only, never the logger/OTLP) and `MustChangePassword=true`, **only** if the default account has no Owner; a re-run is a silent no-op. While the flag is set, `MustChangePasswordMiddleware` 403s everything except `auth/change-password` and `auth/logout`. → [`283-first-run-admin-provisioning.md`](../docs/decisions/283-first-run-admin-provisioning.md) - **Break-glass: `recover-admin` (#265).** Same run-then-exit shape as `seed`, but deliberately **not** environment-gated — it must work against a real Production database. One transaction: freshly generated temp password (never one passed on the command line), rotated security stamp, every refresh token revoked, and a `User.BreakGlassReset` audit row carrying `--reason`. → [`265-break-glass-recovery.md`](../docs/decisions/265-break-glass-recovery.md) · [runbook](../docs/runbooks/break-glass-account-recovery.md) - **Nothing writes an audit event without an actor (#500).** `AuditWriter` throws on an unresolved `ICurrentUser`; system callers declare a `SystemActors` identity and both seeders require an Owner. `ICurrentUser` is an **authorization input, not a label** — `FlockScopeGuard` reads its roles, so an actor built from a literal rather than `UserManager.GetRolesAsync` can fail an entire seed. → [`500-audit-actor.md`](../docs/decisions/500-audit-actor.md) diff --git a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs index c6bbe4c48..7cb00b119 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs @@ -191,17 +191,17 @@ public static CluckworkIdentityRegistration AddCluckworkIdentity( return new(OAuthServer: oauthIssuer is not null); } - // #795 — Production never runs the authorization server: no client registration - // (#797) and no consent with step-up (#798) exist yet. Elsewhere a configured issuer - // turns it on. The issuer is never derived from the request's Host, because discovery - // would then vary with it (#538). + // #798 — a Production serving process always runs the authorization server, so it + // needs its public https URL. Elsewhere a configured issuer turns the server on. The + // issuer is never derived from the request's Host, because discovery would then vary + // with it (#538). private static Uri? OAuthIssuer( IConfiguration configuration, IHostEnvironment environment, ProcessRole role) { var issuer = configuration["OAuth:Issuer"]; - if (role is not ProcessRole.Serving || environment.IsProduction() - || string.IsNullOrWhiteSpace(issuer)) - return null; + if (role is not ProcessRole.Serving) return null; + if (environment.IsProduction()) return EnsureOAuthIssuer(issuer); + if (string.IsNullOrWhiteSpace(issuer)) return null; return Uri.TryCreate(issuer, UriKind.Absolute, out var uri) ? uri @@ -209,6 +209,21 @@ public static CluckworkIdentityRegistration AddCluckworkIdentity( $"OAuth:Issuer must be an absolute URI, and '{issuer}' is not."); } + private static Uri EnsureOAuthIssuer(string? issuer) + { + if (string.IsNullOrWhiteSpace(issuer)) + throw new InvalidOperationException( + "OAuth:Issuer is not configured. Set it to this deployment's public https URL, " + + "for example https://farm.example/."); + + // Discovery builds every endpoint URL under it, which a query or fragment would break. + return Uri.TryCreate(issuer, UriKind.Absolute, out var uri) && uri.Scheme == Uri.UriSchemeHttps + && uri.Query.Length == 0 && uri.Fragment.Length == 0 + ? uri + : throw new InvalidOperationException( + $"OAuth:Issuer must be an absolute https URL, and '{issuer}' is not."); + } + private static void AddUserFeatures(IServiceCollection services) { services.AddScoped< diff --git a/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs index fc673c7f2..bee50f8d7 100644 --- a/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs +++ b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs @@ -2,10 +2,15 @@ using Cluckwork.Api.Hosting; using Cluckwork.Api.Middleware; using Cluckwork.Api.RateLimiting; +using Cluckwork.Api.Modules.Access.Auth; using Cluckwork.Application.Common; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Infrastructure.Persistence; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore; using Microsoft.AspNetCore.Authentication; using OpenIddict.Abstractions; +using OpenIddict.Server; using OpenIddict.Server.AspNetCore; using static OpenIddict.Abstractions.OpenIddictConstants; @@ -24,8 +29,11 @@ public static class OAuthEndpoints public static RouteGroupBuilder MapOAuthEndpoints(this RouteGroupBuilder group) { + // #798 — anonymous so a browser navigation, which carries no bearer, reaches the + // handler and is handed to the SPA's consent route. The SPA then calls it with its + // session bearer to decide. group.MapGet("/authorize", Authorize) - .RequireAuthorization() + .AllowAnonymous() .RequireRateLimiting(RateLimitingOptions.OAuthAuthorizePolicyName) .ExcludeFromDescription(); @@ -75,34 +83,131 @@ public static bool AcceptsOAuthTokens(Endpoint? endpoint) => private sealed class AcceptsOAuthTokensMarker; - // #795 — OpenIddict has already validated the request. No consent screen exists - // until #798, so a signed-in caller approves its own, with the scopes it asked for. - // OpenIddict refuses any scope it does not register, and it registers none yet. + public const string ConsentHeaderName = "X-Cluckwork-Consent"; + + // #798 — OpenIddict has already validated the client, the redirect URI, PKCE and the + // scopes. Consent is all or nothing and always needs the user's password through a + // step-up grant. When one of their valid permanent authorizations for this app already + // holds every scope asked for, only the permission explanation is skipped. private static async Task Authorize( - HttpContext context, ICurrentUser currentUser, IOpenIddictApplicationManager applications, + HttpContext context, + ICurrentUser currentUser, + TenantContext tenant, + FlockScope flockScope, + IAccessModule access, + IOpenIddictApplicationManager applications, + IOpenIddictAuthorizationManager authorizations, + [FromHeader(Name = AuthEndpoints.StepUpHeaderName)] string? stepUpToken, + [FromHeader(Name = ConsentHeaderName)] string? consent, CancellationToken ct) { - if (!currentUser.IsResolved) return Results.Unauthorized(); + if (!currentUser.IsResolved) + { + // An expired session: the SPA refreshes and asks again. + if (context.Request.Headers.Authorization.ToString().StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)) + return Results.Unauthorized(); + // The sign-in screen names the app it continues to. The name is the app's + // own public choice (#797), so it needs no session. + if (consent == "preview") + return Results.Json(new + { + clientName = await applications.GetDisplayNameAsync( + (await applications.FindByClientIdAsync(context.GetOpenIddictServerRequest()!.ClientId!, ct))!, ct), + }); + // A browser navigation. A relative path cannot leave this origin, and the + // query is the request OpenIddict just validated. + return Results.Redirect("/connect" + context.Request.QueryString); + } + + if (consent == "deny") + return Results.Forbid( + new AuthenticationProperties(new Dictionary + { + [OpenIddictServerAspNetCoreConstants.Properties.Error] = Errors.AccessDenied, + [OpenIddictServerAspNetCoreConstants.Properties.ErrorDescription] = "The user declined the request.", + }), + [OpenIddictServerAspNetCoreDefaults.AuthenticationScheme]); var request = context.GetOpenIddictServerRequest()!; + // RFC 6749 §3.3 lets a server default an empty request. The default is the narrower + // scope, and consent shows it. + var scopes = request.GetScopes() is { IsEmpty: false } asked ? asked : [OAuthScopes.ReadFarm]; + var application = (await applications.FindByClientIdAsync(request.ClientId!, ct))!; + var applicationId = (await applications.GetIdAsync(application, ct))!; + var subject = currentUser.UserId.ToString(); + + object? covering = null; + var allowed = new HashSet(StringComparer.Ordinal); + await foreach (var authorization in authorizations.FindAsync( + subject, applicationId, Statuses.Valid, AuthorizationTypes.Permanent, scopes: null, ct)) + { + var granted = await authorizations.GetScopesAsync(authorization, ct); + allowed.UnionWith(granted); + if (covering is null && scopes.All(granted.Contains)) covering = authorization; + } + + // Every code costs a spent step-up grant, a reconnect included: the code becomes a + // token that never expires, so a session bearer alone must never mint one. A + // reconnect only skips the permissions, because the user already approved them. + if (stepUpToken is null) + return Results.Json(new ConsentRequest( + request.ClientId!, + await applications.GetDisplayNameAsync(application, ct), + new Uri(ValidatedRedirectUri(context)).Host, + scopes, + [.. scopes.Where(allowed.Contains)], + AlreadyApproved: covering is not null, + AssignedFlocks: flockScope.IsUnrestricted ? null + : [.. (await access.ListFlockAssignmentsAsync(currentUser.UserId, ct)) + .Select(assignment => assignment.FlockName).OfType()])); + + var proof = await access.ConsumeStepUpGrantAsync(tenant.AccountId, currentUser.UserId, stepUpToken, ct); + if (proof.IsFailure) + return Results.Problem(proof.Error.Description, + statusCode: StatusCodes.Status403Forbidden, title: proof.Error.Code); + var identity = new ClaimsIdentity( context.User.Claims.Where(claim => SessionClaimTypes.Contains(claim.Type)), OpenIddictServerAspNetCoreDefaults.AuthenticationScheme, Claims.Name, Claims.Role); - identity.SetScopes(request.GetScopes()); + identity.SetScopes(scopes); // #800 — the name rides in the token, as the user's email does, so an audit row // snapshots it without a lookup per write. Registration never renames a client. - var application = await applications.FindByClientIdAsync(request.ClientId!, ct); - identity.SetClaim(ClientNameClaim, - application is null ? null : await applications.GetDisplayNameAsync(application, ct)); + identity.SetClaim(ClientNameClaim, await applications.GetDisplayNameAsync(application, ct)); identity.SetDestinations(static _ => [Destinations.AccessToken]); + // One permanent authorization per approval, reused by every later connection that + // asks for no more. Disconnect revokes it, and with it every token it issued (#796). + covering ??= await authorizations.CreateAsync( + identity, subject, applicationId, AuthorizationTypes.Permanent, scopes, ct); + identity.SetAuthorizationId(await authorizations.GetIdAsync(covering, ct)); return Results.SignIn( new ClaimsPrincipal(identity), authenticationScheme: OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } + // What the consent screen shows. The name is the app's own choice (#797). When + // AlreadyApproved, an earlier approval holds every scope and the SPA asks only for the + // password. AssignedFlocks is the user's flock scope as this request resolved it: null + // for every flock, else the names of the flocks they are assigned to. + private sealed record ConsentRequest( + string ClientId, + string? ClientName, + string RedirectHost, + IReadOnlyList Scopes, + IReadOnlyList AlreadyAllowed, + bool AlreadyApproved, + IReadOnlyList? AssignedFlocks); + + // A client with one registered redirect URI may omit redirect_uri; OpenIddict then + // validates that one and keeps it on the validation context, not on the request. + private static string ValidatedRedirectUri(HttpContext context) => + context.Features.Get()!.Transaction! + .GetProperty( + typeof(OpenIddictServerEvents.ValidateAuthorizationRequestContext).FullName!)! + .RedirectUri!; + // OpenIddict has validated the client, the code and its authorization before this // runs; the code's principal becomes the access token's. private static async Task Token(HttpContext context) => Results.SignIn( diff --git a/src/Cluckwork.Application/Modules/Access/Contracts/IAccessModule.cs b/src/Cluckwork.Application/Modules/Access/Contracts/IAccessModule.cs index c49ff8479..4ad9be847 100644 --- a/src/Cluckwork.Application/Modules/Access/Contracts/IAccessModule.cs +++ b/src/Cluckwork.Application/Modules/Access/Contracts/IAccessModule.cs @@ -70,6 +70,11 @@ Task RevokeRefreshTokenAsync( Task> IssueStepUpGrantAsync( Guid accountId, Guid userId, string currentPassword, CancellationToken ct); + // #798 — spends a step-up grant on a decision outside user administration: approving + // a connected app. Validates and consumes it exactly as the gated user operations do. + Task ConsumeStepUpGrantAsync( + Guid accountId, Guid userId, string? stepUpToken, CancellationToken ct); + // Jwt:RefreshTokenDays, the refresh cookie's lifetime. Only this value // leaves the JWT options; the signing keys stay in Identity. int RefreshTokenLifetimeDays { get; } diff --git a/src/Cluckwork.Application/Modules/Access/Contracts/OAuthScopes.cs b/src/Cluckwork.Application/Modules/Access/Contracts/OAuthScopes.cs new file mode 100644 index 000000000..4e1577418 --- /dev/null +++ b/src/Cluckwork.Application/Modules/Access/Contracts/OAuthScopes.cs @@ -0,0 +1,11 @@ +namespace Cluckwork.Application.Modules.Access.Contracts; + +// #788 — the two scopes a connected app can ask for. A scope only subtracts from the +// user's role (#796); #806 maps the tools to them. +public static class OAuthScopes +{ + public const string ReadFarm = "farm:read"; + public const string WriteDailyEntries = "daily-entries:write"; + + public static IReadOnlyList All { get; } = [ReadFarm, WriteDailyEntries]; +} diff --git a/src/Cluckwork.Infrastructure/Modules/Access/Identity/AccessModule.cs b/src/Cluckwork.Infrastructure/Modules/Access/Identity/AccessModule.cs index 4c0407d7f..495e75acf 100644 --- a/src/Cluckwork.Infrastructure/Modules/Access/Identity/AccessModule.cs +++ b/src/Cluckwork.Infrastructure/Modules/Access/Identity/AccessModule.cs @@ -129,5 +129,9 @@ public Task> IssueStepUpGrantAsync( Guid accountId, Guid userId, string currentPassword, CancellationToken ct) => stepUp.IssueAsync(accountId, userId, currentPassword, ct); + public Task ConsumeStepUpGrantAsync( + Guid accountId, Guid userId, string? stepUpToken, CancellationToken ct) => + stepUp.ValidateAsync(accountId, userId, stepUpToken, ct); + public int RefreshTokenLifetimeDays => jwt.Value.RefreshTokenDays; } diff --git a/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs b/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs index ac9cf4c68..1c9df4da8 100644 --- a/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs +++ b/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs @@ -2,7 +2,9 @@ using Cluckwork.Infrastructure.Persistence; using Microsoft.AspNetCore; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Primitives; using OpenIddict.Server; using OpenIddict.Validation; using static OpenIddict.Abstractions.OpenIddictConstants; @@ -33,6 +35,11 @@ public static IServiceCollection AddAccessOAuthServer( .RequireProofKeyForCodeExchange() .UseReferenceAccessTokens() .SetAccessTokenLifetime(null) + .RegisterScopes([.. OAuthScopes.All]) + // Every client registers itself anonymously (#797), so a per-client + // scope permission would be granted to all of them anyway. The user's + // consent is what limits a connection's scopes (#798). + .IgnoreScopePermissions() // OpenIddict refuses to start without both keys, but with the Data // Protection format they only ever sign identity tokens, which this // server never issues. Codes and access tokens are protected by the @@ -48,8 +55,39 @@ public static IServiceCollection AddAccessOAuthServer( // A plain challenge is the verifier itself, so it protects nothing once // the authorization request leaks. OAuth 2.1 clients send S256. options.CodeChallengeMethods.Remove(CodeChallengeMethods.Plain); + // The consent route hands the client's redirect to the SPA as a URL + // (below), which only a query-mode response is. + options.ResponseModes.Remove(ResponseModes.FormPost); + options.ResponseModes.Remove(ResponseModes.Fragment); }); + // #798 — the SPA's consent route asks with its session bearer through fetch, + // which cannot read a cross-origin redirect. It gets the client's redirect as + // JSON and navigates there itself. A browser navigation carries no bearer and + // keeps the standard redirect. + server.AddEventHandler(handler => handler + .UseInlineHandler(static async context => + { + var request = context.Transaction.GetHttpRequest()!; + if (context.RedirectUri is null || !HasBearer(request)) + return; + + var location = context.RedirectUri; + foreach (var (name, value) in context.Response.GetParameters()) + foreach (var item in (StringValues)value) + if (!string.IsNullOrEmpty(item)) + location = QueryHelpers.AddQueryString(location, name, item); + + // OpenIddict has already set 400 for an error, which the client + // still receives through the redirect. + var response = request.HttpContext.Response; + response.StatusCode = StatusCodes.Status200OK; + response.Headers.CacheControl = "no-store"; + await response.WriteAsJsonAsync(new { redirectUri = location }, request.HttpContext.RequestAborted); + context.HandleRequest(); + }) + .SetOrder(Authentication.ProcessQueryResponse.Descriptor.Order - 1)); + // OpenIddict also accepts a POSTed authorization request, which matches no // endpoint and so no rate-limit policy or body cap. Refused before the body // is read or a client looked up (#796). @@ -62,13 +100,18 @@ public static IServiceCollection AddAccessOAuthServer( }) .SetOrder(ExtractGetOrPostRequest.Descriptor.Order - 1)); - // #797 — OpenIddict has no registration endpoint; the API maps one beside - // authorize, and discovery points clients at it. + // #798 — discovery names endpoints under the configured issuer, not under the + // Host the request arrived with: behind a proxy that Host can be an internal + // name. #797 — OpenIddict has no registration endpoint; the API maps one + // beside authorize, and discovery points clients at it. + var publicBase = issuer.AbsoluteUri.EndsWith('/') ? issuer : new Uri(issuer.AbsoluteUri + "/"); server.AddEventHandler(handler => handler .UseInlineHandler(context => { + context.AuthorizationEndpoint = new Uri(publicBase, context.BaseUri!.MakeRelativeUri(context.AuthorizationEndpoint!)); + context.TokenEndpoint = new Uri(publicBase, context.BaseUri!.MakeRelativeUri(context.TokenEndpoint!)); context.Metadata["registration_endpoint"] = - new Uri(context.AuthorizationEndpoint!, "register").AbsoluteUri; + new Uri(context.AuthorizationEndpoint, "register").AbsoluteUri; return default; }) .SetOrder(OpenIddictServerHandlers.Discovery.AttachEndpoints.Descriptor.Order + 1)); @@ -108,4 +151,7 @@ public static IServiceCollection AddAccessOAuthServer( return services; } + + private static bool HasBearer(HttpRequest request) => + request.Headers.Authorization.ToString().StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase); } diff --git a/tests/Cluckwork.Api.IntegrationTests/AuditConnectedAppTests.cs b/tests/Cluckwork.Api.IntegrationTests/AuditConnectedAppTests.cs index d2e9ab200..b6409f8ed 100644 --- a/tests/Cluckwork.Api.IntegrationTests/AuditConnectedAppTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/AuditConnectedAppTests.cs @@ -248,9 +248,8 @@ private static async Task ConnectAsync(WebApplicationFactory ho var verifier = OAuthServerTests.NewCodeVerifier(); var query = OAuthServerTests.AuthorizeQuery(clientId, verifier); query["scope"] = WriteScope; - using var authorize = await OAuthServerTests.SendAuthorizeAsync(host, jwt, query); - Assert.Equal(HttpStatusCode.Redirect, authorize.StatusCode); - var code = QueryHelpers.ParseQuery(authorize.Headers.Location!.Query)["code"].ToString(); + var location = await OAuthServerTests.ApproveAsync(host, jwt, query); + var code = QueryHelpers.ParseQuery(location.Query)["code"].ToString(); using var client = OAuthServerTests.HttpsClient(host, bearer: null); using var token = await client.PostAsync("/api/v1/oauth/token", new FormUrlEncodedContent( diff --git a/tests/Cluckwork.Api.IntegrationTests/CredentialEpochTests.cs b/tests/Cluckwork.Api.IntegrationTests/CredentialEpochTests.cs index 218b64756..311d2f045 100644 --- a/tests/Cluckwork.Api.IntegrationTests/CredentialEpochTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/CredentialEpochTests.cs @@ -389,8 +389,10 @@ private Task UserIdForAsync(Guid accountId, string email) => factory.WithTenantScopeAsync(accountId, async db => await db.Users.Where(user => user.Email == email).Select(user => user.Id).SingleAsync()); - internal static string CreateAccessToken(Guid userId, Guid accountId, string? credentialEpoch) + internal static string CreateAccessToken( + Guid userId, Guid accountId, string? credentialEpoch, DateTime? expiresUtc = null) { + var expires = expiresUtc ?? DateTime.UtcNow.AddMinutes(5); using var rsa = RSA.Create(); rsa.ImportFromPem(TestJwtKeys.PrivateKeyPem.Replace("\\n", "\n", StringComparison.Ordinal)); var claims = new List @@ -408,7 +410,7 @@ internal static string CreateAccessToken(Guid userId, Guid accountId, string? cr }; var token = new JwtSecurityToken( "cluckwork-test", "cluckwork-api-test", claims, - DateTime.UtcNow.AddMinutes(-1), DateTime.UtcNow.AddMinutes(5), + expires.AddMinutes(-6), expires, credentials); return new JwtSecurityTokenHandler().WriteToken(token); } diff --git a/tests/Cluckwork.Api.IntegrationTests/FixturePortRegistrationTests.cs b/tests/Cluckwork.Api.IntegrationTests/FixturePortRegistrationTests.cs index e486968ca..9fa7c1403 100644 --- a/tests/Cluckwork.Api.IntegrationTests/FixturePortRegistrationTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/FixturePortRegistrationTests.cs @@ -70,6 +70,7 @@ public async Task FixturePortsResolveOnlyOutsideProduction(string? environment, psi.Environment["DataProtection__CertificatePem"] = TestDataProtectionCertificate.CertificatePem; psi.Environment["DataProtection__PrivateKeyPem"] = TestDataProtectionCertificate.PrivateKeyPem; psi.Environment["RateLimiting__TrustedProxies__0"] = "10.0.0.0/8"; + psi.Environment["OAuth__Issuer"] = "https://farm.example/"; using var process = Process.Start(psi)!; var stdout = process.StandardOutput.ReadToEndAsync(); diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthClientRegistrationTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthClientRegistrationTests.cs index 66c991921..04a344899 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthClientRegistrationTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthClientRegistrationTests.cs @@ -243,6 +243,8 @@ public async Task Registration_IsRateLimitedPerClientIp_OnTheSharedCounter() using var host = factory.WithWebHostBuilder(builder => { builder.UseSetting("RateLimiting:OAuthRegister:PermitLimit", "2"); + // Clock-aligned window: a day keeps the requests inside one (see OAuthFailClosedTests.Host). + builder.UseSetting("RateLimiting:OAuthRegister:WindowSeconds", "86400"); builder.UseSetting("RateLimiting:TrustedProxies:0", $"{proxy}/32"); builder.ConfigureTestServices(services => { diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthConsentTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthConsentTests.cs new file mode 100644 index 000000000..2ffd59d8b --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthConsentTests.cs @@ -0,0 +1,412 @@ +using System.IdentityModel.Tokens.Jwt; +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Domain.Modules.Farm.Accounts; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using OpenIddict.Abstractions; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace Cluckwork.Api.IntegrationTests; + +// #798 — consent: all or nothing, approved with the user's password through a step-up +// grant, skipped when the app asks for nothing new, asked again when it asks for more. +[Collection(IntegrationCollection.Name)] +public sealed class OAuthConsentTests(CluckworkWebApplicationFactory factory) +{ + private const string Read = OAuthScopes.ReadFarm; + private const string Write = OAuthScopes.WriteDailyEntries; + + [Fact] + public async Task FirstRequest_AsksForConsent_AndIssuesNothing() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await RegisterNamedClientAsync("Claude Desktop"); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read, Write)); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.False(body.TryGetProperty("redirectUri", out _), "a code was issued without consent"); + Assert.Equal(clientId, body.GetProperty("clientId").GetString()); + Assert.Equal("Claude Desktop", body.GetProperty("clientName").GetString()); + Assert.Equal("client.example", body.GetProperty("redirectHost").GetString()); + Assert.Equal([Read, Write], Strings(body, "scopes")); + Assert.Empty(Strings(body, "alreadyAllowed")); + Assert.False(body.GetProperty("alreadyApproved").GetBoolean(), "a first request skipped the permissions"); + Assert.Equal(JsonValueKind.Null, body.GetProperty("assignedFlocks").ValueKind); + Assert.Equal(0, await CountAuthorizationsAsync(userId)); + } + + // Details describes the user's real reach: a Worker with assignments sees only those + // flocks; a Worker with none, like every other role, sees every flock (#388, #612). + [Fact] + public async Task Payload_NamesTheAssignedFlocks_OfAScopedWorker() + { + var (accountId, jwt) = await SeedWorkerAsync(assign: true); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read)); + + var flocks = (await response.Content.ReadFromJsonAsync()).GetProperty("assignedFlocks"); + var name = await factory.WithTenantScopeAsync(accountId, db => + db.Flocks.Select(flock => flock.Name).FirstAsync()); + Assert.True(flocks.ValueKind == JsonValueKind.Array, "a flock-scoped worker was told every flock"); + Assert.Equal([name], flocks.EnumerateArray().Select(flock => flock.GetString()!).ToArray()); + } + + [Fact] + public async Task Payload_SaysEveryFlock_ForAnUnassignedWorker() + { + var (_, jwt) = await SeedWorkerAsync(assign: false); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read)); + + Assert.Equal(JsonValueKind.Null, + (await response.Content.ReadFromJsonAsync()).GetProperty("assignedFlocks").ValueKind); + } + + [Fact] + public async Task Approval_WithoutAValidGrant_IsRefused() + { + const string grant = "not-a-grant"; + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read), stepUp: grant); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal(StepUpErrorCodes.Required, + (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString()); + Assert.Equal(0, await CountAuthorizationsAsync(userId)); + } + + // Single use: the grant that approved one app cannot approve another. + [Fact] + public async Task Approval_SpendsTheGrant() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var first = await OAuthServerTests.RegisterClientAsync(factory.Services); + var second = await OAuthServerTests.RegisterClientAsync(factory.Services); + var grant = await OAuthServerTests.StepUpAsync(factory, jwt); + + using var approved = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(first, Read), stepUp: grant); + using var replayed = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(second, Read), stepUp: grant); + + Assert.Equal(HttpStatusCode.OK, approved.StatusCode); + Assert.Equal(HttpStatusCode.Forbidden, replayed.StatusCode); + } + + // fetch cannot read a cross-origin redirect, so the SPA gets it as JSON, carrying + // everything the standard redirect would. + [Fact] + public async Task Approval_ReturnsTheClientRedirect_AsJson() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + var verifier = OAuthServerTests.NewCodeVerifier(); + + var location = await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, [Read], verifier)); + + Assert.Equal(OAuthServerTests.RedirectUri, location.GetLeftPart(UriPartial.Path)); + var parameters = QueryHelpers.ParseQuery(location.Query); + Assert.Equal("state-795", parameters["state"].ToString()); + Assert.Equal("https://localhost/", parameters["iss"].ToString()); + using var redeemed = await RedeemAsync(clientId, parameters["code"].ToString(), verifier); + Assert.Equal(HttpStatusCode.OK, redeemed.StatusCode); + } + + // A reconnect skips the permissions, never the password (the maintainer, 2026-10-09). + [Fact] + public async Task ApprovedApp_AsksOnlyForThePassword() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, Read, Write)); + + using var bearerOnly = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read)); + var body = await bearerOnly.Content.ReadFromJsonAsync(); + var location = await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, Read)); + + Assert.False(body.TryGetProperty("redirectUri", out _), "a session bearer alone minted a code for an approved app"); + Assert.True(body.GetProperty("alreadyApproved").GetBoolean(), "an approved app was shown its permissions again"); + Assert.False(string.IsNullOrEmpty(QueryHelpers.ParseQuery(location.Query)["code"]), "the reconnect issued no code"); + Assert.Equal(1, await CountAuthorizationsAsync(userId)); + } + + [Fact] + public async Task Reconnect_SpendsTheGrant() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, Read)); + var grant = await OAuthServerTests.StepUpAsync(factory, jwt); + + using var reconnected = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read), stepUp: grant); + using var replayed = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read), stepUp: grant); + + Assert.Equal(HttpStatusCode.OK, reconnected.StatusCode); + Assert.Equal(HttpStatusCode.Forbidden, replayed.StatusCode); + } + + [Fact] + public async Task MoreScopes_AsksAgain_NamingWhatIsAlreadyAllowed() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, Read)); + + using var wider = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read, Write)); + + var body = await wider.Content.ReadFromJsonAsync(); + Assert.False(body.GetProperty("alreadyApproved").GetBoolean(), "a wider request skipped the permissions"); + Assert.Equal([Read, Write], Strings(body, "scopes")); + Assert.Equal([Read], Strings(body, "alreadyAllowed")); + } + + [Fact] + public async Task AnotherUsersApproval_DoesNotSkip() + { + var flows = new OAuthServerTests(factory); + var (_, approver) = await flows.SeedUserAsync(); + var (_, other) = await flows.SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + await OAuthServerTests.ApproveAsync(factory, approver, Query(clientId, Read)); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, other, Query(clientId, Read)); + + Assert.False((await response.Content.ReadFromJsonAsync()).GetProperty("alreadyApproved").GetBoolean(), + "another user's approval skipped the permissions"); + } + + [Fact] + public async Task DisconnectedApproval_DoesNotSkip() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + await OAuthServerTests.ApproveAsync(factory, jwt, Query(clientId, Read)); + await using (var scope = factory.Services.CreateAsyncScope()) + { + var authorizations = scope.ServiceProvider.GetRequiredService(); + await foreach (var authorization in authorizations.FindBySubjectAsync(userId.ToString())) + Assert.True(await authorizations.TryRevokeAsync(authorization), "the authorization was not revoked"); + } + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read)); + + var body = await response.Content.ReadFromJsonAsync(); + Assert.False(body.GetProperty("alreadyApproved").GetBoolean(), "a revoked approval skipped the permissions"); + Assert.Empty(Strings(body, "alreadyAllowed")); + } + + [Fact] + public async Task Cancel_SendsAccessDeniedToTheClient_AndRecordsNothing() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, Read), consent: "deny"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var location = await RedirectFromAsync(response, "cancel did not send the user back to the client"); + var parameters = QueryHelpers.ParseQuery(location.Query); + Assert.Equal(Errors.AccessDenied, parameters["error"].ToString()); + Assert.Equal("state-795", parameters["state"].ToString()); + Assert.Equal(0, await CountAuthorizationsAsync(userId)); + } + + // The SPA refreshes on a 401 and asks again; a redirect to /connect would hand fetch + // the SPA's HTML instead. The token is the user's real session, signed, and expired + // past the 30-second clock skew. + [Fact] + public async Task ExpiredSession_IsUnauthorized_NotARedirect() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var session = new JwtSecurityTokenHandler().ReadJwtToken(jwt); + var expired = CredentialEpochTests.CreateAccessToken( + userId, + Guid.Parse(session.Claims.First(claim => claim.Type == "account_id").Value), + session.Claims.First(claim => claim.Type == "credential_epoch").Value, + expiresUtc: DateTime.UtcNow.AddMinutes(-2)); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, expired, Query(clientId, Read)); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task MalformedBearer_IsUnauthorized_NotARedirect() + { + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, "not.a.session", Query(clientId, Read)); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // A client with one registered redirect URI may omit redirect_uri; OpenIddict then + // uses the registered one. + [Fact] + public async Task OmittedRedirectUri_UsesTheRegisteredOne() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + var query = Query(clientId, Read); + query.Remove("redirect_uri"); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, query); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("client.example", + (await response.Content.ReadFromJsonAsync()).GetProperty("redirectHost").GetString()); + } + + [Fact] + public async Task NoScope_AsksForReadOnly() + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId)); + + Assert.Equal([Read], Strings(await response.Content.ReadFromJsonAsync(), "scopes")); + } + + // Only the two registered scopes exist; OpenIddict refuses anything else before the + // redirect URI is trusted, so it answers directly. + [Fact] + public async Task UnknownScope_IsRefused() + { + var (userId, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, Query(clientId, "farm:admin")); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Contains($"error:{Errors.InvalidScope}", await response.Content.ReadAsStringAsync()); + Assert.Equal(0, await CountAuthorizationsAsync(userId)); + } + + // The SPA receives the client's redirect as one URL, which only query mode is. + // Login names the app before anyone signs in; that answer carries the name and + // nothing else, and never a code. + [Fact] + public async Task Preview_WithoutASession_NamesTheAppOnly() + { + var clientId = await RegisterNamedClientAsync("Claude Desktop"); + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt: null, Query(clientId, Read), consent: "preview"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.Equal(["clientName"], body.EnumerateObject().Select(property => property.Name).ToArray()); + Assert.Equal("Claude Desktop", body.GetProperty("clientName").GetString()); + } + + [Theory] + [InlineData(ResponseModes.FormPost)] + [InlineData(ResponseModes.Fragment)] + public async Task ResponseModeOtherThanQuery_IsRefused(string mode) + { + var (_, jwt) = await new OAuthServerTests(factory).SeedUserAsync(); + var clientId = await OAuthServerTests.RegisterClientAsync(factory.Services); + var query = Query(clientId, Read); + query["response_mode"] = mode; + + using var response = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, query); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + // Behind a proxy the request's Host can be an internal name; clients must be sent to + // the public one. + [Fact] + public async Task Discovery_NamesEndpointsUnderTheIssuer() + { + using var client = factory.CreateClient(new WebApplicationFactoryClientOptions + { + // A loopback name host filtering admits, and not the issuer's host. + BaseAddress = new Uri("https://127.0.0.1"), + }); + + var metadata = await client.GetFromJsonAsync("/.well-known/oauth-authorization-server"); + + Assert.Equal("https://localhost/api/v1/oauth/authorize", metadata.GetProperty("authorization_endpoint").GetString()); + Assert.Equal("https://localhost/api/v1/oauth/token", metadata.GetProperty("token_endpoint").GetString()); + Assert.Equal("https://localhost/api/v1/oauth/register", metadata.GetProperty("registration_endpoint").GetString()); + } + + private static async Task RedirectFromAsync(HttpResponseMessage response, string otherwise) + { + var body = await response.Content.ReadFromJsonAsync(); + Assert.True(body.TryGetProperty("redirectUri", out _), otherwise); + return OAuthServerTests.RedirectOf(body); + } + + private static Dictionary Query(string clientId, params string[] scopes) => + Query(clientId, scopes, OAuthServerTests.NewCodeVerifier()); + + private static Dictionary Query(string clientId, string[] scopes, string verifier) + { + var query = OAuthServerTests.AuthorizeQuery(clientId, verifier); + if (scopes.Length > 0) query["scope"] = string.Join(' ', scopes); + return query; + } + + private async Task<(Guid AccountId, string Jwt)> SeedWorkerAsync(bool assign) + { + var email = $"oauth-worker-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync($"oauth-owner-{Guid.NewGuid():N}@test.local"); + await factory.SeedUserAsync(accountId, email, role: null); + if (assign) + { + var flockId = await factory.SeedFlockAsync(accountId, Guid.NewGuid()); + await factory.WithTenantScopeAsync(accountId, async db => + { + var userId = await db.Users.Where(user => user.Email == email).Select(user => user.Id).SingleAsync(); + db.UserRoleAssignments.Add(UserRoleAssignment.Create(Guid.NewGuid(), accountId, userId, null, null, flockId)); + await db.SaveChangesAsync(); + }); + } + return (accountId, await factory.LoginForAccessTokenAsync(email)); + } + + private async Task RegisterNamedClientAsync(string name) + { + using var response = await OAuthServerTests.HttpsClient(factory, bearer: null).PostAsJsonAsync( + "/api/v1/oauth/register", new { client_name = name, redirect_uris = new[] { OAuthServerTests.RedirectUri } }); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync()).GetProperty("client_id").GetString()!; + } + + private Task RedeemAsync(string clientId, string code, string verifier) => + OAuthServerTests.HttpsClient(factory, bearer: null).PostAsync("/api/v1/oauth/token", new FormUrlEncodedContent( + new Dictionary + { + ["grant_type"] = GrantTypes.AuthorizationCode, + ["client_id"] = clientId, + ["code"] = code, + ["redirect_uri"] = OAuthServerTests.RedirectUri, + ["code_verifier"] = verifier, + })); + + private async Task CountAuthorizationsAsync(Guid userId) + { + await using var scope = factory.Services.CreateAsyncScope(); + var count = 0; + await foreach (var _ in scope.ServiceProvider.GetRequiredService() + .FindBySubjectAsync(userId.ToString())) + count++; + return count; + } + + private static string[] Strings(JsonElement body, string name) => + [.. body.GetProperty(name).EnumerateArray().Select(item => item.GetString()!)]; +} diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthFailClosedTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthFailClosedTests.cs index 084235a89..ca1dfff8f 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthFailClosedTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthFailClosedTests.cs @@ -323,7 +323,7 @@ public async Task AuthorizeEndpoint_IsRateLimited() using var first = await Client(host, user.Jwt).GetAsync(AuthorizeUri(clientId, NewVerifier(), ReadScope)); using var second = await Client(host, user.Jwt).GetAsync(AuthorizeUri(clientId, NewVerifier(), ReadScope)); - Assert.Equal(HttpStatusCode.Redirect, first.StatusCode); + Assert.Equal(HttpStatusCode.OK, first.StatusCode); Assert.Equal(HttpStatusCode.TooManyRequests, second.StatusCode); } @@ -475,9 +475,8 @@ await scope.ServiceProvider.GetRequiredService(). private static async Task AuthorizeAsync( WebApplicationFactory host, string jwt, string clientId, string verifier, params string[] scopes) { - using var response = await Client(host, jwt).GetAsync(AuthorizeUri(clientId, verifier, scopes)); - Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); - return QueryHelpers.ParseQuery(response.Headers.Location!.Query)["code"].ToString(); + var location = await OAuthServerTests.ApproveAsync(host, jwt, AuthorizeParameters(clientId, verifier, scopes)); + return QueryHelpers.ParseQuery(location.Query)["code"].ToString(); } private static string AuthorizeUri(string clientId, string verifier, params string[] scopes) => @@ -518,7 +517,12 @@ private WebApplicationFactory Host(params (string Policy, int PermitLim factory.WithWebHostBuilder(builder => { foreach (var (policy, permitLimit) in limits) + { builder.UseSetting($"RateLimiting:{policy}:PermitLimit", permitLimit.ToString()); + // The window is clock-aligned, so two requests straddling a 60 s boundary + // both pass; a day makes that negligible (#840's MultiInstanceRateLimitTests). + builder.UseSetting($"RateLimiting:{policy}:WindowSeconds", "86400"); + } builder.ConfigureTestServices(services => { services.AddSingleton(); diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs index 9ed4561d5..5e88da143 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs @@ -1,10 +1,11 @@ using System.Net; using System.Net.Http.Json; +using System.Text.Json; using Cluckwork.Api.IntegrationTests.Infrastructure; using Cluckwork.Application.Modules.Access.Contracts; using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.DependencyInjection; -using OpenIddict.Abstractions; +using Microsoft.AspNetCore.WebUtilities; +using static OpenIddict.Abstractions.OpenIddictConstants; namespace Cluckwork.Api.IntegrationTests; @@ -18,31 +19,60 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) } } -// #795 — Production cannot issue an OAuth token until consent with step-up (#798) -// exists; client registration (#797) stays behind the same gate. The base factory configures an issuer, so this -// host proves the environment gate holds even when the issuer is set. +// #798 — Production runs the authorization server: a client registers itself, the user +// approves it with their password, and the client redeems the code. Without its public +// issuer URL a Production host refuses to start. The base factory sets the issuer. public sealed class OAuthServerProductionTests(OAuthProductionFactory factory) : IClassFixture { [Fact] - public async Task Production_RunsNoAuthorizationServer() + public async Task Production_ServesTheConnectFlow() { var client = OAuthServerTests.HttpsClient(factory, bearer: null); + var email = $"oauth-prod-{Guid.NewGuid():N}@test.local"; + await factory.SeedAccountWithUserAsync(email); + var jwt = await factory.LoginForAccessTokenAsync(email); + using var registered = await client.PostAsJsonAsync("/api/v1/oauth/register", + new { client_name = "Claude Desktop", redirect_uris = new[] { OAuthServerTests.RedirectUri } }); + Assert.Equal(HttpStatusCode.Created, registered.StatusCode); + var clientId = (await registered.Content.ReadFromJsonAsync()).GetProperty("client_id").GetString()!; + var verifier = OAuthServerTests.NewCodeVerifier(); + var query = OAuthServerTests.AuthorizeQuery(clientId, verifier); + query["scope"] = OAuthScopes.ReadFarm; + using var navigation = await OAuthServerTests.SendAuthorizeAsync(factory, jwt: null, query); + using var consent = await OAuthServerTests.SendAuthorizeAsync(factory, jwt, query); + var location = await OAuthServerTests.ApproveAsync(factory, jwt, query); using var token = await client.PostAsync("/api/v1/oauth/token", new FormUrlEncodedContent( - new Dictionary { ["grant_type"] = "authorization_code" })); - using var authorize = await client.GetAsync("/api/v1/oauth/authorize?response_type=code"); - using var metadata = await client.GetAsync("/.well-known/oauth-authorization-server"); - using var register = await client.PostAsJsonAsync("/api/v1/oauth/register", - new { redirect_uris = new[] { OAuthServerTests.RedirectUri } }); - - Assert.Equal(HttpStatusCode.NotFound, token.StatusCode); - Assert.Equal(HttpStatusCode.NotFound, authorize.StatusCode); - Assert.Equal(HttpStatusCode.NotFound, metadata.StatusCode); - Assert.Equal(HttpStatusCode.NotFound, register.StatusCode); - Assert.Null(factory.Services.GetService()); - // So OAuthPurgeSweep finds nothing to run. - await using var scope = factory.Services.CreateAsyncScope(); - Assert.Null(scope.ServiceProvider.GetService()); + new Dictionary + { + ["grant_type"] = GrantTypes.AuthorizationCode, + ["client_id"] = clientId, + ["code"] = QueryHelpers.ParseQuery(location.Query)["code"].ToString(), + ["redirect_uri"] = OAuthServerTests.RedirectUri, + ["code_verifier"] = verifier, + })); + + Assert.Equal(HttpStatusCode.Redirect, navigation.StatusCode); + Assert.StartsWith("/connect?", navigation.Headers.Location!.OriginalString); + Assert.Equal("Claude Desktop", + (await consent.Content.ReadFromJsonAsync()).GetProperty("clientName").GetString()); + Assert.Equal(HttpStatusCode.OK, token.StatusCode); + Assert.False(string.IsNullOrEmpty( + (await token.Content.ReadFromJsonAsync()).GetProperty("access_token").GetString())); + } + + [Theory] + [InlineData("", "OAuth:Issuer is not configured")] + [InlineData("http://farm.example/", "OAuth:Issuer must be an absolute https URL")] + [InlineData("https://farm.example/?tenant=1", "OAuth:Issuer must be an absolute https URL")] + [InlineData("https://farm.example/#", "OAuth:Issuer must be an absolute https URL")] + public void Production_WithoutAnHttpsIssuer_RefusesToStart(string issuer, string message) + { + using var host = factory.WithWebHostBuilder(builder => builder.UseSetting("OAuth:Issuer", issuer)); + + var failure = Assert.ThrowsAny(() => host.CreateClient()); + + Assert.Contains(message, failure.ToString()); } } diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthServerTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthServerTests.cs index ad15adbb0..700c7c9e2 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthServerTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthServerTests.cs @@ -6,6 +6,8 @@ using System.Text; using System.Text.Json; using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Api.Modules.Access.Auth; +using Cluckwork.Api.Modules.Access.OAuth; using Cluckwork.Infrastructure.Persistence; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; @@ -114,16 +116,19 @@ public async Task OpenIdScope_IsRefused() Assert.Contains($"error:{Errors.InvalidScope}", await response.Content.ReadAsStringAsync()); } + // #798 — a browser navigation carries no session bearer, so the SPA's consent route + // takes the validated request over, unchanged. [Fact] - public async Task AuthorizationRequest_WithoutASignedInUser_IsRefused() + public async Task AuthorizationRequest_WithoutASignedInUser_GoesToTheConsentRoute() { using var host = WithResourceProbe(factory); var clientId = await RegisterClientAsync(host.Services); + var query = AuthorizeQuery(clientId, NewCodeVerifier()); - using var response = await SendAuthorizeAsync(host, jwt: null, - AuthorizeQuery(clientId, NewCodeVerifier())); + using var response = await SendAuthorizeAsync(host, jwt: null, query); - Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal(QueryHelpers.AddQueryString("/connect", query), response.Headers.Location!.OriginalString); } // OpenIddict's keys here are ephemeral, so two hosts never share them. A code one @@ -265,13 +270,31 @@ private static async Task AuthorizeAsync( WebApplicationFactory host, string jwt, string clientId, string verifier, string redirectUri = RedirectUri) { - using var response = await SendAuthorizeAsync(host, jwt, AuthorizeQuery(clientId, verifier, redirectUri)); - Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); - var location = response.Headers.Location!; + var location = await ApproveAsync(host, jwt, AuthorizeQuery(clientId, verifier, redirectUri)); Assert.StartsWith(redirectUri.Split('?')[0], location.OriginalString); return QueryHelpers.ParseQuery(location.Query)["code"].ToString(); } + // The consent route's Allow (#798): the user re-enters their password for a step-up + // grant, and the approval comes back as the client's redirect. + internal static async Task ApproveAsync( + WebApplicationFactory host, string jwt, Dictionary query) + { + using var response = await SendAuthorizeAsync(host, jwt, query, await StepUpAsync(host, jwt)); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + return RedirectOf(await response.Content.ReadFromJsonAsync()); + } + + internal static Uri RedirectOf(JsonElement body) => new(body.GetProperty("redirectUri").GetString()!); + + internal static async Task StepUpAsync(WebApplicationFactory host, string jwt) + { + using var response = await HttpsClient(host, jwt).PostAsJsonAsync( + "/api/v1/auth/step-up", new { password = TestHarness.Password }); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync()).GetProperty("token").GetString()!; + } + internal static Dictionary AuthorizeQuery( string clientId, string verifier, string redirectUri = RedirectUri) => new() { @@ -284,8 +307,14 @@ private static async Task AuthorizeAsync( }; internal static Task SendAuthorizeAsync( - WebApplicationFactory host, string? jwt, Dictionary query) => - HttpsClient(host, jwt).GetAsync(QueryHelpers.AddQueryString("/api/v1/oauth/authorize", query)); + WebApplicationFactory host, string? jwt, Dictionary query, + string? stepUp = null, string? consent = null) + { + var request = new HttpRequestMessage(HttpMethod.Get, QueryHelpers.AddQueryString("/api/v1/oauth/authorize", query)); + if (stepUp is not null) request.Headers.Add(AuthEndpoints.StepUpHeaderName, stepUp); + if (consent is not null) request.Headers.Add(OAuthEndpoints.ConsentHeaderName, consent); + return HttpsClient(host, jwt).SendAsync(request); + } private static Task RedeemAsync( WebApplicationFactory host, string clientId, string code, string verifier, diff --git a/tests/Cluckwork.Api.IntegrationTests/OtlpSubprocessExporterTests.cs b/tests/Cluckwork.Api.IntegrationTests/OtlpSubprocessExporterTests.cs index a5941a93e..e76d3d0b4 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OtlpSubprocessExporterTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OtlpSubprocessExporterTests.cs @@ -317,6 +317,7 @@ private static ProcessStartInfo BuildStartInfo(string connectionString, Action

psi.Environment["SharedState__Redis__ConnectionString"] = "abortConnect=false", Satisfy: psi => psi.Environment.Remove("SharedState__Redis__ConnectionString")), + + // #798 — Production runs the authorization server, which needs its public URL. + new("#798 issuer missing", "OAuth:Issuer is not configured", + Violate: psi => psi.Environment.Remove("OAuth__Issuer"), + Satisfy: SatisfyOAuthIssuer), + + new("#798 issuer not https", "OAuth:Issuer must be an absolute https URL", + Violate: psi => psi.Environment["OAuth__Issuer"] = "http://farm.example/", + Satisfy: SatisfyOAuthIssuer), ]; + private static void SatisfyOAuthIssuer(ProcessStartInfo psi) => + psi.Environment["OAuth__Issuer"] = "https://farm.example/"; + // Exposed so ServingGuardCoverageTests can hold this table against the source // it claims to enumerate — without that, deleting a row silently deletes an // arm and the suite stays green (#347 review round 3). diff --git a/tests/Cluckwork.Api.IntegrationTests/ServingGuardCoverageTests.cs b/tests/Cluckwork.Api.IntegrationTests/ServingGuardCoverageTests.cs index 643a35b5f..2be0b0927 100644 --- a/tests/Cluckwork.Api.IntegrationTests/ServingGuardCoverageTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/ServingGuardCoverageTests.cs @@ -90,6 +90,8 @@ public void EveryServingBootGuardMethod_HasACoveringRow() ["EnsureNoPlaintextDataProtectionKeysAsync"] = ["Data Protection key(s) stored without encryption (key id: 00000000-0000-0000-0000-000000000794)"], ["EnsureSharedStateConnectionValid"] = ["SharedState:Redis:ConnectionString is set but not a valid"], + ["EnsureOAuthIssuer"] = + ["OAuth:Issuer is not configured", "OAuth:Issuer must be an absolute https URL"], }; Assert.NotEmpty(guardMethods); diff --git a/tests/Cluckwork.Application.Tests/Architecture/Data/coupling-matrix.md b/tests/Cluckwork.Application.Tests/Architecture/Data/coupling-matrix.md index ac866c88f..bfb659a10 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/Data/coupling-matrix.md +++ b/tests/Cluckwork.Application.Tests/Architecture/Data/coupling-matrix.md @@ -12,7 +12,7 @@ | GeneralInventory | — | R (6) | R (6) fk:3 | R (4) fk:2 | — | — | — | — | P | | Finance | — | R (4) | R (3) fk:1 | — | — | — | — | — | P | | Insights | — | R (1) | R (2) | R (2) | R (2) | R (1) | R (2) | — | P | -| Platform | A (30) | A (25) | A (23) | A (25) | A (31) | A (18) | A (9) | A (25) | — | +| Platform | A (31) | A (25) | A (23) | A (25) | A (31) | A (18) | A (9) | A (25) | — | ## Cross-owner foreign keys @@ -63,7 +63,7 @@ After-commit events and read-model queries are not syntactically observable, and | GeneralInventory | Insights | E | — | | Finance | Insights | E | — | | Insights | Platform | Q/P | P | -| Platform/adapters | Access | W | A (30) | +| Platform/adapters | Access | W | A (31) | | Platform/adapters | Farm | W | A (25) | | Platform/adapters | FlockManagement | W | A (23) | | Platform/adapters | EggOperations | W | A (25) | diff --git a/tests/Cluckwork.Application.Tests/Architecture/RealModuleLedger.Adapters.cs b/tests/Cluckwork.Application.Tests/Architecture/RealModuleLedger.Adapters.cs index ebe12ec93..81cc544db 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/RealModuleLedger.Adapters.cs +++ b/tests/Cluckwork.Application.Tests/Architecture/RealModuleLedger.Adapters.cs @@ -171,6 +171,7 @@ internal static partial class RealModuleLedger new("Cluckwork.Api.Modules.Access.Me.MeEndpoints.GetMe", ["Access"]), new("Cluckwork.Api.Modules.Access.Me.MeEndpoints.SetLanguage", ["Access"]), new("Cluckwork.Api.Modules.Access.Me.MeEndpoints.SetStepperUnit", ["Access"]), + new("Cluckwork.Api.Modules.Access.OAuth.OAuthEndpoints.Authorize", ["Access"]), new("Cluckwork.Api.Modules.Insights.Reports.ReportEndpoints.Expenses", ["Insights"]), new("Cluckwork.Api.Modules.Insights.Reports.ReportEndpoints.Production", [ "FlockManagement", diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 4ee475b19..d9e134d8c 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # -# tools/oauth/mutation-check.sh — proves each #795, #796 and #797 OAuth claim has a test that +# tools/oauth/mutation-check.sh — proves each #795 to #798 OAuth claim has a test that # fails when the claim breaks. # # Baseline green, then per mutant: apply one exact-string edit (it must match once), @@ -41,12 +41,36 @@ MUST_CHANGE=src/Cluckwork.Api/Middleware/MustChangePasswordMiddleware.cs TESTS=tests/Cluckwork.Api.IntegrationTests TEST_NS=Cluckwork.Api.IntegrationTests SUITE='FullyQualifiedName~OAuth' -SUITE_MIN=79 +SUITE_MIN=105 # name # expect # file # find # replace # test # declared failure text # ('#' because C# anchors contain '|'; '\n' in a find or replace is a newline) MUTANTS=$(cat <<'EOF' -production-gate#kill#IDENTITY# if (role is not ProcessRole.Serving || environment.IsProduction()# if (role is not ProcessRole.Serving#OAuthServerProductionTests.Production_RunsNoAuthorizationServer#Expected: NotFound +production-issuer-optional#kill#IDENTITY# if (environment.IsProduction()) return EnsureOAuthIssuer(issuer);\n##OAuthServerProductionTests.Production_WithoutAnHttpsIssuer_RefusesToStart#No exception was thrown +production-http-issuer#kill#IDENTITY# && uri.Scheme == Uri.UriSchemeHttps##OAuthServerProductionTests.Production_WithoutAnHttpsIssuer_RefusesToStart#No exception was thrown +production-issuer-query#kill#IDENTITY#\n && uri.Query.Length == 0 && uri.Fragment.Length == 0##OAuthServerProductionTests.Production_WithoutAnHttpsIssuer_RefusesToStart#No exception was thrown +scope-permissions-enforced#kill#SERVER# .IgnoreScopePermissions()\n##OAuthServerProductionTests.Production_ServesTheConnectFlow#Expected: OK +reconnect-without-password#kill#ENDPOINT# if (stepUpToken is null)\n return Results.Json(new ConsentRequest(\n request.ClientId!,\n await applications.GetDisplayNameAsync(application, ct),\n new Uri(ValidatedRedirectUri(context)).Host,\n scopes,\n [.. scopes.Where(allowed.Contains)],\n AlreadyApproved: covering is not null,\n AssignedFlocks: flockScope.IsUnrestricted ? null\n : [.. (await access.ListFlockAssignmentsAsync(currentUser.UserId, ct))\n .Select(assignment => assignment.FlockName).OfType()]));\n\n var proof = await access.ConsumeStepUpGrantAsync(tenant.AccountId, currentUser.UserId, stepUpToken, ct);\n if (proof.IsFailure)\n return Results.Problem(proof.Error.Description,\n statusCode: StatusCodes.Status403Forbidden, title: proof.Error.Code);\n# if (covering is null)\n {\n if (stepUpToken is null)\n return Results.Json(new ConsentRequest(request.ClientId!, null, "", scopes, [], false, null));\n var proof = await access.ConsumeStepUpGrantAsync(tenant.AccountId, currentUser.UserId, stepUpToken, ct);\n if (proof.IsFailure)\n return Results.Problem(proof.Error.Description,\n statusCode: StatusCodes.Status403Forbidden, title: proof.Error.Code);\n }\n#OAuthConsentTests.ApprovedApp_AsksOnlyForThePassword#a session bearer alone minted a code +reconnect-grant-unspent#kill#ENDPOINT# var proof = await access.ConsumeStepUpGrantAsync(# var proof = covering is not null ? Cluckwork.Domain.Common.Result.Success() : await access.ConsumeStepUpGrantAsync(#OAuthConsentTests.Reconnect_SpendsTheGrant#Expected: Forbidden +redirect-host-unvalidated#kill#ENDPOINT#new Uri(ValidatedRedirectUri(context)).Host#new Uri(request.RedirectUri!).Host#OAuthConsentTests.OmittedRedirectUri_UsesTheRegisteredOne#Expected: OK +session-lifetime-unchecked#kill#IDENTITY# ValidateLifetime = true,# ValidateLifetime = false,#OAuthConsentTests.ExpiredSession_IsUnauthorized_NotARedirect#Expected: Unauthorized +grant-unchecked#kill#ENDPOINT# if (proof.IsFailure)# if (proof.IsFailure && stepUpToken.Length < 0)#OAuthConsentTests.Approval_WithoutAValidGrant_IsRefused#Expected: Forbidden +ad-hoc-authorization#kill#ENDPOINT# identity, subject, applicationId, AuthorizationTypes.Permanent, scopes, ct);# identity, subject, applicationId, AuthorizationTypes.AdHoc, scopes, ct);#OAuthConsentTests.ApprovedApp_AsksOnlyForThePassword#an approved app was shown its permissions again +wider-request-skipped#kill#ENDPOINT# if (covering is null && scopes.All(granted.Contains)) covering = authorization;# if (covering is null) covering = authorization;#OAuthConsentTests.MoreScopes_AsksAgain_NamingWhatIsAlreadyAllowed#a wider request skipped the permissions +any-users-approval#kill#ENDPOINT# subject, applicationId, Statuses.Valid, AuthorizationTypes.Permanent, scopes: null, ct))# null, applicationId, Statuses.Valid, AuthorizationTypes.Permanent, scopes: null, ct))#OAuthConsentTests.AnotherUsersApproval_DoesNotSkip#another user's approval skipped the permissions +revoked-approval-skips#kill#ENDPOINT# subject, applicationId, Statuses.Valid, AuthorizationTypes.Permanent, scopes: null, ct))# subject, applicationId, null, AuthorizationTypes.Permanent, scopes: null, ct))#OAuthConsentTests.DisconnectedApproval_DoesNotSkip#a revoked approval skipped the permissions +cancel-ignored#kill#ENDPOINT# if (consent == "deny")# if (consent == "never")#OAuthConsentTests.Cancel_SendsAccessDeniedToTheClient_AndRecordsNothing#cancel did not send the user back +expired-bearer-redirected#kill#ENDPOINT# return Results.Unauthorized();# return Results.Redirect("/connect");#OAuthConsentTests.ExpiredSession_IsUnauthorized_NotARedirect#Expected: Unauthorized +preview-leaks-request#kill#ENDPOINT# return Results.Json(new\n {\n clientName = # return Results.Json(new\n {\n clientId = context.GetOpenIddictServerRequest()!.ClientId,\n clientName = #OAuthConsentTests.Preview_WithoutASession_NamesTheAppOnly#Collections differ +scoped-worker-unrestricted#kill#ENDPOINT#AssignedFlocks: flockScope.IsUnrestricted ? null#AssignedFlocks: flockScope.IsResolved ? null#OAuthConsentTests.Payload_NamesTheAssignedFlocks_OfAScopedWorker#a flock-scoped worker was told every flock +unassigned-worker-scoped#kill#ENDPOINT#AssignedFlocks: flockScope.IsUnrestricted ? null#AssignedFlocks: !flockScope.IsResolved ? null#OAuthConsentTests.Payload_SaysEveryFlock_ForAnUnassignedWorker#Expected: Null +navigation-refused#kill#ENDPOINT# return Results.Redirect("/connect" + context.Request.QueryString);# return Results.Unauthorized();#OAuthServerTests.AuthorizationRequest_WithoutASignedInUser_GoesToTheConsentRoute#Unauthorized +widest-default-scope#kill#ENDPOINT#asked : [OAuthScopes.ReadFarm];#asked : [.. OAuthScopes.All];#OAuthConsentTests.NoScope_AsksForReadOnly#Collections differ +redirect-not-json#kill#SERVER#if (context.RedirectUri is null || !HasBearer(request))#if (context.RedirectUri is null || HasBearer(request))#OAuthConsentTests.Approval_ReturnsTheClientRedirect_AsJson#Expected: OK +error-keeps-400#kill#SERVER# response.StatusCode = StatusCodes.Status200OK;\n##OAuthConsentTests.Cancel_SendsAccessDeniedToTheClient_AndRecordsNothing#Expected: OK +form-post-allowed#kill#SERVER# options.ResponseModes.Remove(ResponseModes.FormPost);\n##OAuthConsentTests.ResponseModeOtherThanQuery_IsRefused#Expected: BadRequest +fragment-allowed#kill#SERVER# options.ResponseModes.Remove(ResponseModes.Fragment);\n##OAuthConsentTests.ResponseModeOtherThanQuery_IsRefused#Expected: BadRequest +discovery-follows-host#kill#SERVER#new Uri(publicBase, context.BaseUri!.MakeRelativeUri(context.AuthorizationEndpoint!));\n context.TokenEndpoint = new Uri(publicBase, #new Uri(context.BaseUri!, context.BaseUri!.MakeRelativeUri(context.AuthorizationEndpoint!));\n context.TokenEndpoint = new Uri(context.BaseUri!, #OAuthConsentTests.Discovery_NamesEndpointsUnderTheIssuer#Strings differ pkce-optional#kill#SERVER# .RequireProofKeyForCodeExchange()\n##OAuthServerTests.AuthorizationRequestWithoutPkce_IsRefused#Expected: BadRequest plain-pkce-allowed#kill#SERVER# options.CodeChallengeMethods.Remove(CodeChallengeMethods.Plain);\n##OAuthServerTests.PlainCodeChallenge_IsRefused#Expected: BadRequest self-contained-tokens#kill#SERVER# .UseReferenceAccessTokens()\n##OAuthServerTests.AuthorizationCodeWithPkce_IssuesAReferenceTokenTheResourceSideAccepts#the access token has no ReferenceId @@ -82,17 +106,17 @@ post-authorize-allowed#kill#SERVER# if (!HttpMethods.IsGe redirect-any-http-host#kill#REGISTRATION#uri.Host.Length != 0 || IsLoopback(uri));#uri.Host.Length != 0 || uri.Scheme == Uri.UriSchemeHttp);#OAuthClientRegistrationTests.WiderRedirectUri_IsRefused#Expected: BadRequest redirect-fragment#hold#REGISTRATION# && uri.Fragment.Length == 0\n##OAuthClientRegistrationTests.WiderRedirectUri_IsRefused# localhost-refused#kill#REGISTRATION#uri.Host is "localhost" or "127.0.0.1" or "[::1]"#uri.Host is "127.0.0.1" or "[::1]"#OAuthClientRegistrationTests.AllowedRedirectUri_IsRegistered#Expected: Created -loopback-not-native#kill#REGISTRATION# if (native)\n descriptor.ApplicationType = ApplicationTypes.Native;\n##OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: Found -loopback-port-kept#kill#REGISTRATION#native ? new UriBuilder(uri) { Port = -1 }.Uri : uri#uri#OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: Found -loopback-query-dropped#kill#REGISTRATION#new UriBuilder(uri) { Port = -1 }#new UriBuilder(uri) { Port = -1, Query = "" }#OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: Found +loopback-not-native#kill#REGISTRATION# if (native)\n descriptor.ApplicationType = ApplicationTypes.Native;\n##OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: OK +loopback-port-kept#kill#REGISTRATION#native ? new UriBuilder(uri) { Port = -1 }.Uri : uri#uri#OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: OK +loopback-query-dropped#kill#REGISTRATION#new UriBuilder(uri) { Port = -1 }#new UriBuilder(uri) { Port = -1, Query = "" }#OAuthClientRegistrationTests.LoopbackClient_AuthorizesOnAnotherPort#Expected: OK loopback-host-merged#kill#REGISTRATION#new UriBuilder(uri) { Port = -1 }#new UriBuilder(uri) { Port = -1, Host = "localhost" }#OAuthClientRegistrationTests.LoopbackClient_OnAnotherPort_StillMatchesTheRest#Expected: BadRequest -response-normalized#kill#ENDPOINT#uri => uri.OriginalString)#uri => uri.AbsoluteUri)#OAuthClientRegistrationTests.ReturnedRedirectUri_IsUsable#Expected: Found +response-normalized#kill#ENDPOINT#uri => uri.OriginalString)#uri => uri.AbsoluteUri)#OAuthClientRegistrationTests.ReturnedRedirectUri_IsUsable#Expected: OK reserved-parameter-500#kill#ENDPOINT#catch (OpenIddictExceptions.ValidationException exception)#catch (OpenIddictExceptions.ValidationException exception) when (exception.Results.IsDefault)#OAuthClientRegistrationTests.ReservedRedirectParameter_IsARegistrationError#Expected: BadRequest grant-any#kill#REGISTRATION# || grants.Any(grant => grant is not (GrantTypes.AuthorizationCode or GrantTypes.RefreshToken))))#))#OAuthClientRegistrationTests.WiderGrant_IsRefused#Expected: BadRequest auth-method-any#kill#REGISTRATION#request.TokenEndpointAuthMethod is not (null or ClientAuthenticationMethods.None)#request.TokenEndpointAuthMethod is ""#OAuthClientRegistrationTests.WiderGrant_IsRefused#Expected: BadRequest name-keeps-bidi#kill#REGISTRATION# or UnicodeCategory.Format or UnicodeCategory.PrivateUse# or UnicodeCategory.PrivateUse#OAuthClientRegistrationTests.ClientName_IsSanitized#Strings differ name-uncapped#kill#REGISTRATION#elements.MoveNext() && capped.Length + elements.GetTextElement().Length <= MaxNameLength#elements.MoveNext()#OAuthClientRegistrationTests.ClientName_IsSanitized#Strings differ -discovery-silent#kill#SERVER# context.Metadata["registration_endpoint"] =\n new Uri(context.AuthorizationEndpoint!, "register").AbsoluteUri;\n##OAuthClientRegistrationTests.Discovery_AdvertisesTheRegistrationEndpoint#discovery does not advertise registration_endpoint +discovery-silent#kill#SERVER# context.Metadata["registration_endpoint"] =\n new Uri(context.AuthorizationEndpoint, "register").AbsoluteUri;\n##OAuthClientRegistrationTests.Discovery_AdvertisesTheRegistrationEndpoint#discovery does not advertise registration_endpoint register-unlimited#kill#ENDPOINT# .RequireRateLimiting(RateLimitingOptions.OAuthRegisterPolicyName)\n##OAuthClientRegistrationTests.Registration_IsRateLimitedPerClientIp_OnTheSharedCounter#Collections differ register-process-local#kill#LIMITS# limiter.AddPolicy(\n RateLimitingOptions.OAuthRegisterPolicyName,\n new DistributedFixedWindowPolicy(\n RateLimitingOptions.OAuthRegisterPolicyName,\n rateLimiting.OAuthRegister.PermitLimit,\n TimeSpan.FromSeconds(rateLimiting.OAuthRegister.WindowSeconds)));# limiter.AddPolicy(RateLimitingOptions.OAuthRegisterPolicyName, context => RateLimitPartition.GetFixedWindowLimiter(RateLimitKey.ForClient(context.Connection.RemoteIpAddress), _ => new FixedWindowRateLimiterOptions { PermitLimit = rateLimiting.OAuthRegister.PermitLimit, Window = TimeSpan.FromSeconds(rateLimiting.OAuthRegister.WindowSeconds) }));#OAuthClientRegistrationTests.Registration_IsRateLimitedPerClientIp_OnTheSharedCounter#the registration limit did not count on the shared counter register-reads-bearer#kill#ENDPOINT# .WithMetadata(new IgnoresAmbientPrincipalAttribute())\n##OAuthClientRegistrationTests.Registration_IgnoresASessionBearer#Expected: Created diff --git a/tools/simulation/bootstrap.sh b/tools/simulation/bootstrap.sh index 563232270..54d07c6b1 100755 --- a/tools/simulation/bootstrap.sh +++ b/tools/simulation/bootstrap.sh @@ -236,6 +236,12 @@ Jwt__PrivateKeyPem="${JWT_PRIVATE_PEM}" DataProtection__CertificatePem="${DP_CERT_PEM}" DataProtection__PrivateKeyPem="${DP_KEY_PEM}" +# --- OAuth issuer (#798) — required by the Production serving boot. The sim has +# no public https URL, so this names the sim host. OpenIddict refuses plain-http +# protocol requests outside Development, so the connect flow does not run on this +# http stack; only the boot needs the value. --- +OAuth__Issuer=https://cluckwork-sim.local/ + # --- First-run admin (#283) — SCRIPT-LEVEL values, not app config (no # double-underscore key, so docker-compose's env-file parser does NOT expose # these to the app as ASP.NET config; the app never reads a "seed" credential diff --git a/tools/simulation/docker-compose.sim.yml b/tools/simulation/docker-compose.sim.yml index 7ff2d492d..f72c7f6e3 100644 --- a/tools/simulation/docker-compose.sim.yml +++ b/tools/simulation/docker-compose.sim.yml @@ -106,6 +106,7 @@ services: Jwt__PrivateKeyPem: ${Jwt__PrivateKeyPem} DataProtection__CertificatePem: ${DataProtection__CertificatePem} DataProtection__PrivateKeyPem: ${DataProtection__PrivateKeyPem} + OAuth__Issuer: ${OAuth__Issuer} # Access-token lifetime in minutes. The default is Production's (15); # the CI e2e job sets 2 in its environment so the real-expiry spec waits # under three minutes instead of sixteen. Local reset.sh and the k6 diff --git a/tools/simulation/verify-harness.sh b/tools/simulation/verify-harness.sh index 398e83f09..60de30599 100755 --- a/tools/simulation/verify-harness.sh +++ b/tools/simulation/verify-harness.sh @@ -247,6 +247,21 @@ else: else: ok.append(f"AllowedHosts OK ({'; '.join(hosts)})") +# --- #798 OAuth issuer ------------------------------------------------- +# Checked as CluckworkIdentityServiceCollectionExtensions.EnsureOAuthIssuer does: +# present, and an absolute https URL with no query or fragment. +issuer = str(env.get("OAuth__Issuer") or "").strip() +parsed = urlsplit(issuer) +if not issuer: + fail.append("OAuth__Issuer is not set on the app service — #798 fails the Production boot") +# .NET keeps a bare "?" or "#" in Uri.Query or Uri.Fragment, so the boot guard refuses the +# delimiter even with nothing after it; urlsplit drops it, so test the raw string. +elif parsed.scheme != "https" or not parsed.netloc or "?" in issuer or "#" in issuer: + fail.append(f"OAuth__Issuer={issuer!r} is not an absolute https URL without a query or fragment — " + "#798 fails the Production boot") +else: + ok.append(f"OAuth__Issuer OK ({issuer})") + # --- #261/#262 TLS opt-out ---------------------------------------------- # The EFFECTIVE value on the app service, not a grep of the file: a matching # line elsewhere in the YAML says nothing about what this container receives. diff --git a/web/scripts/verify-sw.mjs b/web/scripts/verify-sw.mjs index e64d5a4ad..d665f8264 100644 --- a/web/scripts/verify-sw.mjs +++ b/web/scripts/verify-sw.mjs @@ -308,7 +308,14 @@ check(missingJs.length === 0, `emitted JavaScript missing from precache: ${missi // that slice. The remaining headroom is reserved for the consent screen (#798) // and the connected-apps screens (#799). The next raise needs a new // maintainer decision. -const PRECACHE_CEILING_KIB = 1920; +// +// 2026-10-09, later the same day: the maintainer approved a second milestone +// raise, to 1,950 KiB. #800 measures 1,903.38 KiB and the consent screen +// (#798) adds 19.15 KiB on the same base, about 1,922.5 KiB together. The rest +// is reserved for #799. The planned way to win the space back is to stop +// precaching the languages a device does not use (es/tl), tracked separately. +// Any further raise needs a new maintainer decision. +const PRECACHE_CEILING_KIB = 1950; let precacheBytes = 0; const missingOnDisk = []; for (const url of precached) { diff --git a/web/src/App.tsx b/web/src/App.tsx index b7f6f4504..c7af87bd5 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,4 +1,4 @@ -import { lazy } from "react"; +import { lazy, Suspense } from "react"; import { BrowserRouter, Navigate, Route, Routes } from "react-router"; import { AuthProvider } from "./auth/AuthContext"; import { FarmThemeProvider } from "./theme/FarmThemeProvider"; @@ -9,6 +9,7 @@ import { ProtectedRoute } from "./routes/ProtectedRoute"; import { AppLayout } from "./routes/AppLayout"; import { Login } from "./routes/Login"; +const ConnectPage = lazy(() => import("./routes/ConnectPage").then(({ ConnectPage }) => ({ default: ConnectPage }))); const Dashboard = lazy(() => import("./routes/Dashboard").then(({ Dashboard }) => ({ default: Dashboard }))); const DailyEntryPage = lazy(() => import("./routes/DailyEntryPage").then(({ DailyEntryPage }) => ({ default: DailyEntryPage }))); const StockPage = lazy(() => import("./routes/StockPage").then(({ StockPage }) => ({ default: StockPage }))); @@ -49,6 +50,9 @@ export function App() { } /> }> + {/* #798 — a connected app's consent screen: the user's language and + farm (SessionProvider), without the app shell. */} + } /> {/* Inside the auth gate (it needs a token to read /account) and outside the shell, so the sidebar's branding slot and every screen's date fields read the same farm (#123). SessionProvider diff --git a/web/src/api/oauth.ts b/web/src/api/oauth.ts new file mode 100644 index 000000000..4c2bd2529 --- /dev/null +++ b/web/src/api/oauth.ts @@ -0,0 +1,46 @@ +import { apiFetch, STEP_UP_HEADER } from "./client"; + +// #798 — the authorize endpoint answers the SPA's consent route with JSON: +// what the screen shows, or where to send the browser next. +export interface ConsentRequest { + clientId: string; + clientName: string | null; + redirectHost: string; + scopes: string[]; + alreadyAllowed: string[]; + alreadyApproved: boolean; + // Null when the user reaches every flock; else the flocks they are assigned to. + assignedFlocks: string[] | null; +} + +export type AuthorizeAnswer = { redirectUri: string } | ConsentRequest; + +const authorizePath = (search: string) => `/oauth/authorize${search}`; + +export function askConsent(search: string): Promise { + return apiFetch(authorizePath(search), { method: "GET" }); +} + +export function approveConsent(search: string, stepUpToken: string): Promise { + return apiFetch(authorizePath(search), { + method: "GET", headers: { [STEP_UP_HEADER]: stepUpToken }, + }); +} + +export function declineConsent(search: string): Promise { + return apiFetch(authorizePath(search), { + method: "GET", headers: { "X-Cluckwork-Consent": "deny" }, + }); +} + +// Before sign-in: only the app's self-chosen name, or null when the request is +// not one the server accepts. +export async function previewConsent(search: string): Promise { + try { + const res = await fetch(`/api/v1${authorizePath(search)}`, { headers: { "X-Cluckwork-Consent": "preview" } }); + if (!res.ok) return null; + return ((await res.json()) as { clientName: string | null }).clientName; + } catch { + return null; + } +} diff --git a/web/src/auth/returnPath.test.ts b/web/src/auth/returnPath.test.ts new file mode 100644 index 000000000..ce6a7b788 --- /dev/null +++ b/web/src/auth/returnPath.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { returnPath } from "./returnPath"; + +describe("returnPath (#798)", () => { + it.each([ + ["https://evil.example/connect", ""], + ["//evil.example", ""], + ["//evil.example/connect", "?client_id=x"], + ["/\\evil.example", ""], + ["\\\\evil.example", ""], + ["https:evil.example", ""], + ])("refuses %s, which leaves this origin", (pathname, search) => { + expect(returnPath({ pathname, search })).toBe("/"); + }); + + it("keeps the consent route's request", () => { + expect(returnPath({ pathname: "/connect", search: "?client_id=abc&scope=farm%3Aread" })) + .toBe("/connect?client_id=abc&scope=farm%3Aread"); + }); + + it("returns any other path without its query, as before", () => { + expect(returnPath({ pathname: "/sales", search: "?x=1" })).toBe("/sales"); + }); + + it("goes home with nowhere to return to", () => { + expect(returnPath(undefined)).toBe("/"); + }); +}); diff --git a/web/src/auth/returnPath.ts b/web/src/auth/returnPath.ts new file mode 100644 index 000000000..8283c36c9 --- /dev/null +++ b/web/src/auth/returnPath.ts @@ -0,0 +1,10 @@ +// #798 — where sign-in returns to. Only a path on this origin: anything that resolves +// elsewhere (an absolute URL, a protocol-relative //host, a backslash that a browser +// reads as a slash) goes home instead. The consent route keeps its query, which is the +// connected app's request; every other path returns without one, as before. +export function returnPath(from: { pathname: string; search?: string } | undefined): string { + if (!from) return "/"; + const target = new URL(from.pathname + (from.search ?? ""), window.location.origin); + if (target.origin !== window.location.origin) return "/"; + return target.pathname === "/connect" ? target.pathname + target.search : target.pathname; +} diff --git a/web/src/components/AuthShell.test.tsx b/web/src/components/AuthShell.test.tsx index 4848c2d37..1126738d6 100644 --- a/web/src/components/AuthShell.test.tsx +++ b/web/src/components/AuthShell.test.tsx @@ -38,4 +38,14 @@ describe("AuthShell (#976 round 2)", () => { meta.remove(); } }); + + // #798 (login B) — the next step replaces the tagline in the brand panel and + // repeats above the form, where a phone shows it. + it("puts the next step in the brand panel and above the form", () => { + const { getAllByText, queryByText } = render( + Next: approve Claude Desktop}>content); + + expect(getAllByText("Next: approve Claude Desktop")).toHaveLength(2); + expect(queryByText("Daily entry · Stock · Sales")).not.toBeInTheDocument(); + }); }); diff --git a/web/src/components/AuthShell.tsx b/web/src/components/AuthShell.tsx index 4479e4a0f..7fe87bbd4 100644 --- a/web/src/components/AuthShell.tsx +++ b/web/src/components/AuthShell.tsx @@ -5,9 +5,12 @@ import { Box, Paper, Typography } from "@mui/material"; import { ThemeToggle } from "./ThemeToggle"; import { resyncThemeColorMeta } from "../theme/metaThemeColor"; +// nextStep (#798, login B) names what follows sign-in: in the brand panel in place +// of the tagline on a wide screen, and above the form on a phone, where the +// panel is a short band. export function AuthShell({ - children, footerNote, bannerSlot, -}: { children: ReactNode; footerNote: string; bannerSlot?: ReactNode }) { + children, footerNote, bannerSlot, nextStep, +}: { children: ReactNode; footerNote: string; bannerSlot?: ReactNode; nextStep?: ReactNode }) { const { t } = useTranslation("auth"); // #976 round 2 — this screen fills the whole viewport in --surface-2, with @@ -56,15 +59,29 @@ export function AuthShell({ {t("title")} - - {t("shellTagline")} - + {nextStep ? ( + {nextStep} + ) : ( + + {t("shellTagline")} + + )} {bannerSlot} {footerNote} + {nextStep && ( + {nextStep} + )} {children} diff --git a/web/src/components/StepUpPasswordField.tsx b/web/src/components/StepUpPasswordField.tsx new file mode 100644 index 000000000..6259623f3 --- /dev/null +++ b/web/src/components/StepUpPasswordField.tsx @@ -0,0 +1,21 @@ +import { TextField } from "@mui/material"; +import type { TextFieldProps } from "@mui/material"; + +// #308/#798 — the current-password field every step-up uses. The password lives +// only in the caller's state, which clears it as soon as /auth/step-up settles. +export function StepUpPasswordField({ + value, onChange, ...field +}: Omit & { + value: string; + onChange: (value: string) => void; +}) { + return ( + onChange(e.target.value)} + /> + ); +} diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 6defa10eb..7178c036e 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -82,6 +82,9 @@ export const en = { clearFiltersButton: "Clear filters", }, auth: { + loginNextHeading: "Next: approve {{app}}", + loginNextHeadingUnnamed: "Next: approve an app", + loginNextBody: "After you sign in, you see what it asks for and choose whether to allow it.", title: "Cluckwork", shellEyebrow: "Poultry farm management", shellTagline: "Daily entry · Stock · Sales", @@ -156,6 +159,51 @@ export const en = { setPasswordMismatchError: "The new passwords don't match.", setPasswordTooShortError: "The new password must be at least {{min}} characters.", }, + // #798 — the consent screen for connected apps (/connect), compact direction D. + connect: { + title: "Let {{app}} act as you?", + titleMore: "Let {{app}} do more?", + titleReconnect: "Reconnect {{app}}?", + reconnectBody: "You allowed it before. Enter your password to confirm it's you.", + unnamedApp: "An unnamed app", + unverified: "Unverified app", + signedInAs: "Signed in as", + returnsHere: "Returns to this computer", + returnsTo: "Returns to {{host}}", + scopesLabel: "It asks to", + scopeRead: "Read farm data", + scopeReadLine: "Flocks, stock, entries and sales", + scopeWrite: "Record daily entries", + scopeBlocked: "Not allowed for your role", + scopeNew: "New", + scopeHad: "Already allowed", + acts: "Acts as you, with only your access", + roleSr: "Your role:", + password: "Your current password", + allow: "Allow", + cancel: "Cancel", + wrongPassword: "Wrong password. Nothing was connected.", + tooManyAttempts: "Too many attempts. Wait a few minutes and try again.", + failed: "Something went wrong, and nothing was connected. Try again.", + undo: "Disconnect it anytime in Account › Connected apps.", + details: "Details", + detailsActs: "It acts as you, so it can never do more than you can.", + detailsReadAll: "It can read farm data for every flock.", + detailsReadSome: "It can read farm data only for the flocks assigned to you: {{flocks}}.", + detailsWriteAll: "It can record daily entries for any flock.", + detailsWriteSome: "It can record daily entries only for the flocks assigned to you: {{flocks}}.", + detailsWriteBlocked: "It asks to record daily entries, but your role ({{role}}) cannot, so it cannot either.", + detailsName: "{{app}} chose its own name. Cluckwork does not check it, so allow it only if you just started connecting {{app}}.", + detailsReturnHere: "Afterwards your browser goes back to {{host}}, an address on this computer.", + detailsReturnTo: "Afterwards your browser goes to {{host}}.", + detailsScopes: "Reading covers flocks, stock, daily entries, customers, orders and payments.", + notYou: "Not you? Sign out", + refusedTitle: "Nothing to approve", + refusedBody: "This request expired or isn't valid, so nothing was connected. Start again from the app.", + refusedButton: "Go to Cluckwork", + loading: "Loading the request…", + leaving: "Going back to the app…", + }, account: { eyebrow: "Personal preferences", preferences: "Preferences", @@ -3080,6 +3128,13 @@ export const en = { + "changing a login email, disabling a user, re-enabling a user, assigning a worker to a flock, and " + "removing a worker's flock assignment. This confirms it's really you before " + "handing out access or cutting someone else's off. Display-name changes do not ask again.", + signingInConnectApp: + "Connecting an app. An outside app, such as an AI assistant, can ask to act as you. " + + "Cluckwork shows what it asks for, Read farm data, Record daily entries or both, " + + "and you answer with Allow after typing Your current password, or with " + + "Cancel. It can never do more than your own role. When an app you already allowed connects " + + "again, only your password is asked. The app chose its own name, so allow it only if you just started " + + "connecting it. Details has the longer explanation.", signingInCredentialEpoch: "When an administrator resets a password, your existing sign-in can be invalidated immediately. If you " + "see a message that your credentials changed, sign in again with your current password.", @@ -3876,6 +3931,11 @@ export const en = { + "re-runs with the same dates you picked. Each farm has its own allowance, so " + "another farm's reports never use up yours.", // #308/#356/#360 + glossaryAppConsentTerm: "Connecting an app", + glossaryAppConsentDef: + "Approving an outside app, such as an AI assistant, to act as you. You see what it asks for and allow it with " + + "your current password; it can never do more than your role. An app already allowed asks only for the " + + "password when it connects again.", glossaryStepUpAuthTerm: "Step-up authentication", glossaryStepUpAuthDef: "An extra check on top of being signed in: before creating any user, resetting any user's password, " diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 1bc7e425e..68dd175ff 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -47,6 +47,9 @@ export const es = { clearFiltersButton: "Borrar filtros", }, auth: { + loginNextHeading: "Siguiente: aprobar {{app}}", + loginNextHeadingUnnamed: "Siguiente: aprobar una aplicación", + loginNextBody: "Después de iniciar sesión, verás lo que pide y decidirás si lo permites.", title: "Cluckwork", shellEyebrow: "Gestión de granjas avícolas", shellTagline: "Registro diario · Existencias · Ventas", @@ -101,6 +104,50 @@ export const es = { setPasswordMismatchError: "Las nuevas contraseñas no coinciden.", setPasswordTooShortError: "La nueva contraseña debe tener al menos {{min}} caracteres.", }, + connect: { + title: "¿Dejar que {{app}} actúe como tú?", + titleMore: "¿Dejar que {{app}} haga más?", + titleReconnect: "¿Volver a conectar {{app}}?", + reconnectBody: "Ya la permitiste antes. Escribe tu contraseña para confirmar que eres tú.", + unnamedApp: "Una aplicación sin nombre", + unverified: "Aplicación no verificada", + signedInAs: "Sesión iniciada como", + returnsHere: "Vuelve a este equipo", + returnsTo: "Vuelve a {{host}}", + scopesLabel: "Pide", + scopeRead: "Leer datos de la granja", + scopeReadLine: "Lotes, existencias, registros y ventas", + scopeWrite: "Anotar registros diarios", + scopeBlocked: "Tu rol no lo permite", + scopeNew: "Nuevo", + scopeHad: "Ya permitido", + acts: "Actúa como tú, solo con tu acceso", + roleSr: "Tu rol:", + password: "Tu contraseña actual", + allow: "Permitir", + cancel: "Cancelar", + wrongPassword: "Contraseña incorrecta. No se conectó nada.", + tooManyAttempts: "Demasiados intentos. Espera unos minutos y vuelve a intentarlo.", + failed: "Algo salió mal y no se conectó nada. Vuelve a intentarlo.", + undo: "Desconéctala cuando quieras en Cuenta › Aplicaciones conectadas.", + details: "Detalles", + detailsActs: "Actúa como tú, así que nunca puede hacer más que tú.", + detailsReadAll: "Puede leer los datos de la granja de todos los lotes.", + detailsReadSome: "Solo puede leer los datos de la granja de los lotes que tienes asignados: {{flocks}}.", + detailsWriteAll: "Puede anotar registros diarios de cualquier lote.", + detailsWriteSome: "Solo puede anotar registros diarios de los lotes que tienes asignados: {{flocks}}.", + detailsWriteBlocked: "Pide anotar registros diarios, pero tu rol ({{role}}) no puede, así que tampoco puede ella.", + detailsName: "{{app}} eligió su propio nombre. Cluckwork no lo comprueba, así que permítela solo si acabas de empezar a conectar {{app}}.", + detailsReturnHere: "Después, tu navegador vuelve a {{host}}, una dirección de este equipo.", + detailsReturnTo: "Después, tu navegador va a {{host}}.", + detailsScopes: "Leer incluye lotes, existencias, registros diarios, clientes, pedidos y pagos.", + notYou: "¿No eres tú? Cerrar sesión", + refusedTitle: "No hay nada que aprobar", + refusedBody: "Esta solicitud venció o no es válida, así que no se conectó nada. Empieza de nuevo desde la aplicación.", + refusedButton: "Ir a Cluckwork", + loading: "Cargando la solicitud…", + leaving: "Volviendo a la aplicación…", + }, account: { eyebrow: "Preferencias personales", preferences: "Preferencias", @@ -2343,6 +2390,13 @@ export const es = { + "volver a habilitar un usuario, asignar un trabajador a un lote, y quitarle a un trabajador la asignación " + "de un lote. Esto confirma que realmente es usted antes de otorgar acceso o retirárselo " + "a otra persona. Los cambios de nombre para mostrar no vuelven a pedirla.", + signingInConnectApp: + "Conectar una aplicación. Una aplicación externa, como un asistente de IA, puede pedir actuar " + + "como tú. Cluckwork muestra lo que pide, Leer datos de la granja, Anotar registros " + + "diarios o ambos, y respondes con Permitir después de escribir Tu contraseña " + + "actual, o con Cancelar. Nunca puede hacer más que tu propio rol. Cuando una aplicación " + + "que ya permitiste se conecta otra vez, solo se pide tu contraseña. La aplicación eligió su propio nombre, así " + + "que permítela solo si acabas de empezar a conectarla. Detalles tiene la explicación completa.", signingInCredentialEpoch: "Cuando un administrador restablece una contraseña, su sesión actual puede invalidarse inmediatamente. Si " + "ve un mensaje indicando que sus credenciales cambiaron, inicie sesión de nuevo con su contraseña actual.", @@ -3182,6 +3236,11 @@ export const es = { + "el código de la granja que desea.", // #308/#356/#360 (machine-drafted, pending native review) + glossaryAppConsentTerm: "Conectar una aplicación", + glossaryAppConsentDef: + "Aprobar que una aplicación externa, como un asistente de IA, actúe como tú. Ves lo que pide y la permites con tu " + + "contraseña actual; nunca puede hacer más que tu rol. Una aplicación ya permitida solo pide la contraseña " + + "cuando se conecta otra vez.", glossaryStepUpAuthTerm: "Autenticación reforzada (step-up)", glossaryStepUpAuthDef: "Una comprobación adicional además de haber iniciado sesión: antes de crear cualquier usuario, " diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 1e3a5c812..f6d2383ee 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -53,6 +53,9 @@ export const tl = { clearFiltersButton: "I-clear ang mga filter", }, auth: { + loginNextHeading: "Susunod: aprubahan ang {{app}}", + loginNextHeadingUnnamed: "Susunod: aprubahan ang isang app", + loginNextBody: "Pagkatapos mong mag-sign in, makikita mo ang hinihingi nito at pipiliin mo kung papayagan ito.", title: "Cluckwork", shellEyebrow: "Pamamahala ng poultry farm", shellTagline: "Araw-araw na entry · Stock · Sales", @@ -107,6 +110,50 @@ export const tl = { setPasswordMismatchError: "Hindi magkatugma ang mga bagong password.", setPasswordTooShortError: "Dapat hindi bababa sa {{min}} karakter ang bagong password.", }, + connect: { + title: "Hayaan ang {{app}} na kumilos bilang ikaw?", + titleMore: "Hayaan ang {{app}} na gumawa ng higit pa?", + titleReconnect: "Ikonektang muli ang {{app}}?", + reconnectBody: "Pinayagan mo na ito dati. Ilagay ang password mo para kumpirmahing ikaw ito.", + unnamedApp: "Isang app na walang pangalan", + unverified: "Hindi beripikadong app", + signedInAs: "Naka-sign in bilang", + returnsHere: "Babalik sa computer na ito", + returnsTo: "Babalik sa {{host}}", + scopesLabel: "Hinihingi nito", + scopeRead: "Basahin ang datos ng bukid", + scopeReadLine: "Mga flock, stock, tala at benta", + scopeWrite: "Itala ang araw-araw na tala", + scopeBlocked: "Hindi pinapayagan ng role mo", + scopeNew: "Bago", + scopeHad: "Pinayagan na", + acts: "Kumikilos bilang ikaw, gamit lang ang access mo", + roleSr: "Ang role mo:", + password: "Ang kasalukuyan mong password", + allow: "Payagan", + cancel: "Kanselahin", + wrongPassword: "Mali ang password. Walang naikonekta.", + tooManyAttempts: "Masyadong maraming subok. Maghintay ng ilang minuto at subukang muli.", + failed: "May nangyaring mali, at walang naikonekta. Subukang muli.", + undo: "Idiskonekta ito kahit kailan sa Account › Mga nakakonektang app.", + details: "Mga detalye", + detailsActs: "Kumikilos ito bilang ikaw, kaya hindi ito makakagawa ng higit sa kaya mo.", + detailsReadAll: "Mababasa nito ang datos ng bukid para sa bawat flock.", + detailsReadSome: "Mababasa lang nito ang datos ng bukid para sa mga flock na naka-assign sa iyo: {{flocks}}.", + detailsWriteAll: "Makakapagtala ito ng araw-araw na tala para sa alinmang flock.", + detailsWriteSome: "Makakapagtala lang ito ng araw-araw na tala para sa mga flock na naka-assign sa iyo: {{flocks}}.", + detailsWriteBlocked: "Hinihingi nitong magtala ng araw-araw na tala, pero hindi ito kaya ng role mo ({{role}}), kaya hindi rin ito makakapagtala.", + detailsName: "Ang {{app}} ang pumili ng sarili nitong pangalan. Hindi ito sinusuri ng Cluckwork, kaya payagan lang ito kung kasisimula mo pa lang ikonekta ang {{app}}.", + detailsReturnHere: "Pagkatapos, babalik ang browser mo sa {{host}}, isang address sa computer na ito.", + detailsReturnTo: "Pagkatapos, pupunta ang browser mo sa {{host}}.", + detailsScopes: "Saklaw ng pagbasa ang mga flock, stock, araw-araw na tala, customer, order at bayad.", + notYou: "Hindi ikaw? Mag-sign out", + refusedTitle: "Walang dapat aprubahan", + refusedBody: "Nag-expire o hindi wasto ang kahilingang ito, kaya walang naikonekta. Magsimulang muli mula sa app.", + refusedButton: "Pumunta sa Cluckwork", + loading: "Nilo-load ang kahilingan…", + leaving: "Bumabalik sa app…", + }, account: { eyebrow: "Personal na kagustuhan", preferences: "Mga Kagustuhan", @@ -2435,6 +2482,14 @@ export const tl = { + "sa pag-sign in, ang pag-disable ng user, ang muling pag-enable ng user, ang pag-assign ng manggagawa sa " + "isang kawan, at ang pag-alis ng assignment ng manggagawa sa isang kawan. Kinukumpirma nito na ikaw " + "talaga bago magbigay o mag-alis ng access. Hindi muling hinihingi ito sa mga pagbabago ng display name.", + signingInConnectApp: + "Pagkonekta ng app. Puwedeng humiling ang isang panlabas na app, gaya ng AI assistant, na " + + "kumilos bilang ikaw. Ipinapakita ng Cluckwork ang hinihingi nito, Basahin ang datos ng bukid, " + + "Itala ang araw-araw na tala o pareho, at sasagot ka ng Payagan pagkatapos " + + "ilagay ang Ang kasalukuyan mong password, o ng Kanselahin. Hindi ito " + + "makakagawa ng higit sa sarili mong role. Kapag kumonekta ulit ang app na pinayagan mo na, password mo lang ang " + + "hihingin. Ang app ang pumili ng sarili nitong pangalan, kaya payagan lang ito kung kasisimula mo pa lang " + + "ikonekta ito. Nasa Mga detalye ang mas mahabang paliwanag.", signingInCredentialEpoch: "Kapag ni-reset ng administrator ang password, maaaring agad ma-invalid ang kasalukuyan mong sign-in. Kung " + "makakita ka ng mensaheng nagbago ang iyong credentials, mag-sign in muli gamit ang kasalukuyan mong password.", @@ -3306,6 +3361,11 @@ export const tl = { + "manghula; piliin ang code ng bukid na gusto mo.", // #308/#356/#360 (machine-drafted, pending native review) + glossaryAppConsentTerm: "Pagkonekta ng app", + glossaryAppConsentDef: + "Pag-apruba sa isang panlabas na app, gaya ng AI assistant, na kumilos bilang ikaw. Makikita mo ang hinihingi " + + "nito at papayagan mo ito gamit ang kasalukuyan mong password; hindi ito makakagawa ng higit sa role mo. Ang " + + "app na pinayagan na ay password lang ang hihingin kapag kumonekta ulit.", glossaryStepUpAuthTerm: "Karagdagang pagpapatunay (step-up)", glossaryStepUpAuthDef: "Isang karagdagang tsek bukod sa pagiging naka-sign in: bago gumawa ng kahit anong user, mag-reset ng " diff --git a/web/src/i18n/translations-status.ts b/web/src/i18n/translations-status.ts index 0983df36a..4a9310a48 100644 --- a/web/src/i18n/translations-status.ts +++ b/web/src/i18n/translations-status.ts @@ -19,6 +19,7 @@ import type { en } from "./en"; export const TRANSLATED_NAMESPACES = [ "common", "auth", + "connect", "account", "errors", "sales", diff --git a/web/src/routes/ConnectPage.test.tsx b/web/src/routes/ConnectPage.test.tsx new file mode 100644 index 000000000..7480a3b48 --- /dev/null +++ b/web/src/routes/ConnectPage.test.tsx @@ -0,0 +1,234 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, fireEvent, screen, waitFor, within } from "@testing-library/react"; +import { Route, Routes } from "react-router"; +import { ConnectPage } from "./ConnectPage"; +import { renderWithProviders } from "../test/renderWithProviders"; +import { ApiError, logout, stepUp } from "../api/client"; +import { approveConsent, askConsent, declineConsent } from "../api/oauth"; +import type { ConsentRequest } from "../api/oauth"; + +vi.mock("../api/client", async (importOriginal) => ({ + ...(await importOriginal()), + stepUp: vi.fn(), + logout: vi.fn(), + setOnTokensChanged: vi.fn(), + setOnUnauthenticated: vi.fn(), +})); +vi.mock("../api/oauth", () => ({ + askConsent: vi.fn(), + approveConsent: vi.fn(), + declineConsent: vi.fn(), + previewConsent: vi.fn(), +})); + +const SEARCH = "?client_id=c1&scope=farm%3Aread%20daily-entries%3Awrite"; +const REQUEST: ConsentRequest = { + clientId: "c1", clientName: "Claude Desktop", redirectHost: "127.0.0.1", + scopes: ["farm:read", "daily-entries:write"], alreadyAllowed: [], alreadyApproved: false, assignedFlocks: null, +}; +const assign = vi.fn(); +const originalLocation = window.location; + +beforeEach(() => { + Object.defineProperty(window, "location", { configurable: true, value: { ...originalLocation, assign } }); +}); +afterEach(() => { + cleanup(); + vi.clearAllMocks(); + Object.defineProperty(window, "location", { configurable: true, value: originalLocation }); +}); + +async function show(request: ConsentRequest, role: string | null = "Manager") { + vi.mocked(askConsent).mockResolvedValue(request); + renderWithProviders( + } />, + { + route: `/connect${SEARCH}`, + token: role ? { sub: "u1", role } : { sub: "u1" }, + me: { id: "u1", email: "ana.reyes@meadowlark.farm", name: null, role: role ?? "Worker", language: null, preferredStepperUnit: null }, + }, + ); + await screen.findByRole("heading", { level: 1 }); +} + +async function allowWith(password: string) { + fireEvent.change(screen.getByLabelText(/Your current password/), { target: { value: password } }); + await act(async () => { fireEvent.click(screen.getByRole("button", { name: "Allow" })); }); +} + +describe("ConnectPage (#798, consent D)", () => { + it("asks for the request it was opened with and shows the compact card", async () => { + await show(REQUEST); + + expect(askConsent).toHaveBeenCalledWith(SEARCH); + expect(screen.getByRole("heading", { name: "Let Claude Desktop act as you?" })).toBeInTheDocument(); + expect(screen.getByText("Unverified app")).toBeInTheDocument(); + expect(screen.getByText("Returns to this computer")).toBeInTheDocument(); + const rows = within(screen.getByRole("list", { name: "It asks to" })).getAllByRole("listitem"); + expect(rows.map((row) => row.textContent)).toEqual([ + "Read farm dataFlocks, stock, entries and sales", + "Record daily entries", + ]); + expect(screen.getByText("Acts as you, with only your access")).toBeInTheDocument(); + expect(screen.getByText("Your role:").parentElement).toHaveTextContent("Your role: Manager"); + expect(screen.getByText("Disconnect it anytime in Account › Connected apps.")).toBeInTheDocument(); + expect(screen.getByText("Details").closest("details")).not.toHaveAttribute("open"); + }); + + it("names the signed-in account prominently, not as a muted caption", async () => { + await show(REQUEST); + + const email = screen.getByText("Signed in as").parentElement!; + expect(email).toHaveTextContent("Signed in as ana.reyes@meadowlark.farm"); + expect(getComputedStyle(email).fontWeight).toBe("600"); + }); + + it("spends a step-up grant on Allow and sends the browser to the app", async () => { + vi.mocked(stepUp).mockResolvedValue({ token: "grant-1", expiresAt: "" }); + vi.mocked(approveConsent).mockResolvedValue({ redirectUri: "http://127.0.0.1:5000/cb?code=abc" }); + await show(REQUEST); + + await allowWith("pw"); + + expect(stepUp).toHaveBeenCalledWith("pw"); + expect(approveConsent).toHaveBeenCalledWith(SEARCH, "grant-1"); + expect(assign).toHaveBeenCalledWith("http://127.0.0.1:5000/cb?code=abc"); + }); + + it("says a wrong password connected nothing and asks nothing of the server", async () => { + vi.mocked(stepUp).mockRejectedValue(new ApiError(400, "Users.CurrentPasswordIncorrect", "Wrong")); + await show(REQUEST); + + await allowWith("nope"); + + expect(await screen.findByRole("alert")).toHaveTextContent("Wrong password. Nothing was connected."); + expect(approveConsent).not.toHaveBeenCalled(); + expect(screen.getByLabelText(/Your current password/)).toHaveValue(""); + }); + + it("puts the new permission first when the app asks for more", async () => { + await show({ ...REQUEST, alreadyAllowed: ["farm:read"] }); + + expect(screen.getByRole("heading", { name: "Let Claude Desktop do more?" })).toBeInTheDocument(); + const rows = within(screen.getByRole("list", { name: "It asks to" })).getAllByRole("listitem"); + expect(rows.map((row) => row.textContent)).toEqual([ + "Record daily entries New", + "Read farm dataAlready allowed", + ]); + }); + + it("asks an already-approved app for the password only", async () => { + await show({ ...REQUEST, alreadyAllowed: REQUEST.scopes, alreadyApproved: true }); + + expect(screen.getByRole("heading", { name: "Reconnect Claude Desktop?" })).toBeInTheDocument(); + expect(screen.getByText("You allowed it before. Enter your password to confirm it's you.")).toBeInTheDocument(); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); + expect(screen.queryByText("Details")).not.toBeInTheDocument(); + expect(screen.getByText("Signed in as").parentElement).toHaveTextContent("ana.reyes@meadowlark.farm"); + expect(screen.getByLabelText(/Your current password/)).toBeInTheDocument(); + }); + + it("keeps the longer explanations and the sign-out under Details", async () => { + await show(REQUEST, null); + + fireEvent.click(screen.getByText("Details")); + + const details = screen.getByText("Details").closest("details")!; + expect(details).toHaveTextContent("It acts as you, so it can never do more than you can."); + expect(details).toHaveTextContent("Claude Desktop chose its own name."); + expect(details).toHaveTextContent("Afterwards your browser goes back to 127.0.0.1, an address on this computer."); + await act(async () => { fireEvent.click(within(details).getByRole("button", { name: "Not you? Sign out" })); }); + expect(logout).toHaveBeenCalled(); + }); + + async function detailsFor(request: ConsentRequest, role: string | null) { + await show(request, role); + fireEvent.click(screen.getByText("Details")); + return screen.getByText("It acts as you, so it can never do more than you can.").parentElement!; + } + + it("limits a flock-scoped Worker's app to the assigned flocks", async () => { + const details = await detailsFor({ ...REQUEST, assignedFlocks: ["House A", "House B"] }, null); + + expect(details).toHaveTextContent( + "It can read farm data only for the flocks assigned to you: House A, House B. " + + "It can record daily entries only for the flocks assigned to you: House A, House B."); + }); + + it("tells a Worker with no assignments that the app reaches every flock", async () => { + const details = await detailsFor(REQUEST, null); + + expect(details).toHaveTextContent( + "It can read farm data for every flock. It can record daily entries for any flock."); + }); + + it("promises no recording when the app asks only to read", async () => { + const details = await detailsFor({ ...REQUEST, scopes: ["farm:read"] }, "Manager"); + + expect(details).toHaveTextContent("It can read farm data for every flock."); + expect(details).not.toHaveTextContent("record"); + }); + + it("lets a Manager's app read and record for every flock", async () => { + const details = await detailsFor(REQUEST, "Manager"); + + expect(details).toHaveTextContent( + "It can read farm data for every flock. It can record daily entries for any flock."); + }); + + it("starts focus at the request, not the password", async () => { + await show(REQUEST); + + expect(screen.getByRole("heading", { level: 1 })).toHaveFocus(); + expect(screen.getByLabelText(/Your current password/)).not.toHaveFocus(); + }); + + it("starts a reconnect at the request too", async () => { + await show({ ...REQUEST, alreadyAllowed: REQUEST.scopes, alreadyApproved: true }); + + expect(screen.getByRole("heading", { name: "Reconnect Claude Desktop?" })).toHaveFocus(); + }); + + it("returns focus to the password after a wrong one", async () => { + vi.mocked(stepUp).mockRejectedValue(new ApiError(400, "Users.CurrentPasswordIncorrect", "Wrong")); + await show(REQUEST); + + await allowWith("nope"); + + await waitFor(() => expect(screen.getByLabelText(/Your current password/)).toHaveFocus()); + }); + + it("marks recording entries as not allowed for a role that cannot record them", async () => { + await show(REQUEST, "ReadOnly"); + + const write = within(screen.getByRole("list", { name: "It asks to" })).getAllByRole("listitem")[1]; + expect(write).toHaveTextContent("Not allowed for your role"); + }); + + it("names a web app's host instead of this computer", async () => { + await show({ ...REQUEST, redirectHost: "app.example" }); + + expect(screen.getByText("Returns to app.example")).toBeInTheDocument(); + }); + + it("sends Cancel to the server, which answers with the app's redirect", async () => { + vi.mocked(declineConsent).mockResolvedValue({ redirectUri: "http://127.0.0.1:5000/cb?error=access_denied" }); + await show(REQUEST); + + await act(async () => { fireEvent.click(screen.getByRole("button", { name: "Cancel" })); }); + + expect(declineConsent).toHaveBeenCalledWith(SEARCH); + expect(assign).toHaveBeenCalledWith("http://127.0.0.1:5000/cb?error=access_denied"); + }); + + it("shows nothing to approve when the server refuses the request", async () => { + vi.mocked(askConsent).mockRejectedValue(new ApiError(400, "invalid_request", "bad")); + renderWithProviders( + } />, + { route: `/connect${SEARCH}`, token: { sub: "u1" } }, + ); + + expect(await screen.findByRole("heading", { name: "Nothing to approve" })).toBeInTheDocument(); + await waitFor(() => expect(screen.queryByLabelText(/Your current password/)).not.toBeInTheDocument()); + }); +}); diff --git a/web/src/routes/ConnectPage.tsx b/web/src/routes/ConnectPage.tsx new file mode 100644 index 000000000..5a3727798 --- /dev/null +++ b/web/src/routes/ConnectPage.tsx @@ -0,0 +1,260 @@ +import { useEffect, useRef, useState } from "react"; +import type { FormEvent, ReactNode, Ref } from "react"; +import { useLocation, useNavigate } from "react-router"; +import { useTranslation } from "react-i18next"; +import { Eye, Laptop, PenLine, UserRound } from "lucide-react"; +import { Alert, Box, Button, Paper, Stack, Typography } from "@mui/material"; +import { ApiError, stepUp } from "../api/client"; +import { approveConsent, askConsent, declineConsent } from "../api/oauth"; +import type { AuthorizeAnswer, ConsentRequest } from "../api/oauth"; +import { useAuth } from "../auth/useAuth"; +import { currentUserRole } from "../auth/claims"; +import { BusyButton } from "../components/BusyButton"; +import { StepUpPasswordField } from "../components/StepUpPasswordField"; +import { usePendingAction } from "../components/usePendingAction"; +import { useFarm } from "../farm/useFarm"; +import { roleLabel } from "../i18n/enums"; +import { useMe } from "../session/SessionContext"; + +const READ = "farm:read"; +const WRITE = "daily-entries:write"; +const LOOPBACK = new Set(["127.0.0.1", "localhost", "[::1]", "::1"]); +// The roles whose policy lets them record daily entries (ProductionWrite). +const RECORDS_ENTRIES = new Set(["Admin", "Manager", "Worker"]); + +type View = + | { kind: "loading" } + | { kind: "refused" } + | { kind: "leaving" } + | { kind: "ask"; request: ConsentRequest }; + +// #798 — the consent screen a connected app sends the user to (compact +// direction D, docs/designs/788-oauth-screens/SELECTION.md). Every Allow spends +// a step-up grant; an app already approved for everything it asks skips the +// permission rows and asks for the password only. +export function ConnectPage() { + const { t } = useTranslation("connect"); + const { search } = useLocation(); + const navigate = useNavigate(); + const { logout } = useAuth(); + const me = useMe(); + const { farm } = useFarm(); + const [view, setView] = useState({ kind: "loading" }); + const [password, setPassword] = useState(""); + const [error, setError] = useState(null); + const { busy, run } = usePendingAction(); + const headingRef = useRef(null); + const passwordRef = useRef(null); + const wrongPassword = error === t("wrongPassword"); + + // A screen reader starts at the request, and a phone keeps its keyboard closed until + // the user reaches the password. Only a wrong password sends focus back to the field. + useEffect(() => { + if (view.kind === "ask") headingRef.current?.focus(); + }, [view.kind]); + useEffect(() => { + if (wrongPassword && !busy) passwordRef.current?.focus(); + }, [wrongPassword, busy]); + + function follow(answer: AuthorizeAnswer) { + if ("redirectUri" in answer) { + setView({ kind: "leaving" }); + window.location.assign(answer.redirectUri); + } else { + setView({ kind: "ask", request: answer }); + } + } + + useEffect(() => { + let live = true; + askConsent(search).then( + (answer) => { if (live) follow(answer); }, + () => { if (live) setView({ kind: "refused" }); }); + return () => { live = false; }; + }, [search]); + + async function allow(e: FormEvent) { + e.preventDefault(); + await run("allow", async () => { + setError(null); + const entered = password; + setPassword(""); + let grant: string; + try { + grant = (await stepUp(entered)).token; + } catch (err) { + setError(err instanceof ApiError && err.status === 400 ? t("wrongPassword") + : err instanceof ApiError && err.status === 429 ? t("tooManyAttempts") : t("failed")); + return; + } + try { + follow(await approveConsent(search, grant)); + } catch { + setError(t("failed")); + } + }); + } + + async function cancel() { + await run("cancel", async () => { + setError(null); + try { + follow(await declineConsent(search)); + } catch { + setError(t("failed")); + } + }); + } + + const email = me?.email ?? ""; + + return ( + + + + {farm?.name} + {/* body1 sets tabular figures, which space an address's hyphens like gaps. */} + + {t("signedInAs")} {email} + + + + {view.kind === "loading" && {t("loading")}} + {view.kind === "leaving" && {t("leaving")}} + {view.kind === "refused" && ( + + {t("refusedTitle")} + {t("refusedBody")} + + + )} + {view.kind === "ask" && ( + + + {!view.request.alreadyApproved && } + {view.request.alreadyApproved && {t("reconnectBody")}} + + {error && {error}} + + {t("allow")} + + + {t("undo")} + {!view.request.alreadyApproved && ( +

void logout()} /> + )} + + )} + + + + ); +} + +function Headline({ request, ref }: { request: ConsentRequest; ref: Ref }) { + const { t } = useTranslation("connect"); + const app = request.clientName ?? t("unnamedApp"); + const wider = !request.alreadyApproved && request.alreadyAllowed.length > 0; + const title = request.alreadyApproved ? "titleReconnect" : wider ? "titleMore" : "title"; + return ( + + + {t(title, { app })} + + {t("unverified")} + {!request.alreadyApproved && ( + }> + {LOOPBACK.has(request.redirectHost) ? t("returnsHere") : t("returnsTo", { host: request.redirectHost })} + + )} + + ); +} + +function Line({ icon, children }: { icon: ReactNode; children: ReactNode }) { + return ( + + {icon}{children} + + ); +} + +function Permissions({ request }: { request: ConsentRequest }) { + const { t } = useTranslation("connect"); + const had = (scope: string) => request.alreadyAllowed.includes(scope); + // A request for more puts what is new first; what was allowed before drops to a muted line. + const scopes = [...request.scopes].sort((a, b) => Number(had(a)) - Number(had(b))); + const role = currentUserRole(); + const canRecord = RECORDS_ENTRIES.has(role); + return ( + <> + + {scopes.map((scope) => { + const old = had(scope); + const isNew = request.alreadyAllowed.length > 0 && !old; + const read = scope === READ; + const blocked = scope === WRITE && !canRecord; + return ( + + {read ? : } + + + {read ? t("scopeRead") : scope === WRITE ? t("scopeWrite") : scope} + {isNew && <> {t("scopeNew")}} + + {(old || blocked || read) && ( + + {old ? t("scopeHad") + : blocked ? {t("scopeBlocked")} : t("scopeReadLine")} + + )} + + + ); + })} + + + + {t("acts")} + + {t("roleSr")} {roleLabel(role)} + + + + ); +} + +function Details({ request, email, onSignOut }: { request: ConsentRequest; email: string; onSignOut: () => void }) { + const { t } = useTranslation("connect"); + const app = request.clientName ?? t("unnamedApp"); + const role = currentUserRole(); + const host = request.redirectHost; + // What this request would get: its scopes, within the user's role and flock scope. + const flocks = request.assignedFlocks?.join(", "); + const reads = request.scopes.includes(READ) && (flocks === undefined ? t("detailsReadAll") : t("detailsReadSome", { flocks })); + const writes = request.scopes.includes(WRITE) && (!RECORDS_ENTRIES.has(role) + ? t("detailsWriteBlocked", { role: roleLabel(role) }) + : flocks === undefined ? t("detailsWriteAll") : t("detailsWriteSome", { flocks })); + return ( + + {t("details")} + {t("detailsActs")} {[reads, writes].filter(Boolean).join(" ")} + {t("detailsName", { app })} + + {LOOPBACK.has(host) ? t("detailsReturnHere", { host }) : t("detailsReturnTo", { host })} + + {t("detailsScopes")} + + {email} · + + + ); +} diff --git a/web/src/routes/HelpPage.tsx b/web/src/routes/HelpPage.tsx index a1dc15d87..004dddc64 100644 --- a/web/src/routes/HelpPage.tsx +++ b/web/src/routes/HelpPage.tsx @@ -360,6 +360,9 @@ export function HelpPage() {
  • }} />
    • +
  • + }} /> +
  • }} />
    • diff --git a/web/src/routes/Login.test.tsx b/web/src/routes/Login.test.tsx index f247c2b9a..a2714a3dc 100644 --- a/web/src/routes/Login.test.tsx +++ b/web/src/routes/Login.test.tsx @@ -1,10 +1,11 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { screen, fireEvent, act, cleanup, waitFor } from "@testing-library/react"; -import { Routes, Route } from "react-router"; +import { Routes, Route, useLocation } from "react-router"; import { Login } from "./Login"; import { ProtectedRoute } from "./ProtectedRoute"; import { renderWithProviders } from "../test/renderWithProviders"; import { login as apiLogin, ApiError, setOnUnauthenticated } from "../api/client"; +import { previewConsent } from "../api/oauth"; import { setStoredToken } from "../test/jwt"; import { bindAccount, bindFarm, farmBindingToken, clearBoundAccount } from "../auth/tokenStore"; import { cacheBannerBytes, readCachedBannerBlob } from "../lib/bannerCache"; @@ -23,18 +24,25 @@ vi.mock("../api/client", async (importOriginal) => { }; }); +vi.mock("../api/oauth", () => ({ previewConsent: vi.fn() })); + const mockApiLogin = vi.mocked(apiLogin); const mockSetOnUnauthenticated = vi.mocked(setOnUnauthenticated); // /dashboard is behind the real ProtectedRoute, so navigation there only // succeeds if login actually established authenticated state — a bare public // route would false-green if login stopped authenticating. +function ConnectProbe() { + return
    connect {useLocation().search}
    ; +} + function tree() { return ( } /> }> dashboard (protected)} /> + } /> ); @@ -48,7 +56,10 @@ function fillCredentials(email: string, password: string) { // resetAllMocks (not clearAllMocks) so a per-test implementation never leaks // into the next case. -beforeEach(() => vi.resetAllMocks()); +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(previewConsent).mockResolvedValue(null); +}); describe("Login", () => { it("uses the flat inset Field Console auth frame", async () => { @@ -101,6 +112,49 @@ describe("Login", () => { expect(await screen.findByText("dashboard (protected)")).toBeInTheDocument(); }); + // #798 — the consent route's query is the connected app's request, so sign-in must + // hand it back intact. + it("returns to the consent route with its request after sign-in", async () => { + mockApiLogin.mockImplementation(async () => { + setStoredToken({ sub: "u1", role: "Sales" }); + }); + renderWithProviders(tree(), { route: "/connect?client_id=abc&scope=farm%3Aread", token: null }); + + expect(await screen.findByRole("button", { name: "Sign in" })).toBeInTheDocument(); + fillCredentials("owner@farm.co", "pw"); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Sign in" })); + }); + + expect(await screen.findByText("connect ?client_id=abc&scope=farm%3Aread")).toBeInTheDocument(); + }); + + // #798 (login B) — the sign-in that continues to a connected app names it, + // in the brand panel on a wide screen and above the form on a phone. + it("names the app it continues to when sign-in leads to consent", async () => { + vi.mocked(previewConsent).mockResolvedValue("Claude Desktop"); + renderWithProviders(tree(), { route: "/connect?client_id=abc&scope=farm%3Aread", token: null }); + + expect(await screen.findAllByText("Next: approve Claude Desktop")).toHaveLength(2); + expect(previewConsent).toHaveBeenCalledWith("?client_id=abc&scope=farm%3Aread"); + expect(screen.getAllByText( + "After you sign in, you see what it asks for and choose whether to allow it.")).toHaveLength(2); + }); + + it("says an app is next even when the app has no name", async () => { + renderWithProviders(tree(), { route: "/connect?client_id=abc", token: null }); + + expect(await screen.findAllByText("Next: approve an app")).toHaveLength(2); + }); + + it("shows no next step on an ordinary sign-in", async () => { + renderWithProviders(tree(), { route: "/dashboard", token: null }); + + await screen.findByRole("button", { name: "Sign in" }); + expect(screen.queryByText(/Next: approve/)).not.toBeInTheDocument(); + expect(previewConsent).not.toHaveBeenCalled(); + }); + it("redirects an ALREADY-authenticated visit AT /login away from the form (#145 silent-refresh restore)", async () => { // A seeded token = authenticated at mount; mounting Login itself must not // strand the user on the form — its effect navigates to home. diff --git a/web/src/routes/Login.tsx b/web/src/routes/Login.tsx index 20973f531..fbc0a1791 100644 --- a/web/src/routes/Login.tsx +++ b/web/src/routes/Login.tsx @@ -13,10 +13,12 @@ import { usePendingAction } from "../components/usePendingAction"; import i18n from "../i18n"; import { canonicalFarmCode, readFarmCodes, removeFarmCode } from "../auth/farmCodeCache"; import { applyDeviceBrand } from "../lib/brand"; +import { returnPath } from "../auth/returnPath"; +import { previewConsent } from "../api/oauth"; import { useCachedBannerUrl } from "../lib/bannerCache"; interface LocationState { - from?: { pathname: string }; + from?: { pathname: string; search?: string }; } // #283 follow-up — the error code a failed sign-in carries when the default @@ -78,7 +80,18 @@ export function Login() { const { login, isAuthenticated, isLoading, unauthenticatedReason } = useAuth(); const navigate = useNavigate(); const location = useLocation(); - const from = (location.state as LocationState | null)?.from?.pathname ?? "/"; + const from = returnPath((location.state as LocationState | null)?.from); + + // #798 (login B) — sign-in that continues to a connected app's request names the + // app it continues to. Undefined while asking, null when unnamed or refused. + const connectSearch = from.startsWith("/connect?") ? from.slice("/connect".length) : null; + const [nextApp, setNextApp] = useState(undefined); + useEffect(() => { + if (connectSearch === null) return; + let live = true; + void previewConsent(connectSearch).then((name) => { if (live) setNextApp(name); }); + return () => { live = false; }; + }, [connectSearch]); // If the load-time silent refresh (#145) restores a session while we're on // /login, don't strand the user on the form — send them to their destination. @@ -217,6 +230,14 @@ export function Login() { return ( + + {nextApp ? t("loginNextHeading", { app: nextApp }) : t("loginNextHeadingUnnamed")} + + {t("loginNextBody")} + + ) : undefined} bannerSlot={cachedBanner !== null ? (

    {t("stepUpCreateHint")}

    - setCreateStepUpPassword(e.target.value)} + onChange={setCreateStepUpPassword} /> @@ -1005,12 +1004,10 @@ export function UsersPage() {

    {t("assignmentsWorkerOnlyHint")}

    )}

    {t("stepUpFlockHint")}

    - setFlockStepUpPassword(e.target.value)} + onChange={setFlockStepUpPassword} /> @@ -1074,12 +1071,10 @@ export function UsersPage() { onChange={(e) => setPwConfirm(e.target.value)} />

    {t("stepUpResetHint")}

    - setPwStepUpPassword(e.target.value)} + onChange={setPwStepUpPassword} /> @@ -1116,12 +1111,10 @@ export function UsersPage() { ))}

    {t("stepUpRoleHint")}

    - setRoleStepUpPassword(e.target.value)} + onChange={setRoleStepUpPassword} /> @@ -1169,13 +1162,11 @@ export function UsersPage() { }} />

    {t("stepUpEmailHint")}

    - setEmailStepUpPassword(e.target.value)} + onChange={setEmailStepUpPassword} /> @@ -1233,12 +1224,10 @@ export function UsersPage() {

    {stepUpMode === "disable" ? t("stepUpDisableHint") : t("stepUpEnableHint")}

    - setStepUpPassword(e.target.value)} + onChange={setStepUpPassword} /> diff --git a/web/src/routes/helpGlossary.ts b/web/src/routes/helpGlossary.ts index 9e3ee1b68..7eba923d5 100644 --- a/web/src/routes/helpGlossary.ts +++ b/web/src/routes/helpGlossary.ts @@ -36,6 +36,7 @@ const ENTRIES = [ { key: "TooManySignInAttempts", group: "signingIn", spec: "Auth rate limiting" }, { key: "ForcedReauth", group: "signingIn", spec: "Session tokens" }, { key: "StepUpAuth", group: "signingIn", spec: "Step-up authentication" }, + { key: "AppConsent", group: "signingIn", spec: "Connecting an app" }, { key: "Roles", group: "signingIn", spec: "Roles" }, { key: "FlockScoping", group: "signingIn", spec: "Flock scoping" }, { key: "DisabledUser", group: "signingIn", spec: "Disabled user" },