feat: support multi-challenge WWW-Authenticate headers#185
Conversation
|
f17f8a9 to
b644d00
Compare
Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d4417-fcd8-76fd-a3c1-4532e1393c2c
b644d00 to
9c007fe
Compare
src/protocol/core/headers.rs
Outdated
| /// Find the first `tempo` method challenge from one or more WWW-Authenticate | ||
| /// header values. | ||
| /// | ||
| /// This is a convenience wrapper around [`parse_www_authenticate_all`] that | ||
| /// filters for `method="tempo"` and returns the first match. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use mpp::protocol::core::find_tempo_challenge; | ||
| /// | ||
| /// let header = concat!( | ||
| /// r#"Payment id="a", realm="api", method="tempo", intent="charge", request="e30", "#, | ||
| /// r#"Payment id="b", realm="api", method="stripe", intent="charge", request="e30""#, | ||
| /// ); | ||
| /// let challenge = find_tempo_challenge(vec![header]).unwrap(); | ||
| /// assert_eq!(challenge.id, "a"); | ||
| /// assert_eq!(challenge.method.as_str(), "tempo"); | ||
| /// ``` | ||
| pub fn find_tempo_challenge<'a>( | ||
| headers: impl IntoIterator<Item = &'a str>, | ||
| ) -> Result<PaymentChallenge> { | ||
| parse_www_authenticate_all(headers) | ||
| .into_iter() | ||
| .find(|r| r.as_ref().is_ok_and(|c| c.method.as_str() == "tempo")) | ||
| .unwrap_or_else(|| { | ||
| Err(MppError::invalid_challenge_reason( | ||
| "No Payment challenge with method=\"tempo\" found", | ||
| )) | ||
| }) | ||
| } |
There was a problem hiding this comment.
should this be in the tempo module?
Move the Tempo-specific find_tempo_challenge function from protocol::core::headers to protocol::methods::tempo where it belongs. Re-export from protocol::core for backward compatibility. Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
The methods module is gated behind feature = "server" or feature = "tempo", so the re-export must have the same cfg gate. Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
src/protocol/core/headers.rs
Outdated
| #[test] | ||
| fn test_parse_www_authenticate_all_multi_challenge() { | ||
| let header = concat!( | ||
| r#"Payment id="t1", realm="r", method="tempo", intent="charge", request="e30", "#, |
There was a problem hiding this comment.
can we also add a test hwich includes non-payment authentication headers e.g. Bearer
There was a problem hiding this comment.
added some more integration tests with 358cc68
src/protocol/methods/tempo/mod.rs
Outdated
| /// assert_eq!(challenge.id, "a"); | ||
| /// assert_eq!(challenge.method.as_str(), "tempo"); | ||
| /// ``` | ||
| pub fn find_tempo_challenge<'a>( |
There was a problem hiding this comment.
I think we may need something more robust than this
the client should iterate through all challenges and compare against their own set of supported methods (intent/method) -- we could select naively from that.
With this implementation i think there is a chance the server returns (session, charge) but the client only accepts charge
There was a problem hiding this comment.
equivilant mppx flow
https://github.com/wevm/mppx/blob/main/src/client/internal/Fetch.ts#L48-L73
…rsing Verify that Bearer, Basic, and other non-Payment schemes are silently ignored by parse_www_authenticate_all, both as separate header values and mixed within a single header value. Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
The tempo module is gated behind feature = "tempo" only, not feature = "server". Align the re-export cfg accordingly. Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
Replace single-challenge parsing with multi-challenge matching in both fetch.rs and middleware.rs. The client now parses all challenges from the 402 response and selects the first one the provider supports (by method+intent), matching mppx's Fetch.ts behavior. Remove find_tempo_challenge — it was a Tempo-specific shortcut that is superseded by the generic provider.supports() matching. Add HttpError::NoSupportedChallenge for when no challenge matches. Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d445a-5f14-754e-93cc-84e2bf98afc1
Cover the 5 key scenarios for the new challenge selection logic: - Server offers multiple methods, provider selects supported one - First supported challenge is picked when provider supports many - NoSupportedChallenge error when no match exists - Challenges spread across separate WWW-Authenticate headers - Intent mismatch correctly rejected (tempo/session vs tempo/charge) Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Co-Authored-By: grandizzy <38490174+grandizzy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d44fc-2a43-73a8-92b6-167ceb7b388f
Client now matches challenges by
provider.supports(method, intent)instead of assuming a single challenge, mirroring the mppx TypeScript SDK.Changes:
split_payment_challenges: splits a combined header into individual Payment challenge slicesparse_www_authenticate_all: handles both separate headers and combined multi-challenge headersPaymentExt+PaymentMiddleware) iterates all challenges and selects the first one the provider supportsHttpError::NoSupportedChallengefor when no offered challenge matchesPrompted by: georgen