Tighten redirect_uri shape rules (default port, custom scheme)

+2

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

··· 88 88 - **test-oauth-client.AC2.6 Failure:** A public client whose `token_endpoint_auth_method` is anything other than `none` produces a `SpecViolation`. 89 89 - **test-oauth-client.AC2.6b Failure:** A client metadata document declaring `token_endpoint_auth_signing_alg = "none"` produces a `SpecViolation` on `token_endpoint_auth_signing_alg_valid`. The atproto OAuth profile (<https://atproto.com/specs/oauth#client-metadata>) states: *"`none` is never allowed here."* When the field is absent entirely the check emits `Skipped` (the field itself is OPTIONAL overall); the companion `has_key_for_signing_alg` JWKS check handles whether any declared non-`none` value is actually matched by a key in the JWKS. 90 90 - **test-oauth-client.AC2.7 Failure:** A native client whose `redirect_uri` scheme does not match the reverse-domain of the `client_id` host produces a `SpecViolation`. 91 + - **test-oauth-client.AC2.7b Failure:** A native client whose custom-scheme `redirect_uri` uses `scheme://host/path` rather than the spec-required `scheme:/path` (single colon, single forward slash, no authority) produces a `SpecViolation` on `redirect_uris_shape`. See <https://atproto.com/specs/oauth#authorization-request-fields>: *"The URI scheme must be followed by a single colon (`:`) then a single forward slash (`/`) and then a URI path component."* 92 + - **test-oauth-client.AC2.7c Failure:** Any HTTPS `redirect_uri` that explicitly declares the default port (`:443`) produces a `SpecViolation` on `redirect_uris_shape`. The atproto OAuth profile restates the OAuth 2.0 rule: *"The URL may include a port number, but not if it is the default port number."* Applies to web and native HTTPS redirects alike. 91 93 - **test-oauth-client.AC2.8 Failure:** A `scope` field that does not parse against the atproto permission grammar (<https://atproto.com/specs/permission#scope-string-syntax>) produces a `SpecViolation` with the offending token highlighted via miette source span. The grammar is `<resource>[:<positional>][?<query>]`; all three bracketed components are optional independently, so `repo` (bare), `repo:app.bsky.feed.post` (positional only), `repo?collection=foo` (query only), and `repo:app.bsky.feed.post?action=create` (both) are all valid. The resource name is **not** constrained to a fixed set — the spec states it will expand over time and defines `transition:generic`, `transition:chat.bsky`, and `transition:email` as migration-period resources, so the grammar check only flags genuinely syntactic failures (empty resource, malformed `key=value` query pairs, invalid percent-encoding). Unknown-but-well-formed resource names are accepted. 92 94 - **test-oauth-client.AC2.9 Skip:** Every metadata-document validation check on a loopback target emits `Skipped` with reason `"metadata is implicit for loopback clients"`. 93 95

+95

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

··· 1213 1213 if uri.scheme() != "https" { 1214 1214 return Err(RedirectValidationFailure::Other); 1215 1215 } 1216 + if https_has_explicit_default_port(uri_str) { 1217 + return Err(RedirectValidationFailure::Other); 1218 + } 1216 1219 // Check origin (scheme + host + port) matches. 1217 1220 if uri.origin() != client_id.origin() { 1218 1221 return Err(RedirectValidationFailure::Other); ··· 1228 1231 Ok(uri) => { 1229 1232 if uri.scheme() == "https" { 1230 1233 // HTTPS: origin must match. 1234 + if https_has_explicit_default_port(uri_str) { 1235 + return Err(RedirectValidationFailure::Other); 1236 + } 1231 1237 if uri.origin() != client_id.origin() { 1232 1238 return Err(RedirectValidationFailure::Other); 1233 1239 } ··· 1240 1246 if uri.scheme() != expected_scheme { 1241 1247 return Err(RedirectValidationFailure::ReverseDomainMismatch); 1242 1248 } 1249 + // The atproto OAuth profile requires 1250 + // custom-scheme redirect URIs to be 1251 + // `scheme:/path` — a single colon, then a 1252 + // single forward slash, then a path (no 1253 + // authority). Forms like 1254 + // `com.example.app://host/path` are 1255 + // non-conformant. We check the raw string 1256 + // because `url::Url` is permissive on 1257 + // non-special schemes. 1258 + if !custom_scheme_has_single_slash(uri_str) { 1259 + return Err(RedirectValidationFailure::Other); 1260 + } 1243 1261 } 1244 1262 } 1245 1263 Err(_) => return Err(RedirectValidationFailure::Other), ··· 1254 1272 Ok(()) 1255 1273 } 1256 1274 1275 + /// Returns true if the raw HTTPS URI string explicitly includes the 1276 + /// default port (`:443`). Per the atproto OAuth profile 1277 + /// (<https://atproto.com/specs/oauth#authorization-request-fields>), 1278 + /// a `redirect_uri` *"may include a port number, but not if it is 1279 + /// the default port number"*. 1280 + fn https_has_explicit_default_port(uri_str: &str) -> bool { 1281 + // Find scheme delimiter. 1282 + let Some(rest) = uri_str.strip_prefix("https://") else { 1283 + return false; 1284 + }; 1285 + // Take up to the next '/', '?', or '#'. 1286 + let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); 1287 + let authority = &rest[..authority_end]; 1288 + // Skip userinfo (atproto redirects shouldn't have it, but handle 1289 + // defensively). 1290 + let hostport = authority 1291 + .rsplit_once('@') 1292 + .map(|(_, hp)| hp) 1293 + .unwrap_or(authority); 1294 + // For IPv6 literals the host is wrapped in `[...]`; look for the 1295 + // closing bracket and match `:443` after it. 1296 + if let Some(end_bracket) = hostport.find(']') { 1297 + return hostport[end_bracket + 1..] == *":443"; 1298 + } 1299 + // Otherwise, scan for a colon after the host label. 1300 + hostport 1301 + .rsplit_once(':') 1302 + .map(|(_, port)| port == "443") 1303 + .unwrap_or(false) 1304 + } 1305 + 1306 + /// Returns true if a custom-scheme URI has the spec-required shape 1307 + /// `scheme:/path` — exactly one `/` following the colon, no authority 1308 + /// component. `com.example.app:/cb` is compliant; 1309 + /// `com.example.app://host/cb` is not. 1310 + fn custom_scheme_has_single_slash(uri_str: &str) -> bool { 1311 + let Some((_, rest)) = uri_str.split_once(':') else { 1312 + return false; 1313 + }; 1314 + // Must start with exactly one `/`; anything with `//` introduces 1315 + // an authority, which is forbidden. 1316 + rest.starts_with('/') && !rest.starts_with("//") 1317 + } 1318 + 1257 1319 /// Compute the reverse-domain form of a hostname. 1258 1320 /// 1259 1321 /// For example, `app.example.com` becomes `com.example.app`. ··· 1357 1419 #[cfg(test)] 1358 1420 mod tests { 1359 1421 use super::*; 1422 + 1423 + // Redirect-URI shape helpers — atproto profile specifics: 1424 + // - HTTPS URIs must not include the default port (`:443`). 1425 + // - Custom-scheme native URIs must be `scheme:/path` (single 1426 + // slash, no authority), not `scheme://host/path`. 1427 + 1428 + #[test] 1429 + fn https_default_port_is_explicit() { 1430 + assert!(https_has_explicit_default_port( 1431 + "https://example.com:443/cb" 1432 + )); 1433 + assert!(!https_has_explicit_default_port("https://example.com/cb")); 1434 + assert!(!https_has_explicit_default_port( 1435 + "https://example.com:8443/cb" 1436 + )); 1437 + // IPv6 literal with default port. 1438 + assert!(https_has_explicit_default_port( 1439 + "https://[2001:db8::1]:443/cb" 1440 + )); 1441 + assert!(!https_has_explicit_default_port("https://[2001:db8::1]/cb")); 1442 + // Not HTTPS. 1443 + assert!(!https_has_explicit_default_port( 1444 + "http://example.com:443/cb" 1445 + )); 1446 + } 1447 + 1448 + #[test] 1449 + fn custom_scheme_single_slash() { 1450 + assert!(custom_scheme_has_single_slash("com.example.app:/cb")); 1451 + assert!(!custom_scheme_has_single_slash("com.example.app://host/cb")); 1452 + // Reject a bare `scheme:cb` without any slash. 1453 + assert!(!custom_scheme_has_single_slash("com.example.app:cb")); 1454 + } 1360 1455 1361 1456 #[test] 1362 1457 fn scope_parses_atproto_alone() {

+10

tests/fixtures/oauth_client/metadata/native_redirect_double_slash/metadata.json

··· 1 + { 2 + "client_id": "https://app.example.com/oauth-client-metadata.json", 3 + "application_type": "native", 4 + "response_types": ["code"], 5 + "grant_types": ["authorization_code"], 6 + "scope": "atproto", 7 + "redirect_uris": ["com.example.app://host/callback"], 8 + "dpop_bound_access_tokens": true, 9 + "token_endpoint_auth_method": "none" 10 + }

+10

tests/fixtures/oauth_client/metadata/web_redirect_default_port/metadata.json

··· 1 + { 2 + "client_id": "https://client.example.com/metadata.json", 3 + "application_type": "web", 4 + "response_types": ["code"], 5 + "grant_types": ["authorization_code"], 6 + "scope": "atproto", 7 + "redirect_uris": ["https://client.example.com:443/cb"], 8 + "dpop_bound_access_tokens": true, 9 + "token_endpoint_auth_method": "none" 10 + }

+67

tests/oauth_client_metadata.rs

··· 582 582 insta::assert_snapshot!(rendered); 583 583 } 584 584 585 + /// Per <https://atproto.com/specs/oauth#authorization-request-fields>: 586 + /// "The URL may include a port number, but not if it is the default 587 + /// port number." An HTTPS `redirect_uri` that explicitly declares 588 + /// `:443` is non-conformant. 589 + #[tokio::test] 590 + async fn web_redirect_with_explicit_default_port_is_spec_violation() { 591 + let http = common::FakeHttpClient::new(); 592 + let metadata = 593 + include_bytes!("fixtures/oauth_client/metadata/web_redirect_default_port/metadata.json"); 594 + http.add_response( 595 + &Url::parse("https://client.example.com/metadata.json").unwrap(), 596 + 200, 597 + metadata.to_vec(), 598 + ); 599 + 600 + let target = target::parse("https://client.example.com/metadata.json").expect("parse failed"); 601 + let jwks_fetcher = common::FakeJwksFetcher::new(); 602 + let opts = OauthClientOptions { 603 + interactive: None, 604 + http: &http, 605 + jwks: &jwks_fetcher, 606 + verbose: false, 607 + clock: Arc::new(RealClock), 608 + }; 609 + 610 + let report = run_pipeline(target, opts).await; 611 + assert_eq!( 612 + report.exit_code(), 613 + 1, 614 + "explicit default port in HTTPS redirect must be a SpecViolation" 615 + ); 616 + } 617 + 618 + /// Per <https://atproto.com/specs/oauth#authorization-request-fields>: 619 + /// custom-scheme redirect URIs must have "a single colon (`:`) then a 620 + /// single forward slash (`/`) and then a URI path component." Forms 621 + /// with an authority (`scheme://host/path`) are non-conformant. 622 + #[tokio::test] 623 + async fn native_redirect_with_double_slash_is_spec_violation() { 624 + let http = common::FakeHttpClient::new(); 625 + let metadata = 626 + include_bytes!("fixtures/oauth_client/metadata/native_redirect_double_slash/metadata.json"); 627 + http.add_response( 628 + &Url::parse("https://app.example.com/oauth-client-metadata.json").unwrap(), 629 + 200, 630 + metadata.to_vec(), 631 + ); 632 + 633 + let target = 634 + target::parse("https://app.example.com/oauth-client-metadata.json").expect("parse failed"); 635 + let jwks_fetcher = common::FakeJwksFetcher::new(); 636 + let opts = OauthClientOptions { 637 + interactive: None, 638 + http: &http, 639 + jwks: &jwks_fetcher, 640 + verbose: false, 641 + clock: Arc::new(RealClock), 642 + }; 643 + 644 + let report = run_pipeline(target, opts).await; 645 + assert_eq!( 646 + report.exit_code(), 647 + 1, 648 + "native redirect_uri with `scheme://host` must be a SpecViolation" 649 + ); 650 + } 651 + 585 652 /// Per the atproto OAuth profile the client-metadata response must be 586 653 /// served with a JSON Content-Type. A `text/html` response — even one 587 654 /// whose body parses as JSON — fails

Configure Feed

Configure Feed