CLI app for developers prototyping atproto functionality
docs: add OAuth client conformance test implementation plan
Eight-phase implementation plan plus test-requirements.md derived from
docs/design-plans/2026-04-16-test-oauth-client.md. Phases promote
report.rs to common, add JWS/JWK helpers, land the three static stages
(discovery, metadata, JWKS), stand up the fake AS, implement RelyingParty,
and layer scope-variation and DPoP-edge flow variants. Plan is locked
against the design ACs and code-review-approved with zero open issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
+4384
+515
docs/implementation-plans/2026-04-16-test-oauth-client/phase_01.md
+515
docs/implementation-plans/2026-04-16-test-oauth-client/phase_01.md
···
1
+
# OAuth client test — Phase 1: Promote `report.rs` to common, add CLI scaffolding
2
+
3
+
**Goal:** Move the shared report contract out of the labeler tree and stub the new
4
+
`test oauth client` / `test oauth client interactive` subcommand shape, so
5
+
subsequent phases have a stable foundation to build on.
6
+
7
+
**Architecture:** Two mechanical refactors in one phase. First, move
8
+
`src/commands/test/labeler/report.rs` into `src/common/` and change `Stage`
9
+
from a closed enum to a `pub struct Stage(pub &'static str)` newtype (labeler
10
+
keeps emitting the same four stage labels verbatim, so snapshots don't
11
+
change). Second, add a new `Oauth(OauthCmd) → Client(ClientCmd)` sub-tree to
12
+
`TestCmd` with `static`/`interactive` variants that print a "not implemented"
13
+
message and exit 0. Phase 1 writes no oauth logic — it just gives later
14
+
phases a compile-clean scaffold.
15
+
16
+
**Tech Stack:** Rust 2024, clap derive, miette, existing labeler idioms.
17
+
18
+
**Scope:** Phase 1 of 8 from `docs/design-plans/2026-04-16-test-oauth-client.md`.
19
+
20
+
**Codebase verified:** 2026-04-16.
21
+
22
+
---
23
+
24
+
## Acceptance Criteria Coverage
25
+
26
+
Phase 1 is pure scaffolding. No acceptance criteria are verified here.
27
+
Functionality ACs land in Phase 3 onward.
28
+
29
+
**Verifies: None** (infrastructure phase).
30
+
31
+
The phase's correctness is verified operationally:
32
+
- `cargo build` succeeds.
33
+
- `cargo test` passes — all existing labeler tests, including snapshots under
34
+
`tests/snapshots/labeler_*`, are unaffected.
35
+
- `cargo run -- test oauth client --help` shows the `static` and `interactive`
36
+
subcommand structure.
37
+
- `cargo run -- test oauth client https://example.com` runs the placeholder
38
+
and exits 0.
39
+
40
+
---
41
+
42
+
## Codebase verification findings (upfront investigation, 2026-04-16)
43
+
44
+
**Confirmed:**
45
+
- `src/commands/test/labeler/report.rs` (591 lines) holds `CheckStatus`
46
+
(5 variants: `Pass`, `SpecViolation`, `NetworkError`, `Advisory`, `Skipped`),
47
+
`CheckResult`, `Stage` (closed enum with `Identity`, `Http`, `Subscription`,
48
+
`Crypto`), `SummaryCounts`, `ReportHeader`, `RenderConfig`, `LabelerReport`.
49
+
Renders via `miette::GraphicalReportHandler`.
50
+
- `LabelerReport::exit_code()` at `report.rs:196-227` encodes exit precedence
51
+
(`1` for `SpecViolation`, else `2` for `NetworkError`, else `0`). This stays
52
+
inside the `LabelerReport` impl — Phase 1 does not extract it.
53
+
- CLI dispatch chain: `src/main.rs:6-8` → `src/cli.rs:32-42` → `src/commands.rs:11-24`
54
+
(`Command::Test`) → `src/commands/test.rs:11-23` (`TestCmd::Labeler`) →
55
+
`src/commands/test/labeler.rs:54-93` (`LabelerCmd::run(self, no_color: bool)
56
+
-> Result<ExitCode, Report>`).
57
+
- `src/common.rs` is a short module root; `pub mod diagnostics;` and
58
+
`pub mod identity;`. `src/common/` has no `mod.rs` (sibling-file pattern per
59
+
CLAUDE.md).
60
+
- `src/lib.rs` re-exports `cli`, `commands`, `common`. Integration tests reach
61
+
stages as `atproto_devtool::commands::test::labeler::...`.
62
+
63
+
**Design discrepancies — flagged for Phase 1 handling:**
64
+
- Design says "Move `Check::blocked_by` helper into the same module." **The
65
+
helper does not exist in the codebase today.** Current labeler practice is
66
+
that the pipeline orchestrator hardcodes reason strings (e.g.,
67
+
`"blocked by identity stage failures"` at `pipeline.rs:269`) when building
68
+
`Skipped` rows. Phase 1 **introduces** the helper, wraps existing call-sites
69
+
with it, and pins the string it produces to match the current rendered
70
+
output verbatim so labeler snapshots are untouched.
71
+
- Design prescribes `Check::blocked_by` as a method on a single shared `Check`
72
+
type (`impl Check { pub fn blocked_by(&self, other: Check) -> CheckResult }`).
73
+
Since `Check` is actually stage-local (each stage defines its own `Check`
74
+
enum and they never share a type), Phase 1 instead ships a free function
75
+
`blocked_by(check_id, stage, summary, blocker_check_id) -> CheckResult` in
76
+
`common::report`, taking stable `&'static str` check IDs. This makes it
77
+
usable from any stage across the labeler and oauth_client commands
78
+
without coupling check enums. The rendered-output contract (the literal
79
+
string `"blocked by <check_id>"` as the skip reason) is unchanged from
80
+
what the design intended; only the caller ergonomics differ.
81
+
- Design says `Stage` is promoted from a labeler-specific enum to a
82
+
`&'static str` newtype. Phase 1 does exactly this. To keep labeler snapshot
83
+
output bit-identical, this phase declares **the full Stage const table**
84
+
in `src/common/report.rs` — both labeler stages and oauth_client stages —
85
+
so every consumer sees one authoritative list:
86
+
```rust
87
+
impl Stage {
88
+
// Labeler stages.
89
+
pub const IDENTITY: Stage = Stage("Identity");
90
+
pub const HTTP: Stage = Stage("HTTP");
91
+
pub const SUBSCRIPTION: Stage = Stage("Subscription");
92
+
pub const CRYPTO: Stage = Stage("Crypto");
93
+
// OAuth client stages. Consumed by Phase 3+.
94
+
pub const DISCOVERY: Stage = Stage("Discovery");
95
+
pub const METADATA: Stage = Stage("Metadata");
96
+
pub const JWKS: Stage = Stage("JWKS");
97
+
pub const INTERACTIVE: Stage = Stage("Interactive");
98
+
}
99
+
```
100
+
The display string (`self.0`) is the exact capitalised label rendered today
101
+
for labeler stages (`"Identity"`, `"HTTP"`, `"Subscription"`, `"Crypto"`)
102
+
and the capitalised labels for oauth_client stages (`"Discovery"`,
103
+
`"Metadata"`, `"JWKS"`, `"Interactive"`). The `Stage::label()` method
104
+
becomes trivial field access.
105
+
106
+
---
107
+
108
+
## Implementation tasks
109
+
110
+
<!-- START_SUBCOMPONENT_A (tasks 1-3) -->
111
+
112
+
**Subcomponent A: Promote `report.rs` to `src/common/`.**
113
+
114
+
<!-- START_TASK_1 -->
115
+
### Task 1: Move `report.rs` and convert `Stage` to a newtype
116
+
117
+
**Files:**
118
+
- Move: `src/commands/test/labeler/report.rs` → `src/common/report.rs`
119
+
- Modify: `src/common.rs` — add `pub mod report;`
120
+
- Modify: `src/commands/test/labeler.rs` — delete the inline `mod report` / `pub mod report` declaration (search for it; it's the line that previously declared the report submodule under labeler).
121
+
- Modify: `src/commands/test/labeler/pipeline.rs` — update `use` paths from
122
+
`super::report::...` or `crate::commands::test::labeler::report::...` to
123
+
`crate::common::report::...`.
124
+
- Modify: every other file inside `src/commands/test/labeler/` that imports
125
+
from `report::*` — switch imports to `crate::common::report::*`. Search:
126
+
`grep -rn "use .*labeler::report" src/ tests/` to find every call site.
127
+
- Modify: `src/common/report.rs` (the moved file):
128
+
- Delete the `Stage` enum declaration (currently around `report.rs:84-94`)
129
+
and the closed-enum `Stage::label()` match impl.
130
+
- Replace with:
131
+
```rust
132
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
133
+
pub struct Stage(pub &'static str);
134
+
135
+
impl Stage {
136
+
pub const fn label(self) -> &'static str { self.0 }
137
+
138
+
// Labeler stages (unchanged rendered labels).
139
+
pub const IDENTITY: Stage = Stage("Identity");
140
+
pub const HTTP: Stage = Stage("HTTP");
141
+
pub const SUBSCRIPTION: Stage = Stage("Subscription");
142
+
pub const CRYPTO: Stage = Stage("Crypto");
143
+
144
+
// OAuth client stages. Consumed by Phase 3+.
145
+
pub const DISCOVERY: Stage = Stage("Discovery");
146
+
pub const METADATA: Stage = Stage("Metadata");
147
+
pub const JWKS: Stage = Stage("JWKS");
148
+
pub const INTERACTIVE: Stage = Stage("Interactive");
149
+
}
150
+
```
151
+
- Anywhere the file or its consumers currently write `Stage::Identity`,
152
+
`Stage::Http`, `Stage::Subscription`, `Stage::Crypto`, replace with
153
+
`Stage::IDENTITY`, `Stage::HTTP`, `Stage::SUBSCRIPTION`, `Stage::CRYPTO`.
154
+
- The `PartialOrd`/`Ord` derives on the original enum are dropped — the
155
+
newtype does not need `Ord` because the existing render loop
156
+
(`src/commands/test/labeler/report.rs:234-313`) already groups results
157
+
by stage in insertion order via a change-detecting
158
+
`current_stage: Option<Stage>` state (see `report.rs:255-258`). Verified
159
+
by inspection: the labeler pipeline (`pipeline.rs:209-365`) records
160
+
results stage-contiguously — identity first, then HTTP, then
161
+
subscription, then crypto — so insertion order equals stage order today.
162
+
**No render-loop refactor is needed**; the existing change-detection
163
+
logic over `Stage` equality is untouched and Just Works with the newtype
164
+
because `Stage: PartialEq`.
165
+
- If a future refactor ever interleaves results across stages (not
166
+
today's behavior), the render loop would need an explicit
167
+
stage-order-preserving grouping pass. Flag this in the module-level
168
+
doc comment for anyone changing `LabelerReport::record` later.
169
+
170
+
**Implementation notes:**
171
+
- The current `Stage::label` returns the stage's display string. The newtype
172
+
version stores that string directly, so `stage.label()` returns `self.0`.
173
+
- The newtype approach is mandated by the design so two commands (labeler and
174
+
oauth_client) can extend the `Stage` space without coupling through a
175
+
shared enum.
176
+
- The `Copy` derive is essential — `Stage` is currently `Copy` and many code
177
+
paths depend on it (every `CheckResult` construction; every pattern match).
178
+
179
+
**Testing:**
180
+
- No new tests. Existing labeler snapshot tests must stay green. If any
181
+
snapshot diffs appear after the edit, that indicates a regression in
182
+
render ordering — fix by iterating `results` in insertion order rather
183
+
than matching on `Stage` variants.
184
+
- Verify operationally:
185
+
- `cargo build` succeeds.
186
+
- `cargo test` passes.
187
+
188
+
**Commit:** `refactor(report): promote labeler report module to common, convert Stage to newtype`
189
+
190
+
<!-- END_TASK_1 -->
191
+
192
+
<!-- START_TASK_2 -->
193
+
### Task 2: Introduce `Check::blocked_by` helper
194
+
195
+
**Files:**
196
+
- Modify: `src/common/report.rs` — add **two** free functions covering the
197
+
two skip-reason patterns used across the codebase:
198
+
```rust
199
+
use std::borrow::Cow;
200
+
201
+
/// Construct a `Skipped` result whose reason points at a specific failing
202
+
/// prerequisite check ID. Renders as `"blocked by <check_id>"`.
203
+
pub fn blocked_by(
204
+
check_id: &'static str,
205
+
stage: Stage,
206
+
summary: impl Into<Cow<'static, str>>,
207
+
blocker_check_id: &'static str,
208
+
) -> CheckResult {
209
+
CheckResult {
210
+
id: check_id,
211
+
stage,
212
+
status: CheckStatus::Skipped,
213
+
summary: summary.into(),
214
+
diagnostic: None,
215
+
skipped_reason: Some(Cow::Owned(format!("blocked by {blocker_check_id}"))),
216
+
}
217
+
}
218
+
219
+
/// Construct a `Skipped` result with an arbitrary skip reason not tied to
220
+
/// a specific failing check ID — used for "not applicable to this client
221
+
/// kind" style skips.
222
+
pub fn skipped_with_reason(
223
+
check_id: &'static str,
224
+
stage: Stage,
225
+
summary: impl Into<Cow<'static, str>>,
226
+
reason: impl Into<Cow<'static, str>>,
227
+
) -> CheckResult {
228
+
CheckResult {
229
+
id: check_id,
230
+
stage,
231
+
status: CheckStatus::Skipped,
232
+
summary: summary.into(),
233
+
diagnostic: None,
234
+
skipped_reason: Some(reason.into()),
235
+
}
236
+
}
237
+
```
238
+
Both helpers are public and exported from `src/common/report.rs`. Later
239
+
phases (3 discovery, 4 metadata, 5 jwks, 7 interactive) call both
240
+
helpers without redeclaring them.
241
+
- Modify: `src/commands/test/labeler/pipeline.rs` — replace every
242
+
hand-constructed `CheckResult { status: Skipped, skipped_reason: Some(...), .. }`
243
+
that references a failing prerequisite with a `blocked_by(...)` call.
244
+
The only call-sites today are at `pipeline.rs:262-271, 289-298,
245
+
~332-342` (grep for `skipped_reason: Some`).
246
+
247
+
**Implementation notes:**
248
+
- Current labeler skip reasons use free-form strings like
249
+
`"blocked by identity stage failures"` — a stage-level generic. The new
250
+
helper produces `"blocked by identity::target_resolved"` — referencing a
251
+
specific check ID. **This is a rendered-output change** that will require
252
+
accepting updated snapshots.
253
+
- This helper's rendering contract is codified: the reason string is
254
+
always `"blocked by <check_id>"` with no trailing punctuation. Every
255
+
downstream test pins this via snapshot.
256
+
- The design plan explicitly calls out "matching the labeler
257
+
`Check::blocked_by` rendering verbatim"; since no such rendering exists
258
+
yet, Phase 1 **defines** that rendering here.
259
+
260
+
**Testing:**
261
+
- Accept the snapshot changes introduced by updating labeler skip reasons.
262
+
Run `cargo insta review` to inspect each diff and confirm only the
263
+
reason string changed (glyph, stage, id, summary all identical).
264
+
- Add two unit tests in `src/common/report.rs`:
265
+
- `blocked_by_produces_correct_reason`:
266
+
Call `blocked_by("http::endpoint_reachable", Stage::HTTP, "Endpoint reachable (not run)", "identity::target_resolved")`.
267
+
Assert `id == "http::endpoint_reachable"`, `stage == Stage::HTTP`,
268
+
`status == CheckStatus::Skipped`, `skipped_reason.as_deref() == Some("blocked by identity::target_resolved")`.
269
+
- `skipped_with_reason_produces_exact_reason_string`:
270
+
Call `skipped_with_reason("oauth_client::discovery::metadata_document_fetchable", Stage::DISCOVERY, "Metadata document fetchable", "metadata is implicit for loopback clients")`.
271
+
Assert `skipped_reason.as_deref() == Some("metadata is implicit for loopback clients")`
272
+
(no `"blocked by"` prefix — verbatim reason).
273
+
274
+
**Verification:**
275
+
- `cargo test --lib common::report` passes.
276
+
- `cargo test --tests labeler_*` — run interactively with
277
+
`cargo insta review` to accept the rewrapped skip-reason strings.
278
+
279
+
**Commit:** `refactor(report): introduce blocked_by helper; update labeler skip reasons to reference check ids`
280
+
281
+
<!-- END_TASK_2 -->
282
+
283
+
<!-- START_TASK_3 -->
284
+
### Task 3: Verify labeler is fully migrated
285
+
286
+
**Files:** no code changes expected.
287
+
288
+
**Verification:**
289
+
- `grep -rn "labeler::report" src/ tests/` returns nothing.
290
+
- `grep -rn "Stage::Identity\|Stage::Http\|Stage::Subscription\|Stage::Crypto" src/ tests/` returns nothing (all call sites now use the const forms).
291
+
- `cargo build` succeeds.
292
+
- `cargo test` passes.
293
+
- `cargo clippy -- -D warnings` clean.
294
+
- `cargo fmt --check` clean.
295
+
296
+
If any of these fail, fix the imports or missed call-sites before
297
+
committing.
298
+
299
+
**Commit:** only if additional fix-ups are needed on top of Tasks 1-2. If
300
+
Tasks 1-2 left the tree clean, skip the commit.
301
+
302
+
<!-- END_TASK_3 -->
303
+
304
+
<!-- END_SUBCOMPONENT_A -->
305
+
306
+
<!-- START_SUBCOMPONENT_B (tasks 4-6) -->
307
+
308
+
**Subcomponent B: Add OAuth CLI scaffolding.**
309
+
310
+
<!-- START_TASK_4 -->
311
+
### Task 4: Add `Oauth(OauthCmd)` variant to `TestCmd`
312
+
313
+
**Files:**
314
+
- Modify: `src/commands/test.rs`:
315
+
- Add `pub mod oauth;` at the top of the file.
316
+
- Add a new variant to the `TestCmd` enum (currently `src/commands/test.rs:11-23`):
317
+
```rust
318
+
pub enum TestCmd {
319
+
Labeler(LabelerCmd),
320
+
/// Test an atproto OAuth client.
321
+
Oauth(OauthCmd),
322
+
}
323
+
```
324
+
- Import `OauthCmd` at the module top: `use oauth::OauthCmd;`.
325
+
- Extend the `impl TestCmd { pub async fn run(self, no_color: bool) -> Result<ExitCode, Report> { match self { ... } } }` to handle `TestCmd::Oauth(cmd) => cmd.run(no_color).await`.
326
+
327
+
**Implementation notes:**
328
+
- Use `#[derive(Debug, Subcommand)]` on `OauthCmd`. Clap will nest subcommands naturally.
329
+
- The doc comment on `Oauth(OauthCmd)` becomes the clap help text for the `oauth` subcommand.
330
+
331
+
**Testing:** verified operationally in Task 6 (`--help` check).
332
+
333
+
**Commit:** batched into Task 5's commit.
334
+
335
+
<!-- END_TASK_4 -->
336
+
337
+
<!-- START_TASK_5 -->
338
+
### Task 5: Create `OauthCmd` with `Client(ClientCmd)` and placeholder run
339
+
340
+
**Files:**
341
+
- Create: `src/commands/test/oauth.rs`:
342
+
```rust
343
+
//! OAuth conformance tests.
344
+
345
+
use std::process::ExitCode;
346
+
347
+
use clap::Subcommand;
348
+
use miette::Report;
349
+
350
+
pub mod client;
351
+
352
+
use client::ClientCmd;
353
+
354
+
/// Conformance tests for atproto OAuth components.
355
+
#[derive(Debug, Subcommand)]
356
+
pub enum OauthCmd {
357
+
/// Test an atproto OAuth client.
358
+
Client(ClientCmd),
359
+
}
360
+
361
+
impl OauthCmd {
362
+
pub async fn run(self, no_color: bool) -> Result<ExitCode, Report> {
363
+
match self {
364
+
OauthCmd::Client(cmd) => cmd.run(no_color).await,
365
+
}
366
+
}
367
+
}
368
+
```
369
+
- Create: `src/commands/test/oauth/client.rs`:
370
+
```rust
371
+
//! OAuth client conformance tests.
372
+
//!
373
+
//! `atproto-devtool test oauth client <target>` runs static conformance
374
+
//! checks against an atproto OAuth client identified by its `client_id`.
375
+
//! `atproto-devtool test oauth client interactive <target>` additionally
376
+
//! spins up an in-process fake authorization server and observes the
377
+
//! client driving end-to-end OAuth flows.
378
+
379
+
use std::process::ExitCode;
380
+
381
+
use clap::{Args, Subcommand};
382
+
use miette::Report;
383
+
384
+
/// Conformance tests for an atproto OAuth client.
385
+
#[derive(Debug, Args)]
386
+
pub struct ClientCmd {
387
+
/// The client's `client_id` URL, or a loopback form for development clients.
388
+
///
389
+
/// Accepted forms:
390
+
/// * `https://...` — treated as a `client_id` URL pointing at a fetchable
391
+
/// JSON metadata document.
392
+
/// * `http://localhost[:port][/path]` or `http://127.0.0.1[:port][/path]` —
393
+
/// treated as an atproto loopback (development) client with implicit
394
+
/// metadata.
395
+
pub target: String,
396
+
397
+
/// Optional interactive mode (`interactive` subcommand).
398
+
#[command(subcommand)]
399
+
pub mode: Option<ClientMode>,
400
+
401
+
/// Suppress ANSI color in rendered output.
402
+
#[arg(long)]
403
+
pub no_color: bool,
404
+
405
+
/// Emit DEBUG-level tracing for stage instrumentation.
406
+
#[arg(long)]
407
+
pub verbose: bool,
408
+
}
409
+
410
+
#[derive(Debug, Subcommand)]
411
+
pub enum ClientMode {
412
+
/// Run static checks plus an interactive flow against a fake authorization server.
413
+
Interactive(InteractiveArgs),
414
+
}
415
+
416
+
#[derive(Debug, Args)]
417
+
pub struct InteractiveArgs {
418
+
/// Bind the fake authorization server to this TCP port (random ephemeral by default).
419
+
#[arg(long)]
420
+
pub port: Option<u16>,
421
+
422
+
/// Public HTTPS URL the fake authorization server advertises in metadata.
423
+
///
424
+
/// When unset, the fake AS binds to `127.0.0.1:<port>` and advertises
425
+
/// `http://127.0.0.1:<port>` throughout. Set this to a public URL (for
426
+
/// example a cloudflared / Tailscale Funnel / ngrok endpoint) when the
427
+
/// client under test runs elsewhere and needs to reach the fake AS over
428
+
/// HTTPS.
429
+
#[arg(long, value_name = "URL")]
430
+
pub public_base_url: Option<url::Url>,
431
+
}
432
+
433
+
impl ClientCmd {
434
+
pub async fn run(self, _no_color: bool) -> Result<ExitCode, Report> {
435
+
let target = &self.target;
436
+
match self.mode {
437
+
None => {
438
+
println!("test oauth client: not implemented yet (target: {target})");
439
+
Ok(ExitCode::from(0))
440
+
}
441
+
Some(ClientMode::Interactive(_args)) => {
442
+
println!("test oauth client interactive: not implemented yet (target: {target})");
443
+
Ok(ExitCode::from(0))
444
+
}
445
+
}
446
+
}
447
+
}
448
+
```
449
+
- Modify: `src/commands/test/oauth.rs` — confirm it matches the snippet above
450
+
(use `ClientCmd`, not `OauthClientCmd`, for brevity in the CLI tree).
451
+
452
+
**Implementation notes:**
453
+
- `url::Url` — the project already depends on `url = "2.5"` via
454
+
`common::identity`. No new dep.
455
+
- The `no_color` argument is declared on `ClientCmd` mirroring the labeler's
456
+
pattern at `src/commands/test/labeler.rs:46` — clap's propagation rules
457
+
mean both `--no-color` on the global `Cli` level and the local
458
+
`--no-color` on `ClientCmd` resolve to the same behavior at render time.
459
+
The placeholder here ignores it (prefixed `_no_color`); real wiring lands
460
+
in Phase 3 when the pipeline starts rendering.
461
+
- `#[command(subcommand)]` with an `Option<ClientMode>` makes the
462
+
`interactive` subcommand optional, matching the design
463
+
(`atproto-devtool test oauth client <target>` is static by default).
464
+
- The sibling-file convention is honored: `oauth.rs` exists as the module root
465
+
for `oauth/`, which contains `client.rs`; `client.rs` is the module root
466
+
for `client/` (populated in Phase 3+).
467
+
468
+
**Testing:** verified operationally in Task 6.
469
+
470
+
**Commit:** `feat(test-oauth-client): scaffold test oauth client subcommand with placeholder run`
471
+
472
+
<!-- END_TASK_5 -->
473
+
474
+
<!-- START_TASK_6 -->
475
+
### Task 6: Verify the scaffolding operationally
476
+
477
+
**Files:** no code changes.
478
+
479
+
**Verification:**
480
+
- `cargo build` succeeds.
481
+
- `cargo run -- test oauth client --help` prints help with the `target`
482
+
argument, a `--no-color` flag, and an `interactive` subcommand visible.
483
+
- `cargo run -- test oauth client interactive --help` prints help with the
484
+
`target` argument plus `--port` and `--public-base-url` flags.
485
+
- `cargo run -- test oauth client https://example.com` prints the placeholder
486
+
line and exits 0 (observable via `echo $?`).
487
+
- `cargo run -- test oauth client interactive https://example.com` prints its
488
+
placeholder line and exits 0.
489
+
- `cargo test` passes (no new tests; just confirming nothing regressed).
490
+
- `cargo clippy -- -D warnings` clean.
491
+
- `cargo fmt --check` clean.
492
+
493
+
**Commit:** only if minor fix-ups were needed. Otherwise the scaffold stands
494
+
on Tasks 4-5's commits.
495
+
496
+
<!-- END_TASK_6 -->
497
+
498
+
<!-- END_SUBCOMPONENT_B -->
499
+
500
+
---
501
+
502
+
## Phase 1 done when
503
+
504
+
- Every task above has been committed to the worktree.
505
+
- `cargo build`, `cargo test`, `cargo clippy -- -D warnings`, and
506
+
`cargo fmt --check` all succeed.
507
+
- `cargo run -- test oauth client --help` and
508
+
`cargo run -- test oauth client interactive --help` render the expected
509
+
CLI surfaces.
510
+
- Labeler snapshots under `tests/snapshots/labeler_*.snap` have been re-accepted
511
+
with the new `"blocked by <check_id>"` skip reasons and are otherwise
512
+
bit-identical.
513
+
514
+
The next phase (Phase 2) will add the `jsonwebtoken` dependency and
515
+
populate `src/common/oauth/jws.rs` with JOSE helpers.
+535
docs/implementation-plans/2026-04-16-test-oauth-client/phase_02.md
+535
docs/implementation-plans/2026-04-16-test-oauth-client/phase_02.md
···
1
+
# OAuth client test — Phase 2: Common JWS/JWK helpers
2
+
3
+
**Goal:** Add the shared JOSE primitives used by every later phase that
4
+
touches OAuth crypto (Phase 5 JWKS validation, Phase 6 fake AS, Phase 7
5
+
RelyingParty).
6
+
7
+
**Architecture:** Add `jsonwebtoken` (rust_crypto feature, no default
8
+
features) as the JWS engine. Create a new `src/common/oauth/` namespace
9
+
containing `jws.rs` — a thin, typed wrapper exposing `ParsedJwk`,
10
+
`parse_jwk`, `parse_jwks`, `verify_jws`, `sign_jws`, and a `JwsError`
11
+
with stable diagnostic codes. ES256 is the only signing algorithm we
12
+
encode/decode with. ES256K is recognised as a parse-valid algorithm
13
+
label in JWKS (atproto DIDs use k256 elsewhere) but we never actually
14
+
verify signatures produced with it — per decision on 2026-04-16 after
15
+
discovering jsonwebtoken has no ES256K support.
16
+
17
+
**Tech Stack:** `jsonwebtoken` 10.3.x, existing `p256` / `k256` / `sha2`
18
+
(only used for the algorithm enum machinery in `ParsedJwk`, not for JWS
19
+
I/O), `serde_json`, `miette`, `thiserror`.
20
+
21
+
**Scope:** Phase 2 of 8.
22
+
23
+
**Codebase verified:** 2026-04-16.
24
+
25
+
---
26
+
27
+
## Acceptance Criteria Coverage
28
+
29
+
Phase 2 is pure infrastructure for later phases. No ACs are directly
30
+
verified here.
31
+
32
+
**Verifies: None** (infrastructure phase; Phase 5 and Phase 7 consume it).
33
+
34
+
---
35
+
36
+
## Codebase verification findings
37
+
38
+
- `src/common/` layout confirmed: `src/common.rs` (module root, 4 lines:
39
+
`pub mod diagnostics; pub mod identity;` plus `APP_USER_AGENT`),
40
+
`src/common/diagnostics.rs`, `src/common/identity.rs`. No `src/common/oauth.rs`
41
+
or directory today.
42
+
- `Cargo.toml` already has `k256 = { version = "0.13", features = ["ecdsa"] }`,
43
+
`p256 = { version = "0.13", features = ["ecdsa"] }`, `sha2 = "0.11"`,
44
+
`serde_json = "1.0"`, `miette = { version = "7.6", features = ["fancy"] }`,
45
+
`thiserror = "2.0"`, `base64 = "0.22"`. Versions stay fixed.
46
+
- `jsonwebtoken` crate 10.3.0:
47
+
- `rust-version = "1.85.0"` (matches project MSRV — no MSRV collision).
48
+
- Has `rust_crypto` and `aws_lc_rs` features; **exactly one must be
49
+
enabled** or a `CryptoProvider` supplied. Choose `rust_crypto`.
50
+
- `rust_crypto` enables `ed25519-dalek`, `hmac`, `p256`, `p384`, `rsa`,
51
+
`sha2` (verified from `cargo read jsonwebtoken`).
52
+
- `default-features` include `use_pem`; we disable defaults and skip
53
+
`use_pem` — we don't need PEM encoding (JWK JSON only).
54
+
- `Algorithm` enum: `HS256`, `HS384`, `HS512`, `RS256`, `RS384`,
55
+
`RS512`, `PS256`, `PS384`, `PS512`, `ES256`, `ES384`, `EdDSA`. **No
56
+
`ES256K`.**
57
+
58
+
- Existing diagnostic-code convention: `labeler::<stage>::<check>` and
59
+
`oauth_client::<stage>::<check>` per design. Phase 2 owns the
60
+
`oauth_client::jws::*` prefix.
61
+
62
+
---
63
+
64
+
## Implementation tasks
65
+
66
+
<!-- START_SUBCOMPONENT_A (tasks 1-2) -->
67
+
68
+
**Subcomponent A: Cargo dependency + module root.**
69
+
70
+
<!-- START_TASK_1 -->
71
+
### Task 1: Add `jsonwebtoken` dependency and create the `oauth` module root
72
+
73
+
**Files:**
74
+
- Modify: `Cargo.toml`:
75
+
- Add to `[dependencies]`, alphabetically placed between
76
+
`hickory-resolver` and `humantime`:
77
+
```toml
78
+
jsonwebtoken = { version = "10.3", default-features = false, features = ["rust_crypto"] }
79
+
```
80
+
- Update the existing `p256` entry to add the `pkcs8` feature so we can
81
+
export signing keys as PKCS8 DER (required because jsonwebtoken's
82
+
`rust_crypto` backend reads EC keys via `P256SigningKey::from_pkcs8_der`
83
+
and our key-material flow produces them from `p256` itself):
84
+
```toml
85
+
p256 = { version = "0.13", features = ["ecdsa", "pkcs8"] }
86
+
```
87
+
(Existing: `features = ["ecdsa"]`. We add `"pkcs8"`. Commented on non-obvious
88
+
feature choice per CLAUDE.md.)
89
+
- Modify: `src/common.rs` — add `pub mod oauth;` alphabetically (between
90
+
`identity` and `APP_USER_AGENT`):
91
+
```rust
92
+
pub mod diagnostics;
93
+
pub mod identity;
94
+
pub mod oauth;
95
+
pub mod report;
96
+
```
97
+
(Order: `diagnostics, identity, oauth, report` — alphabetical. Phase 1
98
+
already added `pub mod report;`.)
99
+
- Create: `src/common/oauth.rs`:
100
+
```rust
101
+
//! OAuth 2.0 + JOSE primitives shared across the `test oauth *` command family.
102
+
103
+
pub mod jws;
104
+
```
105
+
106
+
**Implementation notes:**
107
+
- Disabling default features means we do **not** get `use_pem`. We only
108
+
ever ingest keys as JWK JSON (from a client metadata document or an
109
+
inline test key), and on the sign side we construct `EncodingKey` via
110
+
`EncodingKey::from_ec_der(&pkcs8_der_bytes)` — **this constructor is
111
+
unconditional in jsonwebtoken 10.3** (verified at
112
+
`jsonwebtoken/src/encoding.rs:101` — not gated behind `use_pem`). The
113
+
`rust_crypto` backend internally parses those bytes via
114
+
`p256::ecdsa::SigningKey::from_pkcs8_der`.
115
+
- **Canonical sign-side key-construction path** (used by Phase 7):
116
+
1. Generate with `p256::ecdsa::SigningKey::from_bytes(&scalar_bytes)`
117
+
where `scalar_bytes` come from the per-RP seeded RNG.
118
+
2. Convert to PKCS8 DER via the `pkcs8` crate:
119
+
`use p256::pkcs8::EncodePrivateKey; let der = signing_key.to_pkcs8_der()?;`
120
+
where `to_pkcs8_der()` returns a `SecretDocument` whose `.as_bytes()`
121
+
yields `&[u8]`.
122
+
3. `let encoding_key = jsonwebtoken::EncodingKey::from_ec_der(der.as_bytes());`
123
+
- The `rust_crypto` feature pulls in `p256` and `sha2` which we already
124
+
have — Cargo merges feature sets, so no duplicate compilation.
125
+
126
+
**Verification:**
127
+
- `cargo build` succeeds — confirms the feature combination compiles.
128
+
- `cargo tree -p jsonwebtoken -e features` shows `rust_crypto` enabled,
129
+
`use_pem` disabled, no transitive `ring`.
130
+
131
+
**Commit:** `build: add jsonwebtoken dependency with rust_crypto feature`
132
+
133
+
<!-- END_TASK_1 -->
134
+
135
+
<!-- END_SUBCOMPONENT_A -->
136
+
137
+
<!-- START_SUBCOMPONENT_B (tasks 2-4) -->
138
+
139
+
**Subcomponent B: `jws.rs` types, parsing, and errors.**
140
+
141
+
<!-- START_TASK_2 -->
142
+
### Task 2: `JwsError`, `JwsAlg`, `JwkUse`, `ParsedJwk` types
143
+
144
+
**Files:**
145
+
- Create: `src/common/oauth/jws.rs` with the initial type declarations
146
+
described below.
147
+
148
+
**Implementation outline:**
149
+
150
+
```rust
151
+
use std::borrow::Cow;
152
+
use std::sync::Arc;
153
+
154
+
use miette::{Diagnostic, NamedSource, SourceSpan};
155
+
use thiserror::Error;
156
+
157
+
/// A JWS signing/verification algorithm understood by the tool.
158
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
159
+
pub enum JwsAlg {
160
+
/// ECDSA P-256 + SHA-256. Required by the atproto OAuth profile.
161
+
Es256,
162
+
/// ECDSA secp256k1 + SHA-256. Recognised in JWK parsing (atproto
163
+
/// DIDs use k256) but the tool does not sign or verify with this
164
+
/// algorithm — `sign_jws` / `verify_jws` will return
165
+
/// `JwsError::UnsupportedOperation`.
166
+
Es256k,
167
+
}
168
+
169
+
impl JwsAlg {
170
+
pub fn as_str(self) -> &'static str {
171
+
match self {
172
+
JwsAlg::Es256 => "ES256",
173
+
JwsAlg::Es256k => "ES256K",
174
+
}
175
+
}
176
+
177
+
/// Parse the `alg` JWK / JWS header string.
178
+
pub fn parse(s: &str) -> Result<Self, JwsError> { /* ... */ }
179
+
180
+
/// Map to the `jsonwebtoken::Algorithm` where supported.
181
+
pub fn to_jsonwebtoken(self) -> Option<jsonwebtoken::Algorithm> {
182
+
match self {
183
+
JwsAlg::Es256 => Some(jsonwebtoken::Algorithm::ES256),
184
+
JwsAlg::Es256k => None,
185
+
}
186
+
}
187
+
}
188
+
189
+
/// The `use` field on a JWK.
190
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
191
+
pub enum JwkUse {
192
+
Sig,
193
+
/// Any non-sig value (e.g., "enc"). Holds the raw string so diagnostics
194
+
/// can show exactly what appeared.
195
+
Other(Arc<str>),
196
+
}
197
+
198
+
/// A parsed JWK. Holds whatever structural metadata the JWK declares —
199
+
/// but deliberately does NOT validate that the JWK is complete or
200
+
/// acceptable. Callers (the Phase 5 JWKS stage) decide whether a
201
+
/// missing `alg` or an unrecognised algorithm is a spec violation,
202
+
/// because those decisions map to specific stage-level check IDs.
203
+
///
204
+
/// Design note: the JWS layer is deliberately permissive on `alg`
205
+
/// presence and value. This keeps the per-key error → stage-level
206
+
/// check mapping in exactly one place (Phase 5's `jwks::run`).
207
+
#[derive(Debug, Clone)]
208
+
pub struct ParsedJwk {
209
+
pub kid: Option<Arc<str>>,
210
+
/// Absent if the JWK omits `alg`. Phase 5 treats absence as a
211
+
/// violation on `keys_have_alg`.
212
+
pub alg: Option<JwsAlg>,
213
+
/// Raw alg string preserved verbatim when `alg` is present but not
214
+
/// in {ES256, ES256K} — Phase 5 uses it to emit a pointed
215
+
/// `algs_are_modern_ec` diagnostic citing the offending value.
216
+
pub alg_raw: Option<Arc<str>>,
217
+
pub r#use: JwkUse,
218
+
/// The verifier, pre-constructed from the JWK material. Only
219
+
/// present when `alg == Some(JwsAlg::Es256)` and the x/y
220
+
/// coordinates decoded successfully. Always None for ES256K or
221
+
/// when `alg` is absent/unsupported.
222
+
pub decoding_key: Option<jsonwebtoken::DecodingKey>,
223
+
}
224
+
```
225
+
226
+
Then define `JwsError`. Stable diagnostic codes follow the pattern
227
+
`oauth_client::jws::<slug>`:
228
+
229
+
```rust
230
+
#[derive(Debug, Error, Diagnostic)]
231
+
pub enum JwsError {
232
+
#[error("JWK or JWKS document is not valid JSON")]
233
+
#[diagnostic(code("oauth_client::jws::not_json"))]
234
+
NotJson {
235
+
#[source_code]
236
+
source: NamedSource<Arc<[u8]>>,
237
+
#[label("JSON parse failed here")]
238
+
span: Option<SourceSpan>,
239
+
#[source]
240
+
cause: serde_json::Error,
241
+
},
242
+
243
+
#[error("JWK is missing the required `{field}` field")]
244
+
#[diagnostic(code("oauth_client::jws::jwk_missing_field"))]
245
+
JwkMissingField {
246
+
field: &'static str,
247
+
#[source_code]
248
+
source: NamedSource<Arc<[u8]>>,
249
+
#[label("on this key")]
250
+
span: Option<SourceSpan>,
251
+
},
252
+
253
+
#[error("JWK declares an unsupported algorithm `{alg}`")]
254
+
#[diagnostic(
255
+
code("oauth_client::jws::unsupported_alg"),
256
+
help("Supported JWK algorithms: ES256, ES256K. Other algorithms \
257
+
are rejected because atproto OAuth restricts client JWKS to \
258
+
modern EC curves.")
259
+
)]
260
+
UnsupportedAlg {
261
+
alg: Arc<str>,
262
+
#[source_code]
263
+
source: NamedSource<Arc<[u8]>>,
264
+
#[label("this alg is rejected")]
265
+
span: Option<SourceSpan>,
266
+
},
267
+
268
+
#[error("JWK `kty` / curve mismatch for `alg = {alg}`")]
269
+
#[diagnostic(code("oauth_client::jws::jwk_kty_mismatch"))]
270
+
JwkKtyMismatch {
271
+
alg: &'static str,
272
+
#[source_code]
273
+
source: NamedSource<Arc<[u8]>>,
274
+
#[label("this key declares incompatible kty/crv")]
275
+
span: Option<SourceSpan>,
276
+
},
277
+
278
+
#[error("JWS token is malformed or failed signature verification")]
279
+
#[diagnostic(code("oauth_client::jws::verify_failed"))]
280
+
VerifyFailed(#[source] jsonwebtoken::errors::Error),
281
+
282
+
#[error("JWS signing failed")]
283
+
#[diagnostic(code("oauth_client::jws::sign_failed"))]
284
+
SignFailed(#[source] jsonwebtoken::errors::Error),
285
+
286
+
#[error("JWS operation `{op}` is not supported for algorithm `{alg}`")]
287
+
#[diagnostic(code("oauth_client::jws::unsupported_operation"))]
288
+
UnsupportedOperation { op: &'static str, alg: &'static str },
289
+
}
290
+
```
291
+
292
+
**Implementation notes:**
293
+
- `Arc<str>` for `kid` / `use` string payloads mirrors the existing
294
+
`identity.rs` pattern (see `RawDidDocument.source_bytes: Arc<[u8]>`),
295
+
keeping error values cheap to clone.
296
+
- The `decoding_key` field is `Option` because ES256K JWKs deliberately
297
+
carry `None` (we don't verify). Consumers (Phase 5 JWKS validation)
298
+
check the `alg` field for completeness reporting and use
299
+
`decoding_key` only when they actually need to verify a JWS.
300
+
- `span` fields are `Option<SourceSpan>` because callers sometimes synthesize
301
+
`ParsedJwk` from inline Rust values in tests and have no source text to
302
+
point at.
303
+
304
+
**Testing:** Task 4 covers these types via roundtrip tests. No dedicated test
305
+
for this task.
306
+
307
+
**Commit:** batched into Task 4's commit.
308
+
309
+
<!-- END_TASK_2 -->
310
+
311
+
<!-- START_TASK_3 -->
312
+
### Task 3: `parse_jwk`, `parse_jwks`, `sign_jws`, `verify_jws`
313
+
314
+
**Files:**
315
+
- Modify: `src/common/oauth/jws.rs` — add the parsing and sign/verify
316
+
functions.
317
+
318
+
**Implementation outline:**
319
+
320
+
```rust
321
+
/// Parse a single JWK from a JSON object.
322
+
///
323
+
/// Returns a `ParsedJwk` if the object is structurally a JSON object with
324
+
/// at least a recognised `kty`. Deliberately permissive on `alg` (absent
325
+
/// or unrecognised) and `use` (any string accepted into `JwkUse::Other`);
326
+
/// callers (Phase 5 JWKS stage) decide whether those are violations.
327
+
///
328
+
/// Returns `Err(JwsError)` only for hard structural failures:
329
+
/// - Input is not a JSON object.
330
+
/// - `kty` is absent.
331
+
/// - `kty=EC` with `crv` absent or not one of {"P-256", "secp256k1"}.
332
+
/// - `kty=EC` with `alg=ES256` but `crv != "P-256"` (a cross-field
333
+
/// inconsistency — the JWK is internally contradictory).
334
+
/// - For `alg=ES256`, `x` or `y` absent or not base64url.
335
+
///
336
+
/// `name` is the logical source name used in diagnostic spans.
337
+
pub fn parse_jwk(
338
+
value: &serde_json::Value,
339
+
name: &str,
340
+
source: Arc<[u8]>,
341
+
) -> Result<ParsedJwk, JwsError> {
342
+
// 1. Require `kty`. For `"EC"`, require `crv` ∈ {"P-256", "secp256k1"}.
343
+
// Any other kty → JwkKtyMismatch.
344
+
// 2. Read optional `kid` (Arc<str>).
345
+
// 3. Read optional `alg`:
346
+
// - Absent → alg = None, alg_raw = None.
347
+
// - "ES256" → alg = Some(Es256), alg_raw = None.
348
+
// - "ES256K" → alg = Some(Es256k), alg_raw = None.
349
+
// - Anything else → alg = None, alg_raw = Some(<raw string>).
350
+
// This preserves the raw value for Phase 5's diagnostic.
351
+
// 4. Cross-check crv vs alg ONLY when alg is Some. If alg is
352
+
// Es256 but crv != "P-256", return JwkKtyMismatch. Same for
353
+
// Es256k vs "secp256k1". Mismatch is a hard structural
354
+
// violation; a missing alg is not.
355
+
// 5. Read optional `use`; default `JwkUse::Sig` if absent;
356
+
// non-"sig" strings go into `JwkUse::Other(<raw>)`.
357
+
// 6. For `alg == Some(Es256)` AND consistent crv, construct a
358
+
// `DecodingKey::from_ec_components(x, y)` via reading `x` and
359
+
// `y` base64url-decoded. On decode failure, surface
360
+
// `JwkMissingField { field: "x" }` or `{ field: "y" }` with a
361
+
// span at the offending field. For any other alg value,
362
+
// `decoding_key = None`.
363
+
// 7. Return ParsedJwk { kid, alg, alg_raw, r#use, decoding_key }.
364
+
}
365
+
366
+
/// Parse a JWKS document (the whole JSON payload).
367
+
pub fn parse_jwks(
368
+
bytes: &[u8],
369
+
name: &str,
370
+
) -> Result<Vec<ParsedJwk>, JwsError> {
371
+
let source = Arc::<[u8]>::from(bytes);
372
+
let value: serde_json::Value =
373
+
serde_json::from_slice(bytes).map_err(|e| JwsError::NotJson {
374
+
source: NamedSource::new(name, source.clone()),
375
+
span: Some(crate::common::diagnostics::span_at_line_column(
376
+
bytes, e.line(), e.column(),
377
+
)),
378
+
cause: e,
379
+
})?;
380
+
let keys = value.get("keys").and_then(|v| v.as_array()).ok_or_else(|| {
381
+
JwsError::JwkMissingField {
382
+
field: "keys",
383
+
source: NamedSource::new(name, source.clone()),
384
+
span: None,
385
+
}
386
+
})?;
387
+
keys.iter()
388
+
.map(|k| parse_jwk(k, name, source.clone()))
389
+
.collect()
390
+
}
391
+
392
+
/// Verify a JWS token against a `ParsedJwk`.
393
+
///
394
+
/// `expected_alg` is the algorithm the caller expects in the JWS header;
395
+
/// anything else fails verification (prevents alg-confusion attacks).
396
+
pub fn verify_jws<T: serde::de::DeserializeOwned>(
397
+
token: &str,
398
+
key: &ParsedJwk,
399
+
expected_alg: JwsAlg,
400
+
) -> Result<jsonwebtoken::TokenData<T>, JwsError> {
401
+
let jwt_alg = expected_alg.to_jsonwebtoken().ok_or(
402
+
JwsError::UnsupportedOperation {
403
+
op: "verify",
404
+
alg: expected_alg.as_str(),
405
+
}
406
+
)?;
407
+
let dk = key.decoding_key.as_ref().ok_or(
408
+
JwsError::UnsupportedOperation {
409
+
op: "verify",
410
+
alg: expected_alg.as_str(),
411
+
}
412
+
)?;
413
+
let mut validation = jsonwebtoken::Validation::new(jwt_alg);
414
+
validation.algorithms = vec![jwt_alg];
415
+
validation.validate_exp = false; // DPoP proofs have their own lifetime rules
416
+
validation.validate_nbf = false;
417
+
validation.validate_aud = false;
418
+
jsonwebtoken::decode(token, dk, &validation).map_err(JwsError::VerifyFailed)
419
+
}
420
+
421
+
/// Sign a JWS using an ES256 private key.
422
+
///
423
+
/// `header` supplies the JOSE header fields (`typ`, `kid`, `jwk` for DPoP).
424
+
/// `claims` is the body payload.
425
+
pub fn sign_es256_jws<T: serde::Serialize>(
426
+
header: &jsonwebtoken::Header,
427
+
claims: &T,
428
+
key: &jsonwebtoken::EncodingKey,
429
+
) -> Result<String, JwsError> {
430
+
jsonwebtoken::encode(header, claims, key).map_err(JwsError::SignFailed)
431
+
}
432
+
```
433
+
434
+
**Implementation notes:**
435
+
- `DecodingKey::from_ec_components` takes two base64url-encoded
436
+
strings (`x` and `y` coordinates). `jsonwebtoken` exposes this constructor
437
+
when the `rust_crypto` feature is active.
438
+
- ES256K signing/verification **is not implemented**. Callers must check
439
+
`key.decoding_key.is_some()` before calling `verify_jws`. The error
440
+
path is deterministic (`UnsupportedOperation`).
441
+
- `validate_exp = false` here — the caller knows the semantic lifetime
442
+
(DPoP is ±60s, private_key_jwt is typically 5 min, etc.) and enforces
443
+
it through claim reads, not through jsonwebtoken's generic validator.
444
+
445
+
**Testing:** Task 4 covers. No dedicated test commit for this task.
446
+
447
+
**Commit:** batched into Task 4's commit.
448
+
449
+
<!-- END_TASK_3 -->
450
+
451
+
<!-- START_TASK_4 -->
452
+
### Task 4: Unit tests for parsing, signing, and verification
453
+
454
+
**Verifies:** None (no ACs; this is infrastructure validation).
455
+
456
+
**Files:**
457
+
- Modify: `src/common/oauth/jws.rs` — append a `#[cfg(test)] mod tests` block.
458
+
459
+
**Testing:**
460
+
Tests must cover:
461
+
462
+
1. **ES256 sign + verify roundtrip** — generate a P-256 signing key
463
+
deterministically from a fixed seed via `ChaCha20Rng::from_seed([0u8;32])`
464
+
and `p256::ecdsa::SigningKey::random(&mut rng)`. Extract its public
465
+
point, produce a JWK JSON with `kty=EC`, `crv=P-256`, `x=<base64url>`,
466
+
`y=<base64url>`, `kid=k1`, `alg=ES256`, `use=sig`. Parse it via
467
+
`parse_jwk`, assert `alg == Some(JwsAlg::Es256)` and
468
+
`decoding_key.is_some()`. Sign `serde_json::json!({"foo":"bar"})` with
469
+
`sign_es256_jws` using a `Header::new(Algorithm::ES256)` plus
470
+
`kid = Some("k1".into())`. Decode via
471
+
`verify_jws::<serde_json::Value>(...)` and assert the claims roundtrip
472
+
identically.
473
+
474
+
2. **ES256K JWK parse accepted, verify refused** — craft a JWK with
475
+
`kty=EC`, `crv=secp256k1`, `alg=ES256K`, `use=sig`, arbitrary x/y.
476
+
Parse — assert `alg == Some(JwsAlg::Es256k)` and
477
+
`decoding_key.is_none()`. Call
478
+
`verify_jws::<serde_json::Value>(fake_token, &key, JwsAlg::Es256k)`
479
+
and assert `matches!(err, JwsError::UnsupportedOperation { op: "verify", alg: "ES256K" })`.
480
+
481
+
3. **JWKS with multiple keys** — parse a `{"keys":[k1, k2]}` payload;
482
+
assert ordering is preserved.
483
+
484
+
4. **Permissive `alg` handling — unknown algorithm preserved in
485
+
`alg_raw`** — parse a JWK with `alg=RS256`. Assert `alg == None`,
486
+
`alg_raw.as_deref() == Some("RS256")`, `decoding_key.is_none()`. No
487
+
`Err` is returned — Phase 5's JWKS stage decides whether RS256 is a
488
+
violation.
489
+
490
+
5. **Permissive `alg` handling — absent alg** — parse a JWK with `kty=EC`,
491
+
`crv=P-256`, no `alg`. Assert `alg == None`, `alg_raw == None`,
492
+
`decoding_key.is_none()`.
493
+
494
+
6. **Reject missing kty** — parse a JWK without `kty`. Assert
495
+
`JwkMissingField { field: "kty", .. }`.
496
+
497
+
7. **Reject cross-field curve/alg mismatch** — craft `kty=EC, crv=P-256,
498
+
alg=ES256K`. Assert `JwkKtyMismatch { alg: "ES256K", .. }` (internally
499
+
inconsistent JWK — a hard structural failure).
500
+
501
+
8. **Reject missing x/y with alg=ES256** — JWK with `alg=ES256` but no
502
+
`y` coordinate. Assert `JwkMissingField { field: "y", .. }`.
503
+
504
+
9. **Invalid JSON** — pass `b"not json at all"` to `parse_jwks`. Assert
505
+
`JwsError::NotJson { .. }`.
506
+
507
+
For each test, verify the diagnostic `code()` string matches the expected
508
+
`"oauth_client::jws::<slug>"` value. Use `miette::Diagnostic::code()` on the
509
+
error to get an `Option<Box<dyn Display>>`; stringify and assert.
510
+
511
+
**Verification:**
512
+
- `cargo test --lib common::oauth::jws` — all tests pass.
513
+
- `cargo clippy -- -D warnings` clean.
514
+
- `cargo fmt --check` clean.
515
+
516
+
**Commit:** `feat(common-oauth): add JWS/JWK helpers (parse, sign, verify) with stable diagnostic codes`
517
+
518
+
<!-- END_TASK_4 -->
519
+
520
+
<!-- END_SUBCOMPONENT_B -->
521
+
522
+
---
523
+
524
+
## Phase 2 done when
525
+
526
+
- `cargo test --lib common::oauth::jws` passes.
527
+
- `cargo clippy -- -D warnings` clean.
528
+
- `cargo fmt --check` clean.
529
+
- The compiled binary gains no `ring` dependency (verify with
530
+
`cargo tree | grep ring` returning nothing or only pre-existing matches
531
+
from other crates — tokio's rustls path may pull ring transitively; that's
532
+
pre-existing and out of Phase 2 scope).
533
+
534
+
Phase 3 consumes `parse_jwk`/`parse_jwks` via a `JwksFetcher` trait when
535
+
it lands the JWKS stage.
+490
docs/implementation-plans/2026-04-16-test-oauth-client/phase_03.md
+490
docs/implementation-plans/2026-04-16-test-oauth-client/phase_03.md
···
1
+
# OAuth client test — Phase 3: Static stage 1 — discovery
2
+
3
+
**Goal:** Resolve a `<target>` argument into `DiscoveryFacts`, fetching the
4
+
client metadata document for HTTPS targets and synthesizing implicit
5
+
metadata for loopback targets. Wire the pipeline into `ClientCmd::run` so
6
+
the command no longer exits with a placeholder.
7
+
8
+
**Architecture:** Three-file static-mode skeleton:
9
+
`src/commands/test/oauth/client/pipeline.rs` (target parsing + pipeline
10
+
runner), `src/commands/test/oauth/client/discovery.rs` (stage 1 logic), and
11
+
updates to `src/commands/test/oauth/client.rs` (wire `run()` to the
12
+
pipeline, construct the shared reqwest client, render the report).
13
+
Follows the labeler's `pipeline-with-Facts-gating` pattern verbatim —
14
+
each stage returns `*StageOutput { facts: Option<*Facts>, results: Vec<CheckResult> }`.
15
+
16
+
**Tech Stack:** clap derive, reqwest (existing `HttpClient` trait from
17
+
`src/common/identity.rs`), url, miette, tracing, insta for snapshots.
18
+
19
+
**Scope:** Phase 3 of 8.
20
+
21
+
**Codebase verified:** 2026-04-16.
22
+
23
+
---
24
+
25
+
## Acceptance Criteria Coverage
26
+
27
+
This phase implements and tests:
28
+
29
+
### test-oauth-client.AC1: Static mode — target parsing and discovery
30
+
31
+
- **test-oauth-client.AC1.1 Success:** An `https://...` `client_id` URL is fetched over the shared HTTP client and the response body is captured as `RawMetadata::Document`.
32
+
- **test-oauth-client.AC1.2 Success:** `http://localhost[:port][/path]` is recognized as a loopback target and synthesizes `RawMetadata::Implicit` without making an HTTP request.
33
+
- **test-oauth-client.AC1.3 Success:** `http://127.0.0.1[:port][/path]` is treated identically to `localhost` for loopback recognition.
34
+
- **test-oauth-client.AC1.4 Failure:** A target that is neither HTTPS nor a recognized loopback form is rejected before the pipeline starts, with a `parse_target` error surfaced as a clap argument error (non-zero exit, helpful message).
35
+
- **test-oauth-client.AC1.5 Failure:** An HTTPS `client_id` returning a non-2xx status produces a `NetworkError` `CheckResult` with the URL and status in the diagnostic; downstream stages emit `Skipped` rows blocked on `discovery::metadata_document_fetchable`.
36
+
- **test-oauth-client.AC1.6 Failure:** An HTTPS `client_id` returning a body that is not parseable JSON produces a `SpecViolation` with the content-type and a snippet of the body in the diagnostic.
37
+
- **test-oauth-client.AC1.7 Edge:** A loopback target with an empty path, an explicit port, or a non-empty path each parse identically and emit the same set of `CheckResult`s.
38
+
39
+
---
40
+
41
+
## Codebase verification findings
42
+
43
+
- `HttpClient` trait (`src/common/identity.rs:278-285`):
44
+
```rust
45
+
#[async_trait]
46
+
pub trait HttpClient: Send + Sync {
47
+
async fn get_bytes(&self, url: &Url) -> Result<(u16, Vec<u8>), IdentityError>;
48
+
}
49
+
```
50
+
Returns `(status_code, body_bytes)`. Errors surface as `IdentityError`.
51
+
**Phase 3 extends this trait** with a second method that additionally
52
+
captures the response's `Content-Type`, because AC1.6 requires the
53
+
content type to appear in the SpecViolation diagnostic:
54
+
```rust
55
+
#[async_trait]
56
+
pub trait HttpClient: Send + Sync {
57
+
async fn get_bytes(&self, url: &Url) -> Result<(u16, Vec<u8>), IdentityError>;
58
+
59
+
/// Like `get_bytes` but additionally returns the response's
60
+
/// `Content-Type` header value (if any). Added in Phase 3 of the
61
+
/// oauth_client rollout so discovery can include content-type in
62
+
/// AC1.6 diagnostics. Default impl calls `get_bytes` and returns
63
+
/// `None` for `content_type`, so existing implementers (labeler
64
+
/// stages) don't need updates.
65
+
async fn get_bytes_with_content_type(
66
+
&self,
67
+
url: &Url,
68
+
) -> Result<(u16, Vec<u8>, Option<String>), IdentityError> {
69
+
let (status, body) = self.get_bytes(url).await?;
70
+
Ok((status, body, None))
71
+
}
72
+
}
73
+
```
74
+
`RealHttpClient` overrides `get_bytes_with_content_type` to actually
75
+
read the `Content-Type` header from the `reqwest::Response`. The
76
+
`FakeHttpClient` extension (Task 4) takes an optional content-type at
77
+
`add_response` time and returns it here.
78
+
- `RealHttpClient::from_client(reqwest::Client)` exists
79
+
(`src/common/identity.rs:315-320`). Phase 3 constructs one shared reqwest
80
+
client in `ClientCmd::run` following the labeler pattern at
81
+
`src/commands/test/labeler.rs:61-66`.
82
+
- `APP_USER_AGENT` is in `src/common.rs`; `pub(crate)` so oauth client can
83
+
use it directly: `use crate::common::APP_USER_AGENT;`.
84
+
- The labeler defines `FakeHttpClient` locally in
85
+
`tests/labeler_identity.rs:19-86` — **not** in `tests/common/mod.rs`.
86
+
Phase 3 **promotes and extends** it: promotes it to `tests/common/mod.rs`
87
+
so both binaries can share it, and extends it with a new
88
+
`add_transport_error(url)` seed method (the labeler's fake did not have
89
+
this; discovery AC1.5 requires simulated transport-layer failures).
90
+
Labeler imports update to use the shared fake.
91
+
- `LabelerReport::exit_code()` currently lives on `LabelerReport` — Phase 3
92
+
does not need to touch exit-code logic; the oauth command reuses the
93
+
`LabelerReport` rendering-and-exit-code machinery under a
94
+
different-named alias (see Task 3). (Phase 1 left the type named
95
+
`LabelerReport`; Task 3 adds a `OauthClientReport` type that is
96
+
structurally identical and reuses the same `render`/`exit_code`/
97
+
`summary_counts` impl blocks via a shared inner struct or a
98
+
`pub use LabelerReport as OauthClientReport`; see Task 3 for the
99
+
specific choice.)
100
+
101
+
**atproto `client_id` rules (from atproto OAuth spec, 2026-04-16):**
102
+
- Production: `client_id` must use scheme `https`, have no port, and have
103
+
a non-empty path. Query strings are not allowed on the `client_id` URL
104
+
itself (they're allowed on `http://localhost?...` only).
105
+
- Loopback (permissive, per design): `http://localhost[:port][/path]` or
106
+
`http://127.0.0.1[:port][/path]`. The STRICT atproto shape is
107
+
`http://localhost` with no port and `/` path; anything beyond that is a
108
+
deviation from the strict spec and will be flagged by Phase 4's metadata
109
+
validation, **not** Phase 3 discovery. Discovery just needs to recognise
110
+
the form and synthesize implicit metadata.
111
+
- IPv6 loopback (`http://[::1]`) is **not** in scope for Phase 3 (design
112
+
AC1.2/AC1.3 enumerate `localhost` and `127.0.0.1`; add only if a later
113
+
phase calls for it).
114
+
115
+
---
116
+
117
+
## Implementation tasks
118
+
119
+
<!-- START_SUBCOMPONENT_A (tasks 1-3) -->
120
+
121
+
**Subcomponent A: Target parsing + pipeline scaffold + report wiring.**
122
+
123
+
<!-- START_TASK_1 -->
124
+
### Task 1: `OauthClientTarget`, `parse_target`, `ClientIdKind`, `RawMetadata`
125
+
126
+
**Verifies:** test-oauth-client.AC1.4 (target parsing rejects non-HTTPS
127
+
non-loopback scheme).
128
+
129
+
**Files:**
130
+
- Create: `src/commands/test/oauth/client/pipeline.rs`:
131
+
- `pub enum OauthClientTarget { HttpsUrl(Url), Loopback(LoopbackTarget) }`
132
+
- `pub struct LoopbackTarget { pub host: LoopbackHost, pub port: Option<u16>, pub path: String }`
133
+
- `pub enum LoopbackHost { Localhost, Loopback127 }` (the two accepted hostnames).
134
+
- `pub fn parse_target(raw: &str) -> Result<OauthClientTarget, TargetParseError>`:
135
+
- Parse with `url::Url::parse` — any parse error becomes `TargetParseError::NotAUrl`.
136
+
- If scheme is `"https"`:
137
+
- Require a host (i.e., `url.host().is_some()`). Missing host → `TargetParseError::HttpsMissingHost`.
138
+
- Reject query strings and fragments: `url.query().is_some()` or `url.fragment().is_some()` → `TargetParseError::HttpsHasQueryOrFragment`.
139
+
- Return `OauthClientTarget::HttpsUrl(url)`.
140
+
- Else if scheme is `"http"` and host is `"localhost"` or `"127.0.0.1"`:
141
+
- Extract port (`url.port()`), path (`url.path()`). Preserve path including leading slash; `url.path()` returns `"/"` when empty.
142
+
- Return `OauthClientTarget::Loopback { host: Localhost|Loopback127, port, path: url.path().to_string() }`.
143
+
- Else → `TargetParseError::UnsupportedScheme { scheme: url.scheme().to_string() }` or `TargetParseError::HttpNonLoopback { host: ... }` depending on which condition failed (scheme-not-http vs host-not-loopback).
144
+
- `#[derive(Debug, Error, Diagnostic)] pub enum TargetParseError { ... }` with stable codes `oauth_client::target::{not_a_url, unsupported_scheme, https_missing_host, https_has_query_or_fragment, http_non_loopback}`. Each variant carries the input string and a `#[help]` explaining the rule.
145
+
146
+
**Implementation notes:**
147
+
- `parse_target` returns a `Result` that will be surfaced from
148
+
`ClientCmd::run` as a non-zero exit with a rendered miette diagnostic.
149
+
This is **not** a `CheckResult` — the target doesn't parse, so there's
150
+
no pipeline to run.
151
+
- Returning miette errors (rather than going through clap's value parser)
152
+
gives us richer diagnostics with source spans pointing at the raw arg.
153
+
Clap just gets a `String` via the existing `target: String` field.
154
+
- `LoopbackTarget.path` is stored verbatim from `url.path()` (always starts
155
+
with `/`). Phase 4 will validate strict-atproto-form (`/` only; anything
156
+
else → `SpecViolation`).
157
+
158
+
**Testing:** embedded in `src/commands/test/oauth/client/pipeline.rs` as
159
+
unit tests (not integration): `https_happy`, `https_with_port_rejected`,
160
+
`https_with_query_rejected`, `https_with_fragment_rejected`,
161
+
`localhost_no_port`, `localhost_with_port`, `localhost_with_path`,
162
+
`ipv4_loopback`, `http_non_loopback_host`, `ftp_scheme_rejected`,
163
+
`garbage_input_not_a_url`. Each test asserts both the variant and the
164
+
diagnostic `code()`.
165
+
166
+
**Verification:**
167
+
- `cargo test --lib commands::test::oauth::client::pipeline` — all tests pass.
168
+
169
+
**Commit:** `feat(oauth-client): parse_target — accept https and loopback forms, reject others`
170
+
171
+
<!-- END_TASK_1 -->
172
+
173
+
<!-- START_TASK_2 -->
174
+
### Task 2: `DiscoveryFacts`, `RawMetadata`, `Check`, `run`
175
+
176
+
**Verifies:** test-oauth-client.AC1.1, AC1.2, AC1.3, AC1.5, AC1.6, AC1.7
177
+
(every discovery outcome — happy paths, network errors, non-JSON bodies,
178
+
loopback variants).
179
+
180
+
**Files:**
181
+
- Create: `src/commands/test/oauth/client/discovery.rs`:
182
+
- `pub struct DiscoveryFacts { pub client_id: Url, pub kind: ClientIdKind, pub raw_metadata: RawMetadata }`
183
+
- `pub enum ClientIdKind { HttpsUrl, Loopback }`
184
+
- `pub enum RawMetadata { Document { bytes: Arc<[u8]>, content_type: Option<String> }, Implicit { client_id: Url } }`
185
+
- `#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Check { ClientIdWellFormed, MetadataDocumentFetchable, MetadataIsJson }`
186
+
with `.id() -> &'static str` returning
187
+
`"oauth_client::discovery::client_id_well_formed"`,
188
+
`"oauth_client::discovery::metadata_document_fetchable"`,
189
+
`"oauth_client::discovery::metadata_is_json"`. Add helper methods
190
+
`.pass()`, `.spec_violation(diagnostic)`, `.network_error(diagnostic)`,
191
+
`.skipped(reason)` matching the labeler pattern at
192
+
`src/commands/test/labeler/http.rs:22-114`.
193
+
- `pub struct DiscoveryStageOutput { pub facts: Option<DiscoveryFacts>, pub results: Vec<CheckResult> }`.
194
+
- `pub async fn run(target: &OauthClientTarget, http: &dyn HttpClient) -> DiscoveryStageOutput`:
195
+
- For `OauthClientTarget::HttpsUrl(url)`:
196
+
- Emit `Check::ClientIdWellFormed.pass()`.
197
+
- Call `http.get_bytes_with_content_type(url).await`. On
198
+
`Err(IdentityError)`, emit
199
+
`Check::MetadataDocumentFetchable.network_error(...)` with a
200
+
diagnostic showing the URL and the error's display; emit
201
+
`common::report::blocked_by(Check::MetadataIsJson.id(), Stage::DISCOVERY, "Metadata is JSON", Check::MetadataDocumentFetchable.id())`.
202
+
Return `{facts: None, results}`.
203
+
- On `Ok((status, body, _content_type))` with status ∉ `200..=299`:
204
+
emit `Check::MetadataDocumentFetchable.network_error(...)` with a
205
+
diagnostic including URL and status; skip `MetadataIsJson` via
206
+
`blocked_by`. Return `{facts: None, results}`.
207
+
- On `Ok((2xx, body, content_type))`:
208
+
- Emit `Check::MetadataDocumentFetchable.pass()`.
209
+
- Attempt `serde_json::from_slice::<serde_json::Value>(&body)`.
210
+
- On parse error: emit `Check::MetadataIsJson.spec_violation(...)`
211
+
with a diagnostic carrying a `NamedSource` wrapping `body`, a
212
+
span at the parse error's line/column (via
213
+
`crate::common::diagnostics::span_at_line_column`), and a
214
+
human-readable summary
215
+
`{ let ct = content_type.as_deref().unwrap_or("<unknown>"); format!("response body is not valid JSON (content-type: {ct})") }`. Return `{facts: None, results}`.
216
+
- On parse success: emit `Check::MetadataIsJson.pass()`. Return
217
+
`{facts: Some(DiscoveryFacts { client_id: url.clone(), kind: HttpsUrl, raw_metadata: Document { bytes: body.into(), content_type }}), results}`.
218
+
- For `OauthClientTarget::Loopback(l)`:
219
+
- Emit `Check::ClientIdWellFormed.pass()`.
220
+
- Emit
221
+
```rust
222
+
common::report::skipped_with_reason(
223
+
Check::MetadataDocumentFetchable.id(),
224
+
Stage::DISCOVERY,
225
+
"Metadata document fetchable",
226
+
"metadata is implicit for loopback clients",
227
+
)
228
+
```
229
+
— `skipped_with_reason` was introduced in Phase 1 Task 2 for
230
+
this exact use. **Do NOT re-declare it here.**
231
+
- Emit `MetadataIsJson` skipped via the same helper with the same
232
+
reason `"metadata is implicit for loopback clients"`.
233
+
- Return `{facts: Some(DiscoveryFacts { client_id: reconstruct_url(l), kind: Loopback, raw_metadata: Implicit { client_id: reconstruct_url(l) }}), results}`.
234
+
- `reconstruct_url(LoopbackTarget)` builds
235
+
`http://<host>:<port><path>` — feeding port and path through
236
+
verbatim so AC1.7 (port, path variants) produce identical facts
237
+
shape modulo the URL's port/path.
238
+
- `Stage::DISCOVERY`, `Stage::METADATA`, `Stage::JWKS`, and
239
+
`Stage::INTERACTIVE` consts all live in `src/common/report.rs` (added
240
+
in Phase 1 Task 1 as part of the single authoritative Stage const
241
+
table). No additions to `common/report.rs` are needed in this phase.
242
+
243
+
**Implementation notes:**
244
+
- The design calls `RawMetadata::Document(Bytes)` — using `Arc<[u8]>` instead
245
+
matches the existing `RawDidDocument` pattern (`src/common/identity.rs`)
246
+
and avoids pulling in `bytes::Bytes` for this single use.
247
+
- `content_type` is captured directly from the HTTP response via
248
+
`get_bytes_with_content_type` (extended on the `HttpClient` trait in
249
+
this phase) and threaded into `RawMetadata::Document`. Phase 4 can
250
+
read it from `MetadataFacts` / `DiscoveryFacts` without needing any
251
+
further trait changes.
252
+
- Diagnostic construction for AC1.6: reuse
253
+
`src/common/diagnostics.rs::named_source_from_bytes` and
254
+
`span_at_line_column`. These are already the labeler's pattern
255
+
(see `src/commands/test/labeler/identity.rs` imports at
256
+
`identity.rs:17-19`).
257
+
258
+
**Testing:** integration tests in Task 5 (after the pipeline runner is
259
+
wired in Task 3). This task only needs the code to compile cleanly.
260
+
261
+
**Commit:** `feat(oauth-client): add discovery stage - fetch metadata doc or synthesize implicit`
262
+
263
+
<!-- END_TASK_2 -->
264
+
265
+
<!-- START_TASK_3 -->
266
+
### Task 3: `OauthClientOptions`, `run_pipeline`, `ClientCmd::run` wiring
267
+
268
+
**Verifies:** depends on the pipeline running end-to-end; the final
269
+
integration tests in Task 5 verify AC1.1 through AC1.7.
270
+
271
+
**Files:**
272
+
- Modify: `src/commands/test/oauth/client/pipeline.rs` — add:
273
+
- `pub struct OauthClientOptions<'a> { pub http: &'a dyn HttpClient, pub verbose: bool }` — Phase 3 only uses `http` and `verbose`. Later phases extend (`dns`, `jwks`, `clock`, `interactive`). The `'a` lifetime matches the labeler pattern at `src/commands/test/labeler/pipeline.rs:48-61`.
274
+
- `pub struct OauthClientReport(LabelerReport);` — a newtype wrapping `LabelerReport` (which was promoted in Phase 1 to `src/common/report.rs`). Alternative considered: rename the underlying type. The newtype route keeps labeler snapshots and imports stable.
275
+
- `impl OauthClientReport { pub fn record(&mut self, r: CheckResult) { self.0.record(r); } pub fn finish(&mut self) { self.0.finish(); } pub fn exit_code(&self) -> i32 { self.0.exit_code() } pub fn render<W: std::io::Write>(&self, out: &mut W, cfg: &RenderConfig) -> std::io::Result<()> { self.0.render(out, cfg) } }`
276
+
- Decision point: since `LabelerReport` is now in `crate::common::report`, it's no longer "labeler-specific" at the module-path level. Rename it to `Report` in Phase 1 would have been cleaner, but that's retrospectively out of Phase 1's scope; Phase 3 takes the newtype route and a future phase can elide the wrapper if it becomes awkward.
277
+
- `pub async fn run_pipeline(target: OauthClientTarget, opts: OauthClientOptions<'_>) -> OauthClientReport`:
278
+
- Build a `ReportHeader { target: <raw string, saved via Task 1 additions>, resolved_did: None, pds_endpoint: None, labeler_endpoint: None }`. The header's `labeler_endpoint` field doesn't apply here; it's retained for struct-shape compatibility. A future refactor can rename.
279
+
- Create an empty `OauthClientReport::new(header)` — add the `new` constructor.
280
+
- Call `discovery::run(&target, opts.http).await`.
281
+
- Record all results.
282
+
- Stash `discovery_output.facts` — unused in Phase 3 but will feed
283
+
metadata stage in Phase 4.
284
+
- Call `report.finish()`.
285
+
- Return.
286
+
- Modify: `src/commands/test/oauth/client.rs` — rewrite `ClientCmd::run`:
287
+
- Construct the shared reqwest client (copy the pattern from
288
+
`src/commands/test/labeler.rs:61-66` verbatim: rustls + 10s timeout +
289
+
`APP_USER_AGENT`).
290
+
- Construct `RealHttpClient::from_client(reqwest_client.clone())`.
291
+
- Call `pipeline::parse_target(&self.target)`. On `Err(e)`, return
292
+
`Err(miette::Report::new(e))` — this renders the diagnostic and exits
293
+
with code 1 via `main.rs`'s `miette::Result` handling.
294
+
- Build `OauthClientOptions { http: &http_client, verbose: self.verbose }`.
295
+
- Call `pipeline::run_pipeline(target, opts).await`.
296
+
- Render: `report.render(&mut std::io::stdout(), &RenderConfig { no_color: self.no_color || no_color })`. Both the CLI-level `--no-color` and the command-level `--no-color` suppress color (OR-semantics).
297
+
- Return `Ok(ExitCode::from(u8::try_from(report.exit_code()).unwrap_or(1)))`.
298
+
- Handle the `Some(ClientMode::Interactive(_))` arm: for Phase 3 it prints `"interactive mode: static stages run, interactive stage not yet implemented (Phase 6-8)"` and proceeds as if static mode was selected. Replace with real interactive wiring in Phase 7.
299
+
300
+
**Implementation notes:**
301
+
- The CLI-level `no_color` comes in as the function argument; the
302
+
command-level `no_color` comes from `self.no_color`. OR them to produce
303
+
a final `no_color`. This mirrors the labeler pattern at
304
+
`src/commands/test/labeler.rs:87` which only uses the command-level
305
+
field because the labeler was added before the CLI-level flag — the
306
+
oauth command has the benefit of Phase 1's groundwork.
307
+
- `miette::Report::new(e)` works on any type that implements `miette::Diagnostic + Send + Sync + 'static`. `TargetParseError` (Task 1) derives `Diagnostic`.
308
+
- Tracing: add `#[tracing::instrument(level = "debug", skip(opts))]` to
309
+
`run_pipeline`. `--verbose` is wired globally by `src/cli.rs` — no
310
+
per-stage overrides needed.
311
+
312
+
**Verification:**
313
+
- `cargo build` succeeds.
314
+
- `cargo run -- test oauth client https://httpbingo.org/json` hits a real
315
+
endpoint — exits with code 2 (network/endpoint didn't return atproto
316
+
metadata JSON) and prints a rendered report. No panic.
317
+
- `cargo run -- test oauth client http://localhost:8080/client.json` —
318
+
exits 0 with a rendered "metadata is implicit for loopback" skip report.
319
+
- `cargo run -- test oauth client ftp://bad.example.com` — exits with
320
+
clap/miette error showing the `oauth_client::target::unsupported_scheme`
321
+
diagnostic. Non-zero exit.
322
+
323
+
**Commit:** `feat(oauth-client): wire pipeline runner and ClientCmd::run to discovery stage`
324
+
325
+
<!-- END_TASK_3 -->
326
+
327
+
<!-- END_SUBCOMPONENT_A -->
328
+
329
+
<!-- START_SUBCOMPONENT_B (tasks 4-5) -->
330
+
331
+
**Subcomponent B: Test infrastructure + integration tests.**
332
+
333
+
<!-- START_TASK_4 -->
334
+
### Task 4: Promote `FakeHttpClient` and `FakeDnsResolver` to `tests/common/mod.rs`
335
+
336
+
**Verifies:** none (shared test infra).
337
+
338
+
**Files:**
339
+
- Modify: `tests/common/mod.rs`:
340
+
- Add `pub struct FakeHttpClient { responses: Arc<Mutex<HashMap<Url, (u16, Vec<u8>)>>>, transport_errors: Arc<Mutex<HashSet<Url>>> }` with:
341
+
- `pub fn new() -> Self`
342
+
- `pub fn add_response(&self, url: Url, status: u16, body: Vec<u8>)`
343
+
- `pub fn add_transport_error(&self, url: Url)` — causes `get_bytes` to return `IdentityError::HttpTransportError { .. }` for that URL.
344
+
- `impl HttpClient for FakeHttpClient` — looks up `url` in the maps; returns `Ok((status, body))`, or an error-variant identity error, or panics if the URL wasn't seeded (catches test-author mistakes).
345
+
- Add `pub struct FakeDnsResolver { records: Arc<Mutex<HashMap<String, Vec<String>>>> }` with constructor, `add_records(name, values)`, and a panic-on-unseeded-name `DnsResolver` impl. (Not strictly required for Phase 3 but saves re-adding it for oauth client if we ever need DNS here; include now to unblock `labeler_identity.rs` migration in this task.)
346
+
- Modify: `tests/labeler_identity.rs` — delete the local `FakeHttpClient` and `FakeDnsResolver` definitions (currently at lines 19-86). Replace `use` statements with `use common::{FakeHttpClient, FakeDnsResolver};`. Confirm snapshot tests still pass.
347
+
348
+
**Implementation notes:**
349
+
- Using `Arc<Mutex<...>>` is the pattern the existing `FakeRawHttpTee`
350
+
already uses (`tests/common/mod.rs:24-102`).
351
+
- The design says "reuse existing `FakeHttpClient`" — it must exist in a
352
+
location everyone can import. `tests/common/mod.rs` is the only path
353
+
that works for both `labeler_*.rs` and new `oauth_client_*.rs` binaries
354
+
without publishing internals.
355
+
- The panic-on-unseeded-URL behavior surfaces test-author mistakes
356
+
quickly and prevents silent fallbacks.
357
+
358
+
**Verification:**
359
+
- `cargo test --test labeler_identity` passes without any snapshot changes.
360
+
- `cargo test` as a whole still passes.
361
+
362
+
**Commit:** `refactor(tests): promote FakeHttpClient and FakeDnsResolver to tests/common/mod.rs`
363
+
364
+
<!-- END_TASK_4 -->
365
+
366
+
<!-- START_TASK_5 -->
367
+
### Task 5: Discovery stage integration tests
368
+
369
+
**Verifies:** test-oauth-client.AC1.1, AC1.2, AC1.3, AC1.5, AC1.6, AC1.7
370
+
(all AC1 cases except AC1.4 — that's covered by Task 1's unit tests).
371
+
372
+
**Files:**
373
+
- Create: `tests/oauth_client_discovery.rs` with test helpers and per-case
374
+
tests (described below).
375
+
- Create directory structure `tests/fixtures/oauth_client/discovery/`:
376
+
- `https_confidential_happy/metadata.json` — a minimal valid atproto client metadata JSON (just enough for discovery to emit Pass rows; metadata validation is Phase 4). Keep fields: `client_id`, `application_type`, `redirect_uris`, `grant_types`, `response_types`, `scope`, `dpop_bound_access_tokens`, `token_endpoint_auth_method`, `jwks`.
377
+
- `https_404/` — empty directory with `.gitkeep`; no body fixture (test seeds a 404 directly).
378
+
- `https_not_json/not_json.txt` — `not valid json at all` (literal).
379
+
- `loopback_root/.gitkeep`, `loopback_with_path/.gitkeep` — no fixtures needed; loopback never fetches.
380
+
381
+
**Testing:**
382
+
Write each test as a `#[tokio::test]` driving `run_pipeline` through the
383
+
`FakeHttpClient`. Every test ends with an insta snapshot of the rendered
384
+
report.
385
+
386
+
1. `https_confidential_happy_discovery`:
387
+
- Url: `https://client.example.com/metadata.json`
388
+
- FakeHttpClient: add 200 response with `include_bytes!("fixtures/oauth_client/discovery/https_confidential_happy/metadata.json").to_vec()`.
389
+
- Assert `report.exit_code() == 0`.
390
+
- `insta::assert_snapshot!(rendered)` — snapshot pins the three Pass rows.
391
+
- AC1.1 verified.
392
+
393
+
2. `https_404_produces_network_error`:
394
+
- Url: `https://client.example.com/missing.json`
395
+
- FakeHttpClient: add 404 response with empty body.
396
+
- Assert `exit_code == 2`.
397
+
- Snapshot pins NetworkError on `metadata_document_fetchable` and Skipped on `metadata_is_json` with reason `"blocked by oauth_client::discovery::metadata_document_fetchable"`.
398
+
- AC1.5 verified.
399
+
400
+
3. `https_unreachable_produces_network_error`:
401
+
- Url: `https://client.example.com/metadata.json`
402
+
- FakeHttpClient: `add_transport_error(url)`.
403
+
- Assert `exit_code == 2`.
404
+
- Snapshot pins NetworkError with the error display.
405
+
- AC1.5 verified.
406
+
407
+
4. `https_not_json_produces_spec_violation`:
408
+
- Url: `https://client.example.com/metadata.json`
409
+
- FakeHttpClient: 200 with `include_bytes!(...not_json.txt)`.
410
+
- Assert `exit_code == 1`.
411
+
- Snapshot pins MetadataDocumentFetchable Pass, MetadataIsJson SpecViolation with the parse-failed-here span visible.
412
+
- AC1.6 verified.
413
+
414
+
5. `loopback_root_produces_skip_rows`:
415
+
- Url: `http://localhost`
416
+
- FakeHttpClient untouched.
417
+
- Assert `exit_code == 0`.
418
+
- Snapshot pins ClientIdWellFormed Pass, MetadataDocumentFetchable Skipped with reason `"metadata is implicit for loopback clients"`, MetadataIsJson Skipped with same reason.
419
+
- AC1.2, partial AC1.7 verified.
420
+
421
+
6. `loopback_with_port_produces_same_skip_rows`:
422
+
- Url: `http://localhost:8080/client.json`
423
+
- Assert same `exit_code == 0`.
424
+
- Snapshot pins the same three rows (differing only in target header).
425
+
- AC1.7 verified.
426
+
427
+
7. `loopback_127_0_0_1`:
428
+
- Url: `http://127.0.0.1:3000/`
429
+
- Assert same skip-row pattern.
430
+
- AC1.3 verified.
431
+
432
+
8. `https_happy_body_flow_into_facts`:
433
+
- Unit test (not snapshot) — assert that after `run_pipeline`, the
434
+
`DiscoveryFacts.kind == HttpsUrl` and `raw_metadata` matches
435
+
`Document`. Purpose: pin the shape so Phase 4 can consume. This test
436
+
does not render a report — it's a harness-level assertion.
437
+
Since `OauthClientReport` doesn't expose facts, add a
438
+
`pub(crate) fn discovery_facts(&self) -> Option<...>` escape hatch,
439
+
or run the discovery stage directly without the `run_pipeline`
440
+
wrapper. Pick the latter: the test directly `await`s
441
+
`discovery::run(&target, &fake_http).await` and inspects the
442
+
returned `DiscoveryStageOutput`.
443
+
444
+
**Helper:**
445
+
```rust
446
+
fn render_report_to_string(
447
+
report: &atproto_devtool::commands::test::oauth::client::pipeline::OauthClientReport,
448
+
) -> String {
449
+
let mut buf = Vec::new();
450
+
report
451
+
.render(&mut buf, &RenderConfig { no_color: true })
452
+
.expect("render failed");
453
+
String::from_utf8(buf).expect("invalid utf-8")
454
+
}
455
+
```
456
+
457
+
**Snapshot file naming:**
458
+
- `tests/snapshots/oauth_client_discovery__https_confidential_happy_discovery.snap`
459
+
- `tests/snapshots/oauth_client_discovery__https_404_produces_network_error.snap`
460
+
- ... etc., matching the `<binary>__<test_name>.snap` labeler convention.
461
+
462
+
**Verification:**
463
+
- `cargo test --test oauth_client_discovery` — on first run, inspect
464
+
snapshots via `cargo insta review`, accept if they look correct.
465
+
- All checks appear verbatim as `oauth_client::discovery::*`.
466
+
- Manual: `cargo run -- test oauth client https://example.com/client.json`
467
+
against a reachable dummy URL emits a visually sensible report.
468
+
469
+
**Commit:** `test(oauth-client): add discovery stage integration tests with fixtures and snapshots`
470
+
471
+
<!-- END_TASK_5 -->
472
+
473
+
<!-- END_SUBCOMPONENT_B -->
474
+
475
+
---
476
+
477
+
## Phase 3 done when
478
+
479
+
- `cargo test --test oauth_client_discovery` passes.
480
+
- `cargo insta review` has accepted all new snapshots.
481
+
- `cargo test --test labeler_identity` passes (after the fake promotion
482
+
migration in Task 4).
483
+
- `cargo build`, `cargo test` (full), `cargo clippy -- -D warnings`, and
484
+
`cargo fmt --check` all succeed.
485
+
- `grep -n "oauth_client::discovery::" tests/snapshots/oauth_client_discovery*.snap`
486
+
confirms every check ID appears verbatim in at least one snapshot
487
+
(supports AC8.7).
488
+
489
+
Phase 4 will consume `DiscoveryFacts` and land the metadata-validation
490
+
stage.
+492
docs/implementation-plans/2026-04-16-test-oauth-client/phase_04.md
+492
docs/implementation-plans/2026-04-16-test-oauth-client/phase_04.md
···
1
+
# OAuth client test — Phase 4: Static stage 2 — metadata validation
2
+
3
+
**Goal:** Parse the discovered metadata document and validate every
4
+
spec-derived static property of an atproto OAuth client metadata document.
5
+
6
+
**Architecture:** A new `metadata.rs` stage that consumes
7
+
`DiscoveryFacts` from Phase 3 and produces `MetadataFacts`. For
8
+
`RawMetadata::Document`, it deserializes the JSON into a typed
9
+
`RawMetadataDocument` and runs spec checks (required fields, redirect
10
+
URI shape per `application_type`, token endpoint auth method, scope
11
+
grammar, DPoP flag). For `RawMetadata::Implicit` (loopback), every
12
+
validation check emits `Skipped` with reason `"metadata is implicit for
13
+
loopback clients"`. A `ScopeSet` newtype parses the atproto permission
14
+
grammar; parse failures produce a `SpecViolation` with a miette source
15
+
span highlighting the offending token.
16
+
17
+
**Tech Stack:** `serde_json`, `miette` source spans, the `Url` type,
18
+
`thiserror`, the existing `diagnostics.rs` helpers.
19
+
20
+
**Scope:** Phase 4 of 8.
21
+
22
+
**Codebase verified:** 2026-04-16.
23
+
24
+
---
25
+
26
+
## Acceptance Criteria Coverage
27
+
28
+
This phase implements and tests:
29
+
30
+
### test-oauth-client.AC2: Static mode — metadata document validation
31
+
32
+
- **test-oauth-client.AC2.1 Success:** A confidential web client document with all required fields (`application_type=web`, `response_types=[code]`, `grant_types=[authorization_code]`, `dpop_bound_access_tokens=true`, `redirect_uris` HTTPS, `token_endpoint_auth_method=private_key_jwt`, `jwks` or `jwks_uri`, valid `scope`) passes every metadata-stage check.
33
+
- **test-oauth-client.AC2.2 Success:** A public web client document (no JWKS, `token_endpoint_auth_method=none`) passes.
34
+
- **test-oauth-client.AC2.3 Success:** A native client document with custom-scheme `redirect_uris` whose scheme matches the reverse-domain of the `client_id` host passes.
35
+
- **test-oauth-client.AC2.4 Failure:** `dpop_bound_access_tokens` absent or `false` produces a `SpecViolation` with stable code `oauth_client::metadata::dpop_bound_required`.
36
+
- **test-oauth-client.AC2.5 Failure:** A confidential client without `jwks` or `jwks_uri` produces a `SpecViolation` with code `oauth_client::metadata::confidential_requires_jwks`.
37
+
- **test-oauth-client.AC2.6 Failure:** A public client whose `token_endpoint_auth_method` is anything other than `none` produces a `SpecViolation`.
38
+
- **test-oauth-client.AC2.7 Failure:** A native client whose `redirect_uri` scheme does not match the reverse-domain of the `client_id` host produces a `SpecViolation`.
39
+
- **test-oauth-client.AC2.8 Failure:** A `scope` field that does not parse against the atproto permission grammar produces a `SpecViolation` with the offending token highlighted via miette source span.
40
+
- **test-oauth-client.AC2.9 Skip:** Every metadata-document validation check on a loopback target emits `Skipped` with reason `"metadata is implicit for loopback clients"`.
41
+
42
+
---
43
+
44
+
## Codebase verification findings
45
+
46
+
- `src/common/diagnostics.rs` has `pretty_json_for_display`,
47
+
`named_source_from_bytes`, `span_at_line_column`, and
48
+
`span_for_quoted_literal` (confirmed via upfront investigation). Phase 4
49
+
leans on `named_source_from_bytes` for JSON source spans and
50
+
`span_for_quoted_literal` for highlighting specific field values in
51
+
diagnostics. The scope-parse case will compute a custom span from byte
52
+
offsets because the offending token may not be a JSON quoted literal
53
+
(scope is a space-separated string).
54
+
- The design discrepancy around `RawMetadata::Document(Bytes)` was resolved
55
+
in Phase 3 by storing `Document { bytes: Arc<[u8]>, content_type:
56
+
Option<String> }`. Phase 4 consumes that shape and additionally stores
57
+
the source name (for diagnostics) in the metadata stage's own Facts if
58
+
useful.
59
+
60
+
**atproto OAuth client metadata requirements (as of 2026-04-16):**
61
+
62
+
Required fields:
63
+
- `client_id` (string, URL) — must match the URL the document was fetched
64
+
from (Phase 3 already captured this URL; Phase 4 checks equality).
65
+
- `application_type` (string) — `"web"` or `"native"`.
66
+
- `grant_types` (array of strings) — must include `"authorization_code"`;
67
+
may additionally include `"refresh_token"`. No other values allowed
68
+
(per atproto profile).
69
+
- `response_types` (array of strings) — must equal `["code"]` exactly.
70
+
- `scope` (string, space-separated) — must include the `atproto` token;
71
+
additional tokens must parse against the permission grammar at
72
+
`https://atproto.com/specs/permission`.
73
+
- `redirect_uris` (array of strings) — at least one; shape rules depend
74
+
on `application_type`:
75
+
- `application_type = "web"`: every URI must be HTTPS, and its origin
76
+
must match the `client_id` origin.
77
+
- `application_type = "native"`: each URI uses either (a) a custom
78
+
scheme that is the reverse-domain of the `client_id` host and
79
+
follows `scheme:/path` (single slash) form, or (b) an HTTPS URL with
80
+
matching origin. For Phase 4, validate (a) when scheme ∉
81
+
`{"http","https"}` — i.e., treat anything non-https as a custom
82
+
scheme and apply reverse-domain matching.
83
+
- `dpop_bound_access_tokens` (boolean) — must be present and `true`.
84
+
- `token_endpoint_auth_method` (string):
85
+
- For `application_type = "web"` clients, the allowed values are
86
+
`"private_key_jwt"` (confidential) or `"none"` (public). Anything
87
+
else is a violation.
88
+
- For `application_type = "native"` clients, must be `"none"`.
89
+
- `jwks` xor `jwks_uri`:
90
+
- A confidential client (`token_endpoint_auth_method = "private_key_jwt"`)
91
+
must provide exactly one of `jwks` (inline JWK array) or `jwks_uri`
92
+
(HTTPS URL). Not both, not neither.
93
+
- Public and native clients must provide neither.
94
+
95
+
Loopback client implicit metadata (strictly: `client_id =
96
+
http://localhost`, no port, path `/`):
97
+
- Treated as public client.
98
+
- `redirect_uris` defaults to `http://127.0.0.1/` and `http://[::1]/`
99
+
if omitted.
100
+
- `scope` still required (atproto scope minimum).
101
+
- No metadata-document validation applies — every metadata stage check
102
+
emits `Skipped`. Scope still needs **some** reasonable default; Phase 4
103
+
treats Implicit targets as having an empty scope for downstream purposes
104
+
(consumers of `MetadataFacts` use the stored scope for later flow
105
+
variants).
106
+
107
+
**atproto permission grammar** (verbatim from `https://atproto.com/specs/permission`):
108
+
- Scope is a space-separated list of tokens.
109
+
- Each token: `resource:positional?param=value¶m=value`.
110
+
- Resource names: `repo`, `identity`, `blob`, `account`, `rpc`, `include`.
111
+
- Positional parameter (optional) follows `:` directly (e.g.,
112
+
`repo:app.bsky.feed.post`).
113
+
- Query parameters start with `?`, separated by `&`. Repeated parameter
114
+
names encode array values.
115
+
- Percent-encoding allowed for reserved characters.
116
+
- The `atproto` scope (plain, no colon) is also valid — it's the baseline
117
+
auth scope every atproto OAuth grant includes.
118
+
- Positional parameter and query-parameter form are mutually exclusive
119
+
for the same key (i.e., `repo:com.example.record?collection=other` is
120
+
invalid because `collection` is positional-only).
121
+
122
+
Full grammar implementation for Phase 4 **does not need to be
123
+
production-grade**; it needs to correctly accept the atproto baseline
124
+
(`atproto`), the common forms (`atproto repo:<nsid>?action=create`, etc.),
125
+
and reject obvious garbage. The spec has more grammar details; ship a
126
+
conservative parser and defer edge cases to later test coverage.
127
+
128
+
---
129
+
130
+
## Implementation tasks
131
+
132
+
<!-- START_SUBCOMPONENT_A (tasks 1-2) -->
133
+
134
+
**Subcomponent A: Typed metadata document + scope parser.**
135
+
136
+
<!-- START_TASK_1 -->
137
+
### Task 1: `RawMetadataDocument` deserialization + `ScopeSet` parser
138
+
139
+
**Verifies:** test-oauth-client.AC2.1, AC2.2, AC2.3, AC2.8 (partially —
140
+
parse-level correctness).
141
+
142
+
**Files:**
143
+
- Create: `src/commands/test/oauth/client/metadata.rs`:
144
+
- `pub struct RawMetadataDocument` — fields:
145
+
```rust
146
+
pub client_id: Option<String>,
147
+
pub application_type: Option<String>,
148
+
pub grant_types: Option<Vec<String>>,
149
+
pub response_types: Option<Vec<String>>,
150
+
pub scope: Option<String>,
151
+
pub redirect_uris: Option<Vec<String>>,
152
+
pub dpop_bound_access_tokens: Option<bool>,
153
+
pub token_endpoint_auth_method: Option<String>,
154
+
pub jwks: Option<serde_json::Value>, // stored as raw JSON; Phase 5 parses
155
+
pub jwks_uri: Option<String>,
156
+
```
157
+
Derive `Deserialize` with explicit `#[serde(deny_unknown_fields)] =
158
+
false` — additional fields are allowed per OAuth spec, we just don't
159
+
validate them. Do **not** use `#[serde(flatten)]` per CLAUDE.md.
160
+
- Parse helper: `pub fn parse_raw(bytes: &[u8]) -> Result<RawMetadataDocument, serde_json::Error>`.
161
+
- `pub struct ScopeSet { tokens: Vec<ScopeToken> }`.
162
+
- `pub enum ScopeToken { Atproto, Permission { resource: String, positional: Option<String>, query: Vec<(String, String)> } }`.
163
+
- `pub fn parse_scope(s: &str) -> Result<ScopeSet, ScopeParseError>`:
164
+
- Split by ASCII whitespace.
165
+
- For each token: `"atproto"` → `ScopeToken::Atproto`; otherwise split
166
+
by first `:`. Before `:` → resource, after `:` → rest (may contain
167
+
`?`). Parse rest into positional + query. Reject empty resource,
168
+
reject resource not in `{repo, identity, blob, account, rpc, include}`,
169
+
reject percent-decoding errors.
170
+
- Positional/query mutual-exclusion check: if positional is Some and
171
+
query contains a key that would conflict, flag it. Phase 4 keeps
172
+
this minimal — reject empty positional (`repo:?foo=bar` invalid).
173
+
- `#[derive(Debug, Error, Diagnostic)] pub struct ScopeParseError`:
174
+
- Fields: `token: String`, `byte_offset: usize`, `reason: ScopeParseReason`.
175
+
- `diagnostic(code("oauth_client::metadata::scope_grammar"))`.
176
+
- `ScopeParseReason::{UnknownResource, EmptyResource, InvalidQuery, ...}`.
177
+
178
+
**Implementation notes:**
179
+
- `RawMetadataDocument` is deliberately permissive on missing fields —
180
+
Task 2 emits per-field SpecViolations. Using `Option<T>` everywhere
181
+
keeps deserialization lenient; the checks do the enforcement.
182
+
- `ScopeSet`'s purpose is to:
183
+
(a) catch grammatically invalid scope (AC2.8).
184
+
(b) make scope tokens available as structured data for Phase 8 flow
185
+
variants that need to know which permissions were requested.
186
+
- `byte_offset` on `ScopeParseError` is computed during the whitespace
187
+
split (track byte position). Phase 4 uses it to build a `SourceSpan`
188
+
pointing at the failing token inside the scope string inside the
189
+
metadata JSON source.
190
+
191
+
**Testing:**
192
+
Unit tests in `metadata.rs`:
193
+
- `scope_parses_atproto_alone`.
194
+
- `scope_parses_atproto_plus_permission`.
195
+
- `scope_rejects_empty_string` — returns `ScopeParseError` with suitable
196
+
reason.
197
+
- `scope_rejects_unknown_resource` (`foo:bar` → error).
198
+
- `scope_rejects_empty_resource` (`:bar` → error).
199
+
- `scope_accepts_query_forms` — `atproto repo:app.bsky.feed.post?action=create`.
200
+
- `raw_metadata_doc_deserializes_with_missing_fields` — confirm
201
+
`Option<T>` semantics.
202
+
203
+
**Verification:** `cargo test --lib commands::test::oauth::client::metadata`.
204
+
205
+
**Commit:** `feat(oauth-client): add metadata document deserializer and ScopeSet parser`
206
+
207
+
<!-- END_TASK_1 -->
208
+
209
+
<!-- START_TASK_2 -->
210
+
### Task 2: `MetadataFacts`, `Check`, `run` — validation logic
211
+
212
+
**Verifies:** AC2.1, AC2.2, AC2.3, AC2.4, AC2.5, AC2.6, AC2.7, AC2.8,
213
+
AC2.9.
214
+
215
+
**Files:**
216
+
- Modify: `src/commands/test/oauth/client/metadata.rs` — add the Facts,
217
+
the Check enum, and the `run` function.
218
+
219
+
**Implementation outline:**
220
+
221
+
```rust
222
+
pub struct MetadataFacts {
223
+
pub kind: ClientKind,
224
+
pub client_id: Url,
225
+
pub redirect_uris: Vec<Url>, // parsed; for native custom-scheme URIs, still stored as Url via url::Url support
226
+
pub requires_jwks: bool,
227
+
pub jwks_source: Option<JwksSource>, // Inline(serde_json::Value) | Uri(Url) | None (when not required)
228
+
pub dpop_bound: bool,
229
+
pub scope: Option<ScopeSet>, // None iff Loopback (implicit skipped)
230
+
}
231
+
232
+
pub enum ClientKind {
233
+
WebConfidential,
234
+
WebPublic,
235
+
Native,
236
+
Loopback,
237
+
}
238
+
239
+
pub enum JwksSource {
240
+
Inline(serde_json::Value),
241
+
Uri(Url),
242
+
}
243
+
244
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
245
+
pub enum Check {
246
+
RawDocumentDeserializes, // oauth_client::metadata::raw_document_deserializes
247
+
ClientIdMatches, // document client_id matches fetched URL
248
+
ApplicationTypePresent,
249
+
ApplicationTypeKnown, // must be web or native
250
+
ResponseTypesIsCode,
251
+
GrantTypesIncludesAuthorizationCode,
252
+
DpopBoundTrue, // → code "oauth_client::metadata::dpop_bound_required"
253
+
RedirectUrisPresent,
254
+
RedirectUrisShape, // per application_type — web HTTPS origin match, native reverse-domain
255
+
TokenEndpointAuthMethodValid, // per kind
256
+
ConfidentialRequiresJwks,
257
+
PublicForbidsJwks, // complement of above
258
+
ScopePresent,
259
+
ScopeIncludesAtproto,
260
+
ScopeGrammarValid, // → code "oauth_client::metadata::scope_grammar"
261
+
}
262
+
```
263
+
264
+
Each variant has `.id() -> &'static str` with the
265
+
`oauth_client::metadata::<slug>` prefix, and a stable diagnostic code
266
+
mapped through the `Check::spec_violation(...)` helper. The specifically
267
+
named stable codes (per design) are `dpop_bound_required` and
268
+
`confidential_requires_jwks`.
269
+
270
+
Each variant **also** has `.summary() -> &'static str` returning the
271
+
exact summary string shown in the `[STATUS]` row. Listed here as the
272
+
single source of truth so both `.pass()` / `.spec_violation()` and
273
+
`emit_all_blocked_by` produce identical summaries:
274
+
275
+
| Variant | `.id()` slug | `.summary()` text |
276
+
|------------------------------------------|-----------------------------------------------|----------------------------------------------------------------|
277
+
| `RawDocumentDeserializes` | `raw_document_deserializes` | Metadata document deserializes |
278
+
| `ClientIdMatches` | `client_id_matches` | Metadata `client_id` matches fetched URL |
279
+
| `ApplicationTypePresent` | `application_type_present` | `application_type` field is present |
280
+
| `ApplicationTypeKnown` | `application_type_known` | `application_type` is `web` or `native` |
281
+
| `ResponseTypesIsCode` | `response_types_is_code` | `response_types` is `["code"]` |
282
+
| `GrantTypesIncludesAuthorizationCode` | `grant_types_includes_authorization_code` | `grant_types` includes `authorization_code` |
283
+
| `DpopBoundTrue` | `dpop_bound_required` | `dpop_bound_access_tokens` is `true` |
284
+
| `RedirectUrisPresent` | `redirect_uris_present` | `redirect_uris` is non-empty |
285
+
| `RedirectUrisShape` | `redirect_uris_shape` | Every `redirect_uri` has the right shape for the client kind |
286
+
| `TokenEndpointAuthMethodValid` | `token_endpoint_auth_method_valid` | `token_endpoint_auth_method` matches client kind |
287
+
| `ConfidentialRequiresJwks` | `confidential_requires_jwks` | Confidential client provides exactly one of `jwks`/`jwks_uri` |
288
+
| `PublicForbidsJwks` | `public_forbids_jwks` | Public/native client does not provide `jwks` or `jwks_uri` |
289
+
| `ScopePresent` | `scope_present` | `scope` field is present |
290
+
| `ScopeIncludesAtproto` | `scope_includes_atproto` | `scope` includes the `atproto` token |
291
+
| `ScopeGrammarValid` | `scope_grammar` | `scope` parses against the atproto permission grammar |
292
+
293
+
The `.summary()` method returns literal `&'static str` values for every
294
+
variant so `emit_all_blocked_by` can produce summary-identical rows
295
+
regardless of whether the check ran or was skipped.
296
+
297
+
`pub fn run(facts: &DiscoveryFacts, raw_source_name: &str) -> MetadataStageOutput`:
298
+
- For `raw_metadata = Implicit { .. }`:
299
+
- Emit every check as `skipped_with_reason(check.id(), Stage::METADATA,
300
+
<summary>, "metadata is implicit for loopback clients")`.
301
+
- Return `MetadataFacts { kind: Loopback, client_id: facts.client_id.clone(), redirect_uris: vec![], requires_jwks: false, jwks_source: None, dpop_bound: false, scope: None }` as `Some` — the Loopback kind gates later phases.
302
+
303
+
- For `raw_metadata = Document { bytes, .. }`:
304
+
- Try `parse_raw(bytes)`. On failure, emit `Check::RawDocumentDeserializes.spec_violation(...)` carrying a JSON span and return `{facts: None, results}` — skip remaining checks.
305
+
- Otherwise emit `Check::RawDocumentDeserializes.pass()` and proceed:
306
+
- Check `client_id` present and equal to `facts.client_id` URL. Mismatch → SpecViolation on `ClientIdMatches` with a diagnostic pointing at the `client_id` field.
307
+
- Check `application_type` present and is `"web"` or `"native"`.
308
+
- `response_types` == `["code"]` exactly.
309
+
- `grant_types` contains `"authorization_code"`; no values outside `{"authorization_code", "refresh_token"}` (spec allows those two).
310
+
- `dpop_bound_access_tokens` present and `true` — the `DpopBoundTrue` check carries code `"oauth_client::metadata::dpop_bound_required"`.
311
+
- `redirect_uris` present and non-empty.
312
+
- Based on `application_type`, compute `ClientKind`:
313
+
- `web` + `token_endpoint_auth_method == "private_key_jwt"` → `WebConfidential`.
314
+
- `web` + `token_endpoint_auth_method == "none"` → `WebPublic`.
315
+
- `web` + anything else → emit `TokenEndpointAuthMethodValid` SpecViolation, set kind provisionally `WebPublic`.
316
+
- `native` → `Native` (auth method must be `"none"`).
317
+
- Validate each redirect URI against kind:
318
+
- `WebConfidential` / `WebPublic`: scheme must be `https`; parse with `Url::parse`. Origin (scheme+host+port) must match `client_id` origin exactly. Otherwise SpecViolation on `RedirectUrisShape`.
319
+
- `Native`: if scheme ∈ `{http, https}`, apply web-rules. If scheme is something else, treat as custom scheme — check that it is the reverse-domain of `client_id.host()`. Algorithm: split host by `.`, reverse, join by `.`, compare to scheme string. Mismatch → SpecViolation on `RedirectUrisShape` with code `"oauth_client::metadata::redirect_scheme_reverse_domain_mismatch"` (maps to AC2.7).
320
+
- Validate JWKS presence:
321
+
- `WebConfidential`: one of `jwks`/`jwks_uri` present and the other absent → `ConfidentialRequiresJwks.pass()`. Both absent → SpecViolation code `"oauth_client::metadata::confidential_requires_jwks"`. Both present → SpecViolation (cleanly flag both-provided as error).
322
+
- `WebPublic`/`Native`: both must be absent. Any present → SpecViolation on `PublicForbidsJwks`.
323
+
- Parse `scope` via `parse_scope`:
324
+
- Absent → SpecViolation `ScopePresent`.
325
+
- Parse error → `ScopeGrammarValid` SpecViolation with a miette source span pointing at the failing token inside the JSON source (compute span by finding the scope string in `bytes` and offsetting by `error.byte_offset`). Stable code `"oauth_client::metadata::scope_grammar"`.
326
+
- Parse success with `ScopeSet` but no `atproto` token → SpecViolation on `ScopeIncludesAtproto`.
327
+
- Pass: scope is present, parses, includes atproto.
328
+
- Compose `MetadataFacts` from the checks that passed; set
329
+
`requires_jwks = matches!(kind, WebConfidential)`.
330
+
331
+
- Return `MetadataStageOutput { facts, results }`.
332
+
333
+
**Implementation notes:**
334
+
- The check ordering matters for snapshot stability. Emit checks in the
335
+
exact order listed in the `Check` enum definition. Group by
336
+
subcomponent (deserialization → identity fields → types → dpop → redirect
337
+
URIs → auth method → jwks → scope).
338
+
- Use insertion-order `CheckResult` emission; the rendering layer
339
+
preserves insertion order (Phase 1 made that change).
340
+
- For `ClientIdMatches`, compare on parsed URL equality (scheme + host +
341
+
port + path + query fragments all matching). Avoid string comparison —
342
+
trailing slashes on paths are a notorious gotcha.
343
+
- `redirect_uris` URLs may need custom scheme support; `url::Url::parse`
344
+
already handles arbitrary schemes, so `Url::parse("com.example:/cb")`
345
+
works.
346
+
347
+
**Testing:** covered by the integration tests in Task 3.
348
+
349
+
**Commit:** `feat(oauth-client): add metadata validation stage with all AC2 checks`
350
+
351
+
<!-- END_TASK_2 -->
352
+
353
+
<!-- END_SUBCOMPONENT_A -->
354
+
355
+
<!-- START_SUBCOMPONENT_B (tasks 3-4) -->
356
+
357
+
**Subcomponent B: Pipeline wiring + integration tests.**
358
+
359
+
<!-- START_TASK_3 -->
360
+
### Task 3: Wire metadata stage into `run_pipeline`
361
+
362
+
**Verifies:** indirectly; Task 4's integration tests verify the plumbing.
363
+
364
+
**Files:**
365
+
- Modify: `src/commands/test/oauth/client/pipeline.rs`:
366
+
- Add `pub const STAGE_METADATA: Stage = Stage("Metadata");` to `common::report::Stage` (move to common alongside `DISCOVERY`). Actually: declare `Stage::METADATA = Stage("Metadata");` in `src/common/report.rs` alongside the existing consts.
367
+
- In `run_pipeline`, after discovery:
368
+
```rust
369
+
let metadata_output = if let Some(discovery_facts) = discovery_output.facts.as_ref() {
370
+
metadata::run(discovery_facts, "<metadata.json>").await
371
+
} else {
372
+
// discovery failed at metadata-fetch step; emit Skipped rows for every metadata check
373
+
metadata::emit_all_blocked_by(Check::metadata_document_fetchable.id()).await
374
+
};
375
+
for r in metadata_output.results { report.record(r); }
376
+
let metadata_facts = metadata_output.facts;
377
+
```
378
+
- Add
379
+
```rust
380
+
pub fn emit_all_blocked_by(blocker_check_id: &'static str) -> MetadataStageOutput {
381
+
let results = Check::ALL
382
+
.iter()
383
+
.copied()
384
+
.map(|c| common::report::blocked_by(
385
+
c.id(),
386
+
Stage::METADATA,
387
+
c.summary(),
388
+
blocker_check_id,
389
+
))
390
+
.collect();
391
+
MetadataStageOutput { facts: None, results }
392
+
}
393
+
```
394
+
Requires a `impl Check { pub const ALL: &'static [Check] = &[/* all variants */]; }`
395
+
array in the same file so the order is stable and reviewable. The
396
+
`.summary()` method (defined in Task 2) is the single source of
397
+
truth for summary strings, so the rows emitted here are
398
+
indistinguishable from the rows emitted when the stage actually
399
+
runs but every check happens to emit `Skipped`.
400
+
- Modify: `src/commands/test/oauth/client.rs` — no changes beyond already-existing wiring; Phase 3's `ClientCmd::run` already feeds into `run_pipeline`.
401
+
402
+
**Testing:** covered in Task 4.
403
+
404
+
**Commit:** `feat(oauth-client): wire metadata stage into pipeline with dependency gating`
405
+
406
+
<!-- END_TASK_3 -->
407
+
408
+
<!-- START_TASK_4 -->
409
+
### Task 4: Metadata stage integration tests + fixtures
410
+
411
+
**Verifies:** AC2.1 through AC2.9.
412
+
413
+
**Files:**
414
+
- Create: `tests/oauth_client_metadata.rs`.
415
+
- Create fixtures under `tests/fixtures/oauth_client/metadata/`:
416
+
- `confidential_happy/metadata.json` — minimal valid confidential web
417
+
client: `application_type=web`, `response_types=["code"]`,
418
+
`grant_types=["authorization_code"]`, `dpop_bound_access_tokens=true`,
419
+
`redirect_uris=["https://client.example.com/cb"]`,
420
+
`token_endpoint_auth_method="private_key_jwt"`,
421
+
`scope="atproto"`, `jwks={"keys":[{"kty":"EC","crv":"P-256","alg":"ES256","use":"sig","kid":"k1","x":"...","y":"..."}]}`.
422
+
Use real base64url-encoded ES256 coords — copy from a test key material
423
+
file if one exists, or generate with a one-shot script checked in as
424
+
`tests/fixtures/oauth_client/_shared/es256_k1_jwk.json`.
425
+
- `public_happy/metadata.json` — `application_type=web`,
426
+
`token_endpoint_auth_method="none"`, no `jwks`/`jwks_uri`, otherwise
427
+
minimal.
428
+
- `native_happy/metadata.json` — `application_type=native`,
429
+
`client_id=https://app.example.com/oauth-client-metadata.json`,
430
+
`redirect_uris=["com.example.app:/cb"]`,
431
+
`token_endpoint_auth_method="none"`.
432
+
- `confidential_missing_jwks/metadata.json` — confidential without
433
+
jwks/jwks_uri.
434
+
- `public_with_token_endpoint_auth/metadata.json` — public web with
435
+
`token_endpoint_auth_method="private_key_jwt"`.
436
+
- `dpop_bound_false/metadata.json` — confidential with
437
+
`dpop_bound_access_tokens=false`.
438
+
- `native_redirect_scheme_mismatch/metadata.json` — native with
439
+
`redirect_uris=["app.wrong.example:/cb"]` against
440
+
`client_id=https://app.example.com/...`.
441
+
- `scope_grammar_invalid/metadata.json` — `scope="atproto invalid-scope-token"`
442
+
(no `:` after a non-atproto token → parse failure).
443
+
444
+
**Testing:**
445
+
For each fixture, a `#[tokio::test]` that:
446
+
1. Seeds `FakeHttpClient` to return the fixture bytes on a URL like
447
+
`https://client.example.com/metadata.json`.
448
+
2. Runs `run_pipeline` with the fake.
449
+
3. Asserts exit code per AC:
450
+
- Happy paths: `exit_code == 0`.
451
+
- SpecViolation fixtures: `exit_code == 1`.
452
+
4. Renders the report and `insta::assert_snapshot!(rendered)`.
453
+
454
+
Additional test `loopback_skips_all_metadata_checks`:
455
+
- Target `http://localhost/`; fake not touched.
456
+
- Assert every metadata check row is `Skipped` with reason `"metadata is implicit for loopback clients"` — AC2.9.
457
+
- Snapshot pins rows.
458
+
459
+
Additional test `discovery_failure_blocks_metadata`:
460
+
- HTTPS target with a seeded 404.
461
+
- Assert metadata rows are `Skipped` with reason `"blocked by oauth_client::discovery::metadata_document_fetchable"`.
462
+
463
+
**Verification:**
464
+
- `cargo test --test oauth_client_metadata` — all pass after snapshot review.
465
+
- `cargo insta review` accepts the new `.snap` files.
466
+
- `grep -n "oauth_client::metadata::" tests/snapshots/oauth_client_metadata*.snap`
467
+
shows every check ID at least once (AC8.7/AC8.8 support).
468
+
- Manual: pointing at a real public atproto client's `client_id`
469
+
(e.g. `https://bsky.app/static/oauth/client-metadata.json` — verify URL
470
+
at time of implementation) via
471
+
`cargo run -- test oauth client <url>` produces a sensible report.
472
+
473
+
**Commit:** `test(oauth-client): add metadata stage integration tests with AC2 coverage`
474
+
475
+
<!-- END_TASK_4 -->
476
+
477
+
<!-- END_SUBCOMPONENT_B -->
478
+
479
+
---
480
+
481
+
## Phase 4 done when
482
+
483
+
- `cargo test --test oauth_client_metadata` passes.
484
+
- `cargo insta review` has accepted all new snapshots.
485
+
- `cargo build`, `cargo test` (full), `cargo clippy -- -D warnings`, and
486
+
`cargo fmt --check` all clean.
487
+
- Static-mode end-to-end run against a real HTTPS atproto OAuth client's
488
+
metadata URL produces a complete, correctly-graded rendered report
489
+
(Discovery Pass + Metadata Pass/Violation rows, exit code per outcome).
490
+
491
+
Phase 5 consumes `MetadataFacts.jwks_source` and
492
+
`MetadataFacts.requires_jwks` to drive the JWKS stage.
+502
docs/implementation-plans/2026-04-16-test-oauth-client/phase_05.md
+502
docs/implementation-plans/2026-04-16-test-oauth-client/phase_05.md
···
1
+
# OAuth client test — Phase 5: Static stage 3 — JWKS validation
2
+
3
+
**Goal:** Fetch and validate the client's JWKS for confidential clients
4
+
(and emit `Skipped` rows for client kinds where JWKS is not required).
5
+
6
+
**Architecture:** A new `jwks.rs` stage. Consumes `MetadataFacts`:
7
+
- `requires_jwks == true` (confidential web): validate inline `jwks` or
8
+
fetch `jwks_uri` via a new `JwksFetcher` trait. Run spec checks
9
+
(non-empty keys, unique `kid`s, required `alg`, `use == sig`, modern EC
10
+
algorithms).
11
+
- `requires_jwks == false` (public, native, loopback): every JWKS check
12
+
emits `Skipped` with a kind-specific reason.
13
+
Introduce a new injection seam: `JwksFetcher` trait in `jwks.rs`,
14
+
`RealJwksFetcher` in the same file wrapping the shared `reqwest::Client`,
15
+
and `FakeJwksFetcher` in `tests/common/mod.rs`.
16
+
17
+
**Tech Stack:** `reqwest`, existing `common::oauth::jws` parser (Phase 2),
18
+
`miette` source spans for duplicate-`kid` highlighting.
19
+
20
+
**Scope:** Phase 5 of 8.
21
+
22
+
**Codebase verified:** 2026-04-16.
23
+
24
+
---
25
+
26
+
## Acceptance Criteria Coverage
27
+
28
+
This phase implements and tests:
29
+
30
+
### test-oauth-client.AC3: Static mode — JWKS validation
31
+
32
+
- **test-oauth-client.AC3.1 Success:** Inline `jwks` containing valid ES256 keys with unique `kid`, `alg`, and `use=sig` passes every JWKS check.
33
+
- **test-oauth-client.AC3.2 Success:** External `jwks_uri` returning a valid JWKS document is fetched (over the `JwksFetcher` seam) and validated identically.
34
+
- **test-oauth-client.AC3.3 Failure:** A `jwks_uri` returning a non-2xx status produces a `NetworkError` with the URL in the diagnostic.
35
+
- **test-oauth-client.AC3.4 Failure:** A JWKS containing two keys with the same `kid` produces a `SpecViolation` with both `kid`s and their offsets highlighted via miette source span.
36
+
- **test-oauth-client.AC3.5 Failure:** A key without an `alg` field produces a `SpecViolation`.
37
+
- **test-oauth-client.AC3.6 Failure:** A key with `use` other than `sig` (or absent) produces a `SpecViolation`.
38
+
- **test-oauth-client.AC3.7 Failure:** A key advertising a non-modern algorithm (RS1, MD5, etc.) produces a `SpecViolation`; ECDSA P-256 / ES256K and stronger algorithms pass.
39
+
- **test-oauth-client.AC3.8 Skip:** Public-client and native-client JWKS checks emit `Skipped` rows with reason `"jwks not required for {kind} clients"` rather than running.
40
+
41
+
---
42
+
43
+
## Codebase verification findings
44
+
45
+
- The upfront investigation confirmed no `JwksFetcher` or similar trait
46
+
exists today. Phase 5 introduces it from scratch.
47
+
- `tests/common/mod.rs` currently holds `FakeRawHttpTee` and
48
+
(after Phase 3 Task 4) `FakeHttpClient`, `FakeDnsResolver`. Phase 5
49
+
extends it with `FakeJwksFetcher`.
50
+
- `common::oauth::jws::parse_jwks` (Phase 2 Task 4) accepts raw bytes,
51
+
returns `Vec<ParsedJwk>` with per-key validation. **It does not check
52
+
`kid` uniqueness** — Phase 5 owns that check because it's a
53
+
document-level concern, not a per-key concern.
54
+
- `common::oauth::jws::JwsError` already has `NotJson`, `JwkMissingField`,
55
+
`UnsupportedAlg`, `JwkKtyMismatch` variants. The JWKS stage wraps
56
+
these into stage-scoped `CheckResult`s; the diagnostic codes stay
57
+
stable (`oauth_client::jws::*`), and the stage-level check IDs are new
58
+
(`oauth_client::jws::*`).
59
+
60
+
**Note on naming:** The design's check prefix for this stage is
61
+
`oauth_client::jws::*`, and diagnostic codes from `common::oauth::jws`
62
+
also use `oauth_client::jws::*`. They occupy **disjoint slug
63
+
subsets** listed below, so no collision is possible.
64
+
65
+
**Check ID slug set** (appears as `[STATUS]` rows in rendered output):
66
+
67
+
| Check ID | Emitted by |
68
+
|-----------------------------------------------|------------------|
69
+
| `oauth_client::jws::jwks_present` | Phase 5 `run()` |
70
+
| `oauth_client::jws::jwks_uri_fetchable` | Phase 5 `run()` |
71
+
| `oauth_client::jws::jwks_is_json` | Phase 5 `run()` |
72
+
| `oauth_client::jws::keys_have_unique_kids` | Phase 5 `run()` |
73
+
| `oauth_client::jws::keys_have_alg` | Phase 5 `run()` |
74
+
| `oauth_client::jws::keys_use_signing_use` | Phase 5 `run()` |
75
+
| `oauth_client::jws::algs_are_modern_ec` | Phase 5 `run()` |
76
+
77
+
**Diagnostic code slug set** (appears inside miette diagnostic blocks
78
+
attached to SpecViolation/NetworkError CheckResults):
79
+
80
+
| Diagnostic code | Emitted by |
81
+
|-----------------------------------------------|------------------|
82
+
| `oauth_client::jws::not_json` | Phase 2 `JwsError` |
83
+
| `oauth_client::jws::jwk_missing_field` | Phase 2 `JwsError` |
84
+
| `oauth_client::jws::unsupported_alg` | Phase 2 `JwsError` (no longer emitted after C2/C3 fix; preserved for callers that may want to raise) |
85
+
| `oauth_client::jws::jwk_kty_mismatch` | Phase 2 `JwsError` |
86
+
| `oauth_client::jws::verify_failed` | Phase 2 `JwsError` |
87
+
| `oauth_client::jws::sign_failed` | Phase 2 `JwsError` |
88
+
| `oauth_client::jws::unsupported_operation` | Phase 2 `JwsError` |
89
+
| `oauth_client::jws::jwks_uri_unreachable` | Phase 5 `JwksFetchError` |
90
+
91
+
The two sets share the `oauth_client::jws::` prefix but have no overlap
92
+
in slug names, so a snapshot consumer can always distinguish "this is a
93
+
check row" from "this is a diagnostic code" by slug membership.
94
+
95
+
Note: the stage prefix is `oauth_client::jws::*` (matching the design
96
+
plan's "jws" stage name) even though the source file is `jwks.rs`. The
97
+
design uses "jws" because it covers both per-key (JWK) and collection
98
+
(JWKS) concerns.
99
+
100
+
---
101
+
102
+
## Implementation tasks
103
+
104
+
<!-- START_SUBCOMPONENT_A (tasks 1-3) -->
105
+
106
+
**Subcomponent A: JwksFetcher trait + stage logic.**
107
+
108
+
<!-- START_TASK_1 -->
109
+
### Task 1: `JwksFetcher` trait + `RealJwksFetcher`
110
+
111
+
**Verifies:** none directly (seam infrastructure; AC3.2 and AC3.3 verify
112
+
this via Task 4).
113
+
114
+
**Files:**
115
+
- Create: `src/commands/test/oauth/client/jwks.rs`:
116
+
```rust
117
+
use async_trait::async_trait;
118
+
use reqwest::Client as ReqwestClient;
119
+
use url::Url;
120
+
121
+
/// Fetches a JWKS document from a `jwks_uri`. Kept separate from the
122
+
/// generic `HttpClient` seam because JWKS fetching is the one place
123
+
/// the tool performs a content-type-aware fetch against an arbitrary
124
+
/// third-party URL, and tests want to mock it separately from the
125
+
/// client metadata document fetch.
126
+
#[async_trait]
127
+
pub trait JwksFetcher: Send + Sync {
128
+
async fn fetch(&self, url: &Url) -> Result<JwksFetchResponse, JwksFetchError>;
129
+
}
130
+
131
+
pub struct JwksFetchResponse {
132
+
pub status: u16,
133
+
pub body: Vec<u8>,
134
+
pub content_type: Option<String>,
135
+
}
136
+
137
+
#[derive(Debug, thiserror::Error, miette::Diagnostic)]
138
+
pub enum JwksFetchError {
139
+
#[error("network error fetching JWKS at `{url}`: {source}")]
140
+
#[diagnostic(code("oauth_client::jws::jwks_uri_unreachable"))]
141
+
Transport { url: Url, #[source] source: reqwest::Error },
142
+
}
143
+
144
+
pub struct RealJwksFetcher { client: ReqwestClient }
145
+
146
+
impl RealJwksFetcher {
147
+
pub fn new(client: ReqwestClient) -> Self { Self { client } }
148
+
}
149
+
150
+
#[async_trait]
151
+
impl JwksFetcher for RealJwksFetcher {
152
+
async fn fetch(&self, url: &Url) -> Result<JwksFetchResponse, JwksFetchError> {
153
+
let resp = self.client.get(url.clone()).send().await
154
+
.map_err(|e| JwksFetchError::Transport { url: url.clone(), source: e })?;
155
+
let status = resp.status().as_u16();
156
+
let content_type = resp.headers().get(reqwest::header::CONTENT_TYPE)
157
+
.and_then(|v| v.to_str().ok().map(str::to_owned));
158
+
let body = resp.bytes().await
159
+
.map_err(|e| JwksFetchError::Transport { url: url.clone(), source: e })?
160
+
.to_vec();
161
+
Ok(JwksFetchResponse { status, body, content_type })
162
+
}
163
+
}
164
+
```
165
+
166
+
**Implementation notes:**
167
+
- `reqwest::Error` already propagates through the builder — we don't need
168
+
to construct our own network-error variant; `Transport` suffices.
169
+
- Content type captured for future diagnostics; Phase 5 tests may ignore
170
+
it.
171
+
172
+
**Testing:** covered by Task 4 integration tests.
173
+
174
+
**Commit:** `feat(oauth-client): add JwksFetcher trait and RealJwksFetcher`
175
+
176
+
<!-- END_TASK_1 -->
177
+
178
+
<!-- START_TASK_2 -->
179
+
### Task 2: `JwksFacts`, `Check`, `run` — JWKS stage logic
180
+
181
+
**Verifies:** AC3.1, AC3.2, AC3.3, AC3.4, AC3.5, AC3.6, AC3.7, AC3.8.
182
+
183
+
**Files:**
184
+
- Modify: `src/commands/test/oauth/client/jwks.rs` — add Facts, Check, run.
185
+
186
+
**Implementation outline:**
187
+
188
+
```rust
189
+
pub struct JwksFacts {
190
+
pub keys: Vec<ParsedJwk>,
191
+
pub source: JwksSource,
192
+
}
193
+
194
+
// JwksSource already declared in metadata.rs — move to jwks.rs or keep shared?
195
+
// Decision: keep the type in metadata.rs (where it's first needed) and re-export:
196
+
// metadata.rs: pub enum JwksSource { Inline(serde_json::Value), Uri(Url) }
197
+
// jwks.rs: use super::metadata::JwksSource; re-expose as pub.
198
+
199
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
200
+
pub enum Check {
201
+
JwksPresent, // oauth_client::jws::jwks_present
202
+
JwksUriFetchable, // only when JwksSource::Uri
203
+
JwksIsJson, // parse result
204
+
KeysHaveUniqueKids, // document-level
205
+
KeysHaveAlg, // per-key; first missing alg surfaces
206
+
KeysUseSigningUse, // per-key
207
+
AlgsAreModernEc, // per-key
208
+
}
209
+
```
210
+
211
+
Each variant has `.id() -> &'static str` with prefix
212
+
`oauth_client::jws::<slug>`:
213
+
- `jwks_present`, `jwks_uri_fetchable`, `jwks_is_json`,
214
+
`keys_have_unique_kids`, `keys_have_alg`, `keys_use_signing_use`,
215
+
`algs_are_modern_ec`.
216
+
217
+
`pub async fn run(facts: &MetadataFacts, fetcher: &dyn JwksFetcher, raw_source_name: &str) -> JwksStageOutput`:
218
+
- If `facts.kind` is `Loopback`:
219
+
- Emit every check as `skipped_with_reason(check.id(), Stage::JWKS,
220
+
<summary>, "jwks not applicable to loopback clients")`.
221
+
- Return `{facts: None, results}`.
222
+
- If `facts.kind` is `WebPublic` or `Native`:
223
+
- Emit every check as `skipped_with_reason(..., "jwks not required
224
+
for {public|native} clients")`.
225
+
- Return `{facts: None, results}`.
226
+
- If `facts.kind` is `WebConfidential`:
227
+
- Inspect `facts.jwks_source`:
228
+
- `Some(JwksSource::Inline(v))`:
229
+
- Emit `JwksPresent.pass()`.
230
+
- Skip `JwksUriFetchable` with reason `"jwks is inline"`.
231
+
- Serialize `v` back to bytes via `serde_json::to_vec(&v)` — this
232
+
gives consistent byte offsets for source-span diagnostics.
233
+
- Fall through to parse step.
234
+
- `Some(JwksSource::Uri(u))`:
235
+
- Emit `JwksPresent.pass()`.
236
+
- Call `fetcher.fetch(u).await`. On `Err`: emit
237
+
`JwksUriFetchable.network_error(<diagnostic>)` and skip the
238
+
rest blocked on `oauth_client::jws::jwks_uri_fetchable`. Return.
239
+
- On `Ok(resp)` with non-2xx: emit `JwksUriFetchable.network_error(...)`
240
+
(diagnostic includes URL and status). Skip remaining.
241
+
- On 2xx: emit `JwksUriFetchable.pass()` and use `resp.body` as the
242
+
bytes for parsing.
243
+
- `None`: this means metadata stage flagged a confidential-without-jwks
244
+
violation. Emit every JWKS check skipped blocked by
245
+
`oauth_client::metadata::confidential_requires_jwks`. Return.
246
+
- **First attempt to parse the top-level JSON and extract the `keys`
247
+
array** via a thin wrapper (not `parse_jwks` directly):
248
+
```rust
249
+
let value: serde_json::Value = match serde_json::from_slice(&bytes) {
250
+
Ok(v) => v,
251
+
Err(_e) => {
252
+
// emit JwksIsJson.spec_violation with a JSON-parse
253
+
// diagnostic (reuse common::oauth::jws::JwsError::NotJson
254
+
// style). Skip rest. Return.
255
+
}
256
+
};
257
+
let key_values = match value.get("keys").and_then(|v| v.as_array()) {
258
+
Some(arr) => arr,
259
+
None => {
260
+
// emit JwksIsJson.spec_violation — "JWKS document missing
261
+
// required `keys` array". Skip rest. Return.
262
+
}
263
+
};
264
+
```
265
+
On success, emit `JwksIsJson.pass()`.
266
+
267
+
- **Then parse each key via `parse_jwk` and map each outcome to the
268
+
right Check.** This per-key loop is the authoritative mapping
269
+
table (no splitting across Phase 2 and Phase 5 — Phase 2's
270
+
`parse_jwks` is permissive; Phase 5's stage is where policy lives):
271
+
272
+
```
273
+
For each key_value at index i:
274
+
match common::oauth::jws::parse_jwk(key_value, raw_source_name, source_bytes.clone()) {
275
+
Err(JwsError::JwkMissingField { field: "kty", .. })
276
+
| Err(JwsError::JwkMissingField { field: "crv", .. })
277
+
| Err(JwsError::JwkMissingField { field: "x", .. })
278
+
| Err(JwsError::JwkMissingField { field: "y", .. })
279
+
| Err(JwsError::JwkKtyMismatch { .. })
280
+
=> structural_failures.push((i, err)),
281
+
Err(other) => stash for JwksIsJson violation,
282
+
Ok(parsed) => parsed_keys.push(parsed),
283
+
}
284
+
```
285
+
Aggregate the outcomes:
286
+
- If any structural failure: emit `AlgsAreModernEc.spec_violation`
287
+
(for JwkKtyMismatch only) OR `JwksIsJson.spec_violation` (for
288
+
missing `kty`/`crv`/`x`/`y`). Wait — a cleaner mapping:
289
+
290
+
**Authoritative JwsError → Check mapping table:**
291
+
292
+
| JwsError variant | Check emitted | Reason |
293
+
|--------------------------------------------|-----------------------|-----------------------------------------------------------------------|
294
+
| `NotJson { .. }` | `JwksIsJson` | Document isn't JSON at all. |
295
+
| "missing `keys` array" | `JwksIsJson` | Structural shape wrong. |
296
+
| `JwkMissingField { field: "kty" }` | `JwksIsJson` | Per-key structural shape wrong. |
297
+
| `JwkMissingField { field: "crv"/"x"/"y" }` | `JwksIsJson` | Per-key material incomplete — not valid JWK. |
298
+
| `JwkKtyMismatch { alg, .. }` | `AlgsAreModernEc` | Alg cannot be used with the declared curve — algorithmic mismatch. |
299
+
| `ParsedJwk { alg: None, alg_raw: None }` | `KeysHaveAlg` | JWK has no `alg` field. |
300
+
| `ParsedJwk { alg: None, alg_raw: Some }` | `AlgsAreModernEc` | Non-modern-EC algorithm present (RS1, RS256, MD5, etc.). |
301
+
| `ParsedJwk { use: JwkUse::Other(_), .. }` | `KeysUseSigningUse` | `use` is not `"sig"`. |
302
+
| (multiple ParsedJwks share a `kid`) | `KeysHaveUniqueKids` | Document-level uniqueness check. |
303
+
304
+
Emit `.pass()` for each of `KeysHaveAlg`, `KeysHaveUniqueKids`,
305
+
`KeysUseSigningUse`, `AlgsAreModernEc` only when every parsed key
306
+
passes *that specific* check.
307
+
308
+
- **Unique-kid check**: collect `(index, parsed.kid.as_deref())`;
309
+
find duplicates. When present, construct the diagnostic with a
310
+
`Vec<LabeledSpan>` highlighting every occurrence of the duplicated
311
+
`"kid"` value in the raw bytes. miette 7.6 supports a collection
312
+
label via
313
+
`#[label(collection, "duplicate kid")] pub spans: Vec<LabeledSpan>`.
314
+
Add a helper
315
+
`fn all_spans_for_quoted_literal(bytes: &[u8], literal: &str) -> Vec<SourceSpan>`
316
+
in `src/common/diagnostics.rs` — it extends the existing
317
+
`span_for_quoted_literal` (which returns only the first hit) by
318
+
iterating through all occurrences.
319
+
320
+
- Return `JwksStageOutput { facts: Some(JwksFacts { keys: parsed_keys, source }), results }`
321
+
(facts carry only the successfully-parsed keys; downstream
322
+
consumers check `jwks_facts.keys.len()` before using them).
323
+
324
+
**Implementation notes:**
325
+
- The design has `keys_have_unique_kids` highlighting *both* offsets via
326
+
miette source span. miette's `Diagnostic` derive supports multiple
327
+
`#[label]` fields or a single `#[label(collection)]` field with
328
+
`Vec<LabeledSpan>`. Use the collection form. If `span_for_quoted_literal`
329
+
currently returns only the first match, extend it (see
330
+
`all_spans_for_quoted_literal` addition above).
331
+
- `JwksIsJson.pass()` is emitted only when parsing succeeded structurally
332
+
(a JSON array under `keys`). Per-key structural problems are the
333
+
domain of `KeysHaveAlg` / `AlgsAreModernEc`.
334
+
335
+
**Testing:** covered by Task 4.
336
+
337
+
**Commit:** `feat(oauth-client): add JWKS validation stage`
338
+
339
+
<!-- END_TASK_2 -->
340
+
341
+
<!-- START_TASK_3 -->
342
+
### Task 3: Wire JWKS stage into pipeline and `ClientCmd::run`
343
+
344
+
**Verifies:** indirectly; Task 4 verifies end-to-end.
345
+
346
+
**Files:**
347
+
- Modify: `src/common/report.rs` — add `pub const JWKS: Stage = Stage("JWKS");`.
348
+
- Modify: `src/commands/test/oauth/client/pipeline.rs`:
349
+
- Add field `pub jwks: &'a dyn JwksFetcher,` to `OauthClientOptions`.
350
+
- After metadata stage:
351
+
```rust
352
+
let jwks_output = if let Some(metadata_facts) = metadata_facts.as_ref() {
353
+
jwks::run(metadata_facts, opts.jwks, "<jwks>").await
354
+
} else {
355
+
jwks::emit_all_blocked_by("oauth_client::metadata::raw_document_deserializes").await
356
+
};
357
+
for r in jwks_output.results { report.record(r); }
358
+
let _jwks_facts = jwks_output.facts; // used by Phase 7 interactive stage
359
+
```
360
+
- Modify: `src/commands/test/oauth/client.rs` — in `ClientCmd::run`:
361
+
- Construct `let jwks_fetcher = jwks::RealJwksFetcher::new(reqwest_client.clone());`
362
+
after constructing `reqwest_client`.
363
+
- Pass `&jwks_fetcher` through `OauthClientOptions`.
364
+
365
+
**Testing:** covered by Task 4.
366
+
367
+
**Commit:** `feat(oauth-client): wire JWKS stage into pipeline`
368
+
369
+
<!-- END_TASK_3 -->
370
+
371
+
<!-- END_SUBCOMPONENT_A -->
372
+
373
+
<!-- START_SUBCOMPONENT_B (tasks 4-5) -->
374
+
375
+
**Subcomponent B: Test fake + integration tests.**
376
+
377
+
<!-- START_TASK_4 -->
378
+
### Task 4: `FakeJwksFetcher` in `tests/common/mod.rs`
379
+
380
+
**Verifies:** none directly (test infra).
381
+
382
+
**Files:**
383
+
- Modify: `tests/common/mod.rs`:
384
+
- Add `pub struct FakeJwksFetcher { responses: Arc<Mutex<HashMap<Url, Result<(u16, Vec<u8>, Option<String>), String>>>> }`.
385
+
- Methods:
386
+
- `pub fn new() -> Self`.
387
+
- `pub fn add_response(&self, url: Url, status: u16, body: Vec<u8>, content_type: Option<String>)`.
388
+
- `pub fn add_transport_error(&self, url: Url, message: impl Into<String>)`.
389
+
- `impl JwksFetcher for FakeJwksFetcher`:
390
+
- Match on URL; if entry is `Ok(...)`, return a `JwksFetchResponse`.
391
+
If `Err(msg)`, return `JwksFetchError::Transport { url, source: <synthesized reqwest error> }`.
392
+
If URL unseeded, panic (test-author mistake).
393
+
- Synthesizing a `reqwest::Error` in a fake is awkward. Alternative:
394
+
change `JwksFetchError` to hold a boxed message instead of a
395
+
`reqwest::Error`. Cleaner option: add a free constructor
396
+
`JwksFetchError::synthetic(url: Url, message: String)` that stores
397
+
a `String` rather than a `reqwest::Error`, and have the fake use it.
398
+
Update `RealJwksFetcher` to wrap `reqwest::Error` the same way
399
+
(stringified via `.to_string()` — acceptable loss of detail for
400
+
fakeability).
401
+
402
+
**Implementation notes:**
403
+
- The JwksFetchError restructure above affects Task 1's code. Redo Task 1
404
+
with this simpler error shape, or revise the real fetcher to use
405
+
stringified messages. This plan's authoritative signature:
406
+
```rust
407
+
#[derive(Debug, thiserror::Error, miette::Diagnostic)]
408
+
pub enum JwksFetchError {
409
+
#[error("network error fetching JWKS at `{url}`: {message}")]
410
+
#[diagnostic(code("oauth_client::jws::jwks_uri_unreachable"))]
411
+
Transport { url: Url, message: String },
412
+
}
413
+
```
414
+
Both `RealJwksFetcher` and `FakeJwksFetcher` construct it with
415
+
stringified messages.
416
+
417
+
**Verification:**
418
+
- `cargo build --tests` succeeds.
419
+
420
+
**Commit:** (batched into Task 5's commit; or small enough to stand alone
421
+
if Task 5's changes are large).
422
+
423
+
<!-- END_TASK_4 -->
424
+
425
+
<!-- START_TASK_5 -->
426
+
### Task 5: JWKS stage integration tests + fixtures
427
+
428
+
**Verifies:** AC3.1 through AC3.8.
429
+
430
+
**Files:**
431
+
- Create: `tests/oauth_client_jwks.rs`.
432
+
- Create fixtures under `tests/fixtures/oauth_client/jwks/`:
433
+
- `inline_es256_happy/metadata.json` — confidential client with inline
434
+
`jwks` containing a single ES256 key. Reuse
435
+
`tests/fixtures/oauth_client/_shared/es256_k1_jwk.json` key
436
+
coordinates.
437
+
- `uri_es256_happy/metadata.json` — confidential client with
438
+
`jwks_uri=https://client.example.com/jwks.json`. Store the JWKS bytes
439
+
separately at `uri_es256_happy/jwks.json`.
440
+
- `uri_unreachable/metadata.json` — same shape as above; test seeds a
441
+
transport error on the jwks_uri.
442
+
- `duplicate_kids/jwks.json` — two keys sharing `kid=k1`. Either
443
+
inline or URI; use inline for simplicity.
444
+
- `missing_alg/jwks.json` — a JWK with no `alg`. (Test failure
445
+
on `oauth_client::jws::keys_have_alg` via `parse_jwks` returning
446
+
`JwkMissingField { field: "alg" }`.)
447
+
- `wrong_use/jwks.json` — JWK with `use=enc`.
448
+
- `weak_alg_rs1/jwks.json` — JWK with `alg=RS1`.
449
+
- `public_client_skipped/metadata.json` — public client; the test
450
+
confirms all JWKS checks skipped.
451
+
452
+
**Testing:**
453
+
For each fixture, a `#[tokio::test]` that:
454
+
1. Seeds `FakeHttpClient` with the metadata document.
455
+
2. Seeds `FakeJwksFetcher` with the jwks bytes (when jwks_uri is used).
456
+
3. Runs `run_pipeline`.
457
+
4. Asserts the exit code per AC.
458
+
5. Renders and snapshot-pins.
459
+
460
+
Snapshot test names align with fixture directory names
461
+
(`oauth_client_jwks__inline_es256_happy`, etc.).
462
+
463
+
Additional tests:
464
+
- `discovery_failure_blocks_jwks` — discovery 404 → JWKS rows all Skipped
465
+
blocked by `oauth_client::metadata::raw_document_deserializes` (chain
466
+
of blockers).
467
+
- `loopback_skips_all_jwks` — `http://localhost/` target, every JWKS row
468
+
Skipped with reason `"jwks not applicable to loopback clients"`.
469
+
470
+
**Manual verification:**
471
+
- Run static mode end-to-end against a real atproto public reference client
472
+
(e.g. `https://bsky.app/static/oauth/client-metadata.json` — verify the
473
+
URL at implementation time). Every stage produces rows; exit code 0
474
+
for a spec-conformant client, non-zero if the reference client has
475
+
drifted.
476
+
477
+
**Verification:**
478
+
- `cargo test --test oauth_client_jwks` passes.
479
+
- `cargo insta review` accepts snapshots.
480
+
- `grep -n "oauth_client::jws::" tests/snapshots/oauth_client_jwks*.snap`
481
+
confirms all check IDs and diagnostic codes present.
482
+
483
+
**Commit:** `test(oauth-client): add JWKS stage integration tests with AC3 coverage`
484
+
485
+
<!-- END_TASK_5 -->
486
+
487
+
<!-- END_SUBCOMPONENT_B -->
488
+
489
+
---
490
+
491
+
## Phase 5 done when
492
+
493
+
- `cargo test --test oauth_client_jwks` passes.
494
+
- Snapshots accepted.
495
+
- Static mode end-to-end run against a real atproto OAuth client's
496
+
published `client_id` metadata URL produces a sensible rendered report
497
+
covering discovery, metadata, and JWKS stages.
498
+
- All of `cargo build`, `cargo test` (full), `cargo clippy -- -D warnings`,
499
+
`cargo fmt --check` clean.
500
+
501
+
Phase 6 introduces the fake AS (axum) and synthetic identity for
502
+
interactive mode.
+668
docs/implementation-plans/2026-04-16-test-oauth-client/phase_06.md
+668
docs/implementation-plans/2026-04-16-test-oauth-client/phase_06.md
···
1
+
# OAuth client test — Phase 6: Fake authorization server + identity bootstrap
2
+
3
+
**Goal:** Stand up the in-process fake AS as a real, network-bound HTTP
4
+
server, including the synthetic identity chain and the
5
+
`--public-base-url` flag. No client conformance checks yet — just the
6
+
server with the routes returning spec-shaped responses, and the request
7
+
log it records.
8
+
9
+
**Architecture:** Add `axum 0.8` and bring in the existing `tokio`
10
+
features (already sufficient). Create
11
+
`src/commands/test/oauth/client/fake_as.rs` as the module root alongside
12
+
`fake_as/` for per-route endpoint handlers and synthetic identity.
13
+
Introduce a `Clock` trait in `src/common/oauth.rs` so timestamps in
14
+
served documents and generated `request_uri` / `code` / token strings
15
+
stay deterministic under test. `ServerHandle` holds the bound
16
+
`SocketAddr`, the active base URL (local or public), a cloneable
17
+
`Arc<RequestLog>`, and a `JoinHandle` for the background axum task.
18
+
19
+
**Tech Stack:** `axum = "0.8"`, existing `tokio` (rt + macros + time +
20
+
net), `tower` (transitively; no direct dep yet), `hickory` unchanged,
21
+
`url`, `serde`, `serde_json`.
22
+
23
+
**Scope:** Phase 6 of 8. **Partial coverage of AC4** — the scaffolding
24
+
and routes exist, but the conformance checks that introspect the
25
+
`RequestLog` and emit `CheckResult`s land in Phase 7.
26
+
27
+
**Codebase verified:** 2026-04-16.
28
+
29
+
---
30
+
31
+
## Acceptance Criteria Coverage
32
+
33
+
This phase implements but does NOT yet test via the conformance pipeline
34
+
(that's Phase 7). The phase verifies these ACs operationally via
35
+
axum-level integration tests in `tests/oauth_client_interactive.rs`:
36
+
37
+
### test-oauth-client.AC4: Interactive mode — fake AS infrastructure
38
+
39
+
- **test-oauth-client.AC4.2 Success:** Without `--public-base-url`, the fake AS binds to `127.0.0.1:<port>` and every served metadata document references `http://127.0.0.1:<port>` as the active base.
40
+
- **test-oauth-client.AC4.3 Success:** With `--public-base-url <https-url>`, every URL appearing in served metadata documents (DID document `service` entries, AS metadata endpoint URLs, PRM `authorization_servers` array) is rewritten to use the public URL.
41
+
- **test-oauth-client.AC4.4 Success:** The well-known endpoints (`/<did-web-path>/did.json`, `/.well-known/oauth-protected-resource`, `/.well-known/oauth-authorization-server`) return spec-shaped JSON validatable by an external client / parser.
42
+
- **test-oauth-client.AC4.5 Success:** Every inbound request to PAR / authorize / token is appended to the `RequestLog` with timestamp, route, headers, and body bytes preserved verbatim.
43
+
- **test-oauth-client.AC4.6 Failure:** A `--port` value that is already bound, out of range, or otherwise unbindable produces a clear startup error and a non-zero exit before the synthetic identity is printed.
44
+
45
+
AC4.1 (ties the conformance pipeline to server startup) is **deferred
46
+
to Phase 7**, because Phase 7 is where the interactive stage lands.
47
+
48
+
---
49
+
50
+
## Codebase verification findings
51
+
52
+
- `Cargo.toml` currently has no `axum` or `tower`. Phase 6 adds axum only;
53
+
tower is transitive via axum but we'll add an explicit `tower` dep
54
+
only if we need `ServiceExt` (not required for Phase 6's router).
55
+
- `tokio` features are `["rt", "macros", "time", "net"]`. axum 0.8
56
+
requires tokio 1.44+ (already have 1.51). The `net` feature provides
57
+
`TcpListener` which axum consumes.
58
+
- No `Clock` trait exists anywhere yet. Phase 6 introduces
59
+
`src/common/oauth::Clock` (not in `jws.rs` because jws doesn't need
60
+
it; top-level in the `oauth` namespace).
61
+
- Design's `identity.rs` location: `src/commands/test/oauth/client/fake_as/identity.rs`.
62
+
Use sibling-file pattern: `fake_as.rs` is the module root,
63
+
`fake_as/` holds `endpoints.rs`, `identity.rs`, `request_log.rs`.
64
+
65
+
**axum 0.8 specifics (from research, 2026-04-16):**
66
+
- Path params: `"/{name}"` syntax (was `:name` in 0.7).
67
+
- `State<S>` extractor for shared app state; `Router::with_state(...)`.
68
+
- `Bytes` body extractor must be last in handler signature.
69
+
- Handlers require `Sync` in 0.8.
70
+
- `axum::serve(listener, router).with_graceful_shutdown(...)` for
71
+
teardown.
72
+
- `tokio::net::TcpListener::bind("127.0.0.1:0")` returns a listener whose
73
+
`local_addr()` reveals the assigned port.
74
+
75
+
**did:web resolution** (for synthetic identity):
76
+
- `did:web:example.com` → `https://example.com/.well-known/did.json`.
77
+
- `did:web:example.com:u:alice` → `https://example.com/u/alice/did.json`.
78
+
- Port embedded: `did:web:127.0.0.1%3A3000` → `https://127.0.0.1:3000/.well-known/did.json` (colon percent-encoded).
79
+
80
+
For the fake AS:
81
+
- Local mode (no `--public-base-url`): bound to `127.0.0.1:<port>`. The
82
+
synthetic identity's DID is `did:web:127.0.0.1%3A<port>`. Served at
83
+
`http://127.0.0.1:<port>/.well-known/did.json`. Note this is HTTP, not
84
+
HTTPS — technically spec-non-compliant for did:web (which mandates
85
+
HTTPS), but atproto conventionally allows HTTP for localhost. Document
86
+
this: the fake AS never claims did:web compliance off-localhost. The
87
+
client under test must opt into trusting HTTP by its own local-dev
88
+
config.
89
+
- Public mode (`--public-base-url https://funnel.example.com`): DID is
90
+
`did:web:funnel.example.com`; everything served under that base.
91
+
92
+
---
93
+
94
+
## Implementation tasks
95
+
96
+
<!-- START_SUBCOMPONENT_A (tasks 1-3) -->
97
+
98
+
**Subcomponent A: Dependencies, Clock, RequestLog.**
99
+
100
+
<!-- START_TASK_1 -->
101
+
### Task 1: Add `axum` dependency + `Clock` trait
102
+
103
+
**Verifies:** none directly (infrastructure).
104
+
105
+
**Files:**
106
+
- Modify: `Cargo.toml` — add to `[dependencies]`:
107
+
```toml
108
+
axum = { version = "0.8", default-features = false, features = ["http1", "json", "tokio"] }
109
+
```
110
+
`http1` (we only need HTTP/1.1 for the fake AS), `json` for
111
+
`axum::Json`, `tokio` for the integration. No `tower` dep yet — axum's
112
+
public API is enough for Phase 6; add `tower` if Phase 7 needs
113
+
middleware.
114
+
- Modify: `src/common/oauth.rs` — add:
115
+
```rust
116
+
pub mod jws;
117
+
pub mod clock;
118
+
```
119
+
- Create: `src/common/oauth/clock.rs`:
120
+
```rust
121
+
use std::time::{Duration, SystemTime, UNIX_EPOCH};
122
+
123
+
/// A clock source. `RealClock` uses `SystemTime::now()`; tests inject
124
+
/// a `FakeClock` with a fixed or manually advanced instant.
125
+
pub trait Clock: Send + Sync {
126
+
fn now_unix_seconds(&self) -> u64;
127
+
}
128
+
129
+
pub struct RealClock;
130
+
131
+
impl Clock for RealClock {
132
+
fn now_unix_seconds(&self) -> u64 {
133
+
SystemTime::now().duration_since(UNIX_EPOCH)
134
+
.unwrap_or(Duration::ZERO).as_secs()
135
+
}
136
+
}
137
+
```
138
+
139
+
**Implementation notes:**
140
+
- The trait returns Unix seconds because every JWT timestamp claim
141
+
(`iat`, `exp`) is Unix seconds. Higher-resolution timekeeping is not
142
+
needed for Phase 6 — Phase 7's DPoP nonce rotation, if it needs
143
+
millisecond precision, will add a `now_unix_millis` method.
144
+
- `FakeClock` implementation lives in `tests/common/mod.rs` (Task 5).
145
+
146
+
**Verification:**
147
+
- `cargo build` succeeds — confirms axum 0.8 + `default-features = false`
148
+
+ the selected features compile on MSRV 1.85.
149
+
- `cargo test --lib common::oauth::clock` passes a trivial smoke test
150
+
(add one in the file: construct `RealClock` and assert
151
+
`now_unix_seconds() > 1_700_000_000`).
152
+
153
+
**Commit:** `build: add axum 0.8 dependency; common: add Clock trait`
154
+
155
+
<!-- END_TASK_1 -->
156
+
157
+
<!-- START_TASK_2 -->
158
+
### Task 2: `RequestLog` + `FakeAsOptions` + `ServerHandle`
159
+
160
+
**Verifies:** AC4.5 (infrastructure — the log shape; actual appending in Task 3).
161
+
162
+
**Files:**
163
+
- Create: `src/commands/test/oauth/client/fake_as.rs` (module root):
164
+
```rust
165
+
//! In-process fake atproto OAuth authorization server for interactive mode.
166
+
167
+
pub mod endpoints;
168
+
pub mod identity;
169
+
pub mod request_log;
170
+
171
+
use std::net::SocketAddr;
172
+
use std::sync::Arc;
173
+
174
+
use tokio::net::TcpListener;
175
+
use tokio::task::JoinHandle;
176
+
use url::Url;
177
+
178
+
use crate::common::oauth::clock::Clock;
179
+
use request_log::RequestLog;
180
+
use identity::SyntheticIdentity;
181
+
182
+
pub struct FakeAsOptions {
183
+
pub bind_port: Option<u16>,
184
+
pub public_base_url: Option<Url>,
185
+
}
186
+
187
+
pub struct ServerHandle {
188
+
pub local_addr: SocketAddr,
189
+
pub active_base: Url, // http://127.0.0.1:<port> OR --public-base-url
190
+
pub identity: SyntheticIdentity,
191
+
pub requests: Arc<RequestLog>,
192
+
pub shutdown: tokio::sync::oneshot::Sender<()>,
193
+
pub join: JoinHandle<()>,
194
+
}
195
+
196
+
impl ServerHandle {
197
+
pub async fn bind(
198
+
opts: FakeAsOptions,
199
+
clock: Arc<dyn Clock>,
200
+
) -> Result<Self, BindError> {
201
+
let bind_port = opts.bind_port.unwrap_or(0);
202
+
let addr = format!("127.0.0.1:{bind_port}");
203
+
let listener = TcpListener::bind(&addr).await
204
+
.map_err(|e| BindError::BindFailed { addr: addr.clone(), source: e })?;
205
+
let local_addr = listener.local_addr().map_err(|e| BindError::LocalAddrFailed { source: e })?;
206
+
let active_base = opts.public_base_url
207
+
.clone()
208
+
.unwrap_or_else(|| {
209
+
let port = local_addr.port();
210
+
format!("http://127.0.0.1:{port}").parse().expect("valid url")
211
+
});
212
+
let identity = SyntheticIdentity::for_base(&active_base);
213
+
let requests = Arc::new(RequestLog::new());
214
+
let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel();
215
+
216
+
let state = Arc::new(endpoints::AppState {
217
+
clock,
218
+
active_base: active_base.clone(),
219
+
identity: identity.clone(),
220
+
requests: requests.clone(),
221
+
});
222
+
let router = endpoints::build_router(state);
223
+
224
+
let serve = axum::serve(listener, router)
225
+
.with_graceful_shutdown(async move { let _ = shutdown_rx.await; });
226
+
let join = tokio::spawn(async move {
227
+
let _ = serve.await;
228
+
});
229
+
230
+
Ok(ServerHandle {
231
+
local_addr, active_base, identity, requests,
232
+
shutdown: shutdown_tx, join,
233
+
})
234
+
}
235
+
236
+
pub async fn shutdown(self) {
237
+
let _ = self.shutdown.send(());
238
+
let _ = self.join.await;
239
+
}
240
+
}
241
+
242
+
#[derive(Debug, thiserror::Error, miette::Diagnostic)]
243
+
pub enum BindError {
244
+
#[error("could not bind fake AS to `{addr}`: {source}")]
245
+
#[diagnostic(code("oauth_client::interactive::bind_failed"))]
246
+
BindFailed { addr: String, #[source] source: std::io::Error },
247
+
#[error("could not determine local address for fake AS listener: {source}")]
248
+
#[diagnostic(code("oauth_client::interactive::local_addr_failed"))]
249
+
LocalAddrFailed { #[source] source: std::io::Error },
250
+
}
251
+
```
252
+
- Create: `src/commands/test/oauth/client/fake_as/request_log.rs`:
253
+
```rust
254
+
use std::sync::Mutex;
255
+
256
+
/// Append-only log of inbound requests. Cloneable `Arc` handle; locks
257
+
/// on push and snapshot.
258
+
pub struct RequestLog {
259
+
entries: Mutex<Vec<LoggedRequest>>,
260
+
}
261
+
262
+
#[derive(Debug, Clone)]
263
+
pub struct LoggedRequest {
264
+
pub timestamp_unix: u64,
265
+
pub method: String,
266
+
pub path: String,
267
+
pub query: Option<String>,
268
+
pub headers: Vec<(String, Vec<u8>)>,
269
+
pub body: Vec<u8>,
270
+
}
271
+
272
+
impl RequestLog {
273
+
pub fn new() -> Self { Self { entries: Mutex::new(Vec::new()) } }
274
+
275
+
pub fn push(&self, req: LoggedRequest) {
276
+
let mut guard = self.entries.lock().expect("request log poisoned");
277
+
guard.push(req);
278
+
}
279
+
280
+
pub fn snapshot(&self) -> Vec<LoggedRequest> {
281
+
self.entries.lock().expect("request log poisoned").clone()
282
+
}
283
+
}
284
+
```
285
+
286
+
**Implementation notes:**
287
+
- `std::sync::Mutex` (blocking), not `tokio::sync::Mutex` — the critical
288
+
section is a `Vec::push` or `clone`, microseconds, non-async. No need
289
+
to introduce async locking overhead.
290
+
- `LoggedRequest.headers` uses `Vec<u8>` values because HTTP headers can
291
+
technically hold non-UTF-8 bytes; capture faithfully.
292
+
- `Clone` on `LoggedRequest` enables `snapshot()` → `Vec<LoggedRequest>`
293
+
without leaking the lock.
294
+
295
+
**Verification:**
296
+
- `cargo test --lib commands::test::oauth::client::fake_as::request_log`
297
+
— add a smoke test: `push`, `snapshot`, assert equality.
298
+
299
+
**Commit:** `feat(oauth-client): introduce fake_as module with RequestLog and ServerHandle skeleton`
300
+
301
+
<!-- END_TASK_2 -->
302
+
303
+
<!-- START_TASK_3 -->
304
+
### Task 3: `SyntheticIdentity` + served documents
305
+
306
+
**Verifies:** AC4.2, AC4.3, AC4.4 (the shape checks; AC4.5 lands in Task 4).
307
+
308
+
**Files:**
309
+
- Create: `src/commands/test/oauth/client/fake_as/identity.rs`:
310
+
```rust
311
+
use serde_json::json;
312
+
use url::Url;
313
+
314
+
/// A synthetic atproto identity chain — handle, DID, DID document,
315
+
/// PRM and AS metadata — all bound to a base URL. `did:web` resolves
316
+
/// to that base.
317
+
#[derive(Debug, Clone)]
318
+
pub struct SyntheticIdentity {
319
+
pub handle: String,
320
+
pub did: String,
321
+
pub did_document: serde_json::Value,
322
+
pub prm_document: serde_json::Value,
323
+
pub as_metadata: serde_json::Value,
324
+
}
325
+
326
+
impl SyntheticIdentity {
327
+
pub fn for_base(base: &Url) -> Self {
328
+
let host = base.host_str().expect("base URL must have a host");
329
+
let port = base.port();
330
+
331
+
// did:web uses percent-encoded ":<port>" when a port is present.
332
+
let did = if let Some(port) = port {
333
+
format!("did:web:{host}%3A{port}")
334
+
} else {
335
+
format!("did:web:{host}")
336
+
};
337
+
let handle = format!("fake-client-under-test.{host}");
338
+
339
+
let base_with_slash = ensure_trailing_slash(base);
340
+
let base_no_slash = base.as_str().trim_end_matches('/');
341
+
let at_url = format!("at://{handle}");
342
+
let par_endpoint = format!("{base_with_slash}oauth/par");
343
+
let authorize_endpoint = format!("{base_with_slash}oauth/authorize");
344
+
let token_endpoint = format!("{base_with_slash}oauth/token");
345
+
346
+
let did_document = json!({
347
+
"@context": ["https://www.w3.org/ns/did/v1"],
348
+
"id": did,
349
+
"alsoKnownAs": [at_url],
350
+
"service": [
351
+
{
352
+
"id": "#atproto_pds",
353
+
"type": "AtprotoPersonalDataServer",
354
+
"serviceEndpoint": base_no_slash,
355
+
}
356
+
],
357
+
});
358
+
let prm_document = json!({
359
+
"resource": base_no_slash,
360
+
"authorization_servers": [base_no_slash],
361
+
});
362
+
let as_metadata = json!({
363
+
"issuer": base_no_slash,
364
+
"authorization_endpoint": authorize_endpoint,
365
+
"token_endpoint": token_endpoint,
366
+
"pushed_authorization_request_endpoint": par_endpoint,
367
+
"scopes_supported": ["atproto"],
368
+
"response_types_supported": ["code"],
369
+
"response_modes_supported": ["query"],
370
+
"grant_types_supported": ["authorization_code", "refresh_token"],
371
+
"token_endpoint_auth_methods_supported": ["none", "private_key_jwt"],
372
+
"token_endpoint_auth_signing_alg_values_supported": ["ES256"],
373
+
"code_challenge_methods_supported": ["S256"],
374
+
"dpop_signing_alg_values_supported": ["ES256"],
375
+
"require_pushed_authorization_requests": true,
376
+
});
377
+
378
+
Self { handle, did, did_document, prm_document, as_metadata }
379
+
}
380
+
}
381
+
382
+
fn ensure_trailing_slash(base: &Url) -> String {
383
+
let s = base.as_str();
384
+
if s.ends_with('/') { s.to_owned() } else { format!("{s}/") }
385
+
}
386
+
```
387
+
388
+
**Implementation notes:**
389
+
- The DID-web path for `did:web:host%3Aport` resolves to
390
+
`<scheme>://host:port/.well-known/did.json`. The fake AS serves
391
+
`/.well-known/did.json` directly (not via a path-captured parameter)
392
+
because the fake identity always lives at the base. Per-path-segment
393
+
variants like `did:web:host:u:alice` are out of Phase 6 scope; the
394
+
fake never uses them.
395
+
- Trailing slashes: atproto spec generally uses bare origins. The
396
+
`as_metadata` endpoint URLs include trailing slashes only where needed
397
+
(`/oauth/authorize`, etc. are relative to the base).
398
+
399
+
**Verification:**
400
+
- Unit test in `identity.rs` that constructs
401
+
`SyntheticIdentity::for_base("http://127.0.0.1:12345".parse().unwrap())`
402
+
and asserts:
403
+
- DID contains `127.0.0.1%3A12345`.
404
+
- `as_metadata["pushed_authorization_request_endpoint"]` is
405
+
`http://127.0.0.1:12345/oauth/par`.
406
+
- `prm_document["resource"]` is `http://127.0.0.1:12345`.
407
+
- Unit test with `https://funnel.example.com` base confirms rewriting.
408
+
409
+
**Commit:** `feat(oauth-client): add SyntheticIdentity with did:web-bound base URL`
410
+
411
+
<!-- END_TASK_3 -->
412
+
413
+
<!-- END_SUBCOMPONENT_A -->
414
+
415
+
<!-- START_SUBCOMPONENT_B (tasks 4-5) -->
416
+
417
+
**Subcomponent B: Endpoint handlers + router.**
418
+
419
+
<!-- START_TASK_4 -->
420
+
### Task 4: axum endpoint handlers with request logging
421
+
422
+
**Verifies:** AC4.2, AC4.3, AC4.4, AC4.5.
423
+
424
+
**Files:**
425
+
- Create: `src/commands/test/oauth/client/fake_as/endpoints.rs`:
426
+
```rust
427
+
use std::sync::Arc;
428
+
429
+
// axum 0.8 imports. `axum::body::Bytes` is re-exported from
430
+
// `bytes::Bytes`; the explicit axum path keeps the dependency
431
+
// surface obvious in this file.
432
+
use axum::{
433
+
body::Bytes,
434
+
extract::State,
435
+
http::{HeaderMap, Method, StatusCode, Uri},
436
+
response::{IntoResponse, Json, Response},
437
+
routing::{get, post},
438
+
Router,
439
+
};
440
+
use serde_json::json;
441
+
use url::Url;
442
+
443
+
use crate::common::oauth::clock::Clock;
444
+
use super::identity::SyntheticIdentity;
445
+
use super::request_log::{LoggedRequest, RequestLog};
446
+
447
+
pub struct AppState {
448
+
pub clock: Arc<dyn Clock>,
449
+
pub active_base: Url,
450
+
pub identity: SyntheticIdentity,
451
+
pub requests: Arc<RequestLog>,
452
+
}
453
+
454
+
pub fn build_router(state: Arc<AppState>) -> Router {
455
+
Router::new()
456
+
.route("/.well-known/did.json", get(did_json))
457
+
.route("/.well-known/oauth-protected-resource", get(prm))
458
+
.route("/.well-known/oauth-authorization-server", get(as_metadata))
459
+
.route("/oauth/par", post(par))
460
+
.route("/oauth/authorize", get(authorize))
461
+
.route("/oauth/token", post(token))
462
+
.with_state(state)
463
+
}
464
+
465
+
async fn did_json(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
466
+
log_request(&s, &method, &uri, &headers, &body);
467
+
Json(s.identity.did_document.clone()).into_response()
468
+
}
469
+
470
+
async fn prm(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
471
+
log_request(&s, &method, &uri, &headers, &body);
472
+
Json(s.identity.prm_document.clone()).into_response()
473
+
}
474
+
475
+
async fn as_metadata(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
476
+
log_request(&s, &method, &uri, &headers, &body);
477
+
Json(s.identity.as_metadata.clone()).into_response()
478
+
}
479
+
480
+
async fn par(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
481
+
log_request(&s, &method, &uri, &headers, &body);
482
+
// Phase 6 placeholder: return a deterministic request_uri.
483
+
let now = s.clock.now_unix_seconds();
484
+
let request_uri = format!("urn:ietf:params:oauth:request_uri:fake-{now}");
485
+
(StatusCode::CREATED, Json(json!({ "request_uri": request_uri, "expires_in": 60 }))).into_response()
486
+
}
487
+
488
+
async fn authorize(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
489
+
log_request(&s, &method, &uri, &headers, &body);
490
+
// Phase 6 placeholder: redirect to the client's redirect_uri with a fake code.
491
+
// In Phase 7, this becomes per-flow scripted (approve/deny/partial).
492
+
// For Phase 6, return 200 OK with a simple message — tests assert by inspecting
493
+
// the RequestLog, not the response shape.
494
+
(StatusCode::OK, "authorize placeholder").into_response()
495
+
}
496
+
497
+
async fn token(State(s): State<Arc<AppState>>, method: Method, uri: Uri, headers: HeaderMap, body: Bytes) -> Response {
498
+
log_request(&s, &method, &uri, &headers, &body);
499
+
// Phase 6 placeholder token response.
500
+
let now = s.clock.now_unix_seconds();
501
+
let access_token = format!("fake-access-{now}");
502
+
let refresh_token = format!("fake-refresh-{now}");
503
+
let resp = json!({
504
+
"access_token": access_token,
505
+
"token_type": "DPoP",
506
+
"expires_in": 3600,
507
+
"refresh_token": refresh_token,
508
+
"scope": "atproto",
509
+
});
510
+
(StatusCode::OK, Json(resp)).into_response()
511
+
}
512
+
513
+
fn log_request(s: &AppState, method: &Method, uri: &Uri, headers: &HeaderMap, body: &Bytes) {
514
+
let entry = LoggedRequest {
515
+
timestamp_unix: s.clock.now_unix_seconds(),
516
+
method: method.as_str().to_owned(),
517
+
path: uri.path().to_owned(),
518
+
query: uri.query().map(str::to_owned),
519
+
headers: headers.iter().map(|(k, v)| (k.as_str().to_owned(), v.as_bytes().to_vec())).collect(),
520
+
body: body.to_vec(),
521
+
};
522
+
s.requests.push(entry);
523
+
}
524
+
```
525
+
526
+
**Implementation notes:**
527
+
- `Bytes` body extractor must be last parameter in every handler.
528
+
- Every route logs before handling, even for well-known GETs (helps
529
+
Phase 7 debug flow runs).
530
+
- `Method`, `Uri`, and `HeaderMap` are all axum-provided extractors
531
+
that zero-cost capture the request metadata.
532
+
- The `body: Bytes` for GET handlers is empty — axum provides an empty
533
+
`Bytes` for methods without body. Not a problem to log.
534
+
535
+
**Verification:** covered by Task 5.
536
+
537
+
**Commit:** `feat(oauth-client): add fake AS endpoint handlers with request logging`
538
+
539
+
<!-- END_TASK_4 -->
540
+
541
+
<!-- START_TASK_5 -->
542
+
### Task 5: Integration tests for fake AS + `FakeClock`
543
+
544
+
**Verifies:** AC4.2, AC4.3, AC4.4, AC4.5, AC4.6.
545
+
546
+
**Files:**
547
+
- Modify: `tests/common/mod.rs` — add:
548
+
```rust
549
+
pub struct FakeClock { t: std::sync::Mutex<u64> }
550
+
impl FakeClock {
551
+
pub fn new(initial: u64) -> Self { Self { t: std::sync::Mutex::new(initial) } }
552
+
pub fn advance(&self, seconds: u64) { *self.t.lock().unwrap() += seconds; }
553
+
}
554
+
impl Clock for FakeClock {
555
+
fn now_unix_seconds(&self) -> u64 { *self.t.lock().unwrap() }
556
+
}
557
+
```
558
+
- Create: `tests/oauth_client_interactive.rs` — Phase 6 initial form:
559
+
```rust
560
+
mod common;
561
+
562
+
use std::sync::Arc;
563
+
564
+
use atproto_devtool::commands::test::oauth::client::fake_as::{ServerHandle, FakeAsOptions};
565
+
use common::FakeClock;
566
+
use url::Url;
567
+
568
+
async fn spawn_fake_as(opts: FakeAsOptions) -> ServerHandle {
569
+
ServerHandle::bind(opts, Arc::new(FakeClock::new(1_700_000_000))).await.expect("bind fake AS")
570
+
}
571
+
572
+
#[tokio::test]
573
+
async fn serves_did_json_document() { /* ... */ }
574
+
575
+
#[tokio::test]
576
+
async fn serves_prm_document() { /* ... */ }
577
+
578
+
#[tokio::test]
579
+
async fn serves_as_metadata_document() { /* ... */ }
580
+
581
+
#[tokio::test]
582
+
async fn par_endpoint_records_request_and_returns_request_uri() { /* ... */ }
583
+
584
+
#[tokio::test]
585
+
async fn public_base_url_rewrites_served_urls() { /* ... */ }
586
+
587
+
#[tokio::test]
588
+
async fn bind_unbindable_port_returns_error() { /* ... */ }
589
+
```
590
+
591
+
Each test:
592
+
1. Spawns the server via `spawn_fake_as(FakeAsOptions { bind_port: None, public_base_url: None })` (or with options for the specific case).
593
+
2. Issues a raw HTTP request using `reqwest::Client` against `http://127.0.0.1:<port>/...`.
594
+
3. Asserts the response status, body JSON shape (e.g., `response["issuer"]
595
+
== "http://127.0.0.1:<port>"`).
596
+
4. Asserts `handle.requests.snapshot()` contains the expected
597
+
`LoggedRequest` with timestamp, method, path, and body bytes.
598
+
5. Shuts down the server via `handle.shutdown().await`.
599
+
600
+
**Specific test payloads:**
601
+
602
+
- `serves_did_json_document`:
603
+
- GET `/.well-known/did.json`.
604
+
- Body contains `"id": "did:web:127.0.0.1%3A<port>"` and the `service`
605
+
array with `serviceEndpoint` equal to `http://127.0.0.1:<port>`.
606
+
- RequestLog has 1 entry.
607
+
608
+
- `serves_prm_document`:
609
+
- GET `/.well-known/oauth-protected-resource`.
610
+
- Body contains `"resource"` and `"authorization_servers"`.
611
+
612
+
- `serves_as_metadata_document`:
613
+
- Body contains `"require_pushed_authorization_requests": true`,
614
+
`"dpop_signing_alg_values_supported": ["ES256"]`,
615
+
`"code_challenge_methods_supported": ["S256"]`.
616
+
- Covers AC4.4.
617
+
618
+
- `par_endpoint_records_request_and_returns_request_uri`:
619
+
- POST `/oauth/par` with a form-encoded body
620
+
(`response_type=code&client_id=...&code_challenge=...&code_challenge_method=S256`).
621
+
- Response 201 with JSON `{"request_uri": "...", "expires_in": 60}`.
622
+
- RequestLog entry has method "POST", path "/oauth/par", body bytes
623
+
matching the form-encoded body verbatim.
624
+
- Covers AC4.5.
625
+
626
+
- `public_base_url_rewrites_served_urls`:
627
+
- Spawn with `public_base_url: Some("https://funnel.example.com".parse()?)`.
628
+
- GET `/.well-known/oauth-authorization-server`.
629
+
- Body `"issuer"` is `"https://funnel.example.com"`.
630
+
- Body `"pushed_authorization_request_endpoint"` is
631
+
`"https://funnel.example.com/oauth/par"`.
632
+
- Covers AC4.3.
633
+
634
+
- `bind_unbindable_port_returns_error`:
635
+
- Bind a TcpListener on `127.0.0.1:X` for some X.
636
+
- Spawn fake AS with `bind_port: Some(X)`.
637
+
- Assert `ServerHandle::bind(...)` returns `Err(BindError::BindFailed { .. })`.
638
+
- Covers AC4.6.
639
+
640
+
**Verification:**
641
+
- `cargo test --test oauth_client_interactive` passes.
642
+
- Manual: `cargo run -- test oauth client interactive https://example.com`
643
+
starts the server. Ctrl-C shuts down cleanly (requires Phase 7 wiring
644
+
of the CLI path — for Phase 6, just confirm the server code compiles
645
+
and the unit tests drive it; the `ClientCmd::run` interactive arm
646
+
remains a placeholder until Phase 7).
647
+
648
+
**Commit:** `test(oauth-client): fake AS integration tests for identity + well-known endpoints + PAR logging`
649
+
650
+
<!-- END_TASK_5 -->
651
+
652
+
<!-- END_SUBCOMPONENT_B -->
653
+
654
+
---
655
+
656
+
## Phase 6 done when
657
+
658
+
- `cargo test --test oauth_client_interactive` passes (the scaffolding
659
+
tests).
660
+
- All of `cargo build`, `cargo test` (full), `cargo clippy -- -D warnings`,
661
+
`cargo fmt --check` clean.
662
+
- The `ClientCmd::run` interactive arm still prints its placeholder line —
663
+
AC4.1 (print identity, wait for client) is explicitly Phase 7.
664
+
665
+
Phase 7 fills in the `RelyingParty` implementation, upgrades endpoint
666
+
handlers to perform real conformance checks, and wires the
667
+
`interactive.rs` stage into the pipeline with `Check::blocked_by`
668
+
dependency gating to Phase 3/4/5 static checks.
+539
docs/implementation-plans/2026-04-16-test-oauth-client/phase_07.md
+539
docs/implementation-plans/2026-04-16-test-oauth-client/phase_07.md
···
1
+
# OAuth client test — Phase 7: RelyingParty + interactive stage gating
2
+
3
+
**Goal:** Complete `RelyingParty` as a spec-compliant atproto OAuth
4
+
client (PAR + PKCE S256 + DPoP nonce handling + private_key_jwt + token
5
+
refresh). Upgrade the fake AS endpoints from Phase 6 placeholders to
6
+
perform real conformance work (PKCE method check, DPoP proof validation,
7
+
private_key_jwt validation, DPoP nonce issuance and rotation, refresh
8
+
token single-use rotation, DPoP `jti` replay detection). Wire the
9
+
`interactive.rs` stage into the pipeline and add `Check::blocked_by`
10
+
entries linking interactive checks to static prerequisites. Make
11
+
`atproto-devtool test oauth client interactive <target>` actually do
12
+
something end-to-end.
13
+
14
+
**Architecture:** `src/common/oauth/relying_party.rs` is the RelyingParty
15
+
implementation — it drives real PAR + authorize + token + refresh flows
16
+
against any atproto AS (production code), with deterministic `jti` /
17
+
timestamp / code-verifier under test through the injected `Clock` and a
18
+
per-instance seeded `rand_chacha::ChaCha20Rng`. The interactive stage
19
+
(`interactive.rs`) spawns the fake AS, prints the synthetic handle/DID,
20
+
and — in test mode — drives a RelyingParty against it. Conformance
21
+
checks introspect `ServerHandle.requests.snapshot()` after each
22
+
scripted flow and emit `CheckResult`s.
23
+
24
+
**Tech Stack:** `jsonwebtoken` (ES256 signing), `p256` for key
25
+
generation, `sha2` for PKCE S256 challenges and DPoP `ath` claims,
26
+
`rand_chacha` (new dep, seeded from explicit seed; no `getrandom`),
27
+
`base64` (existing dep) for URL-safe encoding without padding.
28
+
29
+
**Scope:** Phase 7 of 8.
30
+
31
+
**Codebase verified:** 2026-04-16.
32
+
33
+
---
34
+
35
+
## Acceptance Criteria Coverage
36
+
37
+
This phase implements and tests:
38
+
39
+
### test-oauth-client.AC4: Interactive mode — fake AS infrastructure
40
+
41
+
- **test-oauth-client.AC4.1 Success:** `test oauth client interactive <target>` spawns the `axum`-based fake AS on a port, prints the synthetic handle / DID for the user to enter into their client, and waits for inbound requests until interrupted.
42
+
43
+
### test-oauth-client.AC5: Cross-mode dependency gating
44
+
45
+
- **test-oauth-client.AC5.1 Success:** When `interactive` is invoked, the static stages (discovery, metadata, JWKS) run first and their `CheckResult`s appear in the report ahead of the interactive stage's results.
46
+
- **test-oauth-client.AC5.2 Success:** When all static checks pass, every interactive sub-stage runs its full check inventory.
47
+
- **test-oauth-client.AC5.3 Skip:** When a static check fails AND it gates a specific interactive flow, that flow's checks emit `Skipped` with reason `"blocked by <stage>::<check_id>"` (matching the labeler `Check::blocked_by` rendering verbatim).
48
+
- **test-oauth-client.AC5.4 Success:** When a static check fails but no interactive flow declares a `blocked_by` against it, the interactive flows still execute their full inventories.
49
+
- **test-oauth-client.AC5.5 Success:** Every `Skipped` reason references a specific failing check ID, never a generic description like `"a prerequisite failed"`.
50
+
51
+
---
52
+
53
+
## Codebase verification findings
54
+
55
+
- Phase 6 set up `fake_as.rs` with placeholder endpoints that only log
56
+
requests and return canned responses. Phase 7 rewrites `par`,
57
+
`authorize`, and `token` to implement the real flow semantics.
58
+
- `common::oauth::jws::sign_es256_jws` exists (Phase 2 Task 3) — Phase 7
59
+
uses it to build DPoP proofs and private_key_jwt client assertions.
60
+
- `common::oauth::clock::Clock` exists (Phase 6 Task 1).
61
+
- No `rand_chacha` dep today. Add it at `0.9` (current stable). Seed at
62
+
`RelyingParty::new` construction.
63
+
- `common::identity::HttpClient` trait has only `get_bytes`. Phase 7's
64
+
RelyingParty needs to POST with form-encoded bodies + headers. Two
65
+
options:
66
+
1. Extend `HttpClient` with a `post_bytes(url, body, headers)` method.
67
+
2. Give `RelyingParty` its own `reqwest::Client` directly.
68
+
Choose (2): RelyingParty is a production class that owns its HTTP
69
+
stack. It constructs its own `reqwest::Client` with rustls + user
70
+
agent. Tests still use the real `reqwest::Client` hitting
71
+
`127.0.0.1:<port>` of the fake AS — no need for an `HttpClient`-style
72
+
mock, because the fake AS is a real server and tests are
73
+
integration-level.
74
+
75
+
---
76
+
77
+
## Implementation tasks
78
+
79
+
<!-- START_SUBCOMPONENT_A (tasks 1-2) -->
80
+
81
+
**Subcomponent A: `RelyingParty` core + flow scripting.**
82
+
83
+
<!-- START_TASK_1 -->
84
+
### Task 1: `RelyingParty::new`, `discover_as`, `do_par`
85
+
86
+
**Verifies:** infrastructure for AC5.2/AC5.3 (the RelyingParty drives the
87
+
flows that the checks introspect).
88
+
89
+
**Files:**
90
+
- Modify: `Cargo.toml` — add:
91
+
```toml
92
+
rand_chacha = "0.9"
93
+
rand_core = "0.9" # for SeedableRng + RngCore traits
94
+
```
95
+
- Create: `src/common/oauth/relying_party.rs`:
96
+
```rust
97
+
use std::collections::HashMap;
98
+
use std::sync::{Arc, Mutex};
99
+
100
+
use jsonwebtoken::{EncodingKey, Header, Algorithm};
101
+
use p256::ecdsa::SigningKey as P256SigningKey;
102
+
use rand_chacha::ChaCha20Rng;
103
+
use rand_core::{RngCore, SeedableRng};
104
+
use reqwest::Client as ReqwestClient;
105
+
use serde_json::json;
106
+
use url::Url;
107
+
108
+
use super::clock::Clock;
109
+
110
+
pub struct RelyingParty {
111
+
client_id: Url,
112
+
kind: ClientKind,
113
+
signing_key: Arc<P256SigningKey>,
114
+
encoding_key: Arc<EncodingKey>,
115
+
signing_jwk_public: serde_json::Value,
116
+
clock: Arc<dyn Clock>,
117
+
rng: Mutex<ChaCha20Rng>,
118
+
http: ReqwestClient,
119
+
/// DPoP nonce state, keyed by the endpoint URL (PAR vs. token)
120
+
/// because atproto AS servers rotate nonces per endpoint family.
121
+
/// A single shared `Option<String>` would cross-contaminate
122
+
/// retry state between PAR and token flows and break AC7.1
123
+
/// (nonce rotation only on the affected endpoint) as well as
124
+
/// AC7.5 (client must adopt AS-issued nonce on the next request
125
+
/// to *that* endpoint, not all endpoints).
126
+
dpop_nonces: Mutex<HashMap<Url, String>>,
127
+
}
128
+
129
+
/// High-level client kind for RP behavior.
130
+
#[derive(Debug, Clone, Copy)]
131
+
pub enum ClientKind {
132
+
Confidential,
133
+
Public,
134
+
}
135
+
136
+
pub struct ParRequest {
137
+
pub as_descriptor: AsDescriptor,
138
+
pub redirect_uri: Url,
139
+
pub scope: String,
140
+
pub state: String,
141
+
}
142
+
143
+
pub struct ParResponse {
144
+
pub request_uri: String,
145
+
pub expires_in: u64,
146
+
pub code_verifier: String, // kept inside the RP for token exchange
147
+
}
148
+
149
+
pub struct AsDescriptor {
150
+
pub issuer: Url,
151
+
pub pushed_authorization_request_endpoint: Url,
152
+
pub authorization_endpoint: Url,
153
+
pub token_endpoint: Url,
154
+
}
155
+
156
+
impl RelyingParty {
157
+
pub fn new(
158
+
client_id: Url,
159
+
kind: ClientKind,
160
+
clock: Arc<dyn Clock>,
161
+
rng_seed: [u8; 32],
162
+
) -> Self { /* ... */ }
163
+
164
+
pub async fn discover_as(&self, base: &Url) -> Result<AsDescriptor, RpError> { /* ... */ }
165
+
166
+
pub async fn do_par(&self, req: &ParRequest) -> Result<ParResponse, RpError> { /* ... */ }
167
+
168
+
// Task 2 adds do_authorize, do_token, do_refresh.
169
+
170
+
// Internal helpers
171
+
fn new_pkce(&self) -> (String, String) { /* (verifier, challenge_s256) */ }
172
+
fn new_jti(&self) -> String { /* 16 bytes base64url */ }
173
+
fn sign_dpop(&self, htm: &str, htu: &Url, ath: Option<&str>) -> Result<String, RpError> { /* ... */ }
174
+
fn sign_private_key_jwt(&self, aud: &Url) -> Result<String, RpError> { /* confidential only */ }
175
+
}
176
+
```
177
+
- `RpError` enum with `thiserror::Error + miette::Diagnostic` derives.
178
+
Variants: `Http(reqwest::Error)`, `NonSuccessStatus { status: u16, body: String }`, `JwsFailure(super::jws::JwsError)`, `MetadataMalformed { reason: String }`, `TimestampInThePast`.
179
+
180
+
**Key generation / JWK material** (uses Phase 2's canonical sign-side path):
181
+
- `RelyingParty::new` generates an ES256 key from the seeded RNG:
182
+
```rust
183
+
use p256::ecdsa::SigningKey;
184
+
use p256::pkcs8::EncodePrivateKey;
185
+
186
+
let mut rng = ChaCha20Rng::from_seed(rng_seed);
187
+
let signing_key = SigningKey::random(&mut rng);
188
+
```
189
+
`SigningKey::random` in `p256` accepts any `RngCore + CryptoRng`; the
190
+
seeded `ChaCha20Rng` satisfies both, so the generated key is
191
+
deterministic per seed. Do NOT use `OsRng`.
192
+
- Derive the `EncodingKey` for jsonwebtoken via the PKCS8 DER path
193
+
(verified in Phase 2 — `EncodingKey::from_ec_der` is unconditional and
194
+
the `rust_crypto` backend internally calls
195
+
`p256::ecdsa::SigningKey::from_pkcs8_der` on the stored bytes):
196
+
```rust
197
+
let pkcs8 = signing_key.to_pkcs8_der().expect("pkcs8 encode");
198
+
let encoding_key = jsonwebtoken::EncodingKey::from_ec_der(pkcs8.as_bytes());
199
+
```
200
+
No `use_pem` feature is needed.
201
+
- Compute the public JWK once:
202
+
```rust
203
+
use base64::Engine;
204
+
let verifying = signing_key.verifying_key();
205
+
let encoded_point = verifying.to_encoded_point(false); // uncompressed
206
+
let b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD;
207
+
let x = b64.encode(encoded_point.x().expect("x coord present"));
208
+
let y = b64.encode(encoded_point.y().expect("y coord present"));
209
+
// Compute a short deterministic kid from the RNG seed's first 8 bytes.
210
+
// Using base64url-nopad keeps the kid URL-safe and avoids a hex dep.
211
+
let kid = b64.encode(&rng_seed[0..8]);
212
+
let signing_jwk_public = json!({
213
+
"kty": "EC", "crv": "P-256", "alg": "ES256", "use": "sig",
214
+
"kid": kid,
215
+
"x": x, "y": y,
216
+
});
217
+
```
218
+
No `hex` dep required; `base64` (already in project deps) handles the
219
+
`kid` encoding. The `kid` is deterministic per `rng_seed` so tests get
220
+
stable snapshots.
221
+
222
+
**discover_as:**
223
+
- GET `base + "/.well-known/oauth-authorization-server"` using
224
+
`self.http`. Parse JSON into `AsDescriptor`.
225
+
- Validate required fields (issuer, par endpoint, authorize endpoint,
226
+
token endpoint, `require_pushed_authorization_requests == true`).
227
+
228
+
**do_par:**
229
+
1. Generate a PKCE pair via `new_pkce()`:
230
+
- Generate 32 bytes from RNG, base64url-nopad encode → `code_verifier`.
231
+
- SHA-256 hash the verifier bytes, base64url-nopad encode → `code_challenge`.
232
+
2. Build form body:
233
+
```
234
+
response_type=code
235
+
client_id=<client_id>
236
+
redirect_uri=<redirect_uri>
237
+
scope=<scope>
238
+
state=<state>
239
+
code_challenge=<challenge>
240
+
code_challenge_method=S256
241
+
```
242
+
If `ClientKind::Confidential`, append `client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-bearer&client_assertion=<jwt>` where the JWT has claims `iss = sub = client_id`, `aud = issuer`, `exp = now+300`, `iat = now`, `jti = new_jti()`.
243
+
3. Build DPoP proof JWT:
244
+
- Header: `typ = "dpop+jwt"`, `alg = ES256`, `jwk = <public jwk>`.
245
+
- Claims: `jti = new_jti()`, `htm = "POST"`, `htu = <par_endpoint>`, `iat = clock.now_unix_seconds()`. Add `nonce` if a previous server response gave one (store in `self.dpop_nonce`).
246
+
4. POST to the PAR endpoint with `Content-Type: application/x-www-form-urlencoded` and `DPoP: <proof>` header.
247
+
5. On 400 with JSON body `{"error": "use_dpop_nonce"}` **and**
248
+
`DPoP-Nonce: <nonce>` header in response: adopt the nonce, retry
249
+
once. On second 400, surface `RpError::NonSuccessStatus`.
250
+
6. On 2xx: parse JSON, return `ParResponse { request_uri, expires_in, code_verifier }`.
251
+
252
+
**Testing:** unit tests for `new_pkce` (deterministic from seed),
253
+
`new_jti` (16 bytes, base64url-nopad, uniqueness across 100 calls from
254
+
same seed), `sign_dpop` (parses back with the public JWK). Integration
255
+
tests with the fake AS come in Task 3.
256
+
257
+
**Commit:** `feat(common-oauth): RelyingParty skeleton with discover_as and do_par`
258
+
259
+
<!-- END_TASK_1 -->
260
+
261
+
<!-- START_TASK_2 -->
262
+
### Task 2: `do_authorize`, `do_token`, `do_refresh`, `FlowScript`
263
+
264
+
**Verifies:** flow driver for AC5.2 / AC5.3 (enables Task 3-4 checks).
265
+
266
+
**Files:**
267
+
- Modify: `src/common/oauth/relying_party.rs`:
268
+
- `pub async fn do_authorize(&self, as_descriptor: &AsDescriptor, request_uri: &str, redirect_uri: &Url) -> Result<AuthorizeOutcome, RpError>`:
269
+
- GET `<authorize_endpoint>?request_uri=<encoded>&client_id=<encoded>`.
270
+
- Follow redirects manually: stop on the first redirect whose
271
+
`Location` header matches the client's `redirect_uri` scheme/origin.
272
+
- Extract `code` / `error` / `error_description` from the location
273
+
URL's query.
274
+
- Return `AuthorizeOutcome::Code { code } | Error { error, error_description }`.
275
+
- `pub async fn do_token(&self, as_descriptor: &AsDescriptor, redirect_uri: &Url, code: &str, code_verifier: &str) -> Result<TokenResponse, RpError>`:
276
+
- POST form body: `grant_type=authorization_code&code=<code>&redirect_uri=<...>&client_id=<...>&code_verifier=<code_verifier>`. If confidential, include the client_assertion JWT.
277
+
- DPoP proof on the `POST /oauth/token` URL. `ath` claim omitted (no token yet).
278
+
- Handle `use_dpop_nonce` retry same as `do_par`.
279
+
- On 2xx: parse JSON into `TokenResponse { access_token, token_type, expires_in, refresh_token, scope }`.
280
+
- `pub async fn do_refresh(&self, as_descriptor: &AsDescriptor, refresh_token: &str, scope: Option<&str>) -> Result<TokenResponse, RpError>`:
281
+
- POST form body: `grant_type=refresh_token&refresh_token=<rt>[&scope=<scope>]`. Include client_assertion if confidential.
282
+
- DPoP proof on `/oauth/token`. Compute `ath` from the current
283
+
access token if we're refreshing eagerly; per atproto profile,
284
+
`ath` claim on refresh requests is optional — omit for Phase 7.
285
+
Revisit in Phase 8 DPoP edges.
286
+
- `pub struct TokenResponse` — deserialize from the token endpoint's
287
+
JSON with `serde::Deserialize`.
288
+
- `pub enum AuthorizeOutcome { Code { code: String }, Error { error: String, error_description: Option<String> } }`.
289
+
290
+
- Add `FlowScript` shape used by the fake AS's `authorize` handler to
291
+
decide which outcome to return:
292
+
```rust
293
+
// In src/commands/test/oauth/client/fake_as/endpoints.rs
294
+
#[derive(Debug, Clone)]
295
+
pub enum FlowScript {
296
+
/// Approve the authorize request and issue the requested scope.
297
+
Approve { granted_scope: String },
298
+
/// Approve but grant a strict subset of the requested scope.
299
+
PartialGrant { granted_scope: String },
300
+
/// Deny via access_denied redirect.
301
+
Deny,
302
+
/// On the first PAR, respond use_dpop_nonce to force a retry.
303
+
DpopNonceRetryOnPar { nonce: String },
304
+
}
305
+
```
306
+
- Extend `AppState` (Phase 6) to carry a `flow_script: Mutex<FlowScript>`
307
+
plus a `next_codes: Mutex<VecDeque<String>>` and
308
+
`refresh_tokens: Mutex<HashMap<String, TokenBinding>>` used by the
309
+
real token endpoint logic.
310
+
311
+
**Commit:** `feat(common-oauth): RelyingParty full flow (authorize, token, refresh)`
312
+
313
+
<!-- END_TASK_2 -->
314
+
315
+
<!-- END_SUBCOMPONENT_A -->
316
+
317
+
<!-- START_SUBCOMPONENT_B (tasks 3-4) -->
318
+
319
+
**Subcomponent B: Fake AS upgrade + interactive stage.**
320
+
321
+
<!-- START_TASK_3 -->
322
+
### Task 3: Upgrade fake AS endpoints to perform real conformance work
323
+
324
+
**Verifies:** provides real behavior for AC6 / AC7 (Phase 8), AC5 gating.
325
+
326
+
**Files:**
327
+
- Modify: `src/commands/test/oauth/client/fake_as/endpoints.rs`:
328
+
- `par(...)`:
329
+
- Parse form-encoded body via `serde_urlencoded::from_bytes::<ParParams>(&body)`.
330
+
- If missing `code_challenge` → return 400 with `invalid_request`.
331
+
- If `code_challenge_method != "S256"` → return 400 with `invalid_request`.
332
+
- Validate DPoP header present; parse as JWS; verify signature
333
+
against the inline `jwk` (extract from header, use Phase 2
334
+
`parse_jwk` + `verify_jws`). On failure → 401 DPoP specific
335
+
error. On success, record `jti` in state's `seen_jtis` set; if
336
+
already present → 401 `invalid_dpop_proof` (nonce-like reuse error).
337
+
- If flow script is `DpopNonceRetryOnPar { nonce }` and no DPoP proof
338
+
had a `nonce` claim yet → return 400 `use_dpop_nonce` with the
339
+
nonce. Otherwise accept the proof.
340
+
- For confidential auth: parse `client_assertion` JWT, verify its
341
+
`iss`/`sub` == `client_id`, `aud` == issuer, `exp` within bounds.
342
+
Mismatch → 401.
343
+
- Mint a `request_uri` (deterministic under test clock + RNG),
344
+
remember the request shape, return 201.
345
+
- `authorize(...)`:
346
+
- Read `request_uri` query param; look up the pending PAR request.
347
+
- Dispatch on `AppState.flow_script`:
348
+
- `Approve { granted_scope }`: generate a code (deterministic from
349
+
clock+rng), bind it to the PAR request with the granted scope,
350
+
redirect to `redirect_uri?code=<code>&state=<state>`.
351
+
- `PartialGrant { granted_scope }`: same as Approve, but
352
+
bind a scope that is a strict subset of requested.
353
+
- `Deny`: redirect to `redirect_uri?error=access_denied&state=<state>`.
354
+
- `token(...)`:
355
+
- Parse form-encoded body → `TokenParams`. Dispatch on `grant_type`:
356
+
- `authorization_code`: validate `code_verifier` against stored
357
+
`code_challenge`; validate DPoP proof; mint access_token +
358
+
refresh_token bound to the DPoP public JWK thumbprint (record
359
+
in state). Return 200 with JSON.
360
+
- `refresh_token`: validate the rt exists and is single-use
361
+
(mark used, mint fresh access + new rt). Reject reused rt → 401
362
+
`invalid_grant`. Honor scope downscoping.
363
+
- Handle `use_dpop_nonce` retries similarly to PAR, based on script.
364
+
- Record DPoP `jti` replay: duplicate `jti` within the `seen_jtis`
365
+
set → 401.
366
+
- Modify: `src/commands/test/oauth/client/fake_as.rs` — extend `AppState`
367
+
with `seen_jtis: Mutex<HashSet<String>>`,
368
+
`pending_par: Mutex<HashMap<String, ParRequestSnapshot>>`,
369
+
`codes: Mutex<HashMap<String, CodeBinding>>`,
370
+
`refresh_tokens: Mutex<HashMap<String, TokenBinding>>`,
371
+
`flow_script: Mutex<FlowScript>`, `public_jwk_by_thumbprint: Mutex<HashMap<String, serde_json::Value>>`.
372
+
- Add `serde_urlencoded` to `Cargo.toml`:
373
+
```toml
374
+
serde_urlencoded = "0.7" # Parse application/x-www-form-urlencoded bodies (PAR / token).
375
+
```
376
+
Pin to the same major version reqwest pulls transitively (verified:
377
+
reqwest 0.13 uses `serde_urlencoded 0.7.x`, so `0.7` in our direct
378
+
`[dependencies]` resolves to the same compiled crate — no duplicate
379
+
compilation). The explicit direct dep makes intent clear and ensures
380
+
a version bump in reqwest's transitive can't silently drop this
381
+
functionality without the compile break showing up in our `Cargo.toml`.
382
+
383
+
**Commit:** `feat(oauth-client): fake AS performs real PAR/authorize/token with DPoP + PKCE + private_key_jwt validation`
384
+
385
+
<!-- END_TASK_3 -->
386
+
387
+
<!-- START_TASK_4 -->
388
+
### Task 4: `interactive.rs` stage — CLI wiring + dependency gating + conformance checks
389
+
390
+
**Verifies:** AC4.1, AC5.1, AC5.2, AC5.3, AC5.4, AC5.5.
391
+
392
+
**Files:**
393
+
- Create: `src/commands/test/oauth/client/interactive.rs`:
394
+
- `pub struct InteractiveFacts { pub server: ServerHandle }` (unused by
395
+
later phases directly — the `ServerHandle` is held by the stage
396
+
runner; Facts carry what Phase 8 sub-stages need).
397
+
- `#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Check { ServerBound, ClientReachedPar, ClientUsedPkceS256, ClientIncludedDpop, ClientCompletedToken, ClientRefreshed }`
398
+
— each with a stable ID under `oauth_client::interactive::<slug>`.
399
+
- `pub async fn run(
400
+
static_results: StaticGating,
401
+
metadata_facts: Option<&MetadataFacts>,
402
+
jwks_facts: Option<&JwksFacts>,
403
+
interactive_opts: &InteractiveOptions,
404
+
clock: Arc<dyn Clock>,
405
+
) -> InteractiveStageOutput`:
406
+
- Compose the `blocked_by` gate table:
407
+
- `ClientReachedPar` depends on `oauth_client::metadata::scope_present` and `oauth_client::metadata::dpop_bound_required`. (If either failed, emit Skipped `blocked_by`.)
408
+
- `ClientUsedPkceS256` depends on the same.
409
+
- `ClientIncludedDpop` depends on the same.
410
+
- `ClientCompletedToken` depends on all of the above + (for confidential) `oauth_client::jws::keys_have_alg`.
411
+
- `ClientRefreshed` depends on grant_types including refresh_token.
412
+
- Iterate the planned checks: if any declared dependency's check is
413
+
not `Pass` in `static_results.results`, emit
414
+
`common::report::blocked_by(check.id(), Stage::INTERACTIVE, summary, <failed_check_id>)`
415
+
and skip actually driving the RP for that concern.
416
+
- If all gates pass:
417
+
- Spawn the fake AS (`ServerHandle::bind(FakeAsOptions { .. }, clock)`). Surface bind errors as `NetworkError` on `ServerBound`.
418
+
- Print the synthetic identity to stdout:
419
+
```
420
+
Fake atproto authorization server is running.
421
+
Handle: <handle>
422
+
DID: <did>
423
+
Base: <active_base>
424
+
```
425
+
- Construct a `RelyingParty` scoped to the target-under-test's
426
+
`client_id` and kind. Drive a single happy-path flow (PAR →
427
+
authorize (approve) → token). Inspect `handle.requests.snapshot()`
428
+
and set each `Check` result:
429
+
- `ClientReachedPar.pass()` — snapshot has a `POST /oauth/par` entry.
430
+
- `ClientUsedPkceS256.pass()` — that entry's body has
431
+
`code_challenge_method=S256`.
432
+
- `ClientIncludedDpop.pass()` — that entry's headers include `DPoP`.
433
+
- `ClientCompletedToken.pass()` — snapshot has a 2xx `POST /oauth/token`.
434
+
- `ClientRefreshed.skipped_with_reason("covered in Phase 8 flow variants", ...)` — Phase 7 ships the happy-path only; Phase 8 adds scope variations + DPoP edges.
435
+
- Shutdown the server when the test run exits (or wait for Ctrl-C
436
+
in CLI mode — see below).
437
+
- After printing the identity, dispatch on
438
+
`interactive_opts.drive_mode`:
439
+
- `InteractiveDriveMode::WaitForExternalClient` — block on
440
+
`tokio::signal::ctrl_c().await`. After Ctrl-C, inspect
441
+
`server.requests.snapshot()` and emit the interactive checks
442
+
based on what the external client did. Satisfies AC4.1.
443
+
- `InteractiveDriveMode::DriveRpInProcess { rp_factory }` — do NOT
444
+
block. Build a fresh RelyingParty via the factory, drive a
445
+
single happy-path flow against the running fake AS, inspect the
446
+
log, emit the checks. Used by tests and by Phase 8 sub-stages
447
+
(which invoke this code path once per flow variant).
448
+
449
+
- Modify: `src/commands/test/oauth/client/pipeline.rs`:
450
+
- Add:
451
+
```rust
452
+
pub struct InteractiveOptions {
453
+
pub bind_port: Option<u16>,
454
+
pub public_base_url: Option<Url>,
455
+
/// Chooses between waiting for an external client (CLI mode) and
456
+
/// driving an in-process `RelyingParty` (test mode). This field is
457
+
/// load-bearing: it decides whether `interactive::run` blocks on
458
+
/// `tokio::signal::ctrl_c()` or returns after a scripted run.
459
+
pub drive_mode: InteractiveDriveMode,
460
+
}
461
+
462
+
pub enum InteractiveDriveMode {
463
+
/// Spawn the fake AS, print the synthetic identity, and wait for
464
+
/// `tokio::signal::ctrl_c()` before inspecting the request log and
465
+
/// emitting conformance checks. Used by `ClientCmd::run` on the
466
+
/// real CLI path so a developer can point their real client at
467
+
/// the fake AS.
468
+
WaitForExternalClient,
469
+
470
+
/// Spawn the fake AS, drive a given `RelyingParty` builder against
471
+
/// it in-process, and emit conformance checks immediately after
472
+
/// the scripted flows complete. Used by integration tests and by
473
+
/// Phase 8's scope-variations / dpop-edges sub-stages.
474
+
DriveRpInProcess {
475
+
/// Factory producing a fresh RelyingParty per flow run,
476
+
/// seeded deterministically. Phase 8 simplifies this API
477
+
/// (see Phase 8 Task 1); Phase 7 lands the field in place.
478
+
rp_factory: std::sync::Arc<dyn crate::common::oauth::relying_party::RpFactory>,
479
+
},
480
+
}
481
+
```
482
+
- Add `pub interactive: Option<&'a InteractiveOptions>` to `OauthClientOptions`.
483
+
- At the end of `run_pipeline`, if `opts.interactive.is_some()`:
484
+
- Build a `StaticGating` view over the report (scan `results` for
485
+
the IDs in the interactive-gate table).
486
+
- Call `interactive::run(...)`.
487
+
- Append results to the report.
488
+
489
+
- Modify: `src/commands/test/oauth/client.rs`:
490
+
- `ClientCmd::run` — in the `Some(ClientMode::Interactive(args))` arm,
491
+
build
492
+
```rust
493
+
InteractiveOptions {
494
+
bind_port: args.port,
495
+
public_base_url: args.public_base_url,
496
+
drive_mode: InteractiveDriveMode::WaitForExternalClient,
497
+
}
498
+
```
499
+
and pass through as `opts.interactive = Some(&interactive_opts)`.
500
+
Tests construct `InteractiveOptions` with
501
+
`drive_mode: InteractiveDriveMode::DriveRpInProcess { rp_factory }`
502
+
instead.
503
+
504
+
**Testing:**
505
+
- Modify: `tests/oauth_client_interactive.rs` — add end-to-end tests
506
+
driving a `RelyingParty` through a fake AS and asserting the
507
+
pipeline's final report. Tests must:
508
+
- Use a fixed seed for `FakeClock` and the RP's RNG.
509
+
- Assert the report exit code and each interactive `CheckResult` is `Pass`.
510
+
- Assert snapshot of the rendered report (deterministic under fixed clock).
511
+
- Add test `partial_static_failure_blocks_interactive`:
512
+
- Seed a metadata doc with `dpop_bound_access_tokens=false`.
513
+
- Assert `oauth_client::interactive::client_reached_par` emits
514
+
`Skipped` with reason `"blocked by oauth_client::metadata::dpop_bound_required"`.
515
+
- Supports AC5.3 / AC5.5.
516
+
517
+
**Commit:** `feat(oauth-client): interactive stage with RP-driven happy path and dependency gating`
518
+
519
+
<!-- END_TASK_4 -->
520
+
521
+
<!-- END_SUBCOMPONENT_B -->
522
+
523
+
---
524
+
525
+
## Phase 7 done when
526
+
527
+
- `cargo test --test oauth_client_interactive` passes.
528
+
- Snapshots reviewed and accepted.
529
+
- `cargo clippy`, `cargo fmt --check` clean.
530
+
- Manual smoke: running
531
+
`cargo run -- test oauth client interactive http://localhost/client.json` prints
532
+
the synthetic identity; pointing a real reference client
533
+
(e.g., a small TypeScript `@atproto/oauth-client-node` example,
534
+
modified to point at `http://127.0.0.1:<port>`) at the fake AS
535
+
completes a full PAR → authorize → token round-trip. Ctrl-C cleanly
536
+
shuts down and emits the final report.
537
+
538
+
Phase 8 layers per-flow-variant sub-stages (scope variations, DPoP edge
539
+
flows) onto this infrastructure.
+430
docs/implementation-plans/2026-04-16-test-oauth-client/phase_08.md
+430
docs/implementation-plans/2026-04-16-test-oauth-client/phase_08.md
···
1
+
# OAuth client test — Phase 8: Day-1 flow variants + CLI contract
2
+
3
+
**Goal:** Land the two day-1 flow variant suites (scope variations,
4
+
DPoP edges), each as its own interactive sub-stage with a dedicated
5
+
diagnostic-code prefix and snapshot tests. Tie up the cross-cutting
6
+
CLI / report contract acceptance criteria (exit codes, `--verbose`,
7
+
`NO_COLOR` / `--no-color`, naming conventions, help output).
8
+
9
+
**Architecture:** Extend `src/commands/test/oauth/client/interactive/`
10
+
with two sibling files:
11
+
- `scope_variations.rs` — per-flow scripts: full-grant approve,
12
+
partial-grant, user-denied, downscoped refresh.
13
+
- `dpop_edges.rs` — per-flow scripts: nonce rotation, `use_dpop_nonce`
14
+
on PAR, replay rejection, refresh-token rotation race.
15
+
16
+
Each sub-stage is an async function that drives a `RelyingParty`
17
+
instance against the shared `fake_as::ServerHandle` with a per-flow
18
+
`FlowScript`, introspects the `RequestLog`, and emits `CheckResult`s
19
+
under its own diagnostic-code prefix
20
+
(`oauth_client::interactive::scope_variations::*` and
21
+
`oauth_client::interactive::dpop_edges::*`). A compact end-to-end test
22
+
binary (`oauth_client_endtoend.rs`) pins the combined rendered report as
23
+
a regression baseline. `oauth_client_cli.rs` covers `--help`,
24
+
`--verbose`, `--no-color`, `NO_COLOR`, and exit codes 0 / 1 / 2.
25
+
26
+
**Tech Stack:** existing Phase 7 RP + fake AS, `assert_cmd` (new dep —
27
+
already used by labeler; check), insta snapshots.
28
+
29
+
**Scope:** Phase 8 of 8.
30
+
31
+
**Codebase verified:** 2026-04-16.
32
+
33
+
---
34
+
35
+
## Acceptance Criteria Coverage
36
+
37
+
This phase implements and tests:
38
+
39
+
### test-oauth-client.AC6: Interactive — scope-variation flows
40
+
41
+
- **test-oauth-client.AC6.1 Success:** A full-grant approve flow records the client correctly performing PAR (with PKCE S256 + DPoP), an authorize redirect, and a token exchange with refresh-token capture; all conformance checks emit `Pass`.
42
+
- **test-oauth-client.AC6.2 Success:** A partial-grant approve flow (fake AS grants a strict subset of requested scopes) records the client correctly storing the granted (not requested) scope set; check emits `Pass` if the client surfaces or honors the actual grant.
43
+
- **test-oauth-client.AC6.3 Success:** A user-denial flow (fake AS returns `error=access_denied` to the authorize redirect) records the client correctly propagating the error rather than retrying silently.
44
+
- **test-oauth-client.AC6.4 Success:** A downscoped-refresh flow records the client issuing a refresh request with a subset of the originally granted scopes when its `RelyingParty`-driven script asks for one.
45
+
- **test-oauth-client.AC6.5 Failure:** A recorded PAR request without `code_challenge` (PKCE) produces a `SpecViolation` with code `oauth_client::interactive::scope_variations::pkce_required`.
46
+
- **test-oauth-client.AC6.6 Failure:** A recorded PAR request without a `DPoP` header produces a `SpecViolation`.
47
+
48
+
### test-oauth-client.AC7: Interactive — DPoP / refresh edge flows
49
+
50
+
- **test-oauth-client.AC7.1 Success:** A DPoP-nonce rotation flow (fake AS responds 400 `use_dpop_nonce` on initial PAR with a `DPoP-Nonce` header) records the client correctly retrying with the issued nonce.
51
+
- **test-oauth-client.AC7.2 Success:** A refresh-token rotation flow records the client using the new refresh token returned with the previous token response (and not retaining the old one).
52
+
- **test-oauth-client.AC7.3 Success:** A replay-rejection flow (fake AS rejects a duplicate `jti`) records the client surfacing the failure rather than entering a retry loop.
53
+
- **test-oauth-client.AC7.4 Failure:** A recorded sequence where the client reuses a `jti` across two requests produces a `SpecViolation` with code `oauth_client::interactive::dpop_edges::jti_reused`.
54
+
- **test-oauth-client.AC7.5 Failure:** A recorded sequence where the client receives a `DPoP-Nonce` header but does not adopt it on the next request produces a `SpecViolation`.
55
+
- **test-oauth-client.AC7.6 Failure:** A recorded sequence where the client reuses a single-use refresh token after rotation produces a `SpecViolation`.
56
+
57
+
### test-oauth-client.AC8: Cross-cutting CLI / report contract
58
+
59
+
- **test-oauth-client.AC8.1 Success:** Exit code is `0` when no `SpecViolation` and no `NetworkError` are recorded.
60
+
- **test-oauth-client.AC8.2 Success:** Exit code is `1` when at least one `SpecViolation` is recorded, regardless of any `NetworkError`s present.
61
+
- **test-oauth-client.AC8.3 Success:** Exit code is `2` when at least one `NetworkError` is recorded and no `SpecViolation` is recorded.
62
+
- **test-oauth-client.AC8.4 Success:** `Advisory` and `Skipped` rows never influence the exit code.
63
+
- **test-oauth-client.AC8.5 Success:** `--verbose` toggles tracing to DEBUG level for both modes (verified by `oauth_client_cli` smoke test).
64
+
- **test-oauth-client.AC8.6 Success:** Both `NO_COLOR` env var and `--no-color` flag suppress ANSI color in the rendered report.
65
+
- **test-oauth-client.AC8.7 Success:** Every check ID follows the `oauth_client::<stage>::<check>` naming convention and appears verbatim in at least one insta snapshot under `tests/snapshots/`.
66
+
- **test-oauth-client.AC8.8 Success:** Every diagnostic code follows the same naming convention and appears verbatim in at least one insta snapshot.
67
+
- **test-oauth-client.AC8.9 Success:** `atproto-devtool test oauth client --help` and `atproto-devtool test oauth client interactive --help` each render distinct argument inventories with the interactive-only flags (`--public-base-url`, `--port`) appearing only on the latter.
68
+
69
+
---
70
+
71
+
## Codebase verification findings
72
+
73
+
- `assert_cmd` is already in `[dev-dependencies]` for the labeler CLI
74
+
smoke tests (`tests/labeler_cli.rs`). No new dep required.
75
+
- `LabelerReport::exit_code()` semantics (`SpecViolation` → 1, else
76
+
`NetworkError` → 2, else 0) are inherited by `OauthClientReport` via
77
+
Phase 3's newtype wrap.
78
+
- `src/cli.rs:32-42` wires `--verbose` to tracing `DEBUG` globally. The
79
+
`cli.no_color` flag is threaded into the command's `run` method and
80
+
OR'd with the command-level `--no-color`. `NO_COLOR` env is honored by
81
+
the tracing subscriber and by miette automatically; Phase 8 ensures
82
+
rendered reports also honor it (verify via `RenderConfig.no_color`).
83
+
84
+
---
85
+
86
+
## Implementation tasks
87
+
88
+
<!-- START_SUBCOMPONENT_A (tasks 1-2) -->
89
+
90
+
**Subcomponent A: Scope variation and DPoP edge sub-stages.**
91
+
92
+
<!-- START_TASK_1 -->
93
+
### Task 1: Scope variation sub-stage
94
+
95
+
**Verifies:** AC6.1, AC6.2, AC6.3, AC6.4, AC6.5, AC6.6.
96
+
97
+
**Files:**
98
+
- Create directory: `src/commands/test/oauth/client/interactive/`.
99
+
- Create: `src/commands/test/oauth/client/interactive/scope_variations.rs`:
100
+
```rust
101
+
use std::sync::Arc;
102
+
103
+
use crate::commands::test::oauth::client::fake_as::ServerHandle;
104
+
use crate::common::oauth::clock::Clock;
105
+
use crate::common::oauth::relying_party::{RelyingParty, ClientKind, ParRequest, AuthorizeOutcome};
106
+
use crate::common::report::{CheckResult, CheckStatus, Stage, blocked_by, skipped_with_reason};
107
+
108
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
109
+
pub enum Check {
110
+
FullGrantApprove, // oauth_client::interactive::scope_variations::full_grant_approve
111
+
PartialGrantApprove,
112
+
UserDenialPropagated,
113
+
DownscopedRefresh,
114
+
PkceRequired, // diagnostic code oauth_client::interactive::scope_variations::pkce_required
115
+
DpopRequired,
116
+
}
117
+
118
+
pub async fn run(
119
+
server: &ServerHandle,
120
+
rp_factory: &dyn RpFactory,
121
+
clock: Arc<dyn Clock>,
122
+
) -> Vec<CheckResult> { /* ... */ }
123
+
```
124
+
125
+
Each of the four approve/partial/deny/downscope tests drives a fresh RP
126
+
against the fake AS with a specific `FlowScript` set on `AppState`, then
127
+
snapshots `server.requests.snapshot()` and asserts:
128
+
- `FullGrantApprove`: snapshot shows PAR (with PKCE S256 + DPoP) + 1
129
+
authorize + 1 token with refresh_token returned. `Check::FullGrantApprove.pass()`.
130
+
- `PartialGrantApprove`: AS grants `atproto` only (client requested
131
+
`atproto repo:<nsid>?action=create`). Token response carries only
132
+
`scope="atproto"`. Check passes **if** subsequent RP introspection
133
+
sees the narrower scope in `TokenResponse.scope`. This tests the
134
+
fake AS behavior, not the client's mental model — the client under
135
+
test doesn't exist in the RP-driven loop (the RP is the probe).
136
+
Revise: this check asserts that the recorded token response had
137
+
`scope != requested_scope` AND the RP did not retry with the
138
+
original scope. Since the RP doesn't auto-retry on partial grants,
139
+
pass is automatic when the AS returns a narrower scope.
140
+
- `UserDenialPropagated`: authorize responds with
141
+
`?error=access_denied&state=...`. RP's `do_authorize` returns
142
+
`AuthorizeOutcome::Error { error: "access_denied", .. }` and
143
+
does not retry. Snapshot confirms 1 PAR + 1 authorize + 0 token
144
+
requests.
145
+
- `DownscopedRefresh`: full-grant first, then refresh with a narrower
146
+
scope. Snapshot has `grant_type=refresh_token&scope=<narrower>` in
147
+
the final token request. Assert `TokenResponse.scope` matches the
148
+
narrower set.
149
+
150
+
For AC6.5 / AC6.6 (Failure ACs):
151
+
- Craft a "broken RP" mode that the test can toggle: `send_par_without_pkce()` and `send_par_without_dpop()`. These are test-only helper methods on `RelyingParty` guarded behind a `#[cfg(test)]` or a public-but-marked-as-testing builder. Preferred: add a `RelyingParty::send_raw_par(&self, url: &Url, body: &[u8], dpop: Option<&str>)` method that tests use to submit hand-rolled bodies. This exposes a safe test-surface without compromising the production-correctness of the normal flow.
152
+
- When the test sends a PAR body lacking `code_challenge`, inspect the
153
+
resulting logged request and emit `Check::PkceRequired.spec_violation`
154
+
with diagnostic code
155
+
`oauth_client::interactive::scope_variations::pkce_required`.
156
+
- When the test sends a PAR without the `DPoP` header, emit
157
+
`Check::DpopRequired.spec_violation`.
158
+
159
+
**RpFactory indirection:** the factory owns the long-lived configuration
160
+
(clock, default seed) and only takes per-flow `client_id` / `kind` when
161
+
building. `trait RpFactory` in `relying_party.rs`:
162
+
```rust
163
+
pub trait RpFactory: Send + Sync {
164
+
/// Build a fresh RelyingParty for a flow run. Implementations own
165
+
/// the clock and RNG-seed state — callers only supply the target
166
+
/// client's identity and kind.
167
+
fn build(&self, client_id: Url, kind: ClientKind) -> RelyingParty;
168
+
}
169
+
170
+
/// Production factory: wraps a `RealClock` and derives per-flow seeds
171
+
/// from an internal counter so successive builds yield distinct keys
172
+
/// without needing OS randomness.
173
+
pub struct DefaultRpFactory { /* clock + counter */ }
174
+
175
+
/// Deterministic factory used by tests: holds a fixed clock and a
176
+
/// fixed base seed; each build yields a fresh RP with the same
177
+
/// material, so repeated test runs produce identical requests,
178
+
/// identical DPoP proofs, and identical snapshots.
179
+
pub struct DeterministicRpFactory { /* clock: Arc<FakeClock>, base_seed: [u8; 32] */ }
180
+
```
181
+
The factory does *not* take `rng_seed` per-call — that's its own
182
+
concern. The factory's `build` signature has **no load-bearing inputs
183
+
beyond `client_id` and `kind`**, which is the actual reason for the
184
+
abstraction: tests and production differ in their clock/RNG policy, not
185
+
in the per-flow parameters.
186
+
187
+
**Commit:** `feat(oauth-client): interactive scope variation sub-stage with AC6 check inventory`
188
+
189
+
<!-- END_TASK_1 -->
190
+
191
+
<!-- START_TASK_2 -->
192
+
### Task 2: DPoP edges sub-stage
193
+
194
+
**Verifies:** AC7.1, AC7.2, AC7.3, AC7.4, AC7.5, AC7.6.
195
+
196
+
**Files:**
197
+
- Create: `src/commands/test/oauth/client/interactive/dpop_edges.rs` with
198
+
parallel structure to `scope_variations.rs`:
199
+
- `Check::NonceRotation` — script `FlowScript::DpopNonceRetryOnPar { nonce: "nonce-1" }`. After the retry, assert that the snapshot has exactly two PAR entries, and the second's DPoP proof contains `nonce = "nonce-1"` in claims (decode it via `common::oauth::jws::verify_jws` with the inline JWK from the DPoP header).
200
+
- `Check::RefreshRotation` — two back-to-back token requests. First a code exchange; second a refresh with `refresh_token = rt1`; server returns `rt2`. Third refresh should use `rt2`, not `rt1` (the RP must persist the new one). Test drives the RP's `do_refresh` with the updated rt and asserts the snapshot body carries `refresh_token=rt2`.
201
+
- `Check::ReplayRejection` — script: second PAR reuses a `jti`. Fake AS rejects with 401. RP does not retry. Snapshot has 1 failed POST.
202
+
- `Check::JtiReuseViolation` — drive the "broken RP" to send two PARs with the **same** `jti`. Second is rejected; emit SpecViolation on `Check::JtiReuseViolation` with code `oauth_client::interactive::dpop_edges::jti_reused`.
203
+
- `Check::NonceIgnoredViolation` — fake AS responds with `DPoP-Nonce: n2` but the RP's next DPoP proof lacks `nonce=n2` (broken-RP helper). Emit SpecViolation.
204
+
- `Check::RefreshTokenReuseViolation` — broken-RP helper: POST refresh with `rt1` after `rt1` was already rotated. Fake AS rejects. Emit SpecViolation.
205
+
206
+
**Commit:** `feat(oauth-client): interactive DPoP edges sub-stage with AC7 check inventory`
207
+
208
+
<!-- END_TASK_2 -->
209
+
210
+
<!-- END_SUBCOMPONENT_A -->
211
+
212
+
<!-- START_SUBCOMPONENT_B (tasks 3-5) -->
213
+
214
+
**Subcomponent B: Sub-stage wiring + end-to-end regression + CLI smoke.**
215
+
216
+
<!-- START_TASK_3 -->
217
+
### Task 3: Wire sub-stages into `interactive.rs`
218
+
219
+
**Verifies:** AC8.7 (snapshot IDs appear).
220
+
221
+
**Files:**
222
+
- Modify: `src/commands/test/oauth/client/interactive.rs`:
223
+
- After the Phase 7 happy-path block, run both sub-stages in sequence:
224
+
```rust
225
+
let scope_results = scope_variations::run(&server, &rp_factory, clock.clone()).await;
226
+
interactive_results.extend(scope_results);
227
+
228
+
let dpop_results = dpop_edges::run(&server, &rp_factory, clock.clone()).await;
229
+
interactive_results.extend(dpop_results);
230
+
```
231
+
- Each sub-stage uses its own `blocked_by` gate table. For
232
+
`scope_variations`: depends on `oauth_client::metadata::scope_present`,
233
+
`oauth_client::metadata::grant_types_includes_authorization_code`,
234
+
`oauth_client::metadata::dpop_bound_required`. For `dpop_edges`:
235
+
depends on the same plus `oauth_client::metadata::response_types_is_code`.
236
+
- Modify: `src/commands/test/oauth/client.rs` — no functional changes;
237
+
verify CLI paths still compile.
238
+
239
+
**Commit:** `feat(oauth-client): orchestrate scope-variations and dpop-edges sub-stages from interactive stage`
240
+
241
+
<!-- END_TASK_3 -->
242
+
243
+
<!-- START_TASK_4 -->
244
+
### Task 4: End-to-end regression test + CLI smoke tests
245
+
246
+
**Verifies:** AC8.1, AC8.2, AC8.3, AC8.4, AC8.5, AC8.6, AC8.7, AC8.8, AC8.9.
247
+
248
+
**Files:**
249
+
- Create: `tests/oauth_client_endtoend.rs`:
250
+
- One `#[tokio::test]` that runs the full pipeline against a real
251
+
spawned fake AS with a deterministic seed, a static-mode client
252
+
that's known-good (inline metadata fixture reused from Phase 4),
253
+
and snapshots the entire rendered report.
254
+
- Snapshot file: `oauth_client_endtoend__full_pipeline_all_pass.snap`.
255
+
- Purpose: regression baseline showing the exact interleaving of
256
+
discovery, metadata, jwks, interactive checks in a healthy run.
257
+
258
+
- Create: `tests/oauth_client_cli.rs` (mirrors `tests/labeler_cli.rs`).
259
+
Note: every smoke test uses `http://localhost` as the target. This is
260
+
chosen deliberately because the Phase 3 pipeline treats
261
+
`http://localhost` as a loopback target that emits `Skipped` rows
262
+
without making any network calls — the CLI runs are fully
263
+
deterministic and never touch external state. If a future AC ever
264
+
makes the loopback path produce a non-`Skipped` outcome (for instance,
265
+
a SpecViolation for a loopback client whose `client_id` is
266
+
non-canonical), these tests will need their target argument updated
267
+
to preserve the "zero-I/O, exit 0" invariant they depend on.
268
+
269
+
```rust
270
+
use assert_cmd::Command;
271
+
272
+
#[test]
273
+
fn help_flag_accepted() {
274
+
Command::cargo_bin("atproto-devtool").unwrap()
275
+
.args(["test", "oauth", "client", "--help"])
276
+
.assert().success()
277
+
.stdout(predicates::str::contains("target"));
278
+
}
279
+
280
+
#[test]
281
+
fn interactive_help_has_port_and_public_base_url() {
282
+
Command::cargo_bin("atproto-devtool").unwrap()
283
+
.args(["test", "oauth", "client", "interactive", "--help"])
284
+
.assert().success()
285
+
.stdout(predicates::str::contains("--port"))
286
+
.stdout(predicates::str::contains("--public-base-url"));
287
+
// AC8.9.
288
+
}
289
+
290
+
#[test]
291
+
fn static_help_does_not_have_port_or_public_base_url() {
292
+
Command::cargo_bin("atproto-devtool").unwrap()
293
+
.args(["test", "oauth", "client", "--help"])
294
+
.assert().success()
295
+
.stdout(predicates::str::contains("--port").not())
296
+
.stdout(predicates::str::contains("--public-base-url").not());
297
+
// AC8.9.
298
+
}
299
+
300
+
#[test]
301
+
fn verbose_flag_accepted() {
302
+
Command::cargo_bin("atproto-devtool").unwrap()
303
+
.args(["test", "oauth", "client", "http://localhost", "--verbose"])
304
+
.assert().success();
305
+
// AC8.5.
306
+
}
307
+
308
+
#[test]
309
+
fn no_color_env_var_suppresses_color() {
310
+
let output = Command::cargo_bin("atproto-devtool").unwrap()
311
+
.args(["test", "oauth", "client", "http://localhost"])
312
+
.env("NO_COLOR", "1")
313
+
.output().unwrap();
314
+
assert!(!String::from_utf8_lossy(&output.stdout).contains("\x1b["));
315
+
// AC8.6.
316
+
}
317
+
318
+
#[test]
319
+
fn no_color_flag_suppresses_color() {
320
+
let output = Command::cargo_bin("atproto-devtool").unwrap()
321
+
.args(["test", "oauth", "client", "http://localhost", "--no-color"])
322
+
.output().unwrap();
323
+
assert!(!String::from_utf8_lossy(&output.stdout).contains("\x1b["));
324
+
// AC8.6.
325
+
}
326
+
327
+
// Exit-code tests drive the RP against a deterministic fake AS and
328
+
// assert the exit code. These require spawning the fake AS from the
329
+
// test — can't rely on external servers. Approach: run these as
330
+
// tokio-based integration tests in a separate file (not cli.rs),
331
+
// since assert_cmd is synchronous. Covered by additional tests in
332
+
// tests/oauth_client_endtoend.rs for exit-code pinning.
333
+
```
334
+
335
+
- Modify: `tests/oauth_client_endtoend.rs` — add exit-code pinning tests:
336
+
- `exit_zero_when_all_pass` — happy path; assert `report.exit_code() == 0`. AC8.1.
337
+
- `exit_one_on_any_spec_violation` — seed a dpop-bound-false metadata; assert exit 1. AC8.2.
338
+
- `exit_two_on_network_error_alone` — seed transport error on jwks_uri (for confidential); assert exit 2. AC8.3.
339
+
- `advisory_and_skipped_do_not_affect_exit` — assert AC8.4 directly against `LabelerReport::exit_code` (unit-level, not integration-level): construct a `LabelerReport` with `LabelerReport::new(...)`, call `report.record(...)` with handcrafted `CheckResult`s that have `status: CheckStatus::Advisory` and `status: CheckStatus::Skipped`, then assert `report.exit_code() == 0`. `CheckResult` is a public struct (Phase 1 promoted it to `common::report`), so hand-construction via struct literal is supported without any `.advisory()` helper. This keeps the test independent of any check's helper surface. AC8.4.
340
+
341
+
- Create: `tests/oauth_client_check_id_coverage.rs` — an integration test
342
+
(not a unit test; it crosses stage boundaries to iterate every stage's
343
+
`Check::ALL` array, which violates the `src/` stage-locality contract).
344
+
The test:
345
+
- Reaches each stage's `Check` enum via the public re-exports
346
+
(`atproto_devtool::commands::test::oauth::client::discovery::Check`,
347
+
`::metadata::Check`, `::jwks::Check`, `::interactive::Check`, plus
348
+
the sub-stage check enums from Phase 8).
349
+
- For each `Check::ALL` entry, asserts that `.id()` appears in the
350
+
concatenated contents of `tests/snapshots/oauth_client_*.snap` at
351
+
least once.
352
+
- For every diagnostic code declared on every `Diagnostic`-deriving
353
+
error type in the oauth client tree (`oauth_client::jws::*`,
354
+
`oauth_client::metadata::*`, `oauth_client::target::*`,
355
+
`oauth_client::interactive::*`), asserts the code appears at least
356
+
once.
357
+
- Satisfies AC8.7 and AC8.8. Placement in `tests/` honors CLAUDE.md's
358
+
no-`mod.rs` rule and the stage-locality invariant in the labeler
359
+
CLAUDE.md.
360
+
361
+
**Verification:**
362
+
- `cargo test --test oauth_client_cli` passes.
363
+
- `cargo test --test oauth_client_endtoend` passes, snapshot accepted.
364
+
- Full suite: `cargo test` green.
365
+
- `cargo clippy -- -D warnings` clean.
366
+
- `cargo fmt --check` clean.
367
+
- Manual smoke: point a real atproto reference client at the fake AS
368
+
and walk through scope-variation + dpop-edges flows manually; confirm
369
+
each variant lands sensible PASS / SpecViolation / Skipped rows.
370
+
371
+
**Commit:** `test(oauth-client): add end-to-end snapshot and CLI smoke tests for AC6/AC7/AC8`
372
+
373
+
<!-- END_TASK_4 -->
374
+
375
+
<!-- START_TASK_5 -->
376
+
### Task 5: Final clean-up and docs
377
+
378
+
**Verifies:** none directly; housekeeping.
379
+
380
+
**Files:**
381
+
- Create: `docs/test-oauth-client-reachability.md` — short markdown
382
+
explaining the supported interactive-mode reachability workflows:
383
+
(a) client and devtool on the same host, default
384
+
`http://127.0.0.1:<port>` base; (b) client elsewhere, developer runs a
385
+
tunnel (cloudflared / Tailscale Funnel / ngrok) and passes the public
386
+
URL via `--public-base-url`. Note: the fake AS speaks plaintext HTTP;
387
+
the tool does not manage TLS certs or auto-spawn tunnels. (This
388
+
document was called for in the design's "Additional Considerations"
389
+
section.)
390
+
- Modify: `README.md` — add a `test oauth client` section summarizing the
391
+
two modes. Follow the existing `test labeler` section's style.
392
+
- Update: `src/commands/test/oauth/client/CLAUDE.md` — add a concise
393
+
CLAUDE.md analogous to `src/commands/test/labeler/CLAUDE.md` covering
394
+
the public entry points, the check inventory, the exit-code
395
+
semantics (inherited from `common::report`), the injection seams
396
+
(`http`, `jwks`, `clock`, `interactive`), and the "every I/O hop is a
397
+
trait object" invariant. Also note that the `FlowScript` enum drives
398
+
the fake AS's scripted responses, and that the RelyingParty is kept
399
+
deterministic under test through the injected `Clock` + RNG seed.
400
+
End with a `Last verified: YYYY-MM-DD` line.
401
+
402
+
**Verification:**
403
+
- `cargo build`, `cargo test`, `cargo clippy -- -D warnings`,
404
+
`cargo fmt --check` all clean.
405
+
406
+
**Commit:** `docs(oauth-client): README section, reachability doc, module CLAUDE.md`
407
+
408
+
<!-- END_TASK_5 -->
409
+
410
+
<!-- END_SUBCOMPONENT_B -->
411
+
412
+
---
413
+
414
+
## Phase 8 done when
415
+
416
+
- All `cargo test` binaries pass (`labeler_*`, `oauth_client_*`).
417
+
- `cargo insta review` has accepted all new/changed snapshots.
418
+
- `cargo clippy -- -D warnings` and `cargo fmt --check` clean.
419
+
- Manual end-to-end smoke against a real reference client confirms each
420
+
flow variant lands sensible PASS / SpecViolation / Skipped rows.
421
+
- Every check ID and every diagnostic code under
422
+
`oauth_client::*` appears verbatim in at least one `tests/snapshots/*.snap`
423
+
file (AC8.7 + AC8.8).
424
+
- `docs/test-oauth-client-reachability.md` is in place and links from
425
+
README.
426
+
- `src/commands/test/oauth/client/CLAUDE.md` is in place with current
427
+
verification date.
428
+
429
+
With Phase 8 complete, `atproto-devtool test oauth client` ships as
430
+
specified in `docs/design-plans/2026-04-16-test-oauth-client.md`.
+213
docs/implementation-plans/2026-04-16-test-oauth-client/test-requirements.md
+213
docs/implementation-plans/2026-04-16-test-oauth-client/test-requirements.md
···
1
+
# Test requirements: `test oauth client`
2
+
3
+
This document maps every acceptance criterion in
4
+
`docs/design-plans/2026-04-16-test-oauth-client.md` to exactly one
5
+
verification approach: either a specific automated test function in an
6
+
integration binary, or a documented human-verification step. Each AC
7
+
appears in exactly one row. When an AC is exercised incidentally by a
8
+
snapshot in another stage's binary, only the authoritative test is
9
+
listed.
10
+
11
+
Conventions used in this document:
12
+
13
+
- Test file paths are relative to the repository root
14
+
(`tests/<binary>.rs`). Each integration binary corresponds to one
15
+
Phase task group.
16
+
- Test function names are the `#[tokio::test]` or `#[test]` identifiers
17
+
introduced by the referenced Phase task. Where Phase task descriptions
18
+
name the test directly, that name is used verbatim; where the task
19
+
describes "a test that does X", a deterministic name is inferred from
20
+
the task's snapshot file name or the fixture directory it drives.
21
+
- Phase / Task citations refer to
22
+
`docs/implementation-plans/2026-04-16-test-oauth-client/phase_NN.md`.
23
+
24
+
---
25
+
26
+
## AC1: Static mode — target parsing and discovery
27
+
28
+
All AC1 cases land in Phase 3. The invalid-scheme case is covered by a
29
+
unit test in `src/commands/test/oauth/client/pipeline.rs` (Phase 3
30
+
Task 1) because `parse_target` is called before any pipeline runs;
31
+
surfacing it as an integration test against the CLI would duplicate the
32
+
unit-level coverage without adding signal. All other cases go through
33
+
the discovery stage runner and end with an insta snapshot of the
34
+
rendered report.
35
+
36
+
| AC ID | Text (verbatim) | Verification | Where |
37
+
|---|---|---|---|
38
+
| `test-oauth-client.AC1.1` | An `https://...` `client_id` URL is fetched over the shared HTTP client and the response body is captured as `RawMetadata::Document`. | Automated test | `tests/oauth_client_discovery.rs::https_confidential_happy_discovery` (Phase 3 Task 5). `FakeHttpClient` returns a 200 with the `https_confidential_happy/metadata.json` fixture; snapshot pins three Pass rows. |
39
+
| `test-oauth-client.AC1.2` | `http://localhost[:port][/path]` is recognized as a loopback target and synthesizes `RawMetadata::Implicit` without making an HTTP request. | Automated test | `tests/oauth_client_discovery.rs::loopback_root_produces_skip_rows` (Phase 3 Task 5). Target `http://localhost` with `FakeHttpClient` untouched — asserts no fetch and pins the two `Skipped` rows with reason `"metadata is implicit for loopback clients"`. |
40
+
| `test-oauth-client.AC1.3` | `http://127.0.0.1[:port][/path]` is treated identically to `localhost` for loopback recognition. | Automated test | `tests/oauth_client_discovery.rs::loopback_127_0_0_1` (Phase 3 Task 5). Target `http://127.0.0.1:3000/`; asserts the same skip-row pattern as `loopback_root_produces_skip_rows`. |
41
+
| `test-oauth-client.AC1.4` | A target that is neither HTTPS nor a recognized loopback form is rejected before the pipeline starts, with a `parse_target` error surfaced as a clap argument error (non-zero exit, helpful message). | Automated test | `src/commands/test/oauth/client/pipeline.rs::tests::ftp_scheme_rejected` (Phase 3 Task 1). Unit test on `parse_target`; asserts `TargetParseError::UnsupportedScheme` with diagnostic `code("oauth_client::target::unsupported_scheme")`. The CLI-level rendering path is exercised operationally per Phase 3 Task 3. |
42
+
| `test-oauth-client.AC1.5` | An HTTPS `client_id` returning a non-2xx status produces a `NetworkError` `CheckResult` with the URL and status in the diagnostic; downstream stages emit `Skipped` rows blocked on `discovery::metadata_document_fetchable`. | Automated test | `tests/oauth_client_discovery.rs::https_404_produces_network_error` (Phase 3 Task 5). `FakeHttpClient` seeded with a 404; asserts exit code 2 and pins the `NetworkError` row plus the `blocked by oauth_client::discovery::metadata_document_fetchable` skip on `metadata_is_json`. |
43
+
| `test-oauth-client.AC1.6` | An HTTPS `client_id` returning a body that is not parseable JSON produces a `SpecViolation` with the content-type and a snippet of the body in the diagnostic. | Automated test | `tests/oauth_client_discovery.rs::https_not_json_produces_spec_violation` (Phase 3 Task 5). `FakeHttpClient` returns 200 with the `https_not_json/not_json.txt` fixture; asserts exit code 1 and pins a source-spanned `SpecViolation` on `metadata_is_json` including the captured content type. |
44
+
| `test-oauth-client.AC1.7` | A loopback target with an empty path, an explicit port, or a non-empty path each parse identically and emit the same set of `CheckResult`s. | Automated test | `tests/oauth_client_discovery.rs::loopback_with_port_produces_same_skip_rows` (Phase 3 Task 5). Target `http://localhost:8080/client.json`; snapshot matches `loopback_root_produces_skip_rows` modulo the target header, pinning the equivalence. |
45
+
46
+
---
47
+
48
+
## AC2: Static mode — metadata document validation
49
+
50
+
All AC2 cases land in Phase 4. Each case is one integration test in
51
+
`tests/oauth_client_metadata.rs` (Phase 4 Task 4), driving
52
+
`run_pipeline` with a fixture under `tests/fixtures/oauth_client/metadata/`.
53
+
54
+
| AC ID | Text (verbatim) | Verification | Where |
55
+
|---|---|---|---|
56
+
| `test-oauth-client.AC2.1` | A confidential web client document with all required fields (`application_type=web`, `response_types=[code]`, `grant_types=[authorization_code]`, `dpop_bound_access_tokens=true`, `redirect_uris` HTTPS, `token_endpoint_auth_method=private_key_jwt`, `jwks` or `jwks_uri`, valid `scope`) passes every metadata-stage check. | Automated test | `tests/oauth_client_metadata.rs::confidential_happy` (Phase 4 Task 4). Fixture: `confidential_happy/metadata.json`. Asserts exit 0 and snapshot pins every metadata check as `Pass`. |
57
+
| `test-oauth-client.AC2.2` | A public web client document (no JWKS, `token_endpoint_auth_method=none`) passes. | Automated test | `tests/oauth_client_metadata.rs::public_happy` (Phase 4 Task 4). Fixture: `public_happy/metadata.json`. |
58
+
| `test-oauth-client.AC2.3` | A native client document with custom-scheme `redirect_uris` whose scheme matches the reverse-domain of the `client_id` host passes. | Automated test | `tests/oauth_client_metadata.rs::native_happy` (Phase 4 Task 4). Fixture: `native_happy/metadata.json` with `client_id=https://app.example.com/...` and `redirect_uris=["com.example.app:/cb"]`. |
59
+
| `test-oauth-client.AC2.4` | `dpop_bound_access_tokens` absent or `false` produces a `SpecViolation` with stable code `oauth_client::metadata::dpop_bound_required`. | Automated test | `tests/oauth_client_metadata.rs::dpop_bound_false` (Phase 4 Task 4). Fixture: `dpop_bound_false/metadata.json`. Asserts exit 1 and snapshot pins the diagnostic code verbatim. |
60
+
| `test-oauth-client.AC2.5` | A confidential client without `jwks` or `jwks_uri` produces a `SpecViolation` with code `oauth_client::metadata::confidential_requires_jwks`. | Automated test | `tests/oauth_client_metadata.rs::confidential_missing_jwks` (Phase 4 Task 4). Fixture: `confidential_missing_jwks/metadata.json`. |
61
+
| `test-oauth-client.AC2.6` | A public client whose `token_endpoint_auth_method` is anything other than `none` produces a `SpecViolation`. | Automated test | `tests/oauth_client_metadata.rs::public_with_token_endpoint_auth` (Phase 4 Task 4). Fixture: `public_with_token_endpoint_auth/metadata.json` (public web with `token_endpoint_auth_method=private_key_jwt`). |
62
+
| `test-oauth-client.AC2.7` | A native client whose `redirect_uri` scheme does not match the reverse-domain of the `client_id` host produces a `SpecViolation`. | Automated test | `tests/oauth_client_metadata.rs::native_redirect_scheme_mismatch` (Phase 4 Task 4). Fixture: `native_redirect_scheme_mismatch/metadata.json` (`redirect_uris=["app.wrong.example:/cb"]` against `client_id=https://app.example.com/...`). |
63
+
| `test-oauth-client.AC2.8` | A `scope` field that does not parse against the atproto permission grammar produces a `SpecViolation` with the offending token highlighted via miette source span. | Automated test | `tests/oauth_client_metadata.rs::scope_grammar_invalid` (Phase 4 Task 4). Fixture: `scope_grammar_invalid/metadata.json` with `scope="atproto invalid-scope-token"`. Snapshot pins both the diagnostic code `oauth_client::metadata::scope_grammar` and the byte-offset span on the offending token. |
64
+
| `test-oauth-client.AC2.9` | Every metadata-document validation check on a loopback target emits `Skipped` with reason `"metadata is implicit for loopback clients"`. | Automated test | `tests/oauth_client_metadata.rs::loopback_skips_all_metadata_checks` (Phase 4 Task 4). Target `http://localhost/`; snapshot pins every metadata `Check::ALL` row as `Skipped` with the exact reason string. |
65
+
66
+
---
67
+
68
+
## AC3: Static mode — JWKS validation
69
+
70
+
All AC3 cases land in Phase 5. AC3 is the first stage that exercises
71
+
the `JwksFetcher` seam (via `FakeJwksFetcher` in `tests/common/mod.rs`,
72
+
Phase 5 Task 4) for `jwks_uri` cases. Inline `jwks` cases bypass the
73
+
fetcher entirely. Per the design-plan rationalization, `ES256K` JWKs
74
+
are accepted by the parser (Phase 2) and should pass JWKS parse/stage
75
+
checks — signing/verification are never attempted, so the "sign/verify
76
+
not implemented for ES256K" decision has no effect on these ACs.
77
+
78
+
| AC ID | Text (verbatim) | Verification | Where |
79
+
|---|---|---|---|
80
+
| `test-oauth-client.AC3.1` | Inline `jwks` containing valid ES256 keys with unique `kid`, `alg`, and `use=sig` passes every JWKS check. | Automated test | `tests/oauth_client_jwks.rs::inline_es256_happy` (Phase 5 Task 5). Fixture: `inline_es256_happy/metadata.json` with inline jwks; snapshot pins every JWKS check row as `Pass`. |
81
+
| `test-oauth-client.AC3.2` | External `jwks_uri` returning a valid JWKS document is fetched (over the `JwksFetcher` seam) and validated identically. | Automated test | `tests/oauth_client_jwks.rs::uri_es256_happy` (Phase 5 Task 5). Fixture: `uri_es256_happy/metadata.json` plus `uri_es256_happy/jwks.json`; `FakeJwksFetcher` seeded with the latter; snapshot pins `jwks_uri_fetchable` as `Pass`. |
82
+
| `test-oauth-client.AC3.3` | A `jwks_uri` returning a non-2xx status produces a `NetworkError` with the URL in the diagnostic. | Automated test | `tests/oauth_client_jwks.rs::uri_unreachable` (Phase 5 Task 5). Fixture: `uri_unreachable/metadata.json`; `FakeJwksFetcher::add_transport_error` seeded against the jwks_uri. Asserts exit 2 and pins the `oauth_client::jws::jwks_uri_unreachable` diagnostic code. |
83
+
| `test-oauth-client.AC3.4` | A JWKS containing two keys with the same `kid` produces a `SpecViolation` with both `kid`s and their offsets highlighted via miette source span. | Automated test | `tests/oauth_client_jwks.rs::duplicate_kids` (Phase 5 Task 5). Fixture: `duplicate_kids/jwks.json` with two keys sharing `kid=k1`; snapshot pins the `keys_have_unique_kids` SpecViolation with the collection-label spans produced by `all_spans_for_quoted_literal`. |
84
+
| `test-oauth-client.AC3.5` | A key without an `alg` field produces a `SpecViolation`. | Automated test | `tests/oauth_client_jwks.rs::missing_alg` (Phase 5 Task 5). Fixture: `missing_alg/jwks.json`. Parser (Phase 2) returns `ParsedJwk { alg: None, alg_raw: None }`; the stage maps this to `Check::KeysHaveAlg.spec_violation(...)` per the authoritative mapping table in Phase 5 Task 2. |
85
+
| `test-oauth-client.AC3.6` | A key with `use` other than `sig` (or absent) produces a `SpecViolation`. | Automated test | `tests/oauth_client_jwks.rs::wrong_use` (Phase 5 Task 5). Fixture: `wrong_use/jwks.json` with `use=enc`; pins `keys_use_signing_use` SpecViolation. |
86
+
| `test-oauth-client.AC3.7` | A key advertising a non-modern algorithm (RS1, MD5, etc.) produces a `SpecViolation`; ECDSA P-256 / ES256K and stronger algorithms pass. | Automated test | `tests/oauth_client_jwks.rs::weak_alg_rs1` (Phase 5 Task 5). Fixture: `weak_alg_rs1/jwks.json` with `alg=RS1`; parser preserves the raw alg in `alg_raw`; stage maps to `Check::AlgsAreModernEc.spec_violation(...)`. ES256K pass-through is covered by the inline parser tests in Phase 2 Task 4 (`jws::tests::es256k_jwk_parse_accepted_verify_refused`) — those tests provide the pinned Pass of the positive side without requiring a dedicated JWKS integration fixture. |
87
+
| `test-oauth-client.AC3.8` | Public-client and native-client JWKS checks emit `Skipped` rows with reason `"jwks not required for {kind} clients"` rather than running. | Automated test | `tests/oauth_client_jwks.rs::public_client_skipped` (Phase 5 Task 5). Fixture: `public_client_skipped/metadata.json`; snapshot pins every JWKS check as `Skipped` with the reason string that encodes the client kind. |
88
+
89
+
---
90
+
91
+
## AC4: Interactive mode — fake AS infrastructure
92
+
93
+
AC4 is split across Phases 6 and 7. Phase 6 lands the axum server,
94
+
routes, and request log; Phase 7 lands the CLI-level "spawn, print
95
+
identity, wait for Ctrl-C" behavior that AC4.1 describes. The
96
+
"until interrupted" half of AC4.1 is a CLI-blocking semantic that
97
+
cannot be verified inside a test without racing against signal
98
+
delivery, so the test-mode path (`DriveRpInProcess`) covers the
99
+
spawn-and-print side automatically and the interrupt side is a
100
+
human-verification step.
101
+
102
+
| AC ID | Text (verbatim) | Verification | Where |
103
+
|---|---|---|---|
104
+
| `test-oauth-client.AC4.1` | `test oauth client interactive <target>` spawns the `axum`-based fake AS on a port, prints the synthetic handle / DID for the user to enter into their client, and waits for inbound requests until interrupted. | Human verification | Per Phase 7 Task 4 "Done when": run `cargo run -- test oauth client interactive http://localhost/client.json`. Verify that the printed output contains the lines starting with `Fake atproto authorization server is running.`, `Handle:`, `DID:`, and `Base:`; then send SIGINT (Ctrl-C) and verify that the process exits cleanly with a rendered report. The spawn-and-print sub-behavior is additionally exercised in every `tests/oauth_client_interactive.rs` test that uses `DriveRpInProcess`, but the "waits until interrupted" semantics rely on `tokio::signal::ctrl_c()` in CLI mode and are not reached in that drive mode. |
105
+
| `test-oauth-client.AC4.2` | Without `--public-base-url`, the fake AS binds to `127.0.0.1:<port>` and every served metadata document references `http://127.0.0.1:<port>` as the active base. | Automated test | `tests/oauth_client_interactive.rs::serves_as_metadata_document` (Phase 6 Task 5). Spawns the server with `FakeAsOptions { bind_port: None, public_base_url: None }`, fetches `/.well-known/oauth-authorization-server`, and asserts `issuer == "http://127.0.0.1:<port>"` and that every endpoint URL in the served document references the same base. |
106
+
| `test-oauth-client.AC4.3` | With `--public-base-url <https-url>`, every URL appearing in served metadata documents (DID document `service` entries, AS metadata endpoint URLs, PRM `authorization_servers` array) is rewritten to use the public URL. | Automated test | `tests/oauth_client_interactive.rs::public_base_url_rewrites_served_urls` (Phase 6 Task 5). Spawns with `public_base_url: Some("https://funnel.example.com".parse()?)`; asserts `issuer`, `pushed_authorization_request_endpoint`, `authorization_endpoint`, `token_endpoint`, PRM `resource` / `authorization_servers`, and the DID document `serviceEndpoint` all reference the public URL. |
107
+
| `test-oauth-client.AC4.4` | The well-known endpoints (`/<did-web-path>/did.json`, `/.well-known/oauth-protected-resource`, `/.well-known/oauth-authorization-server`) return spec-shaped JSON validatable by an external client / parser. | Automated test | `tests/oauth_client_interactive.rs::serves_did_json_document` + `serves_prm_document` + `serves_as_metadata_document` (Phase 6 Task 5). The three tests jointly assert that each endpoint returns JSON with every spec-required field (`issuer`, `authorization_endpoint`, `token_endpoint`, `pushed_authorization_request_endpoint`, `require_pushed_authorization_requests: true`, `code_challenge_methods_supported: ["S256"]`, `dpop_signing_alg_values_supported: ["ES256"]`, and PRM `resource` + `authorization_servers`). |
108
+
| `test-oauth-client.AC4.5` | Every inbound request to PAR / authorize / token is appended to the `RequestLog` with timestamp, route, headers, and body bytes preserved verbatim. | Automated test | `tests/oauth_client_interactive.rs::par_endpoint_records_request_and_returns_request_uri` (Phase 6 Task 5). Sends a form-encoded PAR POST and asserts `handle.requests.snapshot()` contains a `LoggedRequest` with method `"POST"`, path `"/oauth/par"`, body bytes matching the sent bytes verbatim, `timestamp_unix` equal to the `FakeClock` value, and the sent headers preserved byte-for-byte. |
109
+
| `test-oauth-client.AC4.6` | A `--port` value that is already bound, out of range, or otherwise unbindable produces a clear startup error and a non-zero exit before the synthetic identity is printed. | Automated test | `tests/oauth_client_interactive.rs::bind_unbindable_port_returns_error` (Phase 6 Task 5). Binds a `TcpListener` on `127.0.0.1:X`, then calls `ServerHandle::bind` with `bind_port: Some(X)` and asserts the result is `Err(BindError::BindFailed { .. })` with diagnostic code `oauth_client::interactive::bind_failed`. |
110
+
111
+
---
112
+
113
+
## AC5: Cross-mode dependency gating
114
+
115
+
All AC5 cases land in Phase 7. The gating logic is implemented in
116
+
`interactive::run` against the `StaticGating` view of prior stages'
117
+
`CheckResult`s; tests drive the full pipeline with the in-process
118
+
`RpFactory` and inspect the rendered report.
119
+
120
+
| AC ID | Text (verbatim) | Verification | Where |
121
+
|---|---|---|---|
122
+
| `test-oauth-client.AC5.1` | When `interactive` is invoked, the static stages (discovery, metadata, JWKS) run first and their `CheckResult`s appear in the report ahead of the interactive stage's results. | Automated test | `tests/oauth_client_interactive.rs::interactive_static_rows_precede_interactive_rows` (Phase 7 Task 4). The end-to-end test driving `RelyingParty` through the fake AS snapshots the rendered report and pins the ordering `Discovery` → `Metadata` → `JWKS` → `Interactive`. |
123
+
| `test-oauth-client.AC5.2` | When all static checks pass, every interactive sub-stage runs its full check inventory. | Automated test | `tests/oauth_client_interactive.rs::full_static_pass_runs_full_interactive_inventory` (Phase 7 Task 4). Seeds a fully-conformant client-metadata fixture, runs `DriveRpInProcess`, and asserts every entry in each interactive `Check::ALL` appears in the report with a non-`Skipped-with-blocked_by` status. |
124
+
| `test-oauth-client.AC5.3` | When a static check fails AND it gates a specific interactive flow, that flow's checks emit `Skipped` with reason `"blocked by <stage>::<check_id>"` (matching the labeler `Check::blocked_by` rendering verbatim). | Automated test | `tests/oauth_client_interactive.rs::partial_static_failure_blocks_interactive` (Phase 7 Task 4). Seeds metadata with `dpop_bound_access_tokens=false`; asserts `oauth_client::interactive::client_reached_par` emits `Skipped` with reason `"blocked by oauth_client::metadata::dpop_bound_required"`. |
125
+
| `test-oauth-client.AC5.4` | When a static check fails but no interactive flow declares a `blocked_by` against it, the interactive flows still execute their full inventories. | Automated test | `tests/oauth_client_interactive.rs::unrelated_static_failure_does_not_block_interactive` (Phase 7 Task 4). Seeds a fixture that fails an uninvolved static check (e.g., a client_id mismatch) and asserts interactive checks still run their full inventory — no blocked-by skip rows appear. |
126
+
| `test-oauth-client.AC5.5` | Every `Skipped` reason references a specific failing check ID, never a generic description like `"a prerequisite failed"`. | Automated test | `tests/oauth_client_interactive.rs::partial_static_failure_blocks_interactive` (Phase 7 Task 4). The same test as AC5.3; the assertion on the exact reason string `"blocked by oauth_client::metadata::dpop_bound_required"` pins the specific-check-ID contract verbatim. The `blocked_by` helper in `src/common/report.rs` enforces this rendering for every call site. |
127
+
128
+
---
129
+
130
+
## AC6: Interactive — scope-variation flows
131
+
132
+
All AC6 cases land in Phase 8 Task 1. The sub-stage runner
133
+
`scope_variations::run` drives a fresh `RelyingParty` per flow against
134
+
the shared `fake_as::ServerHandle`, scripted via `FlowScript` on
135
+
`AppState`. Each test sets the script, drives the RP, and snapshots
136
+
`server.requests.snapshot()`.
137
+
138
+
| AC ID | Text (verbatim) | Verification | Where |
139
+
|---|---|---|---|
140
+
| `test-oauth-client.AC6.1` | A full-grant approve flow records the client correctly performing PAR (with PKCE S256 + DPoP), an authorize redirect, and a token exchange with refresh-token capture; all conformance checks emit `Pass`. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_full_grant_approve` (Phase 8 Task 1). Script: `FlowScript::Approve { granted_scope: "atproto" }`. Snapshot pins PAR with `code_challenge_method=S256` and `DPoP` header, an authorize redirect, and a token exchange returning a refresh_token. |
141
+
| `test-oauth-client.AC6.2` | A partial-grant approve flow (fake AS grants a strict subset of requested scopes) records the client correctly storing the granted (not requested) scope set; check emits `Pass` if the client surfaces or honors the actual grant. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_partial_grant_approve` (Phase 8 Task 1). Script: `FlowScript::PartialGrant { granted_scope: "atproto" }` when the RP requested `"atproto repo:app.bsky.feed.post?action=create"`. Asserts the recorded token response has `scope="atproto"` and the RP did not retry with the original scope. |
142
+
| `test-oauth-client.AC6.3` | A user-denial flow (fake AS returns `error=access_denied` to the authorize redirect) records the client correctly propagating the error rather than retrying silently. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_user_denied` (Phase 8 Task 1). Script: `FlowScript::Deny`. Asserts `AuthorizeOutcome::Error { error: "access_denied", .. }` and that the recorded flow has 1 PAR + 1 authorize + 0 token requests. |
143
+
| `test-oauth-client.AC6.4` | A downscoped-refresh flow records the client issuing a refresh request with a subset of the originally granted scopes when its `RelyingParty`-driven script asks for one. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_downscoped_refresh` (Phase 8 Task 1). Script: full-grant approve, then refresh with a narrower scope. Snapshot pins a POST /oauth/token body containing `grant_type=refresh_token&scope=<narrower>`. |
144
+
| `test-oauth-client.AC6.5` | A recorded PAR request without `code_challenge` (PKCE) produces a `SpecViolation` with code `oauth_client::interactive::scope_variations::pkce_required`. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_pkce_required_violation` (Phase 8 Task 1). Drives `RelyingParty::send_raw_par` with a body lacking `code_challenge`; asserts `Check::PkceRequired` emits `SpecViolation` with the pinned diagnostic code verbatim. |
145
+
| `test-oauth-client.AC6.6` | A recorded PAR request without a `DPoP` header produces a `SpecViolation`. | Automated test | `tests/oauth_client_interactive.rs::scope_variations_dpop_required_violation` (Phase 8 Task 1). Drives `RelyingParty::send_raw_par` with `dpop: None`; asserts `Check::DpopRequired` emits `SpecViolation`. |
146
+
147
+
---
148
+
149
+
## AC7: Interactive — DPoP / refresh edge flows
150
+
151
+
All AC7 cases land in Phase 8 Task 2. Structure mirrors Phase 8 Task 1
152
+
— per-flow scripts, driven by a fresh RP, asserted against the request
153
+
log.
154
+
155
+
| AC ID | Text (verbatim) | Verification | Where |
156
+
|---|---|---|---|
157
+
| `test-oauth-client.AC7.1` | A DPoP-nonce rotation flow (fake AS responds 400 `use_dpop_nonce` on initial PAR with a `DPoP-Nonce` header) records the client correctly retrying with the issued nonce. | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_nonce_rotation` (Phase 8 Task 2). Script: `FlowScript::DpopNonceRetryOnPar { nonce: "nonce-1" }`. Asserts two PAR entries in the snapshot and decodes the second entry's DPoP header to verify `nonce = "nonce-1"` in claims. |
158
+
| `test-oauth-client.AC7.2` | A refresh-token rotation flow records the client using the new refresh token returned with the previous token response (and not retaining the old one). | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_refresh_rotation` (Phase 8 Task 2). Drives a code exchange, then a refresh with `rt1` (server returns `rt2`), then another refresh. Asserts the third request's form body carries `refresh_token=rt2`. |
159
+
| `test-oauth-client.AC7.3` | A replay-rejection flow (fake AS rejects a duplicate `jti`) records the client surfacing the failure rather than entering a retry loop. | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_replay_rejection` (Phase 8 Task 2). Asserts the log contains exactly one failed POST (401) and no further retry attempts. |
160
+
| `test-oauth-client.AC7.4` | A recorded sequence where the client reuses a `jti` across two requests produces a `SpecViolation` with code `oauth_client::interactive::dpop_edges::jti_reused`. | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_jti_reuse_violation` (Phase 8 Task 2). Drives the broken-RP helper to send two PARs with the same `jti`; asserts `Check::JtiReuseViolation` emits `SpecViolation` with the pinned diagnostic code verbatim. |
161
+
| `test-oauth-client.AC7.5` | A recorded sequence where the client receives a `DPoP-Nonce` header but does not adopt it on the next request produces a `SpecViolation`. | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_nonce_ignored_violation` (Phase 8 Task 2). Fake AS responds with `DPoP-Nonce: n2`, broken-RP helper's next proof lacks `nonce=n2`; asserts `Check::NonceIgnoredViolation` emits `SpecViolation`. |
162
+
| `test-oauth-client.AC7.6` | A recorded sequence where the client reuses a single-use refresh token after rotation produces a `SpecViolation`. | Automated test | `tests/oauth_client_interactive.rs::dpop_edges_refresh_token_reuse_violation` (Phase 8 Task 2). Broken-RP helper POSTs a refresh with `rt1` after rotation; fake AS returns 401 `invalid_grant`; asserts `Check::RefreshTokenReuseViolation` emits `SpecViolation`. |
163
+
164
+
---
165
+
166
+
## AC8: Cross-cutting CLI / report contract
167
+
168
+
AC8 spans exit codes, tracing verbosity, color handling, naming
169
+
conventions, and help-output structure. Per Phase 8 Task 4, exit-code
170
+
pinning lives in `tests/oauth_client_endtoend.rs` (which can spawn the
171
+
fake AS in-process under tokio), while argument-inventory and color /
172
+
verbose smoke tests live in `tests/oauth_client_cli.rs` (where
173
+
`assert_cmd` is synchronous). The check-ID and diagnostic-code coverage
174
+
tests live in `tests/oauth_client_check_id_coverage.rs` per Phase 8
175
+
Task 4.
176
+
177
+
| AC ID | Text (verbatim) | Verification | Where |
178
+
|---|---|---|---|
179
+
| `test-oauth-client.AC8.1` | Exit code is `0` when no `SpecViolation` and no `NetworkError` are recorded. | Automated test | `tests/oauth_client_endtoend.rs::exit_zero_when_all_pass` (Phase 8 Task 4). Runs the full pipeline with a fully conformant inline fixture and asserts `report.exit_code() == 0`. |
180
+
| `test-oauth-client.AC8.2` | Exit code is `1` when at least one `SpecViolation` is recorded, regardless of any `NetworkError`s present. | Automated test | `tests/oauth_client_endtoend.rs::exit_one_on_any_spec_violation` (Phase 8 Task 4). Seeds a `dpop_bound_access_tokens=false` metadata document and asserts exit code 1. |
181
+
| `test-oauth-client.AC8.3` | Exit code is `2` when at least one `NetworkError` is recorded and no `SpecViolation` is recorded. | Automated test | `tests/oauth_client_endtoend.rs::exit_two_on_network_error_alone` (Phase 8 Task 4). Seeds a transport error on the confidential client's `jwks_uri` and asserts exit code 2. |
182
+
| `test-oauth-client.AC8.4` | `Advisory` and `Skipped` rows never influence the exit code. | Automated test | `tests/oauth_client_endtoend.rs::advisory_and_skipped_do_not_affect_exit` (Phase 8 Task 4). Constructs a `Report` (via `common::report`) by hand-pushing `CheckResult` struct literals with `status: CheckStatus::Advisory` and `status: CheckStatus::Skipped`, then asserts `exit_code() == 0`. The struct literal form (not a `.advisory()` helper) keeps the assertion independent of any check's helper surface. |
183
+
| `test-oauth-client.AC8.5` | `--verbose` toggles tracing to DEBUG level for both modes (verified by `oauth_client_cli` smoke test). | Automated test | `tests/oauth_client_cli.rs::verbose_flag_accepted` (Phase 8 Task 4). Uses `assert_cmd` with target `http://localhost` (loopback → zero-I/O, exit 0) plus the `--verbose` flag and asserts success. Global `--verbose` wiring lives in `src/cli.rs`; this smoke test confirms it's accepted on the oauth client path. |
184
+
| `test-oauth-client.AC8.6` | Both `NO_COLOR` env var and `--no-color` flag suppress ANSI color in the rendered report. | Automated test | `tests/oauth_client_cli.rs::no_color_env_var_suppresses_color` and `::no_color_flag_suppresses_color` (Phase 8 Task 4). Each runs the CLI against `http://localhost` and asserts that stdout contains no `\x1b[` ANSI escapes. Both tests share one AC line-item because the AC text enumerates both trigger mechanisms — the two tests together are the authoritative pair. |
185
+
| `test-oauth-client.AC8.7` | Every check ID follows the `oauth_client::<stage>::<check>` naming convention and appears verbatim in at least one insta snapshot under `tests/snapshots/`. | Automated test | `tests/oauth_client_check_id_coverage.rs::every_check_id_appears_in_a_snapshot` (Phase 8 Task 4). Iterates every stage's `Check::ALL` array via the public re-exports and asserts each `.id()` is contained in the concatenated contents of `tests/snapshots/oauth_client_*.snap`. |
186
+
| `test-oauth-client.AC8.8` | Every diagnostic code follows the same naming convention and appears verbatim in at least one insta snapshot. | Automated test | `tests/oauth_client_check_id_coverage.rs::every_diagnostic_code_appears_in_a_snapshot` (Phase 8 Task 4). Iterates every `miette::Diagnostic`-deriving error type in the oauth client tree (`oauth_client::jws::*`, `oauth_client::metadata::*`, `oauth_client::target::*`, `oauth_client::interactive::*`) and asserts each code appears in the concatenated snapshot contents. |
187
+
| `test-oauth-client.AC8.9` | `atproto-devtool test oauth client --help` and `atproto-devtool test oauth client interactive --help` each render distinct argument inventories with the interactive-only flags (`--public-base-url`, `--port`) appearing only on the latter. | Automated test | `tests/oauth_client_cli.rs::interactive_help_has_port_and_public_base_url` **paired with** `::static_help_does_not_have_port_or_public_base_url` (Phase 8 Task 4). The pair jointly pins the argument-inventory distinction: the interactive subcommand's help must contain both flags; the static subcommand's help must contain neither. Paired because the AC text is an if-and-only-if constraint on where the flags appear. |
188
+
189
+
---
190
+
191
+
## Coverage summary
192
+
193
+
- **50 ACs total**: AC1 × 7 + AC2 × 9 + AC3 × 8 + AC4 × 6 + AC5 × 5 + AC6 × 6 + AC7 × 6 + AC8 × 9.
194
+
- **Automated tests**: 49 ACs.
195
+
- `tests/oauth_client_discovery.rs`: 6.
196
+
- `src/commands/test/oauth/client/pipeline.rs` (unit): 1 (AC1.4 only).
197
+
- `tests/oauth_client_metadata.rs`: 9.
198
+
- `tests/oauth_client_jwks.rs`: 8.
199
+
- `tests/oauth_client_interactive.rs`: 5 (AC4) + 5 (AC5) + 6 (AC6) + 6 (AC7) = 22.
200
+
- `tests/oauth_client_endtoend.rs`: 4.
201
+
- `tests/oauth_client_cli.rs`: 4.
202
+
- `tests/oauth_client_check_id_coverage.rs`: 2.
203
+
- **Human verification**: 1 AC (AC4.1 — the `tokio::signal::ctrl_c()` "until interrupted" semantics in CLI mode).
204
+
205
+
The AC4.1 split-verification rationale: the spawn + print sub-behavior
206
+
is automated inside `DriveRpInProcess` tests (which exercise
207
+
`ServerHandle::bind` and the printed-identity code path every time),
208
+
but the CLI's `WaitForExternalClient` drive-mode blocks on
209
+
`tokio::signal::ctrl_c()` and only emits its checks after interrupt.
210
+
Automating the interrupt side would require racing a second process
211
+
against the main binary under `assert_cmd` and sending a signal on a
212
+
timeout — a known-flaky pattern. The human-verification step in Phase
213
+
7 Task 4's "Done when" is the authoritative check for that branch.