CLI app for developers prototyping atproto functionality
1
fork

Configure Feed

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

Honour `response_mode` and use 303 on the authorize redirect

The fake AS's `/oauth/authorize` handler had two bugs that broke
fragment-mode public clients:

- It used `Redirect::permanent`, returning HTTP 308 instead of a
one-shot redirect status. 308 is permanent and cacheable, which is
wrong for OAuth, and some clients silently drop a 308 they never
asked for. The handler now emits 303 (See Other), which
unambiguously demotes the redirect to GET regardless of how the
user-agent reached `/oauth/authorize` — matching what most modern
OAuth providers (Auth0, Okta, etc.) use in practice.

- The `response_mode` parameter from PAR was discarded entirely. A
client that requested `response_mode=fragment` (common for
in-browser public clients) still saw `code` / `state` / `iss`
appended to the redirect URL's query string, and silently lost
the response since it was looking in the URL fragment.

A new `ResponseMode` enum is captured at PAR time and consulted
when building the redirect; both query and fragment modes are
declared in the served AS metadata, so honouring whichever the
client asks for is now consistent with the document.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

+92 -21
+92 -21
src/commands/test/oauth/client/fake_as/endpoints.rs
··· 7 7 extract::{Request, State}, 8 8 http::{HeaderMap, HeaderValue, Method, StatusCode, Uri}, 9 9 middleware::{Next, from_fn}, 10 - response::{IntoResponse, Json, Redirect, Response}, 10 + response::{IntoResponse, Json, Response}, 11 11 routing::{get, post}, 12 12 }; 13 13 use base64::Engine; ··· 63 63 pub code_challenge_method: Option<String>, 64 64 pub client_assertion_type: Option<String>, 65 65 pub client_assertion: Option<String>, 66 + /// `response_mode` selects whether the authorization response 67 + /// parameters land in the redirect URL's query (`query`) or 68 + /// fragment (`fragment`). Atproto AS metadata advertises both; 69 + /// browser-based public clients commonly request `fragment` so 70 + /// the code never reaches the server hosting the redirect_uri. 71 + pub response_mode: Option<String>, 72 + } 73 + 74 + /// Where the authorization response parameters are placed on the 75 + /// redirect URL. 76 + #[derive(Debug, Clone, Copy, PartialEq, Eq)] 77 + pub enum ResponseMode { 78 + /// Append parameters to the URL query string. RFC 6749 default 79 + /// for `response_type = code`. 80 + Query, 81 + /// Append parameters to the URL fragment. Common for in-browser 82 + /// public clients that consume the response without a server 83 + /// round-trip. 84 + Fragment, 85 + } 86 + 87 + impl ResponseMode { 88 + /// Parse the `response_mode` PAR parameter; defaults to `Query` 89 + /// (RFC 6749 default for `response_type = code`). 90 + fn from_par(raw: Option<&str>) -> Self { 91 + match raw { 92 + Some("fragment") => ResponseMode::Fragment, 93 + _ => ResponseMode::Query, 94 + } 95 + } 66 96 } 67 97 68 98 /// Snapshot of a pending PAR request for later authorization. ··· 75 105 pub state: String, 76 106 pub code_challenge: String, 77 107 pub code_challenge_method: String, 108 + pub response_mode: ResponseMode, 78 109 } 79 110 80 111 /// Binding of an authorization code to its request and scope. ··· 552 583 let request_uri = format!("urn:ietf:params:oauth:request_uri:fake-{now}"); 553 584 554 585 // Store PAR request snapshot. 586 + let response_mode = ResponseMode::from_par(params.response_mode.as_deref()); 555 587 let par_snap = ParRequestSnapshot { 556 588 response_type: params.response_type.unwrap_or_default(), 557 589 client_id: params.client_id.unwrap_or_default(), ··· 569 601 state: params.state.unwrap_or_default(), 570 602 code_challenge, 571 603 code_challenge_method, 604 + response_mode, 572 605 }; 573 606 574 607 let mut pending = s.pending_par.lock().unwrap(); ··· 660 693 } 661 694 }; 662 695 663 - let mut redirect_url = par_request.redirect_uri.clone(); 664 - redirect_url.query_pairs_mut().clear(); 665 - 696 + // Atproto OAuth requires the authorization response to include 697 + // `iss` so the client can verify the AS matches the one it 698 + // resolved for the account. See 699 + // <https://atproto.com/specs/oauth#authorization-response>. 700 + // The `BogusIssOnRedirect` script deliberately overrides this 701 + // value to test client-side verification. 666 702 if should_approve { 667 703 // Generate code (deterministic from clock + counter). 668 704 let now = s.clock.now_unix_seconds(); ··· 678 714 codes.insert(code.clone(), code_binding); 679 715 drop(codes); 680 716 681 - // Atproto OAuth requires the authorization response to 682 - // include `iss` so the client can verify the AS matches the 683 - // one it resolved for the account. See 684 - // <https://atproto.com/specs/oauth#authorization-response>. 685 - // The `BogusIssOnRedirect` script deliberately overrides this 686 - // value to test client-side verification. 687 - redirect_url 688 - .query_pairs_mut() 689 - .append_pair("code", &code) 690 - .append_pair("state", &par_request.state) 691 - .append_pair("iss", &iss_to_send); 717 + finalize_authorize_redirect( 718 + &par_request.redirect_uri, 719 + par_request.response_mode, 720 + &[ 721 + ("code", code.as_str()), 722 + ("state", par_request.state.as_str()), 723 + ("iss", iss_to_send.as_str()), 724 + ], 725 + ) 692 726 } else { 693 - redirect_url 694 - .query_pairs_mut() 695 - .append_pair("error", "access_denied") 696 - .append_pair("state", &par_request.state) 697 - .append_pair("iss", &iss_to_send); 727 + finalize_authorize_redirect( 728 + &par_request.redirect_uri, 729 + par_request.response_mode, 730 + &[ 731 + ("error", "access_denied"), 732 + ("state", par_request.state.as_str()), 733 + ("iss", iss_to_send.as_str()), 734 + ], 735 + ) 698 736 } 737 + } 699 738 700 - Redirect::permanent(redirect_url.as_str()).into_response() 739 + /// Build a 303 (See Other) redirect to the registered 740 + /// `redirect_uri`, placing `pairs` in either the query string or the 741 + /// fragment per RFC 6749 §3.1.2 / RFC 6749 §4.2.2. 742 + /// 743 + /// 303 unambiguously demotes the redirect to a GET regardless of how 744 + /// the user-agent reached `/oauth/authorize`, so the client's 745 + /// callback always receives a GET. (308 is permanent and cacheable, 746 + /// which is wrong for one-shot OAuth responses; 302 is method- 747 + /// preserving in practice but historically ambiguous, which is why 748 + /// most modern OAuth providers use 303.) 749 + fn finalize_authorize_redirect( 750 + redirect_uri: &Url, 751 + mode: ResponseMode, 752 + pairs: &[(&str, &str)], 753 + ) -> Response { 754 + let mut url = redirect_uri.clone(); 755 + let encoded = serde_urlencoded::to_string(pairs).unwrap_or_default(); 756 + match mode { 757 + ResponseMode::Query => { 758 + // Replace any existing query string with the OAuth response 759 + // params; the fake AS's registered redirect URIs do not 760 + // carry pre-existing query params today. 761 + url.set_query(Some(&encoded)); 762 + } 763 + ResponseMode::Fragment => { 764 + url.set_fragment(Some(&encoded)); 765 + } 766 + } 767 + ( 768 + StatusCode::SEE_OTHER, 769 + [(axum::http::header::LOCATION, url.as_str())], 770 + ) 771 + .into_response() 701 772 } 702 773 703 774 async fn token(