[rorychatt]

Sign in with Microsoft Entra: the OIDC core (D91, PROPOSED, item 3a)

Draft. Not for merge until the owner has ruled on D91. Nothing in this PR is ratified, and nothing was run against a real Microsoft Entra tenant, a real directory or a real phone: every test uses a stand-in for the provider that is in this repository.

What it does

One provider (Microsoft Entra), one tenant pinned by its GUID. Authorization code flow with PKCE S256 and the scope openid profile (Microsoft sends the object id oid only with profile; the profile claims are ignored and never stored). The id token is checked by a verifier written on ring (RS256 only, key by kid from the pinned tenant's key set, iss, tid, aud, azp, nonce, exp, nbf, iat, oid, amr) and dropped: no token of the provider is stored. No new external dependency (round 3 adds one edge inside the workspace: obop-auth depends on obop-types).

• A person is the triple issuer, tenant and oid; never an e-mail address or a user principal name. Only an administrator writes a binding (user_account.bind_sso, obop admin sso bind, REST); an unbound identity is refused and nothing is created. The first-bind rule is 3b and not built.
• sso_only accounts ("Microsoft only") refuse the password and a passkey alone; a sign-in whose amr contains exactly mfa is their second factor (D53 amendment; ngcmfa alone counts for nothing, on purpose). The account is made Microsoft-only only after one sign-in with the button on its binding, with the provider on and the binding in the provider's tenant, and with at least two other enabled administrators who sign in locally with a working second factor (break-glass). The same rule refuses every later change by a person that would leave fewer than two: disable, offboard, demote, app off, last passkey removed, second factor reset (at the command line these warn instead of refusing).
• A leftover password proves nothing. The hash of a Microsoft-only account is kept (turning the flag off hands the password back), so password_ok answers no for such an account: change password, turn the app off, new recovery codes, and the passkey re-authentication (add, rename, remove, passwordless switch) are refused; a store that cannot be read is Unavailable, never a yes.
• Web leg keeps the bearer: a 60 second one-time code bound to the browser by a cookie, exchanged for the answer of the password step. No cookie session; D64 is not amended.
• Provider configuration is one record with the client secret sealed under OBOP_SECRET_KEY (re-sealed by obop secrets rotate). Security events oidc_*, limits per address (30 starts, 60 callbacks that spent a flow, 120 callbacks with an unknown state, 30 exchanges a minute), a negative cache, and the button and its messages in seven languages (six are machine drafts).
• Surfaces: sso_provider.configure|clear (listed everywhere, refuse agents when run, D41) and user_account.bind_sso|unbind_sso|set_sso_only (agents propose), CLI obop admin sso status|configure|clear|bind|unbind|only, configure --enabled|--disabled.

Fix rounds 1 and 2 (pushed earlier)

Round 1 answered the review of the first build (155 findings, 118 standing). Round 2 (head 95a2f30) answered the second review: the test authority gate (H1), the anonymous log (one line per source and kind, the provider's error mapped to 12 codes or other), the calls to Microsoft (six seconds at each of three call sites, single flight, at most 8 waiters, at most 8 calls in the air, a 45 second negative cache), the mutation table committed in docs/ISSUES.md, and the lows.

Correction of the first round's text (kept from round 2): the first round's body said mutant M11b (the result of the delete in code_take ignored) was already killed. It was not; it survived until the code_take race test of round 2. It dies now (race counts in docs/ISSUES.md).

Fix round 3 (pushed earlier, head a1840ba)

Answers the closure list of the third review (small, list-bound). What changed:

• The wiring of the test authority (m34, m35, CG-1, m28). The read of the process environment is injectable (authority_from_vars(get)), the loopback flag is parsed by one function, obop_types::loopback_allowed (exactly 1 or true), which the outbound client of obop-connectors uses too (Config::from_vars); both are unit-tested, and crates/obop/tests/sso_cli.rs spawns the real binary with env_clear and asserts what it does with and without the flag, including the one stderr line. The checklist item says what is accepted (http://127.0.0.1:<port> or http://[::1]:<port>) and that any other value is ignored.
• The call sites of the waiting (m23 to m26, R2H, x8b, N20 j of round 2). Tests at the two call sites show that at most eight requests wait for the discovery document or the key set that another request has in the air (the ninth is told "unavailable" at once), that a waiter gives up at the limit of a call, and that the key-set fetch takes one of the eight places among the calls in the air (the survivor of round 2, r2-jwks-no-permit, dies now).
• R2H-1, the anonymous log. Three hourly budgets of 60 lines (begun sign-ins, callbacks that belong to no sign-in, sign-ins that went wrong) and a slot per source, kind and sort of line (AnonLine). The split protects the failure lines against begun sign-ins and against callbacks whose state is no flow of ours (unknown, spent or expired); it does not protect them against somebody who takes a sign-in of their own to its end (see round 4).
• Docs and cheap lows. D91 Decision 2 (the provider's error code is mapped, not cleaned), Decisions 5 and 9 (what an unbind and Microsoft-only end: sessions and phone apps, not agent tokens or MCP apps; the three budgets), the thread count (eighteen), the log lines of the unbind and Microsoft-only changes, the test authority text (DEPLOY, D91, the checklist item and CONNECTORS.md), the break-glass caveat on the lost-factor lines of DEPLOY and the site page, the missing-oid hint, the names of the profile tests, the Spanish "Going to Microsoft", a time claim that does not fit in 64 bits is no longer saturated, the test-support comment, N7, N18, N20 (a, b, g, j, k) with their true reasons.
• The unbind and Microsoft-only changes are tested for what they end (sessions, phone apps) and what they leave (agent tokens, MCP apps).
• The timing tests of the sign-in binary no longer wait for a guess of how long a busy machine takes: the stand-in holds a request until the test lets go, the server tells a test how many requests wait, and a test releases the fetch it holds when eight wait and the ninth was told. Before this, 3 of 8 runs of the group failed on this machine (99 percent CPU from other builds); after, 12 of 12 passed, and 6 of 6 more with 64 spinning processes added.

Fix round 4 (pushed, head 778524c)

Answers the closure review of round 3 (no high; 2 medium that were one fault in two places, 3 low, 5 nit). Only tests and docs changed; no product source and no web file.

• The budgets count lines, not addresses (r3gb-1, F1). DEPLOY ("What is recorded") and D91 Decision 9 said "the 61st address"; the budget counts lines. A line is one of a kind and sort, once per address in ten minutes, so an address writes up to six lines of a sort an hour and ten addresses that begin a sign-in every ten minutes use up the 60 start lines of an hour. /start is open to anybody (30 a minute per address); a flow is spent by its first callback and that writes at most one line, but an address can begin as many sign-ins as /start lets it and reach each of five failure lines once in ten minutes (a callback without the cookie, a provider error, a cancellation, a redirect with no code, a code the token endpoint refuses; five is a floor, busy is a sixth), at least thirty an hour from one address, so a few addresses can use up the failure budget of an hour on purpose, and a refusal such as an expired client secret is then not written in that hour (the line that says the budget was reached and the next hour's count are). The split protects the failure lines against begun sign-ins and made-up callbacks, not against someone who plays a flow to its end. "Anybody who sends made-up callbacks" is gone from D91; the budget is said to be a fixed window. Kept as built and recorded as a known limit with its reason in ISSUES N20 (l); the budgets are not redesigned.
• Which budget a sort of line belongs to (r3gb-2, x06). One unit test over AnonLine::ALL (sso_anon_tests in obop-commands) pins the budget of all 16 sorts (a table of decisions, a match without a wildcard that makes the list complete, distinct words, every budget used; AnonGroup::ALL is used there). The busy-hour test now also shows that a browser that is not the one that left, a redirect with no code and a token endpoint that is not well are written after the noise budget is used up. The other call sites that name a sort (settings, discovery, switched off, busy, token unreachable, key set, key set unusable, begin) are not pinned: each is listed in N20 (m) with the mutation that showed it (eleven changes of a call site to NoFlow, all survive) and the reason.
• The cap of eight waiting requests is one cap for both fetches (r3-lt-4). A test with four callbacks waiting for a stuck key-set fetch and four starts waiting for a stuck discovery fetch shows that a fifth start is told "unavailable" at once, which the bound of eighteen threads depends on.
• Process-level test (x04b). It asserts one stderr line about the variable in each case, not that it contains the text.
• Docs. D91 Decision 3 names the one change of the manifests (obop-auth to the workspace crate obop-types) and the 64-bit bound of auth_time (and of exp, nbf, iat); Decision 4 says the phone apps end and the apps connected through the OAuth sign-in for MCP clients (D64) stay; "a callback with no valid state writes nothing" now says "no state, or one that is not shaped like a flow id"; "no new dependency" says "no new external dependency" in D91, Q64, N3; the stale cross-reference in the key-set permit test names its table row. The mutation table gives the path of every file (three crates have an sso.rs), says which rows repeat another row's change (109 rows are 107 changes) and no longer says all r2-h1-* rows were replaced.

Merge of main (pushed, head a4c3d00)

main moved to cf317da (PR 36, the language switch, and PR 38, files in Google Drive) and the PR conflicted with it in one hunk, which stopped GitHub from running CI. main is merged into the branch with a normal merge commit (no rebase, no force). The conflict was in the import lines of web/apps/backoffice/src/App.tsx; both sides are kept:

import { describeFor, setInstanceDefaults, t, useI18n } from "@obop/ui";   // PR 36
import {
  discardSsoReturn,   // this branch
  loadSettings,
  meta,
  restorePlace,       // this branch
  type ModuleMeta,
  type Settings,
} from "./api.ts";

Everything else merged without a conflict (Cargo.lock, CLAUDE.md, docs/CONNECTORS.md, docs/DEPLOY.md, docs/ISSUES.md, crates/obop/src/main.rs). PR 36's source-reading guard over the web tree (i18nGuard.ts, i18nDeps.test.ts) passes on the files of this branch with no i18n-memo-ok marker of mine (the markers in the tree are PR 36's, in ModelPage.tsx and modules/view.ts). The new rows D8 to D14 of docs/ISSUES.md do not collide with the N rows or the mutation table (the three names E1, E2 and M10 that the mutation table shared with issue rows are mt-E1, mt-E2 and mt-M10 since the wording round below).

Second merge of main (pushed, head 7a767ab) and wording corrections (pushed, head 4c2b91c)

main moved to 9555614 (PR 39, the Google Drive browser) and the PR conflicted with it again, in the translation catalogues only (web/apps/backoffice/locales/en/messages.json and de|es|fi|fr|pl|sv/{messages,status}.json, 13 files), which stopped GitHub from running CI. main is merged in with a normal merge commit (no rebase, no force). Both sides only added keys, so each file is the union: 1994 keys in every language (1984 here, 1979 there, 1969 in the base), no value differs and no side removed a key. en is in the order of the extraction; the other languages keep this branch's order and take the keys that only main added right after the key that precedes them there (the target-language files are in no sort order, and a re-sort would have moved whole files); a status is reviewed when either side had it. The diffs against the branch are additions only. Everything else merged by itself (the Rust of PR 39, the docs, the Drive browser of the back office).

The coordinator's check of a4c3d00 found no high and no medium item and five wording items; they are answered in one docs commit (4c2b91c; no code, no test and no web file changed):

Item	What was wrong	What it says now
F1	N20 (m) said every unpinned call site needs a costly state	Discovery, TokenUnreachable, KeySet at its first fetch and SwitchedOff need one line of state each and were left out for time; Busy, Settings, KeySetUnusable, Begin and the second KeySet site (the fetch again for an unknown key) are really costly. No test asserts the line text of Settings, SwitchedOff, Busy, KeySetUnusable or Begin. "The sort names are in one place each" is gone (three sorts have two call sites)
F2	"an address that has begun one can write five failure lines" (DEPLOY, D91 Decision 9, N20 l, this body)	a flow is spent by its first callback and writes at most one line; an address can begin as many as /start allows (30 a minute) and reach each of five failure lines once in ten minutes (a floor, busy is a sixth), at least thirty an hour; "up to six an hour" is per kind and sort
F4	N20 (l) cited review ids found nowhere in the repository (and F1 is another row there); (k) cited "X3 of the second review"	the ids are gone; (l) points at fix round 4 in this description and at the R2H-1 comments of the two tests
F6	D91 Decision 3: "the only change of the Cargo manifests" and null time claims	"the only change of dependencies" (the comment above the test-support feature was reworded too); a JSON null counts as absent (a refusal for exp, nothing for nbf and iat, dropped for auth_time)
r4m-1	mutation-table labels E1, E2, M10 repeated issue rows of the same file	mt-E1, mt-E2, mt-M10 (no test comment or other document cited them)

Docs

docs/DECISIONS.md (D91 PROPOSED, the D53 amendment, D64 cross-reference, D33, relayed owner choices, Q64), docs/DEPLOY.md ("Sign in with Microsoft"), docs/ISSUES.md (section N, rows N1 to N20, "Mutation checks of D91"), docs/CONNECTORS.md, docs/CONVENTIONS.md, docs/GLOSSARY.md, docs/spec/security-privacy.md (decision id range only), CLAUDE.md (code map), the site page on two-factor authentication. The OR rows are untouched.

What I ran (Windows 11)

On the first merged head a4c3d00 (the second merge changed only the translation catalogues and files of PR 39; the rows marked * were run again on it):

What	Result
vp test in web/apps/backoffice*	347 passed in 23 files (main alone has 339), including PR 36's guard over the whole web tree (also on 7a767ab, where the guard reads PR 39's Drive browser too)
vp check in web/apps/backoffice, web/packages/ui, web/packages/client, web/packages/e2e; vp run typecheck in web/packages/e2e; vp run i18n:check in web/packages/ui*	pass; pass; catalogues ok (on 7a767ab: 160 and 60 files formatted, no lint errors; 1994 keys in every language)
cargo check -p obop-http -p obop --tests*	on 7a767ab: finished, no error
cargo fmt --all --check; cargo clippy --workspace --all-targets -- -D warnings	clean
cargo test -p obop-auth -p obop-http	224 and 74 passed, 0 failed
cargo test -p obop --bin obop sso_tests; cargo test -p obop --test sso_cli	36 and 3 passed
obop modules check; obop openapi --out ... then vp run generate:types	8 modules ok; no diff
dist and dist-admin rebuilt (dist-admin from PowerShell), debug binary rebuilt; Playwright tests/sso.spec.ts and tests/language-switch.spec.ts with OBOP_E2E_KEEP=1	27 passed (11 and 16), no teardown error; the kept folder was deleted by name

Not run on the merged tree: the whole Playwright suite (CI runs it), the whole Rust workspace (the Rust that main changed is the new crate obop-blob-gdrive and edits in obop-commands, crates/obop (main.rs, ops.rs, storage_cli.rs), Cargo.toml and Cargo.lock; clippy --workspace, the build and the test groups above cover the merge), PostgreSQL, and the obop-storage tests.

Before the merge, on 778524c: cargo test --workspace outside obop-storage 1995 passed, 0 failed, 5 ignored; obop-storage 278 passed and 4 tls_tests failed because their own postgres:17 container did not start in 60 s on a machine at 99 percent CPU (not touched by this PR; they passed in round 3, 2274 passed in all). On a1840ba (round 3): cargo test -p obop-auth --lib against postgres:18 224 passed; the whole Playwright suite 236 passed, 16 skipped, 1 failed (polish.spec.ts, a 30 s timeout under load, passes 15 of 15 alone).

CI on a4c3d00 (the first merge): green, run https://github.com/SpaceCorps/obop/actions/runs/41338849214. CI on the head 4c2b91c: green, run https://github.com/SpaceCorps/obop/actions/runs/41351132873 (lockfiles, web, rust, e2e and all passed on the first attempt; no re-run was needed). main is 9555614 (PR 39) and GitHub reports the PR mergeable (PR 40 is open and edits the D section of docs/ISSUES.md; this branch edits section N and the mutation table). The PR stays a draft.

Mutation checks. 109 rows in docs/ISSUES.md (107 different changes), each one change to the source, the covering tests run, the file put back byte for byte: 109 die, 0 survive; none dies only because the tests do not compile. Each row names the commit it was made on; the ten rows r4-... are for the new tests (for example r4-group-nocode-is-noise, the x06 of the review, dies in the_list_of_sorts_holds_each_sort_once_and_each_sort_has_the_budget_that_was_decided; r4-waiting-cap-per-fetch, a second counter for the key-set waiters, dies in the new shared-cap test alone; r4-stderr-line-twice is x04b), and seven rows whose killers changed were made again. Eleven more changes (a call site of an unpinned sort moved to NoFlow) were made to show what no test pins: all eleven survive and are listed in N20 (m), not in the table. Race counts are unchanged from round 2.

NOT verified

• Anything against a real Entra tenant (issuer form, claims present in real v2.0 tokens, amr and auth_time under real policies, admin consent behaviour including the admin consent workflow, discovery, key rollover, error texts).
• Any real phone; the phone legs (D64 amendment, D87) and the phone step-up are not in 3a.
• Browsers other than Chromium (Playwright); the Back button is staged with a pageshow event, not a real back/forward cache restore (N20 e).
• The six translations (de, fr, pl, fi, sv, es) are machine drafts.
• The workspace tests on PostgreSQL beyond obop-auth; the obop-storage tls_tests in this round.
• A release build in this round.
• Leaver checks beyond the runbook, expiry of the agent tokens of Microsoft-bound staff (proposed 90 days), group mapping, SCIM, a client certificate instead of a secret: not built (ISSUES N4, N9, N16, N18).

For the owner

D91 is PROPOSED. Open decisions are in ISSUES section N: N12 (retention and data class of the object id in the log), N16 (certificate credential), N3 (the verifier on ring, relayed and closed, can be asked again).

🤖 Generated with Claude Code