Exercise client-side iss/sub verification via broken-AS sub-stage

+1 -1

docs/design-plans/2026-04-16-test-oauth-client.md

··· 116 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 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. 118 118 - **test-oauth-client.AC4.9 Success (fake AS compliance):** The fake AS's authorization redirect always includes `iss=<active_base>` (atproto's `authorization_response_iss_parameter_supported = true` contract), and its token response always includes `sub=<account DID>`. A client library pointed at the fake AS can therefore perform the mandatory issuer and subject verifications against real values. 119 - - **test-oauth-client.AC4.10 Deferred (client-side `iss`/`sub` verification):** The `ClientVerifiedIss` and `ClientVerifiedSub` check variants are declared in `CHECK_ALL` and rendered in every report, but emit `Skipped` with reason `"broken-AS sub-stage for client-side iss/sub verification not yet implemented"`. Fully exercising these requires a broken-AS sub-stage (similar in shape to `dpop_edges`) that emits a bogus `iss` on a redirect or a wrong `sub` in a token response and observes whether the client terminates the flow (`iss`) or discards the token (`sub`). The atproto OAuth profile calls this verification *"critical (mandatory)"* for all clients (<https://atproto.com/specs/oauth#authentication-and-subject>); closing this gap is a future phase. 119 + - **test-oauth-client.AC4.10 Success (client-side `iss`/`sub` verification):** The `ClientVerifiedIss` and `ClientVerifiedSub` checks are exercised by a broken-AS sub-stage (`iss_sub_verification`, similar in shape to `dpop_edges`) that drives two additional scripted flows. `BogusIssOnRedirect` substitutes a deliberately wrong `iss` on the authorization redirect and expects `do_authorize` to return `RpError::IssuerMismatch`; `WrongSubInToken` substitutes a deliberately wrong `sub` in the token response and expects `do_token` to return `RpError::SubMismatch`. The RP itself performs both verifications as part of its spec-compliant behaviour; test-surface helpers (`do_authorize_skipping_iss_verification`, `do_token_skipping_sub_verification`, `do_refresh_skipping_sub_verification`) let the broken-RP suite model non-conformant clients. `Pass` is emitted when the RP aborts at the right step; `SpecViolation` when it silently accepts the broken value; `Skipped` only under `WaitForExternalClient` drive mode (which cannot re-drive a flow with a broken script) or when an earlier happy-path failure prevents the sub-stage from running. The atproto OAuth profile calls this verification *"critical (mandatory)"* for all clients (<https://atproto.com/specs/oauth#authentication-and-subject>). 120 120 121 121 ### test-oauth-client.AC5: Cross-mode dependency gating 122 122

+19 -2

src/commands/test/oauth/client/CLAUDE.md

··· 20 20 4. **Interactive** — (optional) Fake AS server exercises the client's full auth flow (PAR, authorize, token exchange, refresh). 21 21 - **scope_variations** sub-stage — Tests scope grant variants: full approval, partial grant, user denial, refresh scoping. 22 22 - **dpop_edges** sub-stage — Tests DPoP edge cases: nonce rotation, refresh-token rotation, replay rejection. 23 + - **iss_sub_verification** sub-stage — Drives two broken-AS flows 24 + (`FlowScript::BogusIssOnRedirect`, `FlowScript::WrongSubInToken`) 25 + against the shared happy-path RP to exercise client-side `iss` and 26 + `sub` verification. The resulting `ClientVerifiedIss` / 27 + `ClientVerifiedSub` check IDs live at the `oauth_client::interactive::*` 28 + top level (they are not namespaced under `iss_sub_verification::`); 29 + the sub-stage just drives the flows that produce them. 23 30 24 31 ### Diagnostic Code Prefixes 25 32 ··· 27 34 - `oauth_client::discovery::*` — Metadata discovery and reachability. 28 35 - `oauth_client::metadata::*` — Client metadata validation. 29 36 - `oauth_client::jws::*` — JWKS and cryptographic checks. 30 - - `oauth_client::interactive::*` — Happy-path flow checks. 37 + - `oauth_client::interactive::*` — Happy-path flow checks (also where the 38 + `iss_sub_verification` sub-stage emits `ClientVerifiedIss` / 39 + `ClientVerifiedSub` results). 31 40 - `oauth_client::interactive::scope_variations::*` — Scope variant checks. 32 41 - `oauth_client::interactive::dpop_edges::*` — DPoP edge case checks. 33 42 ··· 78 87 request signatures, which is what makes snapshot-based assertions viable 79 88 for the interactive path. 80 89 90 + `fake_as::endpoints::FlowScript` is the knob that selects per-flow 91 + behavior: `Approve`, `PartialGrant`, `Deny`, `DpopNonceRetryOnPar`, 92 + `BogusIssOnRedirect { granted_scope, bogus_iss }` (stamps a wrong `iss` 93 + on the authorize redirect so the client's `iss` check can be exercised), 94 + and `WrongSubInToken { granted_scope, wrong_sub }` (substitutes a 95 + different DID for `sub` in the token response so the client's `sub` 96 + check can be exercised). 97 + 81 98 ## Invariants 82 99 83 100 - Every network request is a trait-object call; no unconstrained I/O. ··· 89 106 90 107 Snapshot files (`tests/snapshots/oauth_client_*.snap`) document the full report rendering for each test case. Check IDs and diagnostic codes must appear in at least one snapshot file for completeness (verified by `oauth_client_check_id_coverage.rs`). 91 108 92 - Last verified: 2026-04-21 109 + Last verified: 2026-04-23

+49 -11

src/commands/test/oauth/client/fake_as/endpoints.rs

··· 31 31 Deny, 32 32 /// On the first PAR, respond use_dpop_nonce to force a retry. 33 33 DpopNonceRetryOnPar { nonce: String }, 34 + /// Approve the authorize request, but emit a deliberately wrong 35 + /// `iss` value on the redirect. Used by the broken-AS sub-stage to 36 + /// verify that clients abort when RFC 9207 / atproto's 37 + /// `authorization_response_iss_parameter_supported` contract is 38 + /// violated. 39 + BogusIssOnRedirect { 40 + granted_scope: String, 41 + bogus_iss: String, 42 + }, 43 + /// Approve the authorize request and issue a token, but emit a 44 + /// deliberately wrong `sub` in the token response. Used by the 45 + /// broken-AS sub-stage to verify that clients discard tokens when 46 + /// the atproto mandatory `sub` verification fails. 47 + WrongSubInToken { 48 + granted_scope: String, 49 + wrong_sub: String, 50 + }, 34 51 } 35 52 36 53 /// Parameters from a PAR request. ··· 570 587 571 588 // Dispatch on flow script. 572 589 let flow_script = s.flow_script.lock().unwrap().clone(); 573 - let (granted_scope, should_approve) = match flow_script { 574 - FlowScript::Approve { granted_scope } => (granted_scope, true), 575 - FlowScript::PartialGrant { granted_scope } => (granted_scope, true), 576 - FlowScript::Deny => ("".to_string(), false), 577 - FlowScript::DpopNonceRetryOnPar { .. } => ("atproto".to_string(), true), 590 + let default_iss = s.active_base.as_str().trim_end_matches('/').to_string(); 591 + let (granted_scope, should_approve, iss_to_send) = match flow_script { 592 + FlowScript::Approve { granted_scope } => (granted_scope, true, default_iss.clone()), 593 + FlowScript::PartialGrant { granted_scope } => (granted_scope, true, default_iss.clone()), 594 + FlowScript::Deny => ("".to_string(), false, default_iss.clone()), 595 + FlowScript::DpopNonceRetryOnPar { .. } => { 596 + ("atproto".to_string(), true, default_iss.clone()) 597 + } 598 + FlowScript::BogusIssOnRedirect { 599 + granted_scope, 600 + bogus_iss, 601 + } => (granted_scope, true, bogus_iss), 602 + FlowScript::WrongSubInToken { granted_scope, .. } => { 603 + (granted_scope, true, default_iss.clone()) 604 + } 578 605 }; 579 606 580 607 let mut redirect_url = par_request.redirect_uri.clone(); ··· 599 626 // include `iss` so the client can verify the AS matches the 600 627 // one it resolved for the account. See 601 628 // <https://atproto.com/specs/oauth#authorization-response>. 602 - let iss = s.active_base.as_str().trim_end_matches('/').to_string(); 629 + // The `BogusIssOnRedirect` script deliberately overrides this 630 + // value to test client-side verification. 603 631 redirect_url 604 632 .query_pairs_mut() 605 633 .append_pair("code", &code) 606 634 .append_pair("state", &par_request.state) 607 - .append_pair("iss", &iss); 635 + .append_pair("iss", &iss_to_send); 608 636 } else { 609 - let iss = s.active_base.as_str().trim_end_matches('/').to_string(); 610 637 redirect_url 611 638 .query_pairs_mut() 612 639 .append_pair("error", "access_denied") 613 640 .append_pair("state", &par_request.state) 614 - .append_pair("iss", &iss); 641 + .append_pair("iss", &iss_to_send); 615 642 } 616 643 617 644 Redirect::permanent(redirect_url.as_str()).into_response() ··· 726 753 }; 727 754 let new_refresh_token = format!("fake-refresh-{now}-{counter}"); 728 755 756 + // Compute the `sub` value to send back. Atproto requires the 757 + // real account DID; the `WrongSubInToken` script substitutes a 758 + // deliberately wrong value to test client-side verification. 759 + let sub_to_send = { 760 + let flow_script = s.flow_script.lock().unwrap().clone(); 761 + match flow_script { 762 + FlowScript::WrongSubInToken { wrong_sub, .. } => wrong_sub, 763 + _ => s.identity.did.to_string(), 764 + } 765 + }; 766 + 729 767 if grant_type == "authorization_code" { 730 768 let code = match params.code.as_ref() { 731 769 Some(c) => c, ··· 788 826 "expires_in": 3600, 789 827 "refresh_token": new_refresh_token, 790 828 "scope": scope, 791 - "sub": s.identity.did.to_string(), 829 + "sub": sub_to_send, 792 830 }); 793 831 794 832 // Store refresh token binding. ··· 844 882 "expires_in": 3600, 845 883 "refresh_token": new_refresh_token, 846 884 "scope": scope, 847 - "sub": s.identity.did.to_string(), 885 + "sub": sub_to_send, 848 886 }); 849 887 850 888 // Store new refresh token binding.

+56 -31

src/commands/test/oauth/client/pipeline/interactive.rs

··· 1 1 //! OAuth client interactive stage — fake AS server, RP-driven flow, conformance checks. 2 2 3 3 pub mod dpop_edges; 4 + pub mod iss_sub_verification; 4 5 pub mod scope_variations; 5 6 6 7 use std::borrow::Cow; ··· 111 112 /// flow with (a protection against AS-mixup attacks per 112 113 /// <https://datatracker.ietf.org/doc/html/rfc9207>). 113 114 /// 114 - /// Fully exercising this check requires a broken-AS sub-stage 115 - /// that emits a bogus `iss` and observes whether the client 116 - /// terminates the flow before hitting `/oauth/token`. That 117 - /// infrastructure is not yet in place; for now the check emits 118 - /// `Skipped` with a reason pointing to the deferred 119 - /// implementation. 115 + /// Exercised by the broken-AS `iss_sub_verification` sub-stage, 116 + /// which drives a flow with a deliberately bogus `iss` and 117 + /// expects the client to terminate before hitting `/oauth/token`. 118 + /// Emits `Pass` if the client aborted with an issuer-mismatch 119 + /// error; `SpecViolation` if it silently continued; `Skipped` 120 + /// only when running under `WaitForExternalClient` drive mode 121 + /// (which cannot re-drive the flow with a broken script) or when 122 + /// the happy-path flow failed before the sub-stage could run. 120 123 ClientVerifiedIss, 121 124 /// The client verifies the `sub` field of the token response 122 125 /// matches the account DID it initiated the flow for. The ··· 127 130 /// account identified by the AS is the one they expected."* 128 131 /// (<https://atproto.com/specs/oauth#authentication-and-subject>) 129 132 /// 130 - /// Fully exercising this check requires a broken-AS sub-stage 131 - /// that emits a wrong `sub` and observes whether the client 132 - /// discards the token. That infrastructure is not yet in place; 133 - /// for now the check emits `Skipped` with a reason pointing to 134 - /// the deferred implementation. 133 + /// Exercised by the broken-AS `iss_sub_verification` sub-stage, 134 + /// which drives a flow with a deliberately wrong `sub` and 135 + /// expects the client to discard the token. Emits `Pass` if the 136 + /// client rejected the response with a sub-mismatch error; 137 + /// `SpecViolation` if it accepted the wrong DID; `Skipped` only 138 + /// under `WaitForExternalClient` mode or when the happy-path 139 + /// flow failed before the sub-stage could run. 135 140 ClientVerifiedSub, 136 141 /// The client completed the authorization-code ↔ token exchange 137 142 /// against `/oauth/token` and received an access token. Mirrors ··· 224 229 } 225 230 } 226 231 227 - /// Reason emitted on `ClientVerifiedIss` / `ClientVerifiedSub`'s 228 - /// `Skipped` rows until broken-AS sub-stages land. 229 - const CLIENT_VERIFICATION_SKIP_REASON: &str = 230 - "broken-AS sub-stage for client-side `iss`/`sub` verification not yet implemented"; 232 + /// Reason emitted on `ClientVerifiedIss` / `ClientVerifiedSub` when 233 + /// the happy-path flow failed before the broken-AS sub-stage could 234 + /// observe verification behaviour. The sub-stage constructs its own 235 + /// fresh RPs, so this only fires when the RP under test cannot even 236 + /// reach the happy-path token-exchange step. 237 + const IS_SUB_SKIP_BLOCKED_REASON: &str = 238 + "blocked by earlier happy-path failure; broken-AS sub-stage not run"; 239 + 240 + /// Reason emitted on `ClientVerifiedIss` / `ClientVerifiedSub` in 241 + /// `WaitForExternalClient` drive mode. The broken-AS sub-stage 242 + /// requires an in-process RP driver to re-run the flow twice with 243 + /// deliberately broken scripts; external-client mode only observes a 244 + /// single live flow and cannot exercise this check. 245 + const IS_SUB_EXTERNAL_DRIVER_REASON: &str = "broken-AS sub-stage requires in-process RP driver; external-client mode only observes a single flow"; 231 246 232 247 /// Returns true if the form-encoded `par_body` contains a `key=` pair. 233 248 /// Used to detect `state=` / `nonce=` without requiring a full ··· 401 416 results.push(Check::ClientIncludedDpop.spec_violation()); 402 417 results.push(Check::ClientIncludedState.spec_violation()); 403 418 results.push(Check::ClientOmittedNonce.spec_violation()); 404 - results.push(Check::ClientVerifiedIss.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 405 - results.push(Check::ClientVerifiedSub.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 419 + results.push(Check::ClientVerifiedIss.skipped(IS_SUB_EXTERNAL_DRIVER_REASON)); 420 + results.push(Check::ClientVerifiedSub.skipped(IS_SUB_EXTERNAL_DRIVER_REASON)); 406 421 results.push(Check::ClientCompletedToken.spec_violation()); 407 422 results.push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); 408 423 } 409 424 InteractiveDriveMode::DriveRpInProcess { rp_factory } => { 410 425 // Drive the RP through the happy path. 411 - let client_id = "http://localhost:3000" 426 + let client_id: url::Url = "http://localhost:3000" 412 427 .parse() 413 428 .expect("statically known-good URL"); 414 429 let rp = rp_factory.build( ··· 425 440 results.push(Check::ClientIncludedDpop.spec_violation()); 426 441 results.push(Check::ClientIncludedState.spec_violation()); 427 442 results.push(Check::ClientOmittedNonce.spec_violation()); 428 - results.push(Check::ClientVerifiedIss.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 429 - results.push(Check::ClientVerifiedSub.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 443 + results.push(Check::ClientVerifiedIss.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 444 + results.push(Check::ClientVerifiedSub.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 430 445 results.push(Check::ClientCompletedToken.spec_violation()); 431 446 results 432 447 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 456 471 results.push(Check::ClientIncludedDpop.spec_violation()); 457 472 results.push(Check::ClientIncludedState.spec_violation()); 458 473 results.push(Check::ClientOmittedNonce.spec_violation()); 459 - results.push(Check::ClientVerifiedIss.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 460 - results.push(Check::ClientVerifiedSub.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 474 + results.push(Check::ClientVerifiedIss.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 475 + results.push(Check::ClientVerifiedSub.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 461 476 results.push(Check::ClientCompletedToken.spec_violation()); 462 477 results 463 478 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 527 542 results.push(Check::ClientOmittedNonce.pass()); 528 543 } 529 544 530 - // ClientVerifiedIss / ClientVerifiedSub require a 531 - // broken-AS sub-stage that emits a bogus `iss` or 532 - // `sub` and observes whether the client aborts the 533 - // flow. That infrastructure isn't in place yet, so 534 - // emit `Skipped` with a reason for now. 535 - results.push(Check::ClientVerifiedIss.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 536 - results.push(Check::ClientVerifiedSub.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 545 + // `ClientVerifiedIss` and `ClientVerifiedSub` are 546 + // emitted by the broken-AS sub-stage further down, 547 + // which drives deliberately non-conformant flows and 548 + // observes whether the RP aborts. 537 549 } else { 538 550 results.push(Check::ClientReachedPar.spec_violation()); 539 551 results.push(Check::ClientUsedPkceS256.spec_violation()); 540 552 results.push(Check::ClientIncludedDpop.spec_violation()); 541 553 results.push(Check::ClientIncludedState.spec_violation()); 542 554 results.push(Check::ClientOmittedNonce.spec_violation()); 543 - results.push(Check::ClientVerifiedIss.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 544 - results.push(Check::ClientVerifiedSub.skipped(CLIENT_VERIFICATION_SKIP_REASON)); 545 555 } 546 556 547 557 // Perform authorize. ··· 551 561 { 552 562 Ok(ao) => ao, 553 563 Err(_e) => { 564 + results.push(Check::ClientVerifiedIss.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 565 + results.push(Check::ClientVerifiedSub.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 554 566 results.push(Check::ClientCompletedToken.spec_violation()); 555 567 results 556 568 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 566 578 let code = match auth_outcome { 567 579 crate::common::oauth::relying_party::AuthorizeOutcome::Code { code } => code, 568 580 crate::common::oauth::relying_party::AuthorizeOutcome::Error { .. } => { 581 + results.push(Check::ClientVerifiedIss.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 582 + results.push(Check::ClientVerifiedSub.skipped(IS_SUB_SKIP_BLOCKED_REASON)); 569 583 results.push(Check::ClientCompletedToken.spec_violation()); 570 584 results 571 585 .push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); ··· 584 598 &par_req.redirect_uri, 585 599 &code, 586 600 &par_resp.code_verifier, 601 + &server.identity.did, 587 602 ) 588 603 .await 589 604 { ··· 597 612 598 613 // Phase 7 doesn't test refresh; skip with reason. 599 614 results.push(Check::ClientRefreshed.skipped("covered in Phase 8 flow variants")); 615 + 616 + // Broken-AS sub-stage: exercise the client's `iss` and 617 + // `sub` verification. Drives two additional flows with 618 + // `BogusIssOnRedirect` / `WrongSubInToken` scripts and 619 + // observes whether the RP aborts. Reuses the happy-path 620 + // RP so its RNG state advances past the already-used 621 + // JTIs; a fresh RP built from the deterministic seed 622 + // would collide with the happy-path PAR's JTI. 623 + let iss_sub_results = iss_sub_verification::run(&server, &rp).await; 624 + results.extend(iss_sub_results); 600 625 601 626 // Phase 8: Gate and run scope_variations sub-stage. 602 627 let scope_gates_pass = static_gating.scope_present == CheckStatus::Pass

+9 -2

src/commands/test/oauth/client/pipeline/interactive/dpop_edges.rs

··· 233 233 match run_full_flow(&rp, server, &client_id).await { 234 234 Ok((rt1, as_desc)) => { 235 235 // Try to refresh with rt1. 236 - match rp.do_refresh(&as_desc, &rt1, None).await { 236 + match rp 237 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 238 + .await 239 + { 237 240 Ok(token_resp) => { 238 241 if let Some(rt2) = token_resp.refresh_token { 239 242 if rt2 != rt1 { 240 243 // Now try to reuse rt1 and expect rejection. 241 - match rp.do_refresh(&as_desc, &rt1, None).await { 244 + match rp 245 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 246 + .await 247 + { 242 248 Ok(_) => { 243 249 // rt1 was accepted when it should be rejected. 244 250 results.push(Check::RefreshRotation.spec_violation()); ··· 460 466 &par_req.redirect_uri, 461 467 &code, 462 468 &par_resp.code_verifier, 469 + &server.identity.did, 463 470 ) 464 471 .await?; 465 472

+167

src/commands/test/oauth/client/pipeline/interactive/iss_sub_verification.rs

··· 1 + //! OAuth client interactive stage — broken-AS `iss` / `sub` verification. 2 + //! 3 + //! Drives two scripted non-conformant flows against the fake AS and 4 + //! observes whether the RP under test correctly aborts: 5 + //! 6 + //! - `BogusIssOnRedirect`: the authorize redirect carries a deliberately 7 + //! wrong `iss` query parameter. A conformant client aborts the flow 8 + //! at the authorize step per RFC 9207 and atproto's 9 + //! `authorization_response_iss_parameter_supported` contract. 10 + //! - `WrongSubInToken`: the token response carries a deliberately wrong 11 + //! `sub` value. A conformant client discards the token per atproto's 12 + //! *"critical (mandatory)"* `sub` verification requirement 13 + //! (<https://atproto.com/specs/oauth#authentication-and-subject>). 14 + //! 15 + //! The RP under test is constructed via the injected `RpFactory` — in 16 + //! tests this is a `DeterministicRpFactory`, in production a 17 + //! `DefaultRpFactory`. Because the fake AS enforces spec-compliant 18 + //! behaviour on all other endpoints, a cooperating RP can reach the 19 + //! final verification step before the broken-AS injection kicks in. 20 + 21 + use crate::commands::test::oauth::client::fake_as::{ServerHandle, endpoints::FlowScript}; 22 + use crate::common::oauth::relying_party::{AuthorizeOutcome, ParRequest, RelyingParty, RpError}; 23 + use crate::common::report::CheckResult; 24 + 25 + use super::Check; 26 + 27 + /// Drive the broken-AS sub-stage and return `ClientVerifiedIss` and 28 + /// `ClientVerifiedSub` results. 29 + /// 30 + /// The caller's `rp` is reused so its RNG state advances monotonically 31 + /// across the two broken-AS flows and the happy-path flow that ran 32 + /// before. Building fresh RPs here would restart the deterministic 33 + /// RNG and collide with the happy-path RP's JTIs in the fake AS's 34 + /// replay cache. 35 + pub async fn run(server: &ServerHandle, rp: &RelyingParty) -> Vec<CheckResult> { 36 + let mut results = Vec::with_capacity(2); 37 + 38 + // ClientVerifiedIss — drive flow with BogusIssOnRedirect, observe 39 + // whether do_authorize returns IssuerMismatch. 40 + { 41 + *server.app_state().flow_script.lock().unwrap() = FlowScript::BogusIssOnRedirect { 42 + granted_scope: "atproto".to_string(), 43 + bogus_iss: "https://evil.example.com".to_string(), 44 + }; 45 + 46 + match drive_to_authorize(rp, server).await { 47 + Ok(()) => { 48 + // RP accepted the bogus iss — it does not verify. 49 + results.push(Check::ClientVerifiedIss.spec_violation()); 50 + } 51 + Err(RpError::IssuerMismatch { .. }) => { 52 + // RP correctly aborted on bogus iss. 53 + results.push(Check::ClientVerifiedIss.pass()); 54 + } 55 + Err(_) => { 56 + // Some other RP error (network, metadata malformed, 57 + // etc.) — this indicates the flow couldn't run to the 58 + // verification step, so we cannot conclude whether the 59 + // client verifies iss or not. 60 + results 61 + .push(Check::ClientVerifiedIss.skipped( 62 + "broken-AS flow failed before reaching the iss verification step", 63 + )); 64 + } 65 + } 66 + } 67 + 68 + // ClientVerifiedSub — drive flow with WrongSubInToken, observe 69 + // whether do_token returns SubMismatch. 70 + { 71 + *server.app_state().flow_script.lock().unwrap() = FlowScript::WrongSubInToken { 72 + granted_scope: "atproto".to_string(), 73 + wrong_sub: "did:web:evil.example.com".to_string(), 74 + }; 75 + 76 + match drive_to_token(rp, server).await { 77 + Ok(()) => { 78 + // RP accepted the wrong sub — it does not verify. 79 + results.push(Check::ClientVerifiedSub.spec_violation()); 80 + } 81 + Err(RpError::SubMismatch { .. }) => { 82 + // RP correctly rejected the wrong sub. 83 + results.push(Check::ClientVerifiedSub.pass()); 84 + } 85 + Err(_) => { 86 + results 87 + .push(Check::ClientVerifiedSub.skipped( 88 + "broken-AS flow failed before reaching the sub verification step", 89 + )); 90 + } 91 + } 92 + } 93 + 94 + results 95 + } 96 + 97 + /// Drive discovery → PAR → authorize. Used for the iss verification 98 + /// flow; succeeds (returns `Ok(())`) only if the RP accepted the 99 + /// redirect's `iss` without aborting. 100 + async fn drive_to_authorize(rp: &RelyingParty, server: &ServerHandle) -> Result<(), RpError> { 101 + let as_desc = rp.discover_as(&server.active_base).await?; 102 + 103 + let par_req = ParRequest { 104 + as_descriptor: as_desc.clone(), 105 + redirect_uri: "http://localhost/callback" 106 + .parse() 107 + .expect("statically known-good URL"), 108 + scope: "atproto".to_string(), 109 + state: "iss-verification-state".to_string(), 110 + }; 111 + 112 + let par_resp = rp.do_par(&par_req).await?; 113 + let outcome = rp 114 + .do_authorize(&as_desc, &par_resp.request_uri, &par_req.redirect_uri) 115 + .await?; 116 + // A conformant AS always returns Code under BogusIssOnRedirect 117 + // because the script still approves the authorization. The only 118 + // reason to abort is iss verification — if we got here, the RP 119 + // skipped it. 120 + match outcome { 121 + AuthorizeOutcome::Code { .. } => Ok(()), 122 + AuthorizeOutcome::Error { error, .. } => Err(RpError::MetadataMalformed { 123 + reason: format!("unexpected authorize error: {error}"), 124 + }), 125 + } 126 + } 127 + 128 + /// Drive discovery → PAR → authorize → token. Used for the sub 129 + /// verification flow; succeeds only if the RP accepted the token 130 + /// response's `sub` without aborting. 131 + async fn drive_to_token(rp: &RelyingParty, server: &ServerHandle) -> Result<(), RpError> { 132 + let as_desc = rp.discover_as(&server.active_base).await?; 133 + 134 + let par_req = ParRequest { 135 + as_descriptor: as_desc.clone(), 136 + redirect_uri: "http://localhost/callback" 137 + .parse() 138 + .expect("statically known-good URL"), 139 + scope: "atproto".to_string(), 140 + state: "sub-verification-state".to_string(), 141 + }; 142 + 143 + let par_resp = rp.do_par(&par_req).await?; 144 + let outcome = rp 145 + .do_authorize(&as_desc, &par_resp.request_uri, &par_req.redirect_uri) 146 + .await?; 147 + let code = match outcome { 148 + AuthorizeOutcome::Code { code } => code, 149 + AuthorizeOutcome::Error { error, .. } => { 150 + return Err(RpError::MetadataMalformed { 151 + reason: format!("unexpected authorize error: {error}"), 152 + }); 153 + } 154 + }; 155 + // Expected sub is the server's synthetic DID; the AS will emit a 156 + // deliberately different value under WrongSubInToken, so the 157 + // verifying RP should reject with SubMismatch. 158 + rp.do_token( 159 + &as_desc, 160 + &par_req.redirect_uri, 161 + &code, 162 + &par_resp.code_verifier, 163 + &server.identity.did, 164 + ) 165 + .await 166 + .map(|_| ()) 167 + }

+9 -1

src/commands/test/oauth/client/pipeline/interactive/scope_variations.rs

··· 357 357 &par_req.redirect_uri, 358 358 &code, 359 359 &par_resp.code_verifier, 360 + &server.identity.did, 360 361 ) 361 362 .await?; 362 363 Ok(()) ··· 394 395 &par_req.redirect_uri, 395 396 &code, 396 397 &par_resp.code_verifier, 398 + &server.identity.did, 397 399 ) 398 400 .await?; 399 401 // Verify that token response scope is narrower than requested. ··· 470 472 &par_req.redirect_uri, 471 473 &code, 472 474 &par_resp.code_verifier, 475 + &server.identity.did, 473 476 ) 474 477 .await?; 475 478 let refresh_token = token_resp ··· 478 481 479 482 // Perform refresh with narrower scope. 480 483 let refreshed = rp 481 - .do_refresh(&as_desc, &refresh_token, Some("atproto")) 484 + .do_refresh( 485 + &as_desc, 486 + &refresh_token, 487 + Some("atproto"), 488 + &server.identity.did, 489 + ) 482 490 .await?; 483 491 // Verify the new token response scope matches narrower scope. 484 492 let granted_scope = refreshed.scope.as_deref().unwrap_or("");

+20 -3

src/common/CLAUDE.md

··· 1 1 # common 2 2 3 - Last verified: 2026-04-21 3 + Last verified: 2026-04-23 4 4 5 5 ## Purpose 6 6 ··· 98 98 - **Exposes from `oauth::relying_party`**: 99 99 - Enum: `ClientKind` (`Confidential` | `Public`). 100 100 - Types: `RelyingParty`, `AsDescriptor`, `ParRequest`, `ParResponse`, 101 - `TokenResponse`, `AuthorizeOutcome`, `RpError` 102 - (`#[derive(Diagnostic)]` with `oauth_client::relying_party::*` codes). 101 + `TokenResponse` (with a public `sub: Option<String>` field — atproto 102 + mandates the token endpoint return the authenticated account's DID), 103 + `AuthorizeOutcome`, `RpError` (`#[derive(Diagnostic)]` with 104 + `oauth_client::relying_party::*` codes). 105 + - `RpError` variants carry stable diagnostic codes; the current set 106 + includes `issuer_mismatch` (`IssuerMismatch { expected, got }`, raised 107 + by `do_authorize` when the AS redirect's `iss` query parameter does 108 + not match `AsDescriptor::issuer`) and `sub_mismatch` 109 + (`SubMismatch { expected, got: Option<String> }`, raised by `do_token` 110 + / `do_refresh` when the token response's `sub` is missing or differs 111 + from the DID the flow is pinned to). 112 + - `do_authorize`, `do_token`, and `do_refresh` perform `iss` / `sub` 113 + verification internally; `do_token` and `do_refresh` take an 114 + `expected_sub: &str` argument. For modeling non-conformant clients, 115 + the sibling helpers `do_authorize_skipping_iss_verification`, 116 + `do_token_skipping_sub_verification`, and 117 + `do_refresh_skipping_sub_verification` return the raw response with 118 + no verification — these are test-surface only and drive the 119 + `iss_sub_verification` interactive sub-stage. 103 120 - Trait: `RpFactory` (`fn build(&self, client_id: Url, kind: ClientKind) 104 121 -> RelyingParty`; callers typically hold an `Arc<dyn RpFactory>`). Two 105 122 implementations: `DefaultRpFactory`

+180

src/common/oauth/relying_party.rs

··· 64 64 #[error("Form encoding failed: {0}")] 65 65 #[diagnostic(code = "oauth_client::relying_party::form_encoding_error")] 66 66 FormEncodingError(#[from] serde_urlencoded::ser::Error), 67 + 68 + /// The `iss` parameter on the authorization redirect did not match 69 + /// the issuer advertised in AS metadata. Atproto mandates this 70 + /// check per RFC 9207 71 + /// (<https://datatracker.ietf.org/doc/html/rfc9207>) and the 72 + /// `authorization_response_iss_parameter_supported = true` 73 + /// contract the fake AS and real atproto ASes both honor. 74 + #[error("authorization redirect `iss` ({got:?}) does not match expected issuer ({expected:?})")] 75 + #[diagnostic(code = "oauth_client::relying_party::issuer_mismatch")] 76 + IssuerMismatch { expected: String, got: String }, 77 + 78 + /// The `sub` field of the token response did not match the account 79 + /// DID the client initiated the flow for. Atproto calls this 80 + /// verification *"critical (mandatory)"*; see 81 + /// <https://atproto.com/specs/oauth#authentication-and-subject>. 82 + #[error("token response `sub` ({got:?}) does not match expected account DID ({expected:?})")] 83 + #[diagnostic(code = "oauth_client::relying_party::sub_mismatch")] 84 + SubMismatch { 85 + expected: String, 86 + got: Option<String>, 87 + }, 67 88 } 68 89 69 90 /// Authorization server metadata needed for OAuth flows. ··· 100 121 pub expires_in: u64, 101 122 pub refresh_token: Option<String>, 102 123 pub scope: Option<String>, 124 + /// Account DID identified by the AS. Atproto requires this field 125 + /// on every initial and refresh token response; see 126 + /// <https://atproto.com/specs/oauth#authentication-and-subject>. 127 + pub sub: Option<String>, 103 128 } 104 129 105 130 /// Outcome of the authorization request (either code or error). ··· 423 448 } 424 449 425 450 /// Perform authorization request and obtain code. 451 + /// 452 + /// Verifies the `iss` query parameter on the redirect matches 453 + /// `as_descriptor.issuer`. Atproto and RFC 9207 mandate this check 454 + /// to defend against AS-mixup attacks. A mismatch returns 455 + /// `RpError::IssuerMismatch` and aborts the flow before any code 456 + /// exchange. 426 457 pub async fn do_authorize( 427 458 &self, 428 459 as_descriptor: &AsDescriptor, 429 460 request_uri: &str, 430 461 redirect_uri: &Url, 431 462 ) -> Result<AuthorizeOutcome, RpError> { 463 + self.do_authorize_inner(as_descriptor, request_uri, redirect_uri, true) 464 + .await 465 + } 466 + 467 + /// Shared implementation for `do_authorize` and the test-surface 468 + /// variant that deliberately skips `iss` verification. 469 + async fn do_authorize_inner( 470 + &self, 471 + as_descriptor: &AsDescriptor, 472 + request_uri: &str, 473 + redirect_uri: &Url, 474 + verify_iss: bool, 475 + ) -> Result<AuthorizeOutcome, RpError> { 432 476 let mut url = as_descriptor.authorization_endpoint.clone(); 433 477 url.query_pairs_mut() 434 478 .append_pair("request_uri", request_uri) ··· 455 499 let location_url = Url::parse(location_str)?; 456 500 457 501 if location_url.origin() == redirect_uri.origin() { 502 + if verify_iss { 503 + verify_redirect_iss(&location_url, &as_descriptor.issuer)?; 504 + } 458 505 return self.extract_auth_response(&location_url); 459 506 } 460 507 } ··· 466 513 } 467 514 468 515 /// Exchange code for tokens. 516 + /// 517 + /// Verifies the `sub` field of the token response matches 518 + /// `expected_sub`. Atproto calls this verification 519 + /// *"critical (mandatory)"* — the client must confirm the account 520 + /// identified by the AS is the one it initiated the flow for 521 + /// (<https://atproto.com/specs/oauth#authentication-and-subject>). 522 + /// A mismatch or missing `sub` returns `RpError::SubMismatch` and 523 + /// discards the token. 469 524 pub async fn do_token( 470 525 &self, 471 526 as_descriptor: &AsDescriptor, 472 527 redirect_uri: &Url, 473 528 code: &str, 474 529 code_verifier: &str, 530 + expected_sub: &str, 531 + ) -> Result<TokenResponse, RpError> { 532 + self.do_token_inner( 533 + as_descriptor, 534 + redirect_uri, 535 + code, 536 + code_verifier, 537 + Some(expected_sub), 538 + ) 539 + .await 540 + } 541 + 542 + /// Shared implementation for `do_token` and the test-surface 543 + /// variant that deliberately skips `sub` verification. 544 + async fn do_token_inner( 545 + &self, 546 + as_descriptor: &AsDescriptor, 547 + redirect_uri: &Url, 548 + code: &str, 549 + code_verifier: &str, 550 + expected_sub: Option<&str>, 475 551 ) -> Result<TokenResponse, RpError> { 476 552 let mut params = vec![ 477 553 ("grant_type", "authorization_code"), ··· 517 593 } 518 594 519 595 let token_response: TokenResponse = response.json().await?; 596 + if let Some(sub) = expected_sub { 597 + verify_token_sub(&token_response, sub)?; 598 + } 520 599 Ok(token_response) 521 600 } 522 601 523 602 /// Refresh an access token using a refresh token. 603 + /// 604 + /// Verifies the `sub` field of the refresh response matches 605 + /// `expected_sub` for the same reason as `do_token`: atproto 606 + /// requires `sub` on every token response, initial or refresh, so 607 + /// the client can continue to confirm account identity. 524 608 pub async fn do_refresh( 525 609 &self, 526 610 as_descriptor: &AsDescriptor, 527 611 refresh_token: &str, 528 612 scope: Option<&str>, 613 + expected_sub: &str, 614 + ) -> Result<TokenResponse, RpError> { 615 + self.do_refresh_inner(as_descriptor, refresh_token, scope, Some(expected_sub)) 616 + .await 617 + } 618 + 619 + /// Shared implementation for `do_refresh` and the test-surface 620 + /// variant that deliberately skips `sub` verification. 621 + async fn do_refresh_inner( 622 + &self, 623 + as_descriptor: &AsDescriptor, 624 + refresh_token: &str, 625 + scope: Option<&str>, 626 + expected_sub: Option<&str>, 529 627 ) -> Result<TokenResponse, RpError> { 530 628 let mut params = vec![ 531 629 ("grant_type", "refresh_token"), ··· 572 670 } 573 671 574 672 let token_response: TokenResponse = response.json().await?; 673 + if let Some(sub) = expected_sub { 674 + verify_token_sub(&token_response, sub)?; 675 + } 575 676 Ok(token_response) 576 677 } 577 678 ··· 801 902 self.sign_dpop_inner(&jti, htm, htu, ath, false) 802 903 } 803 904 905 + /// Test-surface: drive `/oauth/authorize` exactly like `do_authorize`, 906 + /// but skip the RFC 9207 / atproto `iss` verification step. Used by 907 + /// the broken-AS sub-stage to model a non-conformant client that 908 + /// does not verify the `iss` parameter. 909 + pub async fn do_authorize_skipping_iss_verification( 910 + &self, 911 + as_descriptor: &AsDescriptor, 912 + request_uri: &str, 913 + redirect_uri: &Url, 914 + ) -> Result<AuthorizeOutcome, RpError> { 915 + self.do_authorize_inner(as_descriptor, request_uri, redirect_uri, false) 916 + .await 917 + } 918 + 919 + /// Test-surface: exchange code for tokens exactly like `do_token`, 920 + /// but skip the atproto `sub` verification step. Used by the 921 + /// broken-AS sub-stage to model a non-conformant client that does 922 + /// not verify the `sub` field of the token response. 923 + pub async fn do_token_skipping_sub_verification( 924 + &self, 925 + as_descriptor: &AsDescriptor, 926 + redirect_uri: &Url, 927 + code: &str, 928 + code_verifier: &str, 929 + ) -> Result<TokenResponse, RpError> { 930 + self.do_token_inner(as_descriptor, redirect_uri, code, code_verifier, None) 931 + .await 932 + } 933 + 934 + /// Test-surface: refresh tokens like `do_refresh`, but skip the 935 + /// atproto `sub` verification step. 936 + pub async fn do_refresh_skipping_sub_verification( 937 + &self, 938 + as_descriptor: &AsDescriptor, 939 + refresh_token: &str, 940 + scope: Option<&str>, 941 + ) -> Result<TokenResponse, RpError> { 942 + self.do_refresh_inner(as_descriptor, refresh_token, scope, None) 943 + .await 944 + } 945 + 804 946 /// Extract code or error from authorization response location. 805 947 fn extract_auth_response(&self, location: &Url) -> Result<AuthorizeOutcome, RpError> { 806 948 let query_pairs: HashMap<String, String> = location.query_pairs().into_owned().collect(); ··· 817 959 reason: "authorization response has no code or error".to_string(), 818 960 }) 819 961 } 962 + } 963 + } 964 + 965 + /// Verify that the `iss` query parameter on an authorization 966 + /// redirect matches the expected AS issuer. The comparison strips a 967 + /// single trailing slash from both sides because atproto's 968 + /// `authorization_response_iss_parameter_supported` contract permits 969 + /// either form. 970 + fn verify_redirect_iss(location: &Url, expected_issuer: &Url) -> Result<(), RpError> { 971 + let got = location 972 + .query_pairs() 973 + .find(|(k, _)| k == "iss") 974 + .map(|(_, v)| v.into_owned()); 975 + let expected = expected_issuer.as_str().trim_end_matches('/').to_string(); 976 + match got { 977 + Some(iss) => { 978 + if iss.trim_end_matches('/') == expected { 979 + Ok(()) 980 + } else { 981 + Err(RpError::IssuerMismatch { expected, got: iss }) 982 + } 983 + } 984 + None => Err(RpError::IssuerMismatch { 985 + expected, 986 + got: String::new(), 987 + }), 988 + } 989 + } 990 + 991 + /// Verify the `sub` field of a token response matches the expected 992 + /// account DID. 993 + fn verify_token_sub(token: &TokenResponse, expected_sub: &str) -> Result<(), RpError> { 994 + match token.sub.as_deref() { 995 + Some(sub) if sub == expected_sub => Ok(()), 996 + other => Err(RpError::SubMismatch { 997 + expected: expected_sub.to_string(), 998 + got: other.map(str::to_owned), 999 + }), 820 1000 } 821 1001 } 822 1002

+10 -3

tests/oauth_client_ac_coverage.rs

··· 303 303 &par_req(&as_desc).redirect_uri, 304 304 &code, 305 305 &par.code_verifier, 306 + &server.identity.did, 306 307 ) 307 308 .await?; 308 309 Ok(token) ··· 408 409 &Url::parse("http://localhost/callback").unwrap(), 409 410 &code, 410 411 &par.code_verifier, 412 + &server.identity.did, 411 413 ) 412 414 .await 413 415 .unwrap(); ··· 480 482 481 483 let as_desc = rp.discover_as(&server.active_base).await.unwrap(); 482 484 let _ = rp 483 - .do_refresh(&as_desc, &rt1, Some("atproto")) 485 + .do_refresh(&as_desc, &rt1, Some("atproto"), &server.identity.did) 484 486 .await 485 487 .expect("refresh with narrower scope"); 486 488 ··· 566 568 let rt1 = token1.refresh_token.expect("rt1"); 567 569 568 570 let as_desc = rp.discover_as(&server.active_base).await.unwrap(); 569 - let token2 = rp.do_refresh(&as_desc, &rt1, None).await.expect("rotate"); 571 + let token2 = rp 572 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 573 + .await 574 + .expect("rotate"); 570 575 let rt2 = token2.refresh_token.expect("rt2"); 571 576 assert_ne!(rt2, rt1, "rt2 must differ from rt1"); 572 577 573 578 // Re-using rt1 after rotation must be rejected. 574 - let result = rp.do_refresh(&as_desc, &rt1, None).await; 579 + let result = rp 580 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 581 + .await; 575 582 assert!( 576 583 result.is_err(), 577 584 "RP must receive rejection when reusing a rotated refresh_token"

+214 -8

tests/oauth_client_broken_rp.rs

··· 1 1 //! Conformance tests for the interactive sub-stage Failure-AC checks. 2 2 //! 3 3 //! These tests drive the `RelyingParty` test-surface helpers 4 - //! (`send_raw_par`, `sign_dpop_with_fixed_jti`, `sign_dpop_ignoring_nonce`) to 5 - //! deliberately inject non-conformant requests into the fake AS's request log, 6 - //! then run the corresponding sub-stage and assert the check emits 7 - //! `SpecViolation` as specified by AC6.5, AC6.6, AC7.4, AC7.5, AC7.6. 4 + //! (`send_raw_par`, `sign_dpop_with_fixed_jti`, `sign_dpop_ignoring_nonce`, 5 + //! `do_authorize_skipping_iss_verification`, 6 + //! `do_token_skipping_sub_verification`) to deliberately inject 7 + //! non-conformant behaviour, then either run the corresponding sub-stage 8 + //! and assert the check emits `SpecViolation`, or observe the RP's 9 + //! verification behaviour directly. Covers AC6.5, AC6.6, AC7.4, AC7.5, 10 + //! AC7.6, and AC4.10. 8 11 //! 9 12 //! In normal flows the production `RelyingParty` never emits these violations, 10 13 //! so these tests are the sole guarantee that the check-emission logic can ··· 17 20 use atproto_devtool::commands::test::oauth::client::fake_as::endpoints::FlowScript; 18 21 use atproto_devtool::commands::test::oauth::client::fake_as::{FakeAsOptions, ServerHandle}; 19 22 use atproto_devtool::commands::test::oauth::client::pipeline::interactive::{ 20 - dpop_edges, scope_variations, 23 + dpop_edges, iss_sub_verification, scope_variations, 21 24 }; 22 25 use atproto_devtool::common::oauth::clock::Clock; 23 26 use atproto_devtool::common::oauth::relying_party::{ 24 - ClientKind, DeterministicRpFactory, RelyingParty, RpFactory, 27 + AuthorizeOutcome, ClientKind, DeterministicRpFactory, ParRequest, RelyingParty, RpError, 28 + RpFactory, 25 29 }; 26 30 use atproto_devtool::common::report::CheckStatus; 27 31 use common::FakeClock; ··· 329 333 &par_req.redirect_uri, 330 334 &code, 331 335 &par_resp.code_verifier, 336 + &server.identity.did, 332 337 ) 333 338 .await 334 339 .expect("do_token"); ··· 338 343 // doesn't persist the rotated rt. First call succeeds and rotates; second 339 344 // call with the now-used rt1 is rejected by the fake AS. 340 345 let _refresh_1 = rp 341 - .do_refresh(&as_desc, &rt1, None) 346 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 342 347 .await 343 348 .expect("first refresh should succeed"); 344 - let refresh_2 = rp.do_refresh(&as_desc, &rt1, None).await; 349 + let refresh_2 = rp 350 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 351 + .await; 345 352 assert!( 346 353 refresh_2.is_err(), 347 354 "second refresh with same rt1 must be rejected", ··· 363 370 364 371 server.shutdown().await; 365 372 } 373 + 374 + // ============================================================================= 375 + // AC4.10: Client-side `iss` / `sub` verification 376 + // ============================================================================= 377 + 378 + fn standard_par_request( 379 + as_desc: &atproto_devtool::common::oauth::relying_party::AsDescriptor, 380 + state: &str, 381 + ) -> ParRequest { 382 + ParRequest { 383 + as_descriptor: as_desc.clone(), 384 + redirect_uri: Url::parse("http://localhost/callback").unwrap(), 385 + scope: "atproto".to_string(), 386 + state: state.to_string(), 387 + } 388 + } 389 + 390 + async fn drive_par_and_authorize( 391 + rp: &RelyingParty, 392 + server: &ServerHandle, 393 + state: &str, 394 + ) -> ( 395 + atproto_devtool::common::oauth::relying_party::AsDescriptor, 396 + Url, 397 + atproto_devtool::common::oauth::relying_party::ParResponse, 398 + ) { 399 + let as_desc = rp.discover_as(&server.active_base).await.expect("discover"); 400 + let par_req = standard_par_request(&as_desc, state); 401 + let par_resp = rp.do_par(&par_req).await.expect("par"); 402 + (as_desc, par_req.redirect_uri, par_resp) 403 + } 404 + 405 + #[tokio::test] 406 + async fn ac4_10_iss_bogus_iss_rejected_by_compliant_rp() { 407 + let (server, clock) = spawn_fake_as().await; 408 + let rp = build_rp(clock); 409 + 410 + *server.app_state().flow_script.lock().unwrap() = FlowScript::BogusIssOnRedirect { 411 + granted_scope: "atproto".to_string(), 412 + bogus_iss: "https://evil.example.com".to_string(), 413 + }; 414 + 415 + let (as_desc, redirect_uri, par_resp) = 416 + drive_par_and_authorize(&rp, &server, "state-iss").await; 417 + let outcome = rp 418 + .do_authorize(&as_desc, &par_resp.request_uri, &redirect_uri) 419 + .await; 420 + 421 + match outcome { 422 + Err(RpError::IssuerMismatch { got, .. }) => { 423 + assert!( 424 + got.contains("evil.example.com"), 425 + "IssuerMismatch must carry the bogus iss value (got {got})", 426 + ); 427 + } 428 + other => panic!("expected IssuerMismatch; got {other:?}"), 429 + } 430 + 431 + server.shutdown().await; 432 + } 433 + 434 + #[tokio::test] 435 + async fn ac4_10_iss_non_verifying_rp_accepts_bogus_iss() { 436 + let (server, clock) = spawn_fake_as().await; 437 + let rp = build_rp(clock); 438 + 439 + *server.app_state().flow_script.lock().unwrap() = FlowScript::BogusIssOnRedirect { 440 + granted_scope: "atproto".to_string(), 441 + bogus_iss: "https://evil.example.com".to_string(), 442 + }; 443 + 444 + let (as_desc, redirect_uri, par_resp) = 445 + drive_par_and_authorize(&rp, &server, "state-iss-skip").await; 446 + let outcome = rp 447 + .do_authorize_skipping_iss_verification(&as_desc, &par_resp.request_uri, &redirect_uri) 448 + .await 449 + .expect("non-verifying RP accepts bogus iss"); 450 + 451 + match outcome { 452 + AuthorizeOutcome::Code { .. } => { 453 + // A client that skips verification silently proceeds with 454 + // the bogus iss — this is the behaviour the sub-stage must 455 + // detect as a SpecViolation when it sees a non-verifying RP. 456 + } 457 + other => panic!("expected Code outcome; got {other:?}"), 458 + } 459 + 460 + server.shutdown().await; 461 + } 462 + 463 + #[tokio::test] 464 + async fn ac4_10_sub_wrong_sub_rejected_by_compliant_rp() { 465 + let (server, clock) = spawn_fake_as().await; 466 + let rp = build_rp(clock); 467 + 468 + *server.app_state().flow_script.lock().unwrap() = FlowScript::WrongSubInToken { 469 + granted_scope: "atproto".to_string(), 470 + wrong_sub: "did:web:evil.example.com".to_string(), 471 + }; 472 + 473 + let (as_desc, redirect_uri, par_resp) = 474 + drive_par_and_authorize(&rp, &server, "state-sub").await; 475 + let outcome = rp 476 + .do_authorize(&as_desc, &par_resp.request_uri, &redirect_uri) 477 + .await 478 + .expect("authorize"); 479 + let code = match outcome { 480 + AuthorizeOutcome::Code { code } => code, 481 + other => panic!("expected Code outcome; got {other:?}"), 482 + }; 483 + 484 + let token_result = rp 485 + .do_token( 486 + &as_desc, 487 + &redirect_uri, 488 + &code, 489 + &par_resp.code_verifier, 490 + &server.identity.did, 491 + ) 492 + .await; 493 + 494 + match token_result { 495 + Err(RpError::SubMismatch { got, .. }) => { 496 + assert_eq!( 497 + got.as_deref(), 498 + Some("did:web:evil.example.com"), 499 + "SubMismatch must carry the wrong sub value", 500 + ); 501 + } 502 + other => panic!("expected SubMismatch; got {other:?}"), 503 + } 504 + 505 + server.shutdown().await; 506 + } 507 + 508 + #[tokio::test] 509 + async fn ac4_10_sub_non_verifying_rp_accepts_wrong_sub() { 510 + let (server, clock) = spawn_fake_as().await; 511 + let rp = build_rp(clock); 512 + 513 + *server.app_state().flow_script.lock().unwrap() = FlowScript::WrongSubInToken { 514 + granted_scope: "atproto".to_string(), 515 + wrong_sub: "did:web:evil.example.com".to_string(), 516 + }; 517 + 518 + let (as_desc, redirect_uri, par_resp) = 519 + drive_par_and_authorize(&rp, &server, "state-sub-skip").await; 520 + let outcome = rp 521 + .do_authorize(&as_desc, &par_resp.request_uri, &redirect_uri) 522 + .await 523 + .expect("authorize"); 524 + let code = match outcome { 525 + AuthorizeOutcome::Code { code } => code, 526 + other => panic!("expected Code outcome; got {other:?}"), 527 + }; 528 + 529 + let token = rp 530 + .do_token_skipping_sub_verification(&as_desc, &redirect_uri, &code, &par_resp.code_verifier) 531 + .await 532 + .expect("non-verifying RP accepts wrong sub"); 533 + 534 + assert_eq!( 535 + token.sub.as_deref(), 536 + Some("did:web:evil.example.com"), 537 + "token response carries the wrong sub", 538 + ); 539 + 540 + server.shutdown().await; 541 + } 542 + 543 + #[tokio::test] 544 + async fn ac4_10_iss_sub_verification_sub_stage_emits_pass_for_compliant_rp() { 545 + let (server, clock) = spawn_fake_as().await; 546 + let rp = build_rp(clock); 547 + 548 + let results = iss_sub_verification::run(&server, &rp).await; 549 + 550 + assert_eq!(results.len(), 2, "sub-stage emits iss + sub results"); 551 + assert_eq!( 552 + results[0].id, 553 + "oauth_client::interactive::client_verified_iss", 554 + ); 555 + assert_eq!( 556 + results[0].status, 557 + CheckStatus::Pass, 558 + "compliant RP aborts on bogus iss → Pass", 559 + ); 560 + assert_eq!( 561 + results[1].id, 562 + "oauth_client::interactive::client_verified_sub", 563 + ); 564 + assert_eq!( 565 + results[1].status, 566 + CheckStatus::Pass, 567 + "compliant RP rejects wrong sub → Pass", 568 + ); 569 + 570 + server.shutdown().await; 571 + }

+24

tests/oauth_client_interactive.rs

··· 359 359 CheckStatus::Skipped, 360 360 "ClientRefreshed should be skipped in Phase 7" 361 361 ); 362 + 363 + // AC4.10: the broken-AS sub-stage drives two extra flows and 364 + // expects a compliant RP to abort on bogus iss / wrong sub. 365 + let client_verified_iss = output 366 + .results 367 + .iter() 368 + .find(|r| r.id == "oauth_client::interactive::client_verified_iss") 369 + .expect("ClientVerifiedIss check present"); 370 + assert_eq!( 371 + client_verified_iss.status, 372 + CheckStatus::Pass, 373 + "ClientVerifiedIss should pass when RP aborts on bogus iss" 374 + ); 375 + 376 + let client_verified_sub = output 377 + .results 378 + .iter() 379 + .find(|r| r.id == "oauth_client::interactive::client_verified_sub") 380 + .expect("ClientVerifiedSub check present"); 381 + assert_eq!( 382 + client_verified_sub.status, 383 + CheckStatus::Pass, 384 + "ClientVerifiedSub should pass when RP rejects wrong sub" 385 + ); 362 386 } 363 387 364 388 #[tokio::test]

+8 -2

tests/oauth_client_substage_snapshots.rs

··· 178 178 &Url::parse("http://localhost/callback").unwrap(), 179 179 &code, 180 180 &par.code_verifier, 181 + &server.identity.did, 181 182 ) 182 183 .await 183 184 .unwrap(); 184 185 let rt1 = token.refresh_token.unwrap(); 185 186 186 187 // Rotate via do_refresh (rt1 → rt2) and then intentionally reuse rt1. 187 - let _ = rp.do_refresh(&as_desc, &rt1, None).await.unwrap(); 188 - let _ = rp.do_refresh(&as_desc, &rt1, None).await; // Expected rejection; log captures both. 188 + let _ = rp 189 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 190 + .await 191 + .unwrap(); 192 + let _ = rp 193 + .do_refresh(&as_desc, &rt1, None, &server.identity.did) 194 + .await; // Expected rejection; log captures both. 189 195 190 196 let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED)); 191 197 let results = dpop_edges::run(&server, factory.as_ref(), clock).await;

Configure Feed

Configure Feed