diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ec7ace..3c549c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,13 @@ Reconstructed from git history for everything up to `v0.1.1`; kept by hand from matching the reference PDS. Clients such as Bridgy that call `app.bsky.feed.getAuthorFeed` on the PDS previously got `404 UnknownMethod`. The caller's session is still required. +- **fix**: a managing-app space that refuses a requester now logs why: the + managing app could not be resolved, the `checkUserAccess` request failed, or + the app answered with a status other than an authorized `200`. These denials + were silent, so `UserNotAuthorized` from `getSpaceCredential` could not be + told apart from an unreachable app. +- **fix**: DID resolution for Spaces honors `ZDS_PLC_DIRECTORY`; it previously + always used `https://plc.directory`. - **build**: the container build fetches dependencies with backoff before compiling, like CI. - **change**: the homepage and api reference headers drop the glass pill and diff --git a/src/atproto/space.zig b/src/atproto/space.zig index 0617422..e4149c1 100644 --- a/src/atproto/space.zig +++ b/src/atproto/space.zig @@ -10,6 +10,7 @@ const clock = @import("../core/clock.zig"); const auth = @import("../auth/tokens.zig"); const config = @import("../core/config.zig"); const http_api = @import("../http/api.zig"); +const log = @import("../core/log.zig"); const cbor_json = @import("../internal/cbor_json.zig"); const permissioned = @import("../internal/permissioned_data.zig"); const client_attestation = @import("../internal/client_attestation.zig"); @@ -1102,7 +1103,7 @@ fn signingKeyForDidKid(allocator: std.mem.Allocator, did: []const u8, kid: []con return allocator.dupe(u8, did_key["did:key:".len..]); } const parsed = zat.Did.parse(did) orelse return error.InvalidDid; - var resolver = zat.DidResolver.init(store.currentIo(), allocator); + var resolver = didResolver(allocator); defer resolver.deinit(); var doc = try resolver.resolve(parsed); defer doc.deinit(); @@ -1174,7 +1175,7 @@ fn serviceAudienceMatchesPds(aud: []const u8) bool { fn spaceHostEndpointForDid(allocator: std.mem.Allocator, did: []const u8) ![]const u8 { const parsed = zat.Did.parse(did) orelse return error.InvalidDid; - var resolver = zat.DidResolver.init(store.currentIo(), allocator); + var resolver = didResolver(allocator); defer resolver.deinit(); var doc = try resolver.resolve(parsed); defer doc.deinit(); @@ -1186,12 +1187,18 @@ fn spaceHostEndpointForDid(allocator: std.mem.Allocator, did: []const u8) ![]con return allocator.dupe(u8, doc.pdsEndpoint() orelse return error.MissingSpaceHostEndpoint); } +fn didResolver(allocator: std.mem.Allocator) zat.DidResolver { + var resolver = zat.DidResolver.init(store.currentIo(), allocator); + resolver.plc_url = config.plcDirectory(); + return resolver; +} + fn serviceEndpointForId(allocator: std.mem.Allocator, service_id: []const u8) ![]const u8 { const fragment_index = std.mem.indexOfScalar(u8, service_id, '#'); const did_text = if (fragment_index) |idx| service_id[0..idx] else service_id; const fragment = if (fragment_index) |idx| service_id[idx..] else null; const did = zat.Did.parse(did_text) orelse return error.InvalidServiceId; - var resolver = zat.DidResolver.init(store.currentIo(), allocator); + var resolver = didResolver(allocator); defer resolver.deinit(); var doc = try resolver.resolve(did); defer doc.deinit(); @@ -1237,10 +1244,19 @@ fn managingAppAllowsRequester( access: store.SpaceAccess, client_id: ?[]const u8, ) !bool { - const managing_app = (if (access == .read) space.read_managing_app else space.write_managing_app) orelse return false; - const endpoint = serviceEndpointForId(allocator, managing_app) catch return false; + const managing_app = (if (access == .read) space.read_managing_app else space.write_managing_app) orelse { + log.err("space managing app missing space={s} access={s}\n", .{ space.uri, @tagName(access) }); + return false; + }; + const endpoint = serviceEndpointForId(allocator, managing_app) catch |err| { + log.err("space managing app unresolved space={s} managing_app={s} err={s}\n", .{ space.uri, managing_app, @errorName(err) }); + return false; + }; const parsed = parseSpaceUri(space.uri) orelse return false; - const authority = (try store.findAccount(allocator, parsed.authority_did)) orelse return false; + const authority = (try store.findAccount(allocator, parsed.authority_did)) orelse { + log.err("space managing app check has no local authority space={s}\n", .{space.uri}); + return false; + }; var keypair = try store.signingKeypair(authority.did); const token = try auth.createServiceJwtWithKeypair( allocator, @@ -1260,10 +1276,20 @@ fn managingAppAllowsRequester( .accept = "application/json", .max_response_size = 16 * 1024, .on_redirect = .refuse, - }) catch return false; + }) catch |err| { + log.err("space managing app check failed space={s} managing_app={s} endpoint={s} user={s} err={s}\n", .{ space.uri, managing_app, endpoint, requester_did, @errorName(err) }); + return false; + }; defer result.deinit(allocator); - if (result.status != .ok) return false; - const response = std.json.parseFromSlice(std.json.Value, allocator, result.body, .{}) catch return false; + const authorized = result.status == .ok and managingAppAuthorized(allocator, result.body); + if (!authorized) { + log.info("space managing app denied space={s} managing_app={s} endpoint={s} user={s} access={s} status={d}\n", .{ space.uri, managing_app, endpoint, requester_did, @tagName(access), @backingInt(result.status) }); + } + return authorized; +} + +fn managingAppAuthorized(allocator: std.mem.Allocator, body: []const u8) bool { + const response = std.json.parseFromSlice(std.json.Value, allocator, body, .{}) catch return false; defer response.deinit(); return switch (response.value) { .object => |object| switch (object.get("authorized") orelse return false) { @@ -2035,6 +2061,16 @@ test "space access selects independent read and write policies" { try std.testing.expect(try spacePolicyAllowsRequester(a, space, owner.did, .write, null)); } +test "managing app grants only on an explicit authorized true" { + const a = std.testing.allocator; + try std.testing.expect(managingAppAuthorized(a, "{\"authorized\":true}")); + try std.testing.expect(!managingAppAuthorized(a, "{\"authorized\":false}")); + try std.testing.expect(!managingAppAuthorized(a, "{\"authorized\":\"true\"}")); + try std.testing.expect(!managingAppAuthorized(a, "{}")); + try std.testing.expect(!managingAppAuthorized(a, "[true]")); + try std.testing.expect(!managingAppAuthorized(a, "not json")); +} + test "managing app checks include access and omit client identity for writes" { var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); diff --git a/tools/smoke-permissioned.sh b/tools/smoke-permissioned.sh index 9a1f81e..ed70a88 100755 --- a/tools/smoke-permissioned.sh +++ b/tools/smoke-permissioned.sh @@ -7,6 +7,8 @@ port="${ZDS_PERMISSIONED_SMOKE_PORT:-2586}" db="${work}/zds-permissioned-smoke.sqlite3" blob_root="${work}/zds-permissioned-smoke-blobs" log="${work}/zds-permissioned-smoke.log" +# stand-in PLC directory and managing app, served by tools/smoke-space-access.py +stub_port="${ZDS_PERMISSIONED_SMOKE_STUB_PORT:-$((port + 1))}" base="http://127.0.0.1:${port}" jwt_secret="smoke-jwt-secret" space_uri="at://did:plc:permissionsmoke/space/fm.plyr.privateMedia/self" @@ -55,6 +57,7 @@ ZDS_PERMISSIONED_DATA=true ./zig-out/bin/zds \ --server-did did:web:localhost \ --handle-domains .test \ --admin-token permissioned-smoke-admin-token \ + --plc-directory "http://127.0.0.1:${stub_port}" \ >"$log" 2>&1 & server_pid=$! @@ -405,7 +408,7 @@ other_space_blob_status=$(curl -sS -o "$work/zds-space-other-blob.json" -w '%{ht test "$other_space_blob_status" = "404" grep -q '"error":"BlobNotFound"' "$work/zds-space-other-blob.json" -SMOKE_BASE="$base" SMOKE_TOKEN="$token" SMOKE_BOB_TOKEN="$bob_token" python3 tools/smoke-space-access.py +SMOKE_BASE="$base" SMOKE_TOKEN="$token" SMOKE_BOB_TOKEN="$bob_token" SMOKE_STUB_PORT="$stub_port" python3 tools/smoke-space-access.py if [ -n "${ZDS_SPACE_SDK_DIR:-}" ]; then SMOKE_BASE="$base" SMOKE_TOKEN="$token" SMOKE_BOB_TOKEN="$bob_token" node tools/smoke-space-interop.mjs diff --git a/tools/smoke-space-access.py b/tools/smoke-space-access.py index 43f6439..b5235d9 100644 --- a/tools/smoke-space-access.py +++ b/tools/smoke-space-access.py @@ -1,8 +1,11 @@ #!/usr/bin/env python3 """Exercise independent space access using the disposable smoke server/accounts.""" +import base64 +import http.server import json import os import subprocess +import threading import urllib.error import urllib.parse import urllib.request @@ -63,6 +66,67 @@ for index, (read, write) in enumerate(((True, False), (False, True), (False, Fal expect('com.atproto.space.getSpaceCredential', 200 if read else 403, token=delegation, body={'space': space}, headers=signature_headers) +# A managing-app space asks the app over service auth before granting anyone +# but the authority. The stub is both the PLC directory that resolves the app's +# DID and the app itself. +stub_port = int(os.environ['SMOKE_STUB_PORT']) +managing_app = 'did:plc:spacesmokeapp#smoke_app' +checks = [] +answer = {'status': 200, 'body': {'authorized': True}} + + +class ManagingApp(http.server.BaseHTTPRequestHandler): + def do_GET(self): + url = urllib.parse.urlsplit(self.path) + if url.path == '/did:plc:spacesmokeapp': + status, body = 200, {'id': 'did:plc:spacesmokeapp', 'service': [ + {'id': '#smoke_app', 'type': 'SmokeManagingApp', 'serviceEndpoint': f'http://127.0.0.1:{stub_port}'}]} + elif url.path == '/xrpc/com.atproto.simplespace.checkUserAccess': + token = self.headers.get('authorization', '').removeprefix('Bearer ') + claims = json.loads(base64.urlsafe_b64decode(token.split('.')[1] + '===')) + checks.append((dict(urllib.parse.parse_qsl(url.query)), claims)) + status, body = answer['status'], answer['body'] + else: + status, body = 404, {'message': 'not found'} + payload = json.dumps(body).encode() + self.send_response(status) + self.send_header('content-type', 'application/json') + self.send_header('content-length', str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, *args): + pass + + +stub = http.server.ThreadingHTTPServer(('127.0.0.1', stub_port), ManagingApp) +threading.Thread(target=stub.serve_forever, daemon=True).start() +app_policy = {'$type': 'com.atproto.simplespace.defs#managingAppPolicy', 'managingApp': managing_app} +managed = expect('com.atproto.simplespace.createSpace', 200, body={ + **create, 'spaceType': 'test.access.managed', 'readPolicy': app_policy, 'writePolicy': app_policy})['uri'] + + +def managed_credential(expected): + token = expect('com.atproto.space.getDelegationToken', 200, token=member, params={'space': managed})['token'] + proof = subprocess.check_output(['./zig-out/bin/zds-space-signature', 'Bearer ' + token], text=True, stderr=subprocess.STDOUT) + return expect('com.atproto.space.getSpaceCredential', expected, token=token, body={'space': managed}, + headers=dict(line.split(': ', 1) for line in proof.strip().splitlines())) + + +managed_credential(200) +params, claims = checks[-1] +assert params == {'space': managed, 'user': member_did, 'access': 'read'}, params +assert (claims['iss'], claims['aud'], claims['lxm']) == ( + 'did:plc:permissionsmoke', managing_app, 'com.atproto.simplespace.checkUserAccess'), claims +for denial in ({'status': 200, 'body': {'authorized': False}}, {'status': 200, 'body': {}}, + {'status': 401, 'body': {'error': 'AuthenticationRequired'}}, {'status': 500, 'body': {}}): + answer.update(denial) + asked = len(checks) + assert managed_credential(403)['error'] == 'UserNotAuthorized' + assert len(checks) == asked + 1 +stub.shutdown() + + expect('com.atproto.simplespace.updateSpace', 200, body={'space': space, 'readPolicy': public_policy}) view = expect('com.atproto.simplespace.getSpace', 200, params={'space': space}) assert view['readPolicy'] == public_policy and view['writePolicy'] == member_policy and 'policy' not in view @@ -83,7 +147,6 @@ credential = expect('com.atproto.space.getSpaceCredential', 200, token=delegatio body={'space': space}, headers=exchange)['credential'] expect('com.atproto.space.getSpaceCredential', 401, token=delegation, body={'space': space}, headers=exchange) -import base64 claims = json.loads(base64.urlsafe_b64decode(credential.split('.')[1] + '===')) assert claims['exp'] - claims['iat'] == 600 and claims['cnf']['kid'].startswith('did:key:') authority = 'did:plc:permissionsmoke'