diff --git a/tools/smoke.sh b/tools/smoke.sh --- a/tools/smoke.sh +++ b/tools/smoke.sh @@ -27,11 +27,35 @@ zig build -# stand-in PLC directory so createAccount genesis never reaches plc.directory +# stand-in PLC directory so createAccount genesis never reaches plc.directory; +# it also hosts a public OAuth client's metadata so PAR can be exercised. plc_port="${ZDS_SMOKE_PLC_PORT:-2586}" +client_id="http://127.0.0.1:${plc_port}/oauth-client-metadata.json" python3 -c " +import json from http.server import BaseHTTPRequestHandler, HTTPServer +metadata = json.dumps({ + 'client_id': '$client_id', + 'client_name': 'smoke', + 'redirect_uris': ['http://127.0.0.1:$plc_port/callback'], + 'scope': 'atproto repo:*', + 'grant_types': ['authorization_code', 'refresh_token'], + 'response_types': ['code'], + 'token_endpoint_auth_method': 'none', + 'application_type': 'web', + 'dpop_bound_access_tokens': True, +}).encode() class Stub(BaseHTTPRequestHandler): + def do_GET(self): + if self.path != '/oauth-client-metadata.json': + self.send_response(404) + self.end_headers() + return + self.send_response(200) + self.send_header('content-type', 'application/json') + self.send_header('content-length', str(len(metadata))) + self.end_headers() + self.wfile.write(metadata) def do_POST(self): self.rfile.read(int(self.headers.get('content-length', 0))) self.send_response(200) @@ -108,6 +132,30 @@ test "$invalid_dpop_status" = "401" grep -qi '^www-authenticate: DPoP error="invalid_token", error_description="Token is invalid"' "$invalid_auth_headers" grep -qi '^dpop-nonce:' "$invalid_auth_headers" + +# PAR scope validation must terminate the request: an unregistered scope is a +# 400 invalid_scope, never a 201 (a helper once wrote the error and returned +# success, and the handler overwrote it with a request_uri). +par_body="${TMPDIR:-/tmp}/zds-smoke-par.json" +par() { + curl -sS -o "$par_body" -w '%{http_code}' -X POST "$base/oauth/par" \ + --data-urlencode "client_id=$client_id" \ + --data-urlencode "redirect_uri=http://127.0.0.1:${plc_port}/callback" \ + --data-urlencode 'response_type=code' \ + --data-urlencode 'code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM' \ + --data-urlencode 'code_challenge_method=S256' \ + --data-urlencode 'state=x' \ + --data-urlencode "scope=$1" +} +test "$(par 'atproto garbage:thing')" = "400" +grep -q '"error":"invalid_scope"' "$par_body" +test "$(par 'atproto include:com.example.nope')" = "400" +grep -q '"error":"invalid_scope"' "$par_body" +test "$(par 'atproto transition:generic repo:*')" = "400" +grep -q '"error":"invalid_scope"' "$par_body" +# a valid scope gets past validation to the DPoP check (no proof was sent) +test "$(par 'atproto repo:*')" = "400" +grep -q '"error":"use_dpop_nonce"' "$par_body" describe=$(curl -fsS "$base/xrpc/com.atproto.server.describeServer") printf '%s' "$describe" | grep -q '"inviteCodeRequired":true' diff --git a/src/atproto/oauth.zig b/src/atproto/oauth.zig --- a/src/atproto/oauth.zig +++ b/src/atproto/oauth.zig @@ -1259,10 +1259,15 @@ return clock.nowSeconds(); } +// writes the error response and then fails with HandledResponse, so a helper +// that does `return oauthError(...)` stops its caller too: before this, `try +// validateScope(...)` and `try requireClientAuth(...)` returned success after +// writing a 400 and the handler went on to overwrite it with a 201/200. fn oauthError(request: *http_api.Request, status: http.Status, err: []const u8, description: []const u8) !void { var buf: [512]u8 = undefined; const body = try std.fmt.bufPrint(&buf, "{{\"error\":{f},\"error_description\":{f}}}", .{ std.json.fmt(err, .{}), std.json.fmt(description, .{}) }); try http_api.json(request, status, body); + return error.HandledResponse; } fn handleAuthorizationDpopError(request: *http_api.Request, allocator: std.mem.Allocator, err: anyerror) !void { diff --git a/src/http/server.zig b/src/http/server.zig --- a/src/http/server.zig +++ b/src/http/server.zig @@ -75,6 +75,16 @@ return; } + app.dispatch(route, request) catch |err| switch (err) { + // the handler already wrote its response (an oauth/xrpc error body); + // anything else is a genuine failure and becomes a 500 upstream. + error.HandledResponse => {}, + else => return err, + }; + handler_failed = false; + } + + fn dispatch(app: *App, route: router.Route, request: *httpz.Request) !void { switch (route) { .cors_preflight => try corsPreflight(request), .root => try landing.serve(request), @@ -162,7 +172,6 @@ .proxy_xrpc => unreachable, .not_found => try xrpcError(request, .not_found, "UnknownMethod", "Unknown XRPC method"), } - handler_failed = false; } };