CLI app for developers prototyping atproto functionality
feat(oauth-client): broken-RP test helpers + Failure-AC conformance tests (C1)
Close the C1 gap from Phase 8 code review: the five Failure-AC checks
(AC6.5 pkce_required, AC6.6 dpop_required, AC7.4 jti_reused, AC7.5
nonce_ignored, AC7.6 refresh_token_reused) could not previously be
verified to fire on a broken log, because the production RelyingParty
always produces conforming requests and no test harness exercised the
SpecViolation path.
Added test-surface helpers on RelyingParty (explicitly documented as
conformance-test-only):
- send_raw_par: post a hand-rolled PAR body with an optional DPoP header.
- build_par_body_without_pkce: construct a PAR body lacking PKCE fields.
- build_par_body_with_pkce: construct a well-formed PAR body.
- sign_dpop_with_fixed_jti: sign a DPoP proof with a caller-supplied jti
(for AC7.4 replay tests).
- sign_dpop_ignoring_nonce: sign a DPoP proof that deliberately omits the
cached nonce claim (for AC7.5 tests).
Refactored sign_dpop internally to dispatch through sign_dpop_inner so
the nonce-inclusion policy and jti source can be overridden.
Added tests/oauth_client_broken_rp.rs with 5 integration tests, one per
Failure-AC. Each test spawns a fake AS, uses a broken-RP helper to
inject a non-conformant entry into the request log, then runs the
corresponding sub-stage (scope_variations or dpop_edges) and asserts
the specific Check emits SpecViolation with the stable code.
All 5 tests pass, bringing the Phase 8 Failure-AC checks from
observationally-only to test-enforced.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
+485
-2
+120
-2
src/common/oauth/relying_party.rs
+120
-2
src/common/oauth/relying_party.rs
···
609
609
/// Sign a DPoP proof JWT.
610
610
fn sign_dpop(&self, htm: &str, htu: &Url, ath: Option<&str>) -> Result<String, RpError> {
611
611
let jti = self.new_jti();
612
+
self.sign_dpop_inner(&jti, htm, htu, ath, true)
613
+
}
614
+
615
+
/// Inner helper for DPoP signing that accepts a caller-supplied `jti`.
616
+
///
617
+
/// `include_nonce` controls whether the current endpoint's cached nonce is
618
+
/// included. Test-surface helpers pass `false` to deliberately omit a nonce
619
+
/// that the server has issued (AC7.5 conformance check).
620
+
fn sign_dpop_inner(
621
+
&self,
622
+
jti: &str,
623
+
htm: &str,
624
+
htu: &Url,
625
+
ath: Option<&str>,
626
+
include_nonce: bool,
627
+
) -> Result<String, RpError> {
612
628
let iat = self.clock.now_unix_seconds();
613
629
614
630
// Build header with jwk (DPoP-specific).
···
626
642
});
627
643
628
644
// Include nonce claim if a nonce is stored for this endpoint.
629
-
if let Some(nonce) = self.dpop_nonces.lock().unwrap().get(htu).cloned() {
630
-
claims["nonce"] = json!(nonce);
645
+
if include_nonce {
646
+
if let Some(nonce) = self.dpop_nonces.lock().unwrap().get(htu).cloned() {
647
+
claims["nonce"] = json!(nonce);
648
+
}
631
649
}
632
650
633
651
if let Some(ath_val) = ath {
···
681
699
682
700
let jwt = jws::sign_es256_jws(&header, &claims, &self.encoding_key)?;
683
701
Ok(jwt)
702
+
}
703
+
704
+
// ===== Test-surface helpers =====
705
+
//
706
+
// The following methods are intentionally part of the public API so that
707
+
// conformance tests can submit deliberately malformed requests. They are
708
+
// NOT called by production flows, and should only be used by the
709
+
// `oauth_client_broken_rp.rs` integration test and any follow-up
710
+
// conformance suites.
711
+
712
+
/// Test-surface: send a hand-rolled PAR body with an optional DPoP header.
713
+
///
714
+
/// Returns `(status, response_body_text)` so callers can inspect both the
715
+
/// fake AS's rejection code and its error response body. Unlike `do_par`,
716
+
/// this does NOT retry on `use_dpop_nonce` and does NOT parse the response
717
+
/// as JSON.
718
+
pub async fn send_raw_par(
719
+
&self,
720
+
url: &Url,
721
+
body: &[u8],
722
+
dpop: Option<&str>,
723
+
) -> Result<(u16, String), RpError> {
724
+
let mut req = self
725
+
.http
726
+
.post(url.clone())
727
+
.header("Content-Type", "application/x-www-form-urlencoded")
728
+
.body(body.to_vec());
729
+
if let Some(d) = dpop {
730
+
req = req.header("DPoP", d);
731
+
}
732
+
let response = req.send().await?;
733
+
let status = response.status().as_u16();
734
+
let body_text = response.text().await.unwrap_or_default();
735
+
Ok((status, body_text))
736
+
}
737
+
738
+
/// Test-surface: build a form-encoded PAR body that deliberately omits
739
+
/// `code_challenge` and `code_challenge_method` (AC6.5 negative test).
740
+
pub fn build_par_body_without_pkce(
741
+
&self,
742
+
redirect_uri: &Url,
743
+
scope: &str,
744
+
state: &str,
745
+
) -> Result<String, RpError> {
746
+
let params = [
747
+
("response_type", "code"),
748
+
("client_id", self.client_id.as_str()),
749
+
("redirect_uri", redirect_uri.as_str()),
750
+
("scope", scope),
751
+
("state", state),
752
+
];
753
+
Ok(serde_urlencoded::to_string(params)?)
754
+
}
755
+
756
+
/// Test-surface: build a well-formed PAR body (with PKCE S256) but allow
757
+
/// the caller to choose whether to send a DPoP header (AC6.6 negative test).
758
+
/// Returns `(body, code_verifier)`.
759
+
pub fn build_par_body_with_pkce(
760
+
&self,
761
+
redirect_uri: &Url,
762
+
scope: &str,
763
+
state: &str,
764
+
) -> Result<(String, String), RpError> {
765
+
let (verifier, challenge) = self.new_pkce();
766
+
let params = [
767
+
("response_type", "code"),
768
+
("client_id", self.client_id.as_str()),
769
+
("redirect_uri", redirect_uri.as_str()),
770
+
("scope", scope),
771
+
("state", state),
772
+
("code_challenge", challenge.as_str()),
773
+
("code_challenge_method", "S256"),
774
+
];
775
+
Ok((serde_urlencoded::to_string(params)?, verifier))
776
+
}
777
+
778
+
/// Test-surface: sign a DPoP proof with a caller-supplied `jti`.
779
+
///
780
+
/// Used by AC7.4 (jti reuse) negative tests where two requests must carry
781
+
/// the same jti.
782
+
pub fn sign_dpop_with_fixed_jti(
783
+
&self,
784
+
htm: &str,
785
+
htu: &Url,
786
+
ath: Option<&str>,
787
+
jti: &str,
788
+
) -> Result<String, RpError> {
789
+
self.sign_dpop_inner(jti, htm, htu, ath, true)
790
+
}
791
+
792
+
/// Test-surface: sign a DPoP proof that deliberately omits the `nonce`
793
+
/// claim even if the server has issued one (AC7.5 negative test).
794
+
pub fn sign_dpop_ignoring_nonce(
795
+
&self,
796
+
htm: &str,
797
+
htu: &Url,
798
+
ath: Option<&str>,
799
+
) -> Result<String, RpError> {
800
+
let jti = self.new_jti();
801
+
self.sign_dpop_inner(&jti, htm, htu, ath, false)
684
802
}
685
803
686
804
/// Extract code or error from authorization response location.
+365
tests/oauth_client_broken_rp.rs
+365
tests/oauth_client_broken_rp.rs
···
1
+
//! Conformance tests for the interactive sub-stage Failure-AC checks.
2
+
//!
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.
8
+
//!
9
+
//! In normal flows the production `RelyingParty` never emits these violations,
10
+
//! so these tests are the sole guarantee that the check-emission logic can
11
+
//! fire on a bad log.
12
+
13
+
mod common;
14
+
15
+
use std::sync::Arc;
16
+
17
+
use atproto_devtool::commands::test::oauth::client::fake_as::endpoints::FlowScript;
18
+
use atproto_devtool::commands::test::oauth::client::fake_as::{FakeAsOptions, ServerHandle};
19
+
use atproto_devtool::commands::test::oauth::client::pipeline::interactive::{
20
+
dpop_edges, scope_variations,
21
+
};
22
+
use atproto_devtool::common::oauth::clock::Clock;
23
+
use atproto_devtool::common::oauth::relying_party::{
24
+
ClientKind, DeterministicRpFactory, RelyingParty, RpFactory,
25
+
};
26
+
use atproto_devtool::common::report::CheckStatus;
27
+
use common::FakeClock;
28
+
use url::Url;
29
+
30
+
const SEED: [u8; 32] = [7u8; 32];
31
+
32
+
async fn spawn_fake_as() -> (ServerHandle, Arc<dyn Clock>) {
33
+
let clock: Arc<dyn Clock> = Arc::new(FakeClock::new(1_700_000_000));
34
+
let handle = ServerHandle::bind(
35
+
FakeAsOptions {
36
+
bind_port: None,
37
+
public_base_url: None,
38
+
},
39
+
clock.clone(),
40
+
)
41
+
.await
42
+
.expect("bind fake AS");
43
+
(handle, clock)
44
+
}
45
+
46
+
fn build_rp(clock: Arc<dyn Clock>) -> RelyingParty {
47
+
RelyingParty::new(
48
+
Url::parse("http://localhost:3000").unwrap(),
49
+
ClientKind::Public,
50
+
clock,
51
+
SEED,
52
+
)
53
+
}
54
+
55
+
fn par_url(server: &ServerHandle) -> Url {
56
+
let mut base = server.active_base.clone();
57
+
base.set_path("/oauth/par");
58
+
base
59
+
}
60
+
61
+
fn find_check_status(
62
+
results: &[atproto_devtool::common::report::CheckResult],
63
+
id: &str,
64
+
) -> CheckStatus {
65
+
results
66
+
.iter()
67
+
.find(|r| r.id == id)
68
+
.unwrap_or_else(|| panic!("check {id} not in results"))
69
+
.status
70
+
}
71
+
72
+
// =============================================================================
73
+
// AC6.5: PAR without PKCE -> pkce_required SpecViolation
74
+
// =============================================================================
75
+
76
+
#[tokio::test]
77
+
async fn ac6_5_par_without_pkce_emits_pkce_required_spec_violation() {
78
+
let (server, clock) = spawn_fake_as().await;
79
+
let rp = build_rp(clock.clone());
80
+
let par_url = par_url(&server);
81
+
82
+
// Build a PAR body without code_challenge and ship it with a valid DPoP.
83
+
let body = rp
84
+
.build_par_body_without_pkce(
85
+
&Url::parse("http://localhost/callback").unwrap(),
86
+
"atproto",
87
+
"state123",
88
+
)
89
+
.expect("build body");
90
+
let dpop = rp
91
+
.sign_dpop_with_fixed_jti("POST", &par_url, None, "broken-rp-pkce-jti")
92
+
.expect("sign dpop");
93
+
let (status, _body) = rp
94
+
.send_raw_par(&par_url, body.as_bytes(), Some(&dpop))
95
+
.await
96
+
.expect("send raw par");
97
+
98
+
// Fake AS enforces PKCE and rejects with 400.
99
+
assert_eq!(
100
+
status, 400,
101
+
"fake AS must reject PAR without code_challenge"
102
+
);
103
+
104
+
// Cumulative log now contains a PAR without code_challenge. Run the
105
+
// scope_variations sub-stage and confirm PkceRequired emits SpecViolation.
106
+
let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED));
107
+
let results = scope_variations::run(&server, factory.as_ref(), clock).await;
108
+
109
+
assert_eq!(
110
+
find_check_status(
111
+
&results,
112
+
"oauth_client::interactive::scope_variations::pkce_required",
113
+
),
114
+
CheckStatus::SpecViolation,
115
+
"PkceRequired must be SpecViolation when log contains a PAR without code_challenge",
116
+
);
117
+
118
+
server.shutdown().await;
119
+
}
120
+
121
+
// =============================================================================
122
+
// AC6.6: PAR without DPoP header -> dpop_required SpecViolation
123
+
// =============================================================================
124
+
125
+
#[tokio::test]
126
+
async fn ac6_6_par_without_dpop_emits_dpop_required_spec_violation() {
127
+
let (server, clock) = spawn_fake_as().await;
128
+
let rp = build_rp(clock.clone());
129
+
let par_url = par_url(&server);
130
+
131
+
// Build a well-formed PAR body (with PKCE) but send no DPoP header.
132
+
let (body, _verifier) = rp
133
+
.build_par_body_with_pkce(
134
+
&Url::parse("http://localhost/callback").unwrap(),
135
+
"atproto",
136
+
"state123",
137
+
)
138
+
.expect("build body");
139
+
let (status, _body) = rp
140
+
.send_raw_par(&par_url, body.as_bytes(), None)
141
+
.await
142
+
.expect("send raw par");
143
+
144
+
assert_eq!(status, 400, "fake AS must reject PAR without DPoP header");
145
+
146
+
// Log now contains a PAR without DPoP header; DpopRequired must SpecViolate.
147
+
let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED));
148
+
let results = scope_variations::run(&server, factory.as_ref(), clock).await;
149
+
150
+
assert_eq!(
151
+
find_check_status(
152
+
&results,
153
+
"oauth_client::interactive::scope_variations::dpop_required",
154
+
),
155
+
CheckStatus::SpecViolation,
156
+
"DpopRequired must be SpecViolation when log contains a PAR without DPoP header",
157
+
);
158
+
159
+
server.shutdown().await;
160
+
}
161
+
162
+
// =============================================================================
163
+
// AC7.4: Two PARs with the same jti -> jti_reused SpecViolation
164
+
// =============================================================================
165
+
166
+
#[tokio::test]
167
+
async fn ac7_4_jti_reuse_emits_jti_reused_spec_violation() {
168
+
let (server, clock) = spawn_fake_as().await;
169
+
let rp = build_rp(clock.clone());
170
+
let par_url = par_url(&server);
171
+
172
+
// Two PARs signed with the same fixed jti. The second should be rejected
173
+
// by the fake AS as a replay.
174
+
let fixed_jti = "broken-rp-reused-jti";
175
+
176
+
let (body1, _v1) = rp
177
+
.build_par_body_with_pkce(
178
+
&Url::parse("http://localhost/callback").unwrap(),
179
+
"atproto",
180
+
"state-a",
181
+
)
182
+
.expect("build body");
183
+
let dpop1 = rp
184
+
.sign_dpop_with_fixed_jti("POST", &par_url, None, fixed_jti)
185
+
.expect("sign dpop");
186
+
let (status1, _b1) = rp
187
+
.send_raw_par(&par_url, body1.as_bytes(), Some(&dpop1))
188
+
.await
189
+
.expect("send raw par");
190
+
191
+
let (body2, _v2) = rp
192
+
.build_par_body_with_pkce(
193
+
&Url::parse("http://localhost/callback").unwrap(),
194
+
"atproto",
195
+
"state-b",
196
+
)
197
+
.expect("build body");
198
+
let dpop2 = rp
199
+
.sign_dpop_with_fixed_jti("POST", &par_url, None, fixed_jti)
200
+
.expect("sign dpop");
201
+
let (status2, _b2) = rp
202
+
.send_raw_par(&par_url, body2.as_bytes(), Some(&dpop2))
203
+
.await
204
+
.expect("send raw par");
205
+
206
+
// First succeeds (201), second rejected (401) due to jti replay.
207
+
assert!(status1 == 201, "first PAR should succeed; got {status1}",);
208
+
assert_eq!(status2, 401, "second PAR with reused jti must be rejected");
209
+
210
+
// Log contains two DPoP proofs with the same jti; JtiReuseViolation SpecViolates.
211
+
let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED));
212
+
let results = dpop_edges::run(&server, factory.as_ref(), clock).await;
213
+
214
+
assert_eq!(
215
+
find_check_status(
216
+
&results,
217
+
"oauth_client::interactive::dpop_edges::jti_reused",
218
+
),
219
+
CheckStatus::SpecViolation,
220
+
"JtiReuseViolation must be SpecViolation when log contains two DPoP proofs with same jti",
221
+
);
222
+
223
+
server.shutdown().await;
224
+
}
225
+
226
+
// =============================================================================
227
+
// AC7.5: Response issues DPoP-Nonce but next request omits it -> nonce_ignored SpecViolation
228
+
// =============================================================================
229
+
230
+
#[tokio::test]
231
+
async fn ac7_5_ignoring_dpop_nonce_emits_nonce_ignored_spec_violation() {
232
+
let (server, clock) = spawn_fake_as().await;
233
+
let rp = build_rp(clock.clone());
234
+
let par_url = par_url(&server);
235
+
236
+
// Force the fake AS to issue a DPoP-Nonce on the first PAR.
237
+
*server.app_state().flow_script.lock().unwrap() = FlowScript::DpopNonceRetryOnPar {
238
+
nonce: "issued-nonce".into(),
239
+
};
240
+
241
+
// First PAR: normal proof (no nonce claim), server rejects with 400 + DPoP-Nonce.
242
+
let (body1, _v1) = rp
243
+
.build_par_body_with_pkce(
244
+
&Url::parse("http://localhost/callback").unwrap(),
245
+
"atproto",
246
+
"state-first",
247
+
)
248
+
.expect("build body");
249
+
let dpop1 = rp
250
+
.sign_dpop_with_fixed_jti("POST", &par_url, None, "broken-rp-nonce-jti-1")
251
+
.expect("sign dpop");
252
+
let (status1, _) = rp
253
+
.send_raw_par(&par_url, body1.as_bytes(), Some(&dpop1))
254
+
.await
255
+
.expect("send raw par 1");
256
+
assert_eq!(
257
+
status1, 400,
258
+
"first PAR must be rejected with use_dpop_nonce"
259
+
);
260
+
261
+
// Second PAR: use sign_dpop_ignoring_nonce to deliberately omit the nonce
262
+
// claim, even though a nonce was just issued.
263
+
let (body2, _v2) = rp
264
+
.build_par_body_with_pkce(
265
+
&Url::parse("http://localhost/callback").unwrap(),
266
+
"atproto",
267
+
"state-second",
268
+
)
269
+
.expect("build body");
270
+
let dpop2 = rp
271
+
.sign_dpop_ignoring_nonce("POST", &par_url, None)
272
+
.expect("sign dpop ignoring nonce");
273
+
let (_status2, _) = rp
274
+
.send_raw_par(&par_url, body2.as_bytes(), Some(&dpop2))
275
+
.await
276
+
.expect("send raw par 2");
277
+
278
+
// Log now contains a 2nd PAR whose DPoP proof lacks the nonce claim.
279
+
// NonceIgnoredViolation must emit SpecViolation.
280
+
let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED));
281
+
let results = dpop_edges::run(&server, factory.as_ref(), clock).await;
282
+
283
+
assert_eq!(
284
+
find_check_status(
285
+
&results,
286
+
"oauth_client::interactive::dpop_edges::nonce_ignored",
287
+
),
288
+
CheckStatus::SpecViolation,
289
+
"NonceIgnoredViolation must be SpecViolation when 2nd PAR's DPoP proof omits the issued nonce",
290
+
);
291
+
292
+
server.shutdown().await;
293
+
}
294
+
295
+
// =============================================================================
296
+
// AC7.6: Two token requests with the same refresh_token -> refresh_token_reused SpecViolation
297
+
// =============================================================================
298
+
299
+
#[tokio::test]
300
+
async fn ac7_6_refresh_token_reuse_emits_refresh_token_reused_spec_violation() {
301
+
let (server, clock) = spawn_fake_as().await;
302
+
let rp = build_rp(clock.clone());
303
+
304
+
// Drive a full happy-path flow to obtain rt1.
305
+
*server.app_state().flow_script.lock().unwrap() = FlowScript::Approve {
306
+
granted_scope: "atproto".to_string(),
307
+
};
308
+
309
+
let as_desc = rp.discover_as(&server.active_base).await.expect("discover");
310
+
311
+
let par_req = atproto_devtool::common::oauth::relying_party::ParRequest {
312
+
as_descriptor: as_desc.clone(),
313
+
redirect_uri: Url::parse("http://localhost/callback").unwrap(),
314
+
scope: "atproto".to_string(),
315
+
state: "state-rt".to_string(),
316
+
};
317
+
let par_resp = rp.do_par(&par_req).await.expect("do_par");
318
+
let auth_outcome = rp
319
+
.do_authorize(&as_desc, &par_resp.request_uri, &par_req.redirect_uri)
320
+
.await
321
+
.expect("do_authorize");
322
+
let code = match auth_outcome {
323
+
atproto_devtool::common::oauth::relying_party::AuthorizeOutcome::Code { code } => code,
324
+
_ => panic!("expected authorization code"),
325
+
};
326
+
let token_resp = rp
327
+
.do_token(
328
+
&as_desc,
329
+
&par_req.redirect_uri,
330
+
&code,
331
+
&par_resp.code_verifier,
332
+
)
333
+
.await
334
+
.expect("do_token");
335
+
let rt1 = token_resp.refresh_token.expect("rt1 present");
336
+
337
+
// Call do_refresh twice with the same rt1 — simulating a broken RP that
338
+
// doesn't persist the rotated rt. First call succeeds and rotates; second
339
+
// call with the now-used rt1 is rejected by the fake AS.
340
+
let _refresh_1 = rp
341
+
.do_refresh(&as_desc, &rt1, None)
342
+
.await
343
+
.expect("first refresh should succeed");
344
+
let refresh_2 = rp.do_refresh(&as_desc, &rt1, None).await;
345
+
assert!(
346
+
refresh_2.is_err(),
347
+
"second refresh with same rt1 must be rejected",
348
+
);
349
+
350
+
// Log contains two /oauth/token POSTs carrying the same refresh_token= value.
351
+
// RefreshTokenReuseViolation must emit SpecViolation.
352
+
let factory: Arc<dyn RpFactory> = Arc::new(DeterministicRpFactory::new(clock.clone(), SEED));
353
+
let results = dpop_edges::run(&server, factory.as_ref(), clock).await;
354
+
355
+
assert_eq!(
356
+
find_check_status(
357
+
&results,
358
+
"oauth_client::interactive::dpop_edges::refresh_token_reused",
359
+
),
360
+
CheckStatus::SpecViolation,
361
+
"RefreshTokenReuseViolation must be SpecViolation when log shows rt1 used twice",
362
+
);
363
+
364
+
server.shutdown().await;
365
+
}