CLI app for developers prototyping atproto functionality
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Add interactive checks for `state` presence and `nonce` absence in PAR

The atproto OAuth profile
(<https://atproto.com/specs/oauth#authorization-request-fields>)
makes two rules about the PAR request body that the tester wasn't
observing:

- *"The `state` parameter in client authorization requests is
mandatory."* (Elevated from RFC 6749 §4.1.1's RECOMMENDED.)
- *"The `nonce` value, used in many other OAuth profiles, should not
be included."* (Atproto explicitly forbids the OIDC-style nonce.)

Add two interactive-stage checks alongside `ClientUsedPkceS256` and
`ClientIncludedDpop`:

- `ClientIncludedState`: `state=…` must appear in the PAR
body. Missing → SpecViolation.
- `ClientOmittedNonce`: `nonce=…` must NOT appear in the PAR body.
Present → SpecViolation.

Both inspect the request log via a new `par_body_has_form_key`
helper (simple key-presence test; we don't need full form-decoding
here). The blocked_by paths, PAR-failure early-exits, and the
WaitForExternalClient placeholder all emit the two new variants so
the check inventory stays consistent.

The default `DeterministicRpFactory` already sends `state` and
doesn't send `nonce`, so the happy-path snapshots show both new
checks as `Pass`.

All 334 tests pass; all 16 real-world atproto OAuth clients still
pass statically.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

+98 -53
+2
docs/design-plans/2026-04-16-test-oauth-client.md
··· 113 113 - **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. 114 114 - **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. 115 115 - **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. 116 + - **test-oauth-client.AC4.7 Success:** The PAR request body includes a `state` parameter, and the `ClientIncludedState` check passes. The atproto OAuth profile (<https://atproto.com/specs/oauth#authorization-request-fields>) states: *"The `state` parameter in client authorization requests is mandatory."* A PAR body without `state=…` fails this check. 117 + - **test-oauth-client.AC4.8 Success:** The PAR request body does NOT include a `nonce` parameter, and the `ClientOmittedNonce` check passes. The atproto OAuth profile calls this out explicitly: *"The `nonce` value, used in many other OAuth profiles, should not be included."* A PAR body that carries `nonce=…` fails this check. 116 118 117 119 ### test-oauth-client.AC5: Cross-mode dependency gating 118 120
+89 -48
src/commands/test/oauth/client/pipeline/interactive.rs
··· 87 87 /// access tokens to a client-held key and starts at the PAR 88 88 /// endpoint, not only at the token endpoint. 89 89 ClientIncludedDpop, 90 + /// The PAR request body included a `state` parameter. The atproto 91 + /// OAuth profile 92 + /// (<https://atproto.com/specs/oauth#authorization-request-fields>) 93 + /// restates RFC 6749 §4.1.1 and elevates it to mandatory: *"The 94 + /// `state` parameter in client authorization requests is 95 + /// mandatory."* A missing `state` leaves the client unable to 96 + /// verify the redirect against the request. 97 + ClientIncludedState, 98 + /// The PAR request body did NOT include a `nonce` parameter. 99 + /// The atproto OAuth profile explicitly calls out: *"The `nonce` 100 + /// value, used in many other OAuth profiles, should not be 101 + /// included."* (<https://atproto.com/specs/oauth#authorization-request-fields>) 102 + /// A client that carries an OIDC-style `nonce` into an atproto 103 + /// authorization request is non-conformant. 104 + ClientOmittedNonce, 90 105 /// The client completed the authorization-code ↔ token exchange 91 106 /// against `/oauth/token` and received an access token. Mirrors 92 107 /// RFC 6749 §4.1.3 ··· 112 127 Check::ClientReachedPar => "oauth_client::interactive::client_reached_par", 113 128 Check::ClientUsedPkceS256 => "oauth_client::interactive::client_used_pkce_s256", 114 129 Check::ClientIncludedDpop => "oauth_client::interactive::client_included_dpop", 130 + Check::ClientIncludedState => "oauth_client::interactive::client_included_state", 131 + Check::ClientOmittedNonce => "oauth_client::interactive::client_omitted_nonce", 115 132 Check::ClientCompletedToken => "oauth_client::interactive::client_completed_token", 116 133 Check::ClientRefreshed => "oauth_client::interactive::client_refreshed", 117 134 } ··· 124 141 Check::ClientReachedPar => "Client reached PAR endpoint", 125 142 Check::ClientUsedPkceS256 => "Client used PKCE S256 method", 126 143 Check::ClientIncludedDpop => "Client included DPoP proof", 144 + Check::ClientIncludedState => "Client included `state` parameter in PAR", 145 + Check::ClientOmittedNonce => "Client did not include `nonce` parameter in PAR", 127 146 Check::ClientCompletedToken => "Client completed token exchange", 128 147 Check::ClientRefreshed => "Client refreshed access token", 129 148 } ··· 170 189 } 171 190 } 172 191 192 + /// Returns true if the form-encoded `par_body` contains a `key=` pair. 193 + /// Used to detect `state=` / `nonce=` without requiring a full 194 + /// form-decode (we only care about key presence). Handles both 195 + /// `key=value` and a leading-key case (`key=` at the very start). 196 + fn par_body_has_form_key(par_body: &str, key: &str) -> bool { 197 + par_body 198 + .split('&') 199 + .any(|pair| pair.split('=').next() == Some(key)) 200 + } 201 + 173 202 /// All interactive checks. 174 203 pub const CHECK_ALL: &[Check] = &[ 175 204 Check::ServerBound, 176 205 Check::ClientReachedPar, 177 206 Check::ClientUsedPkceS256, 178 207 Check::ClientIncludedDpop, 208 + Check::ClientIncludedState, 209 + Check::ClientOmittedNonce, 179 210 Check::ClientCompletedToken, 180 211 Check::ClientRefreshed, 181 212 ]; ··· 206 237 // Each check's result is based on its specific gate, not a cascading if-chain. 207 238 208 239 if static_gating.scope_present != CheckStatus::Pass { 209 - results.push(blocked_by( 210 - Check::ClientReachedPar.id(), 211 - Stage::OAUTH_CLIENT_INTERACTIVE, 212 - Check::ClientReachedPar.summary(), 213 - "oauth_client::metadata::scope_present", 214 - )); 215 - results.push(blocked_by( 216 - Check::ClientUsedPkceS256.id(), 217 - Stage::OAUTH_CLIENT_INTERACTIVE, 218 - Check::ClientUsedPkceS256.summary(), 219 - "oauth_client::metadata::scope_present", 220 - )); 221 - results.push(blocked_by( 222 - Check::ClientIncludedDpop.id(), 223 - Stage::OAUTH_CLIENT_INTERACTIVE, 224 - Check::ClientIncludedDpop.summary(), 225 - "oauth_client::metadata::scope_present", 226 - )); 227 - results.push(blocked_by( 228 - Check::ClientCompletedToken.id(), 229 - Stage::OAUTH_CLIENT_INTERACTIVE, 230 - Check::ClientCompletedToken.summary(), 231 - "oauth_client::metadata::scope_present", 232 - )); 240 + for check in [ 241 + Check::ClientReachedPar, 242 + Check::ClientUsedPkceS256, 243 + Check::ClientIncludedDpop, 244 + Check::ClientIncludedState, 245 + Check::ClientOmittedNonce, 246 + Check::ClientCompletedToken, 247 + ] { 248 + results.push(blocked_by( 249 + check.id(), 250 + Stage::OAUTH_CLIENT_INTERACTIVE, 251 + check.summary(), 252 + "oauth_client::metadata::scope_present", 253 + )); 254 + } 233 255 // ClientRefreshed is always skipped in Phase 7. 234 256 results.push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); 235 257 // Emit ServerBound.skipped to maintain consistent 6-check inventory. ··· 244 266 } 245 267 246 268 if static_gating.dpop_bound_required != CheckStatus::Pass { 247 - results.push(blocked_by( 248 - Check::ClientReachedPar.id(), 249 - Stage::OAUTH_CLIENT_INTERACTIVE, 250 - Check::ClientReachedPar.summary(), 251 - "oauth_client::metadata::dpop_bound_required", 252 - )); 253 - results.push(blocked_by( 254 - Check::ClientUsedPkceS256.id(), 255 - Stage::OAUTH_CLIENT_INTERACTIVE, 256 - Check::ClientUsedPkceS256.summary(), 257 - "oauth_client::metadata::dpop_bound_required", 258 - )); 259 - results.push(blocked_by( 260 - Check::ClientIncludedDpop.id(), 261 - Stage::OAUTH_CLIENT_INTERACTIVE, 262 - Check::ClientIncludedDpop.summary(), 263 - "oauth_client::metadata::dpop_bound_required", 264 - )); 265 - results.push(blocked_by( 266 - Check::ClientCompletedToken.id(), 267 - Stage::OAUTH_CLIENT_INTERACTIVE, 268 - Check::ClientCompletedToken.summary(), 269 - "oauth_client::metadata::dpop_bound_required", 270 - )); 269 + for check in [ 270 + Check::ClientReachedPar, 271 + Check::ClientUsedPkceS256, 272 + Check::ClientIncludedDpop, 273 + Check::ClientIncludedState, 274 + Check::ClientOmittedNonce, 275 + Check::ClientCompletedToken, 276 + ] { 277 + results.push(blocked_by( 278 + check.id(), 279 + Stage::OAUTH_CLIENT_INTERACTIVE, 280 + check.summary(), 281 + "oauth_client::metadata::dpop_bound_required", 282 + )); 283 + } 271 284 // ClientRefreshed is always skipped in Phase 7. 272 285 results.push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); 273 286 // Emit ServerBound.skipped to maintain consistent 6-check inventory. ··· 340 353 results.push(Check::ClientReachedPar.spec_violation()); 341 354 results.push(Check::ClientUsedPkceS256.spec_violation()); 342 355 results.push(Check::ClientIncludedDpop.spec_violation()); 356 + results.push(Check::ClientIncludedState.spec_violation()); 357 + results.push(Check::ClientOmittedNonce.spec_violation()); 343 358 results.push(Check::ClientCompletedToken.spec_violation()); 344 359 results.push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); 345 360 } ··· 360 375 results.push(Check::ClientReachedPar.spec_violation()); 361 376 results.push(Check::ClientUsedPkceS256.spec_violation()); 362 377 results.push(Check::ClientIncludedDpop.spec_violation()); 378 + results.push(Check::ClientIncludedState.spec_violation()); 379 + results.push(Check::ClientOmittedNonce.spec_violation()); 363 380 results.push(Check::ClientCompletedToken.spec_violation()); 364 381 results 365 382 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 387 404 results.push(Check::ClientReachedPar.spec_violation()); 388 405 results.push(Check::ClientUsedPkceS256.spec_violation()); 389 406 results.push(Check::ClientIncludedDpop.spec_violation()); 407 + results.push(Check::ClientIncludedState.spec_violation()); 408 + results.push(Check::ClientOmittedNonce.spec_violation()); 390 409 results.push(Check::ClientCompletedToken.spec_violation()); 391 410 results 392 411 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 435 454 } else { 436 455 results.push(Check::ClientIncludedDpop.spec_violation()); 437 456 } 457 + 458 + // Check for `state` presence and `nonce` absence in 459 + // the PAR body. Atproto's profile makes `state` 460 + // mandatory and forbids `nonce`; see 461 + // <https://atproto.com/specs/oauth#authorization-request-fields>. 462 + let par_body = log_snapshot 463 + .iter() 464 + .find(|req| req.method == "POST" && req.path == "/oauth/par") 465 + .map(|req| String::from_utf8_lossy(&req.body).into_owned()) 466 + .unwrap_or_default(); 467 + if par_body_has_form_key(&par_body, "state") { 468 + results.push(Check::ClientIncludedState.pass()); 469 + } else { 470 + results.push(Check::ClientIncludedState.spec_violation()); 471 + } 472 + if par_body_has_form_key(&par_body, "nonce") { 473 + results.push(Check::ClientOmittedNonce.spec_violation()); 474 + } else { 475 + results.push(Check::ClientOmittedNonce.pass()); 476 + } 438 477 } else { 439 478 results.push(Check::ClientReachedPar.spec_violation()); 440 479 results.push(Check::ClientUsedPkceS256.spec_violation()); 441 480 results.push(Check::ClientIncludedDpop.spec_violation()); 481 + results.push(Check::ClientIncludedState.spec_violation()); 482 + results.push(Check::ClientOmittedNonce.spec_violation()); 442 483 } 443 484 444 485 // Perform authorize.
+4 -4
tests/oauth_client_interactive.rs
··· 276 276 let output = interactive::run(static_gating, None, None, &interactive_opts, clock).await; 277 277 278 278 // Verify that all expected checks are present. 279 - // Phase 7: 6 checks (ServerBound, ClientReachedPar, ClientUsedPkceS256, ClientIncludedDpop, ClientCompletedToken, ClientRefreshed). 279 + // Phase 7: 8 checks (ServerBound, ClientReachedPar, ClientUsedPkceS256, ClientIncludedDpop, ClientIncludedState, ClientOmittedNonce, ClientCompletedToken, ClientRefreshed). 280 280 // Phase 8: 6 scope_variations checks + 6 dpop_edges checks. 281 - // Total: 18 checks. 281 + // Total: 20 checks. 282 282 assert_eq!( 283 283 output.results.len(), 284 - 18, 285 - "should have 18 checks (6 phase 7 + 6 scope_variations + 6 dpop_edges)" 284 + 20, 285 + "should have 20 checks (8 phase 7 + 6 scope_variations + 6 dpop_edges)" 286 286 ); 287 287 288 288 // Check that ServerBound passed (server bind was successful).
+3 -1
tests/snapshots/oauth_client_substage_snapshots__interactive_stage_blocked_by_static_failures_snapshot.snap
··· 10 10 [SKIP] Client reached PAR endpoint — blocked by oauth_client::metadata::scope_present 11 11 [SKIP] Client used PKCE S256 method — blocked by oauth_client::metadata::scope_present 12 12 [SKIP] Client included DPoP proof — blocked by oauth_client::metadata::scope_present 13 + [SKIP] Client included `state` parameter in PAR — blocked by oauth_client::metadata::scope_present 14 + [SKIP] Client did not include `nonce` parameter in PAR — blocked by oauth_client::metadata::scope_present 13 15 [SKIP] Client completed token exchange — blocked by oauth_client::metadata::scope_present 14 16 [SKIP] Client refreshed access token — covered in Phase 8 flow variants 15 17 16 - Summary: 0 passed, 0 failed (spec), 0 network errors, 0 advisories, 6 skipped. Exit code: 0 18 + Summary: 0 passed, 0 failed (spec), 0 network errors, 0 advisories, 8 skipped. Exit code: 0