Fix three false positives surfaced by real-world OAuth clients

+35 -13

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

··· 75 75 76 76 ### test-oauth-client.AC2: Static mode — metadata document validation 77 77 78 - - **test-oauth-client.AC2.1 Success:** A confidential web client document with all required fields (`application_type=web`, `response_types=[code]`, `grant_types=[authorization_code]`, `dpop_bound_access_tokens=true`, `redirect_uris` HTTPS, `token_endpoint_auth_method=private_key_jwt`, `jwks` or `jwks_uri`, valid `scope`) passes every metadata-stage check. 79 - - **test-oauth-client.AC2.2 Success:** A public web client document (no JWKS, `token_endpoint_auth_method=none`) passes. 78 + - **test-oauth-client.AC2.1 Success:** A confidential web client document with all required fields (`application_type=web` (optional; defaults to `web` when omitted), `response_types=[code]`, `grant_types=[authorization_code]`, `dpop_bound_access_tokens=true`, `redirect_uris` HTTPS, `token_endpoint_auth_method=private_key_jwt`, `jwks` or `jwks_uri`, valid `scope`) passes every metadata-stage check. 79 + - **test-oauth-client.AC2.2 Success:** A public web client document (no JWKS, `token_endpoint_auth_method=none`) passes. Omitting `application_type` is valid per the atproto OAuth profile, which marks the field OPTIONAL with default `web`; the `application_type_present` check emits `Skipped` (reason: "`application_type` is optional; atproto defaults to `web`") rather than failing. 80 80 - **test-oauth-client.AC2.3 Success:** A native client document with custom-scheme `redirect_uris` whose scheme matches the reverse-domain of the `client_id` host passes. 81 81 - **test-oauth-client.AC2.4 Failure:** `dpop_bound_access_tokens` absent or `false` produces a `SpecViolation` with stable code `oauth_client::metadata::dpop_bound_required`. 82 82 - **test-oauth-client.AC2.5 Failure:** A confidential client without `jwks` or `jwks_uri` produces a `SpecViolation` with code `oauth_client::metadata::confidential_requires_jwks`. 83 83 - **test-oauth-client.AC2.6 Failure:** A public client whose `token_endpoint_auth_method` is anything other than `none` produces a `SpecViolation`. 84 84 - **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`. 85 - - **test-oauth-client.AC2.8 Failure:** A `scope` field that does not parse against the atproto permission grammar produces a `SpecViolation` with the offending token highlighted via miette source span. 85 + - **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. 86 86 - **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"`. 87 87 88 88 ### test-oauth-client.AC3: Static mode — JWKS validation ··· 781 781 **Components:** 782 782 783 783 - `src/commands/test/oauth/client/metadata.rs`: 784 - - `MetadataFacts { kind: ClientKind, redirect_uris: Vec<Url>, requires_jwks: bool, dpop_bound: bool, scope: ScopeSet, ... }`. 784 + - `MetadataFacts { kind: ClientKind, redirect_uris: Vec<Url>, requires_jwks: bool, dpop_bound: bool, scope: ScopeSet, token_endpoint_auth_signing_alg: Option<String>, ... }`. 785 785 - `ClientKind::{WebConfidential, WebPublic, Native, Loopback}`. 786 - - `ScopeSet` newtype wrapping the parsed atproto permission scope grammar 787 - (per `https://atproto.com/specs/permission`). 788 - - `Check` enum with stable IDs covering: required fields present 789 - (`application_type`, `response_types`, `grant_types`, 790 - `dpop_bound_access_tokens`, `redirect_uris`, `scope`), 791 - `redirect_uris` shape per `application_type`, 792 - `token_endpoint_auth_method` matches client kind, scope grammar parses, 793 - URL fields use HTTPS where required. 786 + Derivation uses `application_type` (defaulting to `"web"` when 787 + absent, per the atproto profile) together with 788 + `token_endpoint_auth_method`. 789 + - `ScopeSet` newtype wrapping the parsed atproto permission scope 790 + grammar (per <https://atproto.com/specs/permission>). The parser 791 + implements only the syntactic rules: `<resource>[:<positional>] 792 + [?<query>]` where the colon-positional and question-mark-query 793 + components are independently optional, and the resource name is any 794 + non-empty string (the spec's resource space is explicitly 795 + open-ended, and `transition:*` migration-period scopes are 796 + documented in the atproto OAuth profile). Semantic validation of 797 + resource names is deliberately NOT performed here. 798 + - `Check` enum with stable IDs covering: fields present and 799 + well-formed where the spec requires them (`response_types`, 800 + `grant_types`, `dpop_bound_access_tokens`, `redirect_uris`, 801 + `scope`); `application_type` is optional per the atproto profile 802 + and `Check::ApplicationTypePresent` emits `Skipped` when absent 803 + with reason `"application_type is optional; atproto defaults to 804 + web"`; `redirect_uris` shape per effective `application_type`; 805 + `token_endpoint_auth_method` matches client kind; scope grammar 806 + parses; URL fields use HTTPS where required. 794 807 - `run(facts: &DiscoveryFacts, ...) -> MetadataStageOutput`. For 795 808 `RawMetadata::Implicit` (loopback), validation checks emit `Skipped` 796 809 with reason `"metadata is implicit for loopback clients"`. ··· 805 818 - `public_with_token_endpoint_auth/` (spec violation) 806 819 - `dpop_bound_false/` (spec violation) 807 820 - `native_redirect_scheme_mismatch/` (spec violation) 808 - - `scope_grammar_invalid/` (spec violation) 821 + - `scope_grammar_invalid/` (spec violation — genuinely 822 + syntax-malformed scope token, e.g. a `?…` component with no 823 + `key=value` pair; unknown-but-well-formed resource names do NOT 824 + belong here) 825 + - `transition_scopes/` (regression: `transition:*` migration scopes 826 + must parse cleanly) 827 + - `resource_query_no_positional/` (regression: `<resource>?<query>` 828 + without a positional component must parse cleanly) 829 + - `application_type_omitted/` (regression: missing `application_type` 830 + defaults to `web` and does not fail) 809 831 810 832 **Dependencies:** Phase 3. 811 833

+189 -125

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

··· 59 59 } 60 60 61 61 /// Reason for a scope parse failure. 62 + /// 63 + /// Only syntactic issues are represented here. The atproto permission 64 + /// grammar does not constrain resource names to a fixed set, so 65 + /// unknown-but-syntactically-valid resource names (e.g., 66 + /// `transition:generic`, bare `chat`) are accepted by the parser. 62 67 #[derive(Debug, Clone, Copy, PartialEq, Eq)] 63 68 pub enum ScopeParseReason { 64 - /// Resource name not in the known set. 65 - UnknownResource, 66 69 /// Empty resource name (e.g., `:foo` or `:?param=val`). 67 70 EmptyResource, 68 71 /// Invalid query parameter syntax. ··· 74 77 impl std::fmt::Display for ScopeParseReason { 75 78 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { 76 79 match self { 77 - ScopeParseReason::UnknownResource => write!(f, "unknown resource name"), 78 80 ScopeParseReason::EmptyResource => write!(f, "empty resource name"), 79 81 ScopeParseReason::InvalidQuery => write!(f, "invalid query parameters"), 80 82 ScopeParseReason::InvalidPercentEncoding => { ··· 101 103 /// 102 104 /// The scope string is space-separated. Each token is either: 103 105 /// - The literal string `"atproto"` (baseline scope). 104 - /// - A resource permission: `<resource>[:<positional>][?param=val&param=val...]`. 106 + /// - A resource permission: 107 + /// `<resource>[:<positional>][?param=val&param=val...]`. 105 108 /// 106 - /// Known resources: `repo`, `identity`, `blob`, `account`, `rpc`, `include`. 107 - /// Positional and query parameters are mutually exclusive for the same key. 109 + /// The grammar is defined at 110 + /// <https://atproto.com/specs/permission#scope-string-syntax>: 111 + /// *"There is a resource name part, followed by an **optional** 112 + /// 'positional' part separated by a colon (`:`), followed by optional 113 + /// parameters starting with a question mark (`?`)."* The resource name 114 + /// itself is not constrained to a fixed set — the spec explicitly 115 + /// notes that the resource space is expected to expand over time and 116 + /// lists `transition:*` scopes (`transition:generic`, 117 + /// `transition:chat.bsky`, `transition:email`) as valid — so this 118 + /// function accepts any non-empty resource name. Tokens without a 119 + /// positional but with a query (e.g., `repo?collection=…`) are valid. 108 120 pub fn parse_scope(s: &str) -> Result<ScopeSet, ScopeParseError> { 109 121 let mut tokens = Vec::new(); 110 122 let mut byte_offset = 0; ··· 126 138 Ok(ScopeSet { tokens }) 127 139 } 128 140 129 - /// Result type for parsing positional and query parameters. 130 - type PositionalAndQuery = (Option<String>, Vec<(String, String)>); 131 - 132 141 /// Parse a single permission token. 142 + /// 143 + /// Structural shape, per 144 + /// <https://atproto.com/specs/permission#scope-string-syntax>: 145 + /// `<resource>[:<positional>][?<query>]`. The resource name is any 146 + /// non-empty string; the colon and positional component are OPTIONAL, 147 + /// so `repo?collection=…` (no positional) is valid — the `?` may 148 + /// follow the resource name directly. 133 149 fn parse_permission_token(token: &str, byte_offset: usize) -> Result<ScopeToken, ScopeParseError> { 134 - // Split on the first ':' to separate resource from rest. 135 - let (resource_part, rest) = match token.split_once(':') { 136 - Some((r, rest)) => (r, Some(rest)), 137 - None => (token, None), 150 + // Split resource from the optional remainder. Prefer ':' so a 151 + // token like `repo:app.bsky.feed.post?action=create` goes through 152 + // the positional+query branch; fall back to '?' so `repo?a=b` 153 + // (resource + query, no positional) is also accepted. 154 + let (resource_part, positional, query_part) = if let Some((r, rest)) = token.split_once(':') { 155 + let (pos, q) = match rest.split_once('?') { 156 + Some((pos, q)) => { 157 + let pos = if pos.is_empty() { 158 + None 159 + } else { 160 + Some(pos.to_string()) 161 + }; 162 + (pos, Some(q)) 163 + } 164 + None => { 165 + let pos = if rest.is_empty() { 166 + None 167 + } else { 168 + Some(rest.to_string()) 169 + }; 170 + (pos, None) 171 + } 172 + }; 173 + (r, pos, q) 174 + } else if let Some((r, q)) = token.split_once('?') { 175 + (r, None, Some(q)) 176 + } else { 177 + (token, None, None) 138 178 }; 139 179 140 - // Check that resource is not empty. 141 180 if resource_part.is_empty() { 142 181 return Err(ScopeParseError { 143 182 token: token.to_string(), ··· 146 185 }); 147 186 } 148 187 149 - // Validate resource name. 150 - let known_resources = ["repo", "identity", "blob", "account", "rpc", "include"]; 151 - if !known_resources.contains(&resource_part) { 152 - return Err(ScopeParseError { 153 - token: token.to_string(), 154 - byte_offset, 155 - reason: ScopeParseReason::UnknownResource, 156 - }); 157 - } 158 - 159 - let resource = resource_part.to_string(); 160 - let (positional, query) = if let Some(rest) = rest { 161 - // Parse positional and query from the remainder. 162 - match parse_positional_and_query(rest, byte_offset) { 163 - Ok((pos, q)) => (pos, q), 164 - Err(e) => return Err(e), 165 - } 188 + let query = if let Some(q) = query_part { 189 + parse_query_string(q, byte_offset)? 166 190 } else { 167 - (None, Vec::new()) 191 + Vec::new() 168 192 }; 169 193 170 194 Ok(ScopeToken::Permission { 171 - resource, 195 + resource: resource_part.to_string(), 172 196 positional, 173 197 query, 174 198 }) 175 - } 176 - 177 - /// Parse the positional parameter and query parameters from the post-colon part. 178 - fn parse_positional_and_query( 179 - rest: &str, 180 - byte_offset: usize, 181 - ) -> Result<PositionalAndQuery, ScopeParseError> { 182 - // Split on '?' to separate positional from query. 183 - let (positional_part, query_part) = match rest.split_once('?') { 184 - Some((pos, q)) => { 185 - if pos.is_empty() { 186 - (None, Some(q)) 187 - } else { 188 - (Some(pos.to_string()), Some(q)) 189 - } 190 - } 191 - None => { 192 - if rest.is_empty() { 193 - (None, None) 194 - } else { 195 - (Some(rest.to_string()), None) 196 - } 197 - } 198 - }; 199 - 200 - // Parse query parameters if present. 201 - let query = if let Some(q) = query_part { 202 - parse_query_string(q, byte_offset)? 203 - } else { 204 - Vec::new() 205 - }; 206 - 207 - Ok((positional_part, query)) 208 199 } 209 200 210 201 /// Parse the query string (param=val&param=val...). ··· 330 321 /// client's OAuth identifier; any mismatch breaks trust on 331 322 /// discovery. See <https://atproto.com/specs/oauth#clients>. 332 323 ClientIdMatches, 333 - /// The document declares an `application_type`. OpenID Connect 334 - /// Dynamic Client Registration §2 makes this field OPTIONAL, but 335 - /// the atproto OAuth profile requires it to be explicit so that 336 - /// downstream redirect-URI and auth-method rules can be applied 337 - /// unambiguously. See 338 - /// <https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata> 339 - /// and <https://atproto.com/specs/oauth#clients>. 324 + /// The document declares an `application_type` explicitly. The 325 + /// atproto OAuth profile marks this field OPTIONAL with a default 326 + /// of `"web"` (matching OpenID Connect Dynamic Client Registration 327 + /// §2.1, which `application_type` is inherited from); when absent, 328 + /// the check emits `Skipped` rather than a violation. Clients 329 + /// targeting native platforms must declare `"native"` explicitly 330 + /// for the default to not apply. See 331 + /// <https://atproto.com/specs/oauth#clients> and 332 + /// <https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata>. 340 333 ApplicationTypePresent, 341 - /// `application_type` is either `"web"` or `"native"` — the only 342 - /// two values the atproto OAuth profile recognises. Loopback 343 - /// development clients don't reach this check (their metadata is 344 - /// implicit). 334 + /// `application_type`'s effective value is either `"web"` or 335 + /// `"native"`. When the field is absent it defaults to `"web"` 336 + /// and the check passes against the default; when it is present, 337 + /// any other string fails. Loopback clients skip metadata checks 338 + /// entirely (implicit metadata). 345 339 ApplicationTypeKnown, 346 340 /// `response_types` is exactly `["code"]`. The atproto OAuth 347 341 /// profile only supports the authorization-code flow (RFC 6749 ··· 682 676 } 683 677 } 684 678 685 - // Check ApplicationTypePresent. 686 - let application_type_present = doc.application_type.is_some(); 687 - let app_type_known = if !application_type_present { 688 - let diagnostic: Box<dyn miette::Diagnostic + Send + Sync> = 689 - Box::new(MetadataViolationDiagnostic { 690 - message: "application_type field is missing".to_string(), 691 - code: "oauth_client::metadata::application_type_present", 692 - }); 693 - results 694 - .push(Check::ApplicationTypePresent.spec_violation(Some(diagnostic))); 695 - // Skip ApplicationTypeKnown as blocked. 696 - results.push(blocked_by( 697 - Check::ApplicationTypeKnown.id(), 698 - Stage::OAUTH_CLIENT_METADATA, 699 - Check::ApplicationTypeKnown.summary(), 700 - Check::ApplicationTypePresent.id(), 701 - )); 702 - false 703 - } else { 704 - results.push(Check::ApplicationTypePresent.pass()); 705 - 706 - // Check ApplicationTypeKnown. 707 - let known = matches!( 708 - doc.application_type.as_deref(), 709 - Some("web") | Some("native") 710 - ); 711 - if !known { 712 - let diagnostic: Box<dyn miette::Diagnostic + Send + Sync> = 713 - Box::new(MetadataViolationDiagnostic { 714 - message: format!( 715 - "application_type must be 'web' or 'native', got '{}'", 716 - doc.application_type.as_deref().unwrap_or("unknown") 717 - ), 718 - code: "oauth_client::metadata::application_type_known", 719 - }); 720 - results 721 - .push(Check::ApplicationTypeKnown.spec_violation(Some(diagnostic))); 722 - } else { 679 + // Check ApplicationTypePresent. Per the atproto OAuth 680 + // spec (<https://atproto.com/specs/oauth#clients>) 681 + // `application_type` is OPTIONAL and defaults to 682 + // "web" when omitted, so absence is emitted as 683 + // `Skipped` rather than a violation and 684 + // `ApplicationTypeKnown` passes against the implicit 685 + // default. 686 + let app_type_known = match doc.application_type.as_deref() { 687 + None => { 688 + results.push(skipped_with_reason( 689 + Check::ApplicationTypePresent.id(), 690 + Stage::OAUTH_CLIENT_METADATA, 691 + Check::ApplicationTypePresent.summary(), 692 + "`application_type` is optional; atproto defaults to `web`", 693 + )); 723 694 results.push(Check::ApplicationTypeKnown.pass()); 695 + true 724 696 } 725 - known 697 + Some(value) => { 698 + results.push(Check::ApplicationTypePresent.pass()); 699 + 700 + let known = matches!(value, "web" | "native"); 701 + if !known { 702 + let diagnostic: Box<dyn miette::Diagnostic + Send + Sync> = 703 + Box::new(MetadataViolationDiagnostic { 704 + message: format!( 705 + "application_type must be 'web' or 'native', got '{value}'" 706 + ), 707 + code: "oauth_client::metadata::application_type_known", 708 + }); 709 + results.push( 710 + Check::ApplicationTypeKnown.spec_violation(Some(diagnostic)), 711 + ); 712 + } else { 713 + results.push(Check::ApplicationTypeKnown.pass()); 714 + } 715 + known 716 + } 726 717 }; 727 718 728 719 // Check ResponseTypesIsCode. ··· 799 790 800 791 // Determine ClientKind and TokenEndpointAuthMethodValid. 801 792 // If ApplicationTypeKnown check didn't pass, skip to blocked status. 802 - let client_kind = if !application_type_present || !app_type_known { 793 + let client_kind = if !app_type_known { 803 794 // Skip these checks as blocked. 804 795 results.push(blocked_by( 805 796 Check::RedirectUrisShape.id(), ··· 827 818 )); 828 819 None 829 820 } else { 830 - let app_type = doc.application_type.as_deref().unwrap(); 821 + // Default to "web" when `application_type` is 822 + // absent, matching the atproto OAuth profile's 823 + // implicit default. 824 + let app_type = doc.application_type.as_deref().unwrap_or("web"); 831 825 let auth_method = doc.token_endpoint_auth_method.as_deref(); 832 826 833 827 let (kind, auth_valid) = match app_type { ··· 1334 1328 assert_eq!(result.tokens.len(), 0); 1335 1329 } 1336 1330 1331 + // Bug 2: the atproto permission grammar does not constrain resource 1332 + // names — <https://atproto.com/specs/permission#scope-string-syntax> 1333 + // explicitly notes "The set of resources is expected to expand over 1334 + // time as new functionality is added to the protocol." The parser 1335 + // must therefore accept any non-empty resource name, including the 1336 + // `transition:*` namespace used by real-world clients during the 1337 + // OAuth transition. 1337 1338 #[test] 1338 - fn scope_rejects_unknown_resource() { 1339 - let result = parse_scope("foo:bar"); 1340 - assert!(result.is_err()); 1341 - let err = result.unwrap_err(); 1342 - assert_eq!(err.reason, ScopeParseReason::UnknownResource); 1343 - assert_eq!(err.token, "foo:bar"); 1339 + fn scope_accepts_transition_generic() { 1340 + let result = parse_scope("atproto transition:generic").expect("parse failed"); 1341 + assert_eq!(result.tokens.len(), 2); 1342 + match &result.tokens[1] { 1343 + ScopeToken::Permission { 1344 + resource, 1345 + positional, 1346 + query, 1347 + } => { 1348 + assert_eq!(resource, "transition"); 1349 + assert_eq!(positional.as_deref(), Some("generic")); 1350 + assert!(query.is_empty()); 1351 + } 1352 + _ => panic!("expected Permission token"), 1353 + } 1354 + } 1355 + 1356 + #[test] 1357 + fn scope_accepts_unknown_resource_names() { 1358 + // The spec's own examples include `resource` and bare `chat`-like 1359 + // tokens as syntactically valid; semantic validity is a separate 1360 + // concern. `scope_grammar` is a syntax check, not a semantic one. 1361 + parse_scope("atproto chat").expect("bare unknown resource should parse"); 1362 + parse_scope("atproto transition:email").expect("transition:email should parse"); 1363 + parse_scope("atproto fictional:thing").expect("fictional resource should parse"); 1344 1364 } 1345 1365 1346 1366 #[test] ··· 1351 1371 assert_eq!(err.reason, ScopeParseReason::EmptyResource); 1352 1372 } 1353 1373 1374 + // Bug 1: `<resource>?<query>` with no positional is syntactically valid 1375 + // per <https://atproto.com/specs/permission#scope-string-syntax> — 1376 + // positional is "optional". Real-world clients (blento, greengale, 1377 + // semble, stream.place) advertise `repo?collection=…` tokens. 1378 + #[test] 1379 + fn scope_accepts_resource_with_query_but_no_positional() { 1380 + let result = parse_scope("atproto repo?collection=app.bsky.feed.post") 1381 + .expect("resource?query should parse"); 1382 + assert_eq!(result.tokens.len(), 2); 1383 + match &result.tokens[1] { 1384 + ScopeToken::Permission { 1385 + resource, 1386 + positional, 1387 + query, 1388 + } => { 1389 + assert_eq!(resource, "repo"); 1390 + assert!(positional.is_none()); 1391 + assert_eq!(query.len(), 1); 1392 + assert_eq!( 1393 + query[0], 1394 + ("collection".to_string(), "app.bsky.feed.post".to_string()) 1395 + ); 1396 + } 1397 + _ => panic!("expected Permission token"), 1398 + } 1399 + } 1400 + 1401 + #[test] 1402 + fn scope_accepts_resource_with_query_multi_params() { 1403 + let scope = "atproto repo?collection=app.bsky.feed.post&collection=app.bsky.actor.profile"; 1404 + let result = parse_scope(scope).expect("multi-value query should parse"); 1405 + assert_eq!(result.tokens.len(), 2); 1406 + if let ScopeToken::Permission { query, .. } = &result.tokens[1] { 1407 + assert_eq!(query.len(), 2); 1408 + } else { 1409 + panic!("expected Permission token"); 1410 + } 1411 + } 1412 + 1413 + #[test] 1414 + fn scope_accepts_blob_query_form() { 1415 + parse_scope("atproto blob?accept=*/*").expect("blob?accept=… should parse"); 1416 + } 1417 + 1354 1418 #[test] 1355 1419 fn scope_accepts_query_forms() { 1356 1420 let result = ··· 1400 1464 let err = ScopeParseError { 1401 1465 token: "invalid".to_string(), 1402 1466 byte_offset: 0, 1403 - reason: ScopeParseReason::UnknownResource, 1467 + reason: ScopeParseReason::EmptyResource, 1404 1468 }; 1405 1469 // The derive(Diagnostic) with code = "..." should set the code. 1406 1470 // We can't directly test this without rendering, but the test ensures

+9

tests/fixtures/oauth_client/metadata/application_type_omitted/metadata.json

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

+10

tests/fixtures/oauth_client/metadata/resource_query_no_positional/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 repo?collection=app.bsky.feed.post&collection=app.bsky.actor.profile blob?accept=*/*", 7 + "redirect_uris": ["https://client.example.com/cb"], 8 + "dpop_bound_access_tokens": true, 9 + "token_endpoint_auth_method": "none" 10 + }

+1 -1

tests/fixtures/oauth_client/metadata/scope_grammar_invalid/metadata.json

··· 7 7 "grant_types": [ 8 8 "authorization_code" 9 9 ], 10 - "scope": "atproto invalid-scope-token", 10 + "scope": "atproto repo:app.bsky.feed.post?not-a-key-value-pair", 11 11 "redirect_uris": [ 12 12 "https://client.example.com/cb" 13 13 ],

+10

tests/fixtures/oauth_client/metadata/transition_scopes/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 transition:generic transition:email transition:chat.bsky", 7 + "redirect_uris": ["https://client.example.com/cb"], 8 + "dpop_bound_access_tokens": true, 9 + "token_endpoint_auth_method": "none" 10 + }

+106

tests/oauth_client_metadata.rs

··· 335 335 let rendered = render_report_to_string(&report); 336 336 insta::assert_snapshot!(rendered); 337 337 } 338 + 339 + // ============================================================================= 340 + // Regressions from real-world atproto OAuth clients. 341 + // ============================================================================= 342 + 343 + /// `transition:*` scopes (documented at 344 + /// <https://atproto.com/specs/oauth#authorization-scopes>) are a 345 + /// documented part of the atproto OAuth profile during the migration 346 + /// from the App Password auth model. The parser must accept them. 347 + /// Regression for several clients (aetheros, cocoon, chive, leaflet, 348 + /// nooki) that previously failed against the conformance tester. 349 + #[tokio::test] 350 + async fn transition_scopes_accepted() { 351 + let http = common::FakeHttpClient::new(); 352 + let metadata = include_bytes!("fixtures/oauth_client/metadata/transition_scopes/metadata.json"); 353 + http.add_response( 354 + &Url::parse("https://client.example.com/metadata.json").unwrap(), 355 + 200, 356 + metadata.to_vec(), 357 + ); 358 + 359 + let target = target::parse("https://client.example.com/metadata.json").expect("parse failed"); 360 + let jwks_fetcher = common::FakeJwksFetcher::new(); 361 + let opts = OauthClientOptions { 362 + interactive: None, 363 + http: &http, 364 + jwks: &jwks_fetcher, 365 + verbose: false, 366 + clock: Arc::new(RealClock), 367 + }; 368 + 369 + let report = run_pipeline(target, opts).await; 370 + assert_eq!( 371 + report.exit_code(), 372 + 0, 373 + "transition:* scopes must parse cleanly" 374 + ); 375 + } 376 + 377 + /// `<resource>?<query>` (no colon, no positional) is valid per 378 + /// <https://atproto.com/specs/permission#scope-string-syntax>. Regression 379 + /// for blento, greengale, semble, and stream.place. 380 + #[tokio::test] 381 + async fn resource_query_without_positional_accepted() { 382 + let http = common::FakeHttpClient::new(); 383 + let metadata = 384 + include_bytes!("fixtures/oauth_client/metadata/resource_query_no_positional/metadata.json"); 385 + http.add_response( 386 + &Url::parse("https://client.example.com/metadata.json").unwrap(), 387 + 200, 388 + metadata.to_vec(), 389 + ); 390 + 391 + let target = target::parse("https://client.example.com/metadata.json").expect("parse failed"); 392 + let jwks_fetcher = common::FakeJwksFetcher::new(); 393 + let opts = OauthClientOptions { 394 + interactive: None, 395 + http: &http, 396 + jwks: &jwks_fetcher, 397 + verbose: false, 398 + clock: Arc::new(RealClock), 399 + }; 400 + 401 + let report = run_pipeline(target, opts).await; 402 + assert_eq!( 403 + report.exit_code(), 404 + 0, 405 + "resource?query tokens must parse cleanly" 406 + ); 407 + } 408 + 409 + /// `application_type` is OPTIONAL per 410 + /// <https://atproto.com/specs/oauth#clients>, defaulting to `"web"`. 411 + /// Omission must not produce a SpecViolation. Regression for 412 + /// grain.social. 413 + #[tokio::test] 414 + async fn application_type_omitted_defaults_to_web() { 415 + let http = common::FakeHttpClient::new(); 416 + let metadata = 417 + include_bytes!("fixtures/oauth_client/metadata/application_type_omitted/metadata.json"); 418 + http.add_response( 419 + &Url::parse("https://client.example.com/metadata.json").unwrap(), 420 + 200, 421 + metadata.to_vec(), 422 + ); 423 + 424 + let target = target::parse("https://client.example.com/metadata.json").expect("parse failed"); 425 + let jwks_fetcher = common::FakeJwksFetcher::new(); 426 + let opts = OauthClientOptions { 427 + interactive: None, 428 + http: &http, 429 + jwks: &jwks_fetcher, 430 + verbose: false, 431 + clock: Arc::new(RealClock), 432 + }; 433 + 434 + let report = run_pipeline(target, opts).await; 435 + assert_eq!( 436 + report.exit_code(), 437 + 0, 438 + "missing application_type must default to `web`, not fail" 439 + ); 440 + 441 + let rendered = render_report_to_string(&report); 442 + insta::assert_snapshot!(rendered); 443 + }

+36

tests/snapshots/oauth_client_metadata__application_type_omitted_defaults_to_web.snap

··· 1 + --- 2 + source: tests/oauth_client_metadata.rs 3 + expression: rendered 4 + --- 5 + Target: https://client.example.com/metadata.json 6 + elapsed: XXms 7 + 8 + == Discovery == 9 + [OK] Client ID well-formed 10 + [OK] Metadata document fetchable 11 + [OK] Metadata is valid JSON 12 + == Metadata == 13 + [OK] Metadata document deserializes 14 + [OK] Metadata `client_id` matches fetched URL 15 + [SKIP] `application_type` field is present — `application_type` is optional; atproto defaults to `web` 16 + [OK] `application_type` is `web` or `native` 17 + [OK] `response_types` is `["code"]` 18 + [OK] `grant_types` includes `authorization_code` 19 + [OK] `dpop_bound_access_tokens` is `true` 20 + [OK] `redirect_uris` is non-empty 21 + [OK] Every `redirect_uri` has the right shape for the client kind 22 + [OK] `token_endpoint_auth_method` matches client kind 23 + [OK] Public/native client does not provide `jwks` or `jwks_uri` 24 + [OK] `scope` field is present 25 + [OK] `scope` includes the `atproto` token 26 + [OK] `scope` parses against the atproto permission grammar 27 + == JWKS == 28 + [SKIP] JWKS is present — jwks not required for public clients 29 + [SKIP] JWKS URI is fetchable — jwks not required for public clients 30 + [SKIP] JWKS is valid JSON — jwks not required for public clients 31 + [SKIP] Keys have unique kid values — jwks not required for public clients 32 + [SKIP] JWKS contains a key compatible with `token_endpoint_auth_signing_alg` — jwks not required for public clients 33 + [SKIP] Keys use signing use — jwks not required for public clients 34 + [SKIP] Algorithms are modern EC — jwks not required for public clients 35 + 36 + Summary: 16 passed, 0 failed (spec), 0 network errors, 0 advisories, 8 skipped. Exit code: 0

+4 -4

tests/snapshots/oauth_client_metadata__scope_grammar_invalid.snap

··· 26 26 [FAIL] `scope` parses against the atproto permission grammar 27 27 oauth_client::metadata::scope_grammar 28 28 29 - × scope grammar error: unknown resource name 29 + × scope grammar error: invalid query parameters 30 30 ╭─[metadata.json:27:21] 31 31 26 │ ], 32 - 27 │ "scope": "atproto invalid-scope-token", 33 - · ─────────┬───────── 34 - · ╰── invalid token 32 + 27 │ "scope": "atproto repo:app.bsky.feed.post?not-a-key-value-pair", 33 + · ──────────┬────────── 34 + · ╰── invalid token 35 35 28 │ "token_endpoint_auth_method": "private_key_jwt", 36 36 ╰──── 37 37 == JWKS ==

Configure Feed

Configure Feed