From fbbb35ace25add9dfe7ab570cd00423f6f48ffc9 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Fri, 7 Aug 2026 21:37:27 -0500 Subject: [PATCH] accept space: oauth scopes end to end two bugs made oauth spaces access impossible regardless of what a client requested, which bulleted hit: - isGranularScope did not recognize space:/space? scopes, so PAR rejected any request carrying one with invalid_scope before consent - spaceScopeMatches required a collection match for read_self, but the repo-state reads (getLatestCommit, getRepoState, getRepo, getBlob) pass a null collection, so no scope could ever satisfy them; bare space: scopes also granted read while denying the weaker read_self space:* is now advertised in both oauth metadata documents, unrestricted scopes allow repo-level self reads, and read implies read_self for bare scopes. regression tests cover the PAR gate and the read_self matrix. Co-Authored-By: Claude Fable 5 --- src/atproto/oauth.zig | 10 +++++++--- src/internal/scopes.zig | 19 +++++++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/atproto/oauth.zig b/src/atproto/oauth.zig index a5de7ac..14fd884 100644 --- a/src/atproto/oauth.zig +++ b/src/atproto/oauth.zig @@ -56,7 +56,7 @@ pub fn protectedResource(request: *http_api.Request) !void { else config.publicUrl(); const body = try std.fmt.allocPrint(arena.allocator(), - \\{{"resource":{f},"authorization_servers":[{f}],"scopes_supported":["atproto","transition:generic","transition:email","transition:chat.bsky","repo:*","blob:*/*","rpc:*","account:*","identity:*","include:*"],"bearer_methods_supported":["header"],"resource_documentation":"https://atproto.com"}} + \\{{"resource":{f},"authorization_servers":[{f}],"scopes_supported":["atproto","transition:generic","transition:email","transition:chat.bsky","repo:*","blob:*/*","rpc:*","account:*","identity:*","include:*","space:*"],"bearer_methods_supported":["header"],"resource_documentation":"https://atproto.com"}} , .{ std.json.fmt(resource, .{}), std.json.fmt(config.publicUrl(), .{}) }); try http_api.json(request, .ok, body); } @@ -67,7 +67,7 @@ pub fn authorizationServer(request: *http_api.Request) !void { const allocator = arena.allocator(); const issuer = config.publicUrl(); const body = try std.fmt.allocPrint(allocator, - \\{{"issuer":{f},"request_parameter_supported":true,"request_uri_parameter_supported":true,"require_request_uri_registration":true,"scopes_supported":["atproto","transition:generic","transition:email","transition:chat.bsky","repo:*","blob:*/*","rpc:*","account:*","identity:*","include:*"],"subject_types_supported":["public"],"response_types_supported":["code"],"response_modes_supported":["query","fragment"],"grant_types_supported":["authorization_code","refresh_token"],"code_challenge_methods_supported":["S256"],"authorization_response_iss_parameter_supported":true,"client_id_metadata_document_supported":true,"require_pushed_authorization_requests":true,"token_endpoint_auth_methods_supported":["none","private_key_jwt"],"token_endpoint_auth_signing_alg_values_supported":["ES256","ES256K"],"dpop_signing_alg_values_supported":["ES256","ES256K"],"prompt_values_supported":["none","login","consent","select_account","create"],"protected_resources":[{f}],"jwks_uri":{f},"authorization_endpoint":{f},"token_endpoint":{f},"revocation_endpoint":{f},"introspection_endpoint":{f},"pushed_authorization_request_endpoint":{f}}} + \\{{"issuer":{f},"request_parameter_supported":true,"request_uri_parameter_supported":true,"require_request_uri_registration":true,"scopes_supported":["atproto","transition:generic","transition:email","transition:chat.bsky","repo:*","blob:*/*","rpc:*","account:*","identity:*","include:*","space:*"],"subject_types_supported":["public"],"response_types_supported":["code"],"response_modes_supported":["query","fragment"],"grant_types_supported":["authorization_code","refresh_token"],"code_challenge_methods_supported":["S256"],"authorization_response_iss_parameter_supported":true,"client_id_metadata_document_supported":true,"require_pushed_authorization_requests":true,"token_endpoint_auth_methods_supported":["none","private_key_jwt"],"token_endpoint_auth_signing_alg_values_supported":["ES256","ES256K"],"dpop_signing_alg_values_supported":["ES256","ES256K"],"prompt_values_supported":["none","login","consent","select_account","create"],"protected_resources":[{f}],"jwks_uri":{f},"authorization_endpoint":{f},"token_endpoint":{f},"revocation_endpoint":{f},"introspection_endpoint":{f},"pushed_authorization_request_endpoint":{f}}} , .{ std.json.fmt(issuer, .{}), std.json.fmt(issuer, .{}), @@ -1213,7 +1213,9 @@ fn isGranularScope(scope: []const u8) bool { std.mem.startsWith(u8, scope, "rpc:") or std.mem.startsWith(u8, scope, "account:") or std.mem.startsWith(u8, scope, "identity:") or - std.mem.startsWith(u8, scope, "include:"); + std.mem.startsWith(u8, scope, "include:") or + std.mem.startsWith(u8, scope, "space:") or + std.mem.startsWith(u8, scope, "space?"); } fn percentEncode(allocator: std.mem.Allocator, value: []const u8) ![]const u8 { @@ -1288,6 +1290,8 @@ test "granular scopes accept query-form repo permissions" { try std.testing.expect(isGranularScope("repo?collection=place.stream.live&action=create")); try std.testing.expect(isGranularScope("repo?action=create&collection=place.stream.live")); try std.testing.expect(isGranularScope("rpc:app.bsky.actor.getProfile?aud=did:web:api.bsky.app%23bsky_appview")); + try std.testing.expect(isGranularScope("space:app.bulleted.space?action=read")); + try std.testing.expect(isGranularScope("space?type=app.bulleted.space")); try std.testing.expect(!isGranularScope("transition:generic")); } diff --git a/src/internal/scopes.zig b/src/internal/scopes.zig index 839eb32..3144484 100644 --- a/src/internal/scopes.zig +++ b/src/internal/scopes.zig @@ -258,7 +258,7 @@ fn spaceScopeMatches( const query_start = std.mem.indexOfScalar(u8, scope, '?'); const base = if (query_start) |idx| scope[0..idx] else scope; if (!spaceTypeMatches(base, space_type)) return false; - if (query_start == null) return action == .read; + if (query_start == null) return action == .read or action == .read_self; var saw_action = false; var action_matches = false; var saw_manage = false; @@ -299,7 +299,7 @@ fn spaceScopeMatches( return switch (action) { .manage_create, .manage_update, .manage_delete => saw_manage and manage_matches, .read => if (saw_action) action_matches else true, - .read_self => (if (saw_action) action_matches else true) and saw_collection and collection_matches, + .read_self => (if (saw_action) action_matches else true) and (if (saw_collection) collection_matches else true), .create, .update, .delete => (if (saw_action) action_matches else true) and saw_collection and collection_matches, }; } @@ -372,6 +372,21 @@ test "space scopes constrain type identity key action and collection" { try std.testing.expect(!spaceAllows("repo:*", .read, "fm.plyr.privateMedia", "did:plc:bob", "records", null)); } +test "space read scopes allow repo-level self reads" { + // regression: repo-state endpoints (getLatestCommit, getRepoState, getRepo, + // getBlob) authorize read_self with a null collection; scopes without a + // collection restriction must allow that, and read implies read_self. + try std.testing.expect(spaceAllows("space:app.bulleted.space?action=read", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); + try std.testing.expect(spaceAllows("space:app.bulleted.space", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); + try std.testing.expect(spaceAllows("space:app.bulleted.space?action=read", .read_self, "app.bulleted.space", "did:plc:alice", "self", "app.bulleted.bullet")); + // collection-restricted scopes still deny whole-repo reads and self reads + // of other collections + try std.testing.expect(!spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); + try std.testing.expect(!spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read_self, "app.bulleted.space", "did:plc:alice", "self", "app.other.thing")); + // read_self never grants cross-repo read + try std.testing.expect(!spaceAllows("space:app.bulleted.space?action=read_self", .read, "app.bulleted.space", "did:plc:alice", "self", null)); +} + test "account scopes constrain attribute and action" { try std.testing.expect(accountAllows("account:email?action=manage", .email, .manage)); try std.testing.expect(!accountAllows("account:email?action=read", .email, .manage)); -- 2.51.2