Skip to content

Commit 2a4d4e5

Browse files
refactor(webauthn): narrow RelatedOriginsHttpClient error to WellKnownFetchError
The trait's old return type was the full RelatedOriginsError, but four of its five variants (UnexpectedContentType, MalformedJson, MalformedDocument, NoMatchingOrigin) are produced inside validate_related_origins after the fetch returns. Implementers had no reason to ever emit them. Introduce WellKnownFetchError with the variants a fetcher can actually emit (Transport, Status, BodyTooLarge, NotSupported) and let RelatedOriginsError wrap it via a Fetch variant with #[from]. The reqwest client now distinguishes non-200 status from transport faults and from body-cap hits without stringifying everything. Also drops RelatedOriginsError::kind(); the two debug! call sites switch to logging the Display form of the error directly.
1 parent 41ef219 commit 2a4d4e5

6 files changed

Lines changed: 62 additions & 58 deletions

File tree

libwebauthn/src/ops/webauthn/get_assertion.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ impl FromIdlModel<PublicKeyCredentialRequestOptionsJSON, GetAssertionRequestPars
187187
if let Err(err) =
188188
validate_related_origins(&request_origin.origin, &parsed, psl, http).await
189189
{
190-
debug!(rp_id = %parsed.0, kind = err.kind(), "Related-origins validation failed");
190+
debug!(rp_id = %parsed.0, %err, "Related-origins validation failed");
191191
return Err(GetAssertionRequestParsingError::MismatchingRelyingPartyId(
192192
parsed.0,
193193
effective_rp_id.to_string(),
@@ -664,7 +664,7 @@ mod tests {
664664

665665
use crate::ops::webauthn::psl::MockPublicSuffixList;
666666
use crate::ops::webauthn::related_origins::{
667-
RelatedOriginsError, RelatedOriginsHttpClient, WellKnownResponse,
667+
RelatedOriginsHttpClient, WellKnownFetchError, WellKnownResponse,
668668
};
669669
use crate::ops::webauthn::{GetAssertionRequest, NoRelatedOriginsClient, RequestOrigin};
670670
use crate::proto::ctap2::Ctap2PublicKeyCredentialType;
@@ -674,7 +674,7 @@ mod tests {
674674
/// Test-only HTTP client backed by a fixed response. `panicking` proves the
675675
/// suffix-check short-circuit by failing the test if the fetch is invoked.
676676
struct MockHttpClient {
677-
response: Option<Result<WellKnownResponse, RelatedOriginsError>>,
677+
response: Option<Result<WellKnownResponse, WellKnownFetchError>>,
678678
}
679679

680680
impl MockHttpClient {
@@ -687,7 +687,7 @@ mod tests {
687687
}
688688
}
689689

690-
fn err(e: RelatedOriginsError) -> Self {
690+
fn err(e: WellKnownFetchError) -> Self {
691691
Self {
692692
response: Some(Err(e)),
693693
}
@@ -703,7 +703,7 @@ mod tests {
703703
async fn fetch_well_known(
704704
&self,
705705
_: &RelyingPartyId,
706-
) -> Result<WellKnownResponse, RelatedOriginsError> {
706+
) -> Result<WellKnownResponse, WellKnownFetchError> {
707707
match &self.response {
708708
Some(r) => r.clone(),
709709
None => panic!("fetch_well_known should not be called"),
@@ -912,7 +912,7 @@ mod tests {
912912
async fn related_origins_fetch_error_keeps_mismatch_error() {
913913
let request_origin: RequestOrigin = "https://app.example.org".parse().unwrap();
914914
let req_json = json_field_add(REQUEST_BASE_JSON, "rpId", r#""example.com""#);
915-
let http = MockHttpClient::err(RelatedOriginsError::FetchFailed("simulated".into()));
915+
let http = MockHttpClient::err(WellKnownFetchError::Transport("simulated".into()));
916916

917917
let result = GetAssertionRequest::from_json(
918918
&request_origin,

libwebauthn/src/ops/webauthn/make_credential.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ impl FromIdlModel<PublicKeyCredentialCreationOptionsJSON, MakeCredentialRequestP
401401
if let Err(err) =
402402
validate_related_origins(&request_origin.origin, &rp_id, psl, http).await
403403
{
404-
debug!(rp_id = %rp_id.0, kind = err.kind(), "Related-origins validation failed");
404+
debug!(rp_id = %rp_id.0, %err, "Related-origins validation failed");
405405
return Err(
406406
MakeCredentialRequestParsingError::MismatchingRelyingPartyId(
407407
rp_id.0,
@@ -719,7 +719,7 @@ mod tests {
719719

720720
use crate::ops::webauthn::psl::MockPublicSuffixList;
721721
use crate::ops::webauthn::related_origins::{
722-
RelatedOriginsError, RelatedOriginsHttpClient, WellKnownResponse,
722+
RelatedOriginsHttpClient, WellKnownFetchError, WellKnownResponse,
723723
};
724724
use crate::ops::webauthn::{MakeCredentialRequest, NoRelatedOriginsClient, RequestOrigin};
725725
use crate::proto::ctap2::Ctap2PublicKeyCredentialType;
@@ -729,7 +729,7 @@ mod tests {
729729
/// Test-only HTTP client backed by a fixed response. `panicking` proves the
730730
/// suffix-check short-circuit by failing the test if the fetch is invoked.
731731
struct MockHttpClient {
732-
response: Option<Result<WellKnownResponse, RelatedOriginsError>>,
732+
response: Option<Result<WellKnownResponse, WellKnownFetchError>>,
733733
}
734734

735735
impl MockHttpClient {
@@ -742,7 +742,7 @@ mod tests {
742742
}
743743
}
744744

745-
fn err(e: RelatedOriginsError) -> Self {
745+
fn err(e: WellKnownFetchError) -> Self {
746746
Self {
747747
response: Some(Err(e)),
748748
}
@@ -758,7 +758,7 @@ mod tests {
758758
async fn fetch_well_known(
759759
&self,
760760
_: &RelyingPartyId,
761-
) -> Result<WellKnownResponse, RelatedOriginsError> {
761+
) -> Result<WellKnownResponse, WellKnownFetchError> {
762762
match &self.response {
763763
Some(r) => r.clone(),
764764
None => panic!("fetch_well_known should not be called"),
@@ -1262,7 +1262,7 @@ mod tests {
12621262
"rp",
12631263
r#"{"id": "example.com", "name": "example.com"}"#,
12641264
);
1265-
let http = MockHttpClient::err(RelatedOriginsError::FetchFailed("simulated".into()));
1265+
let http = MockHttpClient::err(WellKnownFetchError::Transport("simulated".into()));
12661266

12671267
let result = MakeCredentialRequest::from_json(
12681268
&request_origin,

libwebauthn/src/ops/webauthn/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ pub use psl::{
3939
};
4040
pub use related_origins::{
4141
validate_related_origins, NoRelatedOriginsClient, RelatedOriginsError,
42-
RelatedOriginsHttpClient, WellKnownResponse,
42+
RelatedOriginsHttpClient, WellKnownFetchError, WellKnownResponse,
4343
};
4444
#[cfg(feature = "related-origins-client")]
4545
pub use related_origins::{HttpPolicy, ReqwestRelatedOriginsClient};

libwebauthn/src/ops/webauthn/related_origins/http.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use futures::StreamExt;
88
use reqwest::redirect::Policy;
99
use reqwest::{Client, StatusCode};
1010

11-
use super::{RelatedOriginsError, RelatedOriginsHttpClient, WellKnownResponse};
11+
use super::{RelatedOriginsHttpClient, WellKnownFetchError, WellKnownResponse};
1212
use crate::ops::webauthn::idl::rpid::RelyingPartyId;
1313

1414
#[derive(Debug, Clone)]
@@ -35,11 +35,11 @@ pub struct ReqwestRelatedOriginsClient {
3535
}
3636

3737
impl ReqwestRelatedOriginsClient {
38-
pub fn new() -> Result<Self, RelatedOriginsError> {
38+
pub fn new() -> Result<Self, WellKnownFetchError> {
3939
Self::with_policy(HttpPolicy::default())
4040
}
4141

42-
pub fn with_policy(policy: HttpPolicy) -> Result<Self, RelatedOriginsError> {
42+
pub fn with_policy(policy: HttpPolicy) -> Result<Self, WellKnownFetchError> {
4343
let max_redirects = policy.max_redirects;
4444
let redirect_policy = Policy::custom(move |attempt| {
4545
if attempt.previous().len() >= max_redirects {
@@ -58,7 +58,7 @@ impl ReqwestRelatedOriginsClient {
5858
.referer(false)
5959
.timeout(policy.request_timeout)
6060
.build()
61-
.map_err(|e| RelatedOriginsError::FetchFailed(e.to_string()))?;
61+
.map_err(|e| WellKnownFetchError::Transport(e.to_string()))?;
6262
Ok(Self {
6363
client,
6464
max_body_bytes: policy.max_body_bytes,
@@ -71,19 +71,16 @@ impl RelatedOriginsHttpClient for ReqwestRelatedOriginsClient {
7171
async fn fetch_well_known(
7272
&self,
7373
rp_id: &RelyingPartyId,
74-
) -> Result<WellKnownResponse, RelatedOriginsError> {
74+
) -> Result<WellKnownResponse, WellKnownFetchError> {
7575
let url = format!("https://{}/.well-known/webauthn", rp_id.0);
7676
let response = self
7777
.client
7878
.get(&url)
7979
.send()
8080
.await
81-
.map_err(|e| RelatedOriginsError::FetchFailed(e.to_string()))?;
81+
.map_err(|e| WellKnownFetchError::Transport(e.to_string()))?;
8282
if response.status() != StatusCode::OK {
83-
return Err(RelatedOriginsError::FetchFailed(format!(
84-
"status {}",
85-
response.status()
86-
)));
83+
return Err(WellKnownFetchError::Status(response.status().as_u16()));
8784
}
8885
let content_type = response
8986
.headers()
@@ -94,11 +91,9 @@ impl RelatedOriginsHttpClient for ReqwestRelatedOriginsClient {
9491
let mut body = Vec::with_capacity(8 * 1024);
9592
let mut stream = response.bytes_stream();
9693
while let Some(chunk) = stream.next().await {
97-
let chunk = chunk.map_err(|e| RelatedOriginsError::FetchFailed(e.to_string()))?;
94+
let chunk = chunk.map_err(|e| WellKnownFetchError::Transport(e.to_string()))?;
9895
if body.len() + chunk.len() > self.max_body_bytes {
99-
return Err(RelatedOriginsError::FetchFailed(
100-
"body exceeded size cap".into(),
101-
));
96+
return Err(WellKnownFetchError::BodyTooLarge);
10297
}
10398
body.extend_from_slice(&chunk);
10499
}

libwebauthn/src/ops/webauthn/related_origins/mod.rs

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,41 @@ pub struct WellKnownResponse {
3232
/// Fetcher for `https://{rp_id}/.well-known/webauthn`, per WebAuthn L3 §5.11.1
3333
/// step 2. Implementations MUST send no credentials, no Referer, refuse
3434
/// non-`https://` redirects, cap the body size, and bound the request duration.
35-
/// Implementations MUST return `Err(FetchFailed)` for any status code other
36-
/// than 200 (after following redirects). Implementations MUST report the wire
37-
/// `Content-Type` header value unmodified (or `None` if absent) and MUST NOT
38-
/// synthesise an `application/json` content type for non-JSON responses.
35+
/// Implementations MUST return `Err(WellKnownFetchError::Status)` for any
36+
/// status code other than 200 (after following redirects). Implementations MUST
37+
/// report the wire `Content-Type` header value unmodified (or `None` if absent)
38+
/// and MUST NOT synthesise an `application/json` content type for non-JSON
39+
/// responses.
3940
#[async_trait]
4041
pub trait RelatedOriginsHttpClient: Send + Sync {
4142
async fn fetch_well_known(
4243
&self,
4344
rp_id: &RelyingPartyId,
44-
) -> Result<WellKnownResponse, RelatedOriginsError>;
45+
) -> Result<WellKnownResponse, WellKnownFetchError>;
46+
}
47+
48+
/// Failure modes for [`RelatedOriginsHttpClient::fetch_well_known`].
49+
#[derive(thiserror::Error, Debug, Clone)]
50+
pub enum WellKnownFetchError {
51+
/// Transport-level failure: TLS, DNS, timeout, rejected redirect, body
52+
/// stream interrupt, client build error, etc.
53+
#[error("transport error: {0}")]
54+
Transport(String),
55+
/// Endpoint replied with a non-200 status (after following redirects).
56+
#[error("HTTP status {0}")]
57+
Status(u16),
58+
/// Body exceeded the implementation's configured size cap before completion.
59+
#[error("body exceeded configured size cap")]
60+
BodyTooLarge,
61+
/// Implementation does not perform fetches (see [`NoRelatedOriginsClient`]).
62+
#[error("client does not support related-origin fetches")]
63+
NotSupported,
4564
}
4665

4766
#[derive(thiserror::Error, Debug, Clone)]
4867
pub enum RelatedOriginsError {
4968
#[error("well-known fetch failed: {0}")]
50-
FetchFailed(String),
69+
Fetch(#[from] WellKnownFetchError),
5170
#[error("unexpected content type: {0:?}")]
5271
UnexpectedContentType(Option<String>),
5372
/// Step 2.b: body did not decode as JSON.
@@ -60,19 +79,6 @@ pub enum RelatedOriginsError {
6079
NoMatchingOrigin,
6180
}
6281

63-
impl RelatedOriginsError {
64-
/// Log-safe variant discriminant; `Debug`/`Display` may carry reqwest/serde text with IPs or body snippets.
65-
pub fn kind(&self) -> &'static str {
66-
match self {
67-
RelatedOriginsError::FetchFailed(_) => "fetch_failed",
68-
RelatedOriginsError::UnexpectedContentType(_) => "unexpected_content_type",
69-
RelatedOriginsError::MalformedJson(_) => "malformed_json",
70-
RelatedOriginsError::MalformedDocument(_) => "malformed_document",
71-
RelatedOriginsError::NoMatchingOrigin => "no_matching_origin",
72-
}
73-
}
74-
}
75-
7682
pub type RelatedOriginsResult = Result<(), RelatedOriginsError>;
7783

7884
#[derive(Debug, Deserialize)]
@@ -192,10 +198,8 @@ impl RelatedOriginsHttpClient for NoRelatedOriginsClient {
192198
async fn fetch_well_known(
193199
&self,
194200
_: &RelyingPartyId,
195-
) -> Result<WellKnownResponse, RelatedOriginsError> {
196-
Err(RelatedOriginsError::FetchFailed(
197-
"this client does not support related origin requests".into(),
198-
))
201+
) -> Result<WellKnownResponse, WellKnownFetchError> {
202+
Err(WellKnownFetchError::NotSupported)
199203
}
200204
}
201205

@@ -205,15 +209,15 @@ mod tests {
205209
use super::*;
206210

207211
struct MockClient {
208-
response: Result<WellKnownResponse, RelatedOriginsError>,
212+
response: Result<WellKnownResponse, WellKnownFetchError>,
209213
}
210214

211215
#[async_trait]
212216
impl RelatedOriginsHttpClient for MockClient {
213217
async fn fetch_well_known(
214218
&self,
215219
_: &RelyingPartyId,
216-
) -> Result<WellKnownResponse, RelatedOriginsError> {
220+
) -> Result<WellKnownResponse, WellKnownFetchError> {
217221
self.response.clone()
218222
}
219223
}
@@ -670,9 +674,9 @@ mod tests {
670674
}
671675

672676
#[tokio::test]
673-
async fn fetch_error_propagates_as_fetch_failed() {
677+
async fn fetch_error_propagates_as_fetch() {
674678
let http = MockClient {
675-
response: Err(RelatedOriginsError::FetchFailed("simulated".into())),
679+
response: Err(WellKnownFetchError::Transport("simulated".into())),
676680
};
677681
let res = validate_related_origins(
678682
&caller("https://example.com"),
@@ -681,7 +685,12 @@ mod tests {
681685
&http,
682686
)
683687
.await;
684-
assert!(matches!(res, Err(RelatedOriginsError::FetchFailed(_))));
688+
assert!(matches!(
689+
res,
690+
Err(RelatedOriginsError::Fetch(WellKnownFetchError::Transport(
691+
_
692+
)))
693+
));
685694
}
686695

687696
#[tokio::test]

libwebauthn/tests/related_origins.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
use async_trait::async_trait;
77

88
use libwebauthn::ops::webauthn::{
9-
GetAssertionRequest, MakeCredentialRequest, PublicSuffixList, RelatedOriginsError,
10-
RelatedOriginsHttpClient, RelyingPartyId, RequestOrigin, WebAuthnIDL, WellKnownResponse,
9+
GetAssertionRequest, MakeCredentialRequest, PublicSuffixList, RelatedOriginsHttpClient,
10+
RelyingPartyId, RequestOrigin, WebAuthnIDL, WellKnownFetchError, WellKnownResponse,
1111
};
1212

1313
const KNOWN_SUFFIXES: &[&str] = &["com", "org"];
@@ -39,7 +39,7 @@ impl RelatedOriginsHttpClient for StaticHttp {
3939
async fn fetch_well_known(
4040
&self,
4141
_: &RelyingPartyId,
42-
) -> Result<WellKnownResponse, RelatedOriginsError> {
42+
) -> Result<WellKnownResponse, WellKnownFetchError> {
4343
Ok(WellKnownResponse {
4444
content_type: Some("application/json".into()),
4545
body: self.body.as_bytes().to_vec(),

0 commit comments

Comments
 (0)