Grandiras/ATProto.NET
1
1
Fork 0
Code Issues Pull requests Projects Releases 9 Packages 8 Wiki Activity Actions

feat(server)!: inbound service-auth verification (proposal 0014) #166

Merged
Grandiras merged 3 commits from issue-125-service-auth into main 2026-09-26 01:36:35 +00:00
Conversation 10 Commits 3 Files changed 62 +3994 -458
Grandiras commented 2026-09-26 00:59:39 +00:00
Owner
Copy link

Closes #125.

The SDK could mint service auth tokens but had no general way to accept them: feed generators, labelers and AppViews on ATProtoNet.Server had nothing to plug in, and the only verifier was the Spaces one for notifyWrite. This PR adds a general verifier and an ASP.NET Core scheme that follow the April 2026 spec revision (proposal 0014), rebuilds the Spaces verifier on them, and fixes the out-of-range exp/iat 500 reported on the issue.

Generator (ServiceAuthGenerator)

  • CreateToken(string audience, Nsid lxm, TimeSpan? expiresIn): lxm is now required, because the spec makes it mandatory on XRPC calls.
  • aud is validated as did or did#serviceId with the reference isDidStringOrService rule: exactly one non-empty fragment. Whitespace and control characters are refused too. The docs prefer did#serviceId and describe the bare DID as deprecated.
  • iss is a Did, which cannot carry a fragment. A test pins this.
  • The new optional constructor parameter keyId (for example #atproto_label) is sent as the kid header. It must be a #fragment without the DID. With no keyId, no kid is sent, which means #atproto.
  • jti was already always set (#118). A test now pins the exact claim set against @atproto/xrpc-server's createServiceJwt: {typ, alg} in the header and iat, iss, aud, exp, lxm, jti in the payload.
  • The shared syntax rules live in the internal Auth/ServiceAuthSyntax, which the generator and the verifier both use.

Verifier (ServiceAuthVerifier, ATProtoNet.Server.Authentication)

VerifyAsync(token, acceptedAudiences, Nsid? method) returns a VerifiedServiceAuth or throws ServiceAuthException. That exception is an XrpcException with status 401 and uses the reference error names in ServiceAuthErrors: BadJwt, BadJwtType, JwtExpired, BadJwtAudience, BadJwtLexiconMethod, BadJwtIss, BadJwtSignature. It checks, in order:

  • Structure and header: well-formed JWS; alg is ES256 or ES256K; typ is not at+jwt, refresh+jwt or dpop+jwt (as the reference refuses them).
  • iss: a bare DID. A fragment is refused with a hint to use kid.
  • kid: must be in AllowedKeyIds, which defaults to {#atproto}. A DID-URL kid naming the issuer's own key is normalized; one naming another DID's key is refused.
  • aud: exact match against the accepted audiences, for example {did#svc, did} during the transition.
  • lxm: must equal the method called. RequireLexiconMethod (default true) refuses a missing lxm; turning it off tolerates a missing one but never a wrong one. Passing method: null skips the binding, for calls that are not XRPC.
  • exp and iat: both are required, integral and range-checked. They are checked with ClockSkew (30 s) and MaxTokenLifetime (5 min, matching lex-server and the generator's cap). The comparisons use differences, so a configured duration cannot overflow DateTimeOffset.
  • jti: required, at most 255 characters (the EF column width).
  • Signature: checked against the issuer's key from the IDidResolver cache. On failure, or when the key is missing, the document is refetched once and the check retried, following the refetch-once pattern from #120. The alg must match the key's curve. A high-S signature is accepted, as @atproto/xrpc-server accepts it on a JWT (allowMalleableSig: true). This goes through a new internal AtProtoCrypto.VerifyJwtSignature over DidKeyCache. Commit verification keeps its low-S rule.
  • Replay: the jti is spent in IJtiReplayStore last, so a forged token cannot burn a genuine token's identifier.

JTI replay store

  • ISpaceReplayStore is generalized into IJtiReplayStore, with InMemoryJtiReplayStore and EfCoreJtiReplayStore<T>. The EF store's entity is JtiReplayEntity on table AtProtoJtiReplay.
  • The new JtiReplayDbContext (with ConfigureJtiReplayModel) is for services without a space server; SpaceDbContext includes the same table.
  • Registration moves to AddAtProtoEfCoreJtiReplayStore<T>(). Service auth and the space server share one registered store, and a test pins that.
  • No Redis implementation was added, as the owner decided. The existing RedisSpaceReplayStore only switches to the new interface so it keeps compiling; #136 deletes it.

ASP.NET Core integration

  • Scheme: AddAuthentication().AddAtProtoServiceAuth(o => o.Audiences.Add("did:web:x#bsky_fg")). It has a default or custom scheme name, and the options are validated at host start (ValidateOnStart), so a missing or malformed audience fails the start.
  • Principal: carries the claims did (also the identity's name), aud and lxm. The names are constants on AtProtoServiceAuthDefaults.
  • XRPC integration: MapXrpcEndpoints() now attaches a public XrpcMethodMetadata(Nsid) to every endpoint, and the handler binds lxm to it automatically.
  • Other endpoints: an endpoint without that metadata refuses service auth before the token is verified, so the token is not spent. Hand-mapped routes can attach the metadata themselves.
  • Requiring the scheme: [RequireServiceAuth] (an AuthorizeAttribute) on a handler, or .RequireServiceAuth() on the group. [AllowAnonymous] still opens a handler.
  • Responses: a challenge answers 401 with the XRPC envelope, WWW-Authenticate: Bearer, and the refusal's error name (AuthenticationRequired when there was no token). Forbid answers 403 Forbidden with the envelope.
  • Defaults it registers: AddAtProtoIdentity() and an in-process IJtiReplayStore, each unless one is already registered.

Spaces

  • SpaceServiceAuthVerifier now delegates to ServiceAuthVerifier with ClockSkew and MaxSingleUseTokenLifetime taken from SpaceServerOptions. It maps ServiceAuthException to SpaceVerificationException(NotAuthorized) and keeps the inner exception.
  • The wrong "jti is optional" code and comment are gone.
  • The audience forms from #113 are unchanged: bare authority DID, #atproto_space_host, and the service DID. Tests cover them.
  • IsRepoHostAsync is unchanged.

Bug fix: an out-of-range exp/iat answered 500

  • The new internal JsonElementExtensions.TryGetNumericDate reads whole seconds within years 1–9999 and reports anything else as invalid.
  • It is used in SpaceTokens.Parse (so TryParse returns false again), in DPoPProofValidator and in the new verifier, which the Spaces service-auth verifier now uses.
  • The error messages say "is not a valid time".
  • Behaviour change: a present iat that is not a whole number now makes SpaceTokens.Parse fail. It used to be ignored silently, which matches how exp was already handled.

Docs

  • docs/xrpc-handlers.md: new "Serving XRPC to other services" section (setup, a table of checks and options, error names, replay store, DID-cache caveat, non-XRPC endpoints, direct verifier use).
  • Updated: crypto.md (generator), spaces.md (store names, a replay-table upgrade note, type table), api-reference.md (new "Service auth" table, Spaces rows), server.md, and aspnet-core.md (a pointer from the legacy handler to service auth).

Breaking changes and migration

  • CreateToken requires lxm. Migration: pass Nsid.Parse("…") for the method being called. An audience whose fragment contains a second # or whitespace is now refused.
  • Replay store renames:
    • ISpaceReplayStore → IJtiReplayStore, now in the ATProtoNet.Server.Authentication namespace.
    • InMemorySpaceReplayStore → InMemoryJtiReplayStore, and EfCoreSpaceReplayStore<T> → EfCoreJtiReplayStore<T>.
    • AddAtProtoEfCoreSpaceReplayStore<T>() → AddAtProtoEfCoreJtiReplayStore<T>().
    • SpaceReplayEntity → JtiReplayEntity, SpaceDbContext.AtProtoSpaceReplay → AtProtoJtiReplay, and SpaceDbContext.ConfigureSpaceReplayModel → JtiReplayDbContext.ConfigureJtiReplayModel.
    • Migration: EF users need a migration for the renamed table. Its rows live for minutes, so drop-and-create is safe, as docs/spaces.md explains.
  • Stricter notifyWrite service auth: jti, lxm and iat are now required, iss must have no fragment, and only a #atproto key is accepted. The SDK's own SpaceWriteNotifier and the reference PDS already send all of these.
  • VerifiedServiceAuth moved and changed shape: it moves to ATProtoNet.Server.Authentication and is a class with init-only properties instead of a positional record. Method is now the token's own lxm, and it adds KeyId, TokenId, IssuedAt and ExpiresAt.

Review fixes

An adversarial review found the following; each fix has a regression test that fails without it (checked by reverting each fix).

  • Replay after exp (high): a token is accepted until exp + skew, but its jti was kept only until exp.
    • The service-auth, delegation-token, client-attestation and DPoP verifiers now keep it until the token stops being accepted. IJtiReplayStore documents that invariant.
    • The EF store no longer sweeps rows from the current second, since it stores whole seconds.
    • A DPoP proof is refused from the instant iat + ProofLifetime is reached.
    • The Spaces half of this was pre-existing, so it has a Security bullet.
  • S ≥ n threw a 500: signatures whose r/s fall outside [1, n−1] are refused before high-S normalization (AtProtoCrypto.HasScalarsInRange), and NormalizeLowSSignature no longer throws. AtProtoKey.Verify checks the range too, as defence in depth; .NET already answered false there.
  • Missing kid bypassed AllowedKeyIds: the implied #atproto is now checked against the allow-list too.
  • The scheme spent tokens on endpoints that did not ask for it. Design:
    • The handler computes the endpoint's authorization policy the way the authorization middleware will (AuthorizationPolicy.CombineAsync). It verifies only when that policy names the scheme, or names none while this is the default scheme.
    • Endpoints with IAllowAnonymous, without authorization, or authorized by other schemes return NoResult, and the token stays unspent.
    • The space server's self-authenticating endpoints (repo-host reads, getSpaceCredential, listRepos, (un)registerNotify, notifyWrite) carry an internal [AuthenticatesItself] marker, which is an IAllowAnonymous. So a group-wide .RequireServiceAuth() or RequireAuthorization(), or a fallback policy, no longer stands in front of them. notifyWrite then checks and spends its token exactly once.
    • The simplespace admin endpoints, which read the caller from the host's own authentication, are unchanged.
    • The auto-default behaviour is documented. Optional authentication is left to calling ServiceAuthVerifier directly.
  • Whitespace or control-character jti gave a 500: refused as malformed. The shared Jwt.IsUsableTokenId rule applies to service auth, space tokens and DPoP proofs (the latter two were pre-existing, under Fixed).
  • typ: any explicitly typed …+jwt is now refused (RFC 8725 §3.11), including space delegation tokens signed with the same #atproto key. JWT or no typ is still accepted.
  • Alignment with lex-server: an iat older than MaxTokenLifetime + ClockSkew is refused, and nbf is honoured with skew.
  • Separate commit fix(syntax): the Did, Handle, Nsid, RecordKey, Tid and did:web host patterns now anchor at \z instead of $. AtUri, AtIdentifier, SpaceUri, Cid and AtDatetime were checked and already refuse line breaks through their components or parsers; tests cover every type.
  • did:web id match: a did:web document's id must now equal the requested DID exactly, as @atproto/identity compares (val.id !== did). Previously it matched case-insensitively.

Verification

  • Unit tests (rebased on 810e2de): 4,219 passed, 0 failed, 5 skipped (4,224 total). New:
    • Server/Authentication/ServiceAuthVerifierTests.cs: every check, refetch-once (rotated key verifies, forged key refused after exactly one refresh, valid token never refreshes), a genuinely high-S signature accepted, the forged-token-cannot-burn-a-jti case, option validation and copying, and error names pinned to the reference.
    • ServiceAuthSchemeTests.cs: TestServer end to end. Covers attribute and group convention, [AllowAnonymous], lxm binding, audience forms, replay, the unmatched NSID, a non-XRPC route (refused without spending the token), a hand-mapped route with XrpcMethodMetadata, 403 via policy, host start failing on bad options, and DI.
    • JtiReplayStoreTests.cs: in-memory store, EF over SQLite with JtiReplayDbContext, table and key pinned, registration order.
    • Out-of-range exp/iat regressions in SpaceTokensTests, DPoPProofValidatorTests, the Spaces service-auth tests, and JwtTests.TryGetNumericDate.
    • New Spaces tests for required jti/lxm/iat, a refused kid, iss with a fragment, and both authority audience forms.
    • Generator tests for the reference claim shape, kid, and audience and kid syntax.
  • Build: the full solution builds with 0 warnings and 0 errors. Integration tests compile.
  • Reference behaviour: checked against @atproto/xrpc-server auth.ts (verifyJwt, createServiceJwt) and @atproto/lex-server service-auth.ts at their main as of 2026-09-26.

Deliberately deferred or out of scope

  • Legacy handler: AtProtoAuthenticationHandler / AddAtProto(AuthenticationBuilder) stays; #136 deletes it. aspnet-core.md now points to service auth.
  • Claim constants: they live on AtProtoServiceAuthDefaults; #136 plans AtProtoClaimTypes and can fold them in.
  • Redis: no Redis replay store (owner decision). RedisSpaceReplayStore only retargets the interface until #136 removes it.
  • In-memory warning: only AddAtProtoSpaces() warns at startup about the in-process replay store. The service-auth scheme documents the multi-instance requirement but logs nothing.
  • Unresolvable DIDs: each distinct issuer DID that does not resolve costs one directory fetch before the signature can be checked; CachingDidResolver then caches the failure for a minute. This is the same as the reference implementation and pre-existing in Spaces.

🤖 Generated with Claude Code

Closes #125. The SDK could mint service auth tokens but had no general way to accept them: feed generators, labelers and AppViews on `ATProtoNet.Server` had nothing to plug in, and the only verifier was the Spaces one for `notifyWrite`. This PR adds a general verifier and an ASP.NET Core scheme that follow the April 2026 spec revision ([proposal 0014](https://github.com/bluesky-social/proposals/tree/main/0014-service-auth-revised)), rebuilds the Spaces verifier on them, and fixes the out-of-range `exp`/`iat` 500 reported on the issue. ## Generator (`ServiceAuthGenerator`) - `CreateToken(string audience, Nsid lxm, TimeSpan? expiresIn)`: `lxm` is now required, because the spec makes it mandatory on XRPC calls. - `aud` is validated as `did` or `did#serviceId` with the reference `isDidStringOrService` rule: exactly one non-empty fragment. Whitespace and control characters are refused too. The docs prefer `did#serviceId` and describe the bare DID as deprecated. - `iss` is a `Did`, which cannot carry a fragment. A test pins this. - The new optional constructor parameter `keyId` (for example `#atproto_label`) is sent as the `kid` header. It must be a `#fragment` without the DID. With no `keyId`, no `kid` is sent, which means `#atproto`. - `jti` was already always set (#118). A test now pins the exact claim set against `@atproto/xrpc-server`'s `createServiceJwt`: `{typ, alg}` in the header and `iat, iss, aud, exp, lxm, jti` in the payload. - The shared syntax rules live in the internal `Auth/ServiceAuthSyntax`, which the generator and the verifier both use. ## Verifier (`ServiceAuthVerifier`, `ATProtoNet.Server.Authentication`) `VerifyAsync(token, acceptedAudiences, Nsid? method)` returns a `VerifiedServiceAuth` or throws `ServiceAuthException`. That exception is an `XrpcException` with status 401 and uses the reference error names in `ServiceAuthErrors`: `BadJwt`, `BadJwtType`, `JwtExpired`, `BadJwtAudience`, `BadJwtLexiconMethod`, `BadJwtIss`, `BadJwtSignature`. It checks, in order: - **Structure and header:** well-formed JWS; `alg` is ES256 or ES256K; `typ` is not `at+jwt`, `refresh+jwt` or `dpop+jwt` (as the reference refuses them). - **`iss`:** a bare DID. A fragment is refused with a hint to use `kid`. - **`kid`:** must be in `AllowedKeyIds`, which defaults to `{#atproto}`. A DID-URL `kid` naming the issuer's own key is normalized; one naming another DID's key is refused. - **`aud`:** exact match against the accepted audiences, for example `{did#svc, did}` during the transition. - **`lxm`:** must equal the method called. `RequireLexiconMethod` (default true) refuses a missing `lxm`; turning it off tolerates a missing one but never a wrong one. Passing `method: null` skips the binding, for calls that are not XRPC. - **`exp` and `iat`:** both are required, integral and range-checked. They are checked with `ClockSkew` (30 s) and `MaxTokenLifetime` (5 min, matching `lex-server` and the generator's cap). The comparisons use differences, so a configured duration cannot overflow `DateTimeOffset`. - **`jti`:** required, at most 255 characters (the EF column width). - **Signature:** checked against the issuer's key from the `IDidResolver` cache. On failure, or when the key is missing, the document is refetched once and the check retried, following the refetch-once pattern from #120. The `alg` must match the key's curve. A high-S signature is accepted, as `@atproto/xrpc-server` accepts it on a JWT (`allowMalleableSig: true`). This goes through a new internal `AtProtoCrypto.VerifyJwtSignature` over `DidKeyCache`. Commit verification keeps its low-S rule. - **Replay:** the `jti` is spent in `IJtiReplayStore` last, so a forged token cannot burn a genuine token's identifier. ## JTI replay store - `ISpaceReplayStore` is generalized into `IJtiReplayStore`, with `InMemoryJtiReplayStore` and `EfCoreJtiReplayStore<T>`. The EF store's entity is `JtiReplayEntity` on table `AtProtoJtiReplay`. - The new `JtiReplayDbContext` (with `ConfigureJtiReplayModel`) is for services without a space server; `SpaceDbContext` includes the same table. - Registration moves to `AddAtProtoEfCoreJtiReplayStore<T>()`. Service auth and the space server share one registered store, and a test pins that. - No Redis implementation was added, as the owner decided. The existing `RedisSpaceReplayStore` only switches to the new interface so it keeps compiling; #136 deletes it. ## ASP.NET Core integration - **Scheme:** `AddAuthentication().AddAtProtoServiceAuth(o => o.Audiences.Add("did:web:x#bsky_fg"))`. It has a default or custom scheme name, and the options are validated at host start (`ValidateOnStart`), so a missing or malformed audience fails the start. - **Principal:** carries the claims `did` (also the identity's name), `aud` and `lxm`. The names are constants on `AtProtoServiceAuthDefaults`. - **XRPC integration:** `MapXrpcEndpoints()` now attaches a public `XrpcMethodMetadata(Nsid)` to every endpoint, and the handler binds `lxm` to it automatically. - **Other endpoints:** an endpoint without that metadata refuses service auth before the token is verified, so the token is not spent. Hand-mapped routes can attach the metadata themselves. - **Requiring the scheme:** `[RequireServiceAuth]` (an `AuthorizeAttribute`) on a handler, or `.RequireServiceAuth()` on the group. `[AllowAnonymous]` still opens a handler. - **Responses:** a challenge answers 401 with the XRPC envelope, `WWW-Authenticate: Bearer`, and the refusal's error name (`AuthenticationRequired` when there was no token). Forbid answers 403 `Forbidden` with the envelope. - **Defaults it registers:** `AddAtProtoIdentity()` and an in-process `IJtiReplayStore`, each unless one is already registered. ## Spaces - `SpaceServiceAuthVerifier` now delegates to `ServiceAuthVerifier` with `ClockSkew` and `MaxSingleUseTokenLifetime` taken from `SpaceServerOptions`. It maps `ServiceAuthException` to `SpaceVerificationException(NotAuthorized)` and keeps the inner exception. - The wrong "jti is optional" code and comment are gone. - The audience forms from #113 are unchanged: bare authority DID, `#atproto_space_host`, and the service DID. Tests cover them. - `IsRepoHostAsync` is unchanged. ## Bug fix: an out-of-range `exp`/`iat` answered 500 - The new internal `JsonElementExtensions.TryGetNumericDate` reads whole seconds within years 1–9999 and reports anything else as invalid. - It is used in `SpaceTokens.Parse` (so `TryParse` returns false again), in `DPoPProofValidator` and in the new verifier, which the Spaces service-auth verifier now uses. - The error messages say "is not a valid time". - Behaviour change: a present `iat` that is not a whole number now makes `SpaceTokens.Parse` fail. It used to be ignored silently, which matches how `exp` was already handled. ## Docs - `docs/xrpc-handlers.md`: new "Serving XRPC to other services" section (setup, a table of checks and options, error names, replay store, DID-cache caveat, non-XRPC endpoints, direct verifier use). - Updated: `crypto.md` (generator), `spaces.md` (store names, a replay-table upgrade note, type table), `api-reference.md` (new "Service auth" table, Spaces rows), `server.md`, and `aspnet-core.md` (a pointer from the legacy handler to service auth). ## Breaking changes and migration - **`CreateToken` requires `lxm`.** Migration: pass `Nsid.Parse("…")` for the method being called. An audience whose fragment contains a second `#` or whitespace is now refused. - **Replay store renames:** - `ISpaceReplayStore` → `IJtiReplayStore`, now in the `ATProtoNet.Server.Authentication` namespace. - `InMemorySpaceReplayStore` → `InMemoryJtiReplayStore`, and `EfCoreSpaceReplayStore<T>` → `EfCoreJtiReplayStore<T>`. - `AddAtProtoEfCoreSpaceReplayStore<T>()` → `AddAtProtoEfCoreJtiReplayStore<T>()`. - `SpaceReplayEntity` → `JtiReplayEntity`, `SpaceDbContext.AtProtoSpaceReplay` → `AtProtoJtiReplay`, and `SpaceDbContext.ConfigureSpaceReplayModel` → `JtiReplayDbContext.ConfigureJtiReplayModel`. - Migration: EF users need a migration for the renamed table. Its rows live for minutes, so drop-and-create is safe, as `docs/spaces.md` explains. - **Stricter `notifyWrite` service auth:** `jti`, `lxm` and `iat` are now required, `iss` must have no fragment, and only a `#atproto` key is accepted. The SDK's own `SpaceWriteNotifier` and the reference PDS already send all of these. - **`VerifiedServiceAuth` moved and changed shape:** it moves to `ATProtoNet.Server.Authentication` and is a class with init-only properties instead of a positional record. `Method` is now the token's own `lxm`, and it adds `KeyId`, `TokenId`, `IssuedAt` and `ExpiresAt`. ## Review fixes An adversarial review found the following; each fix has a regression test that fails without it (checked by reverting each fix). - **Replay after `exp` (high):** a token is accepted until `exp + skew`, but its `jti` was kept only until `exp`. - The service-auth, delegation-token, client-attestation and DPoP verifiers now keep it until the token stops being accepted. `IJtiReplayStore` documents that invariant. - The EF store no longer sweeps rows from the current second, since it stores whole seconds. - A DPoP proof is refused from the instant `iat + ProofLifetime` is reached. - The Spaces half of this was pre-existing, so it has a Security bullet. - **S ≥ n threw a 500:** signatures whose `r`/`s` fall outside [1, n−1] are refused before high-S normalization (`AtProtoCrypto.HasScalarsInRange`), and `NormalizeLowSSignature` no longer throws. `AtProtoKey.Verify` checks the range too, as defence in depth; .NET already answered `false` there. - **Missing `kid` bypassed `AllowedKeyIds`:** the implied `#atproto` is now checked against the allow-list too. - **The scheme spent tokens on endpoints that did not ask for it.** Design: - The handler computes the endpoint's authorization policy the way the authorization middleware will (`AuthorizationPolicy.CombineAsync`). It verifies only when that policy names the scheme, or names none while this is the default scheme. - Endpoints with `IAllowAnonymous`, without authorization, or authorized by other schemes return `NoResult`, and the token stays unspent. - The space server's self-authenticating endpoints (repo-host reads, `getSpaceCredential`, `listRepos`, `(un)registerNotify`, `notifyWrite`) carry an internal `[AuthenticatesItself]` marker, which is an `IAllowAnonymous`. So a group-wide `.RequireServiceAuth()` or `RequireAuthorization()`, or a fallback policy, no longer stands in front of them. `notifyWrite` then checks and spends its token exactly once. - The `simplespace` admin endpoints, which read the caller from the host's own authentication, are unchanged. - The auto-default behaviour is documented. Optional authentication is left to calling `ServiceAuthVerifier` directly. - **Whitespace or control-character `jti` gave a 500:** refused as malformed. The shared `Jwt.IsUsableTokenId` rule applies to service auth, space tokens and DPoP proofs (the latter two were pre-existing, under Fixed). - **`typ`:** any explicitly typed `…+jwt` is now refused (RFC 8725 §3.11), including space delegation tokens signed with the same `#atproto` key. `JWT` or no `typ` is still accepted. - **Alignment with `lex-server`:** an `iat` older than `MaxTokenLifetime + ClockSkew` is refused, and `nbf` is honoured with skew. - **Separate commit `fix(syntax)`:** the `Did`, `Handle`, `Nsid`, `RecordKey`, `Tid` and did:web host patterns now anchor at `\z` instead of `$`. `AtUri`, `AtIdentifier`, `SpaceUri`, `Cid` and `AtDatetime` were checked and already refuse line breaks through their components or parsers; tests cover every type. - **did:web `id` match:** a did:web document's `id` must now equal the requested DID exactly, as `@atproto/identity` compares (`val.id !== did`). Previously it matched case-insensitively. ## Verification - **Unit tests** (rebased on `810e2de`): 4,219 passed, 0 failed, 5 skipped (4,224 total). New: - `Server/Authentication/ServiceAuthVerifierTests.cs`: every check, refetch-once (rotated key verifies, forged key refused after exactly one refresh, valid token never refreshes), a genuinely high-S signature accepted, the forged-token-cannot-burn-a-jti case, option validation and copying, and error names pinned to the reference. - `ServiceAuthSchemeTests.cs`: TestServer end to end. Covers attribute and group convention, `[AllowAnonymous]`, lxm binding, audience forms, replay, the unmatched NSID, a non-XRPC route (refused without spending the token), a hand-mapped route with `XrpcMethodMetadata`, 403 via policy, host start failing on bad options, and DI. - `JtiReplayStoreTests.cs`: in-memory store, EF over SQLite with `JtiReplayDbContext`, table and key pinned, registration order. - Out-of-range `exp`/`iat` regressions in `SpaceTokensTests`, `DPoPProofValidatorTests`, the Spaces service-auth tests, and `JwtTests.TryGetNumericDate`. - New Spaces tests for required `jti`/`lxm`/`iat`, a refused `kid`, `iss` with a fragment, and both authority audience forms. - Generator tests for the reference claim shape, `kid`, and audience and `kid` syntax. - **Build:** the full solution builds with 0 warnings and 0 errors. Integration tests compile. - **Reference behaviour:** checked against `@atproto/xrpc-server` `auth.ts` (`verifyJwt`, `createServiceJwt`) and `@atproto/lex-server` `service-auth.ts` at their `main` as of 2026-09-26. ## Deliberately deferred or out of scope - **Legacy handler:** `AtProtoAuthenticationHandler` / `AddAtProto(AuthenticationBuilder)` stays; #136 deletes it. `aspnet-core.md` now points to service auth. - **Claim constants:** they live on `AtProtoServiceAuthDefaults`; #136 plans `AtProtoClaimTypes` and can fold them in. - **Redis:** no Redis replay store (owner decision). `RedisSpaceReplayStore` only retargets the interface until #136 removes it. - **In-memory warning:** only `AddAtProtoSpaces()` warns at startup about the in-process replay store. The service-auth scheme documents the multi-instance requirement but logs nothing. - **Unresolvable DIDs:** each distinct issuer DID that does not resolve costs one directory fetch before the signature can be checked; `CachingDidResolver` then caches the failure for a minute. This is the same as the reference implementation and pre-existing in Spaces. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(server)!: inbound service-auth verification (proposal 0014) (closes #125)
All checks were successful
CI / pds-integration (pull_request) Successful in 27s
Details
CI / build-and-test (pull_request) Successful in 54s
Details
bc630997a3 
The SDK could mint service auth tokens but not accept them: feed
generators, labelers and AppViews on ATProtoNet.Server had nothing to
plug in, and the only verifier was the Spaces one for notifyWrite.

ServiceAuthVerifier follows the April 2026 spec revision: accepted
audiences (did#serviceId, plus the bare DID during the transition),
lxm bound to the method called, a kid allow-list (#atproto), no
fragment in iss, exp/iat with clock skew and a lifetime ceiling, the
signature checked against the cached DID document with one refetch on
failure, and a required jti spent last. AddAtProtoServiceAuth() wraps it
as an authentication scheme (did/lxm/aud claims) that binds each token
to the NSID MapXrpcEndpoints() now attaches as XrpcMethodMetadata;
[RequireServiceAuth] and .RequireServiceAuth() require it.

ISpaceReplayStore becomes the general IJtiReplayStore (in-memory and EF
Core), shared by service auth and the space server, and the Spaces
service-auth verifier is rebuilt on the general one, which makes jti,
lxm and iat mandatory on notifyWrite. ServiceAuthGenerator now requires
an lxm and can send a kid.

Also fixes the out-of-range exp/iat bug: FromUnixTimeSeconds threw from
SpaceTokens.Parse, DPoPProofValidator and SpaceServiceAuthVerifier, so a
crafted token produced a 500 instead of a 401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
opencode-bot commented 2026-09-26 00:59:41 +00:00
Collaborator
Copy link

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-166/run-2026-09-26T00-59-41-424Z.md) — updating every ~5 minutes while the review runs.
opencode-bot commented 2026-09-26 00:59:44 +00:00
Collaborator
Copy link

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T05:59:41.809Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T05:59:41.809Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f24c65be6ffeRMzoDUjKt6El6O -->
opencode-bot commented 2026-09-26 01:26:21 +00:00
Collaborator
Copy link

🤖 Resolving the merge conflicts — the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the fix runs.

🤖 Resolving the merge conflicts — the reply will link the full session transcript. <!-- opencode-run:conflict --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-166/run-2026-09-26T01-26-20-536Z.md) — updating every ~5 minutes while the fix runs.
opencode-bot commented 2026-09-26 01:26:22 +00:00
Collaborator
Copy link

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T06:26:17.710Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T06:26:17.710Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f24adf6a2ffeVn9kOmI7qbEqB1 -->
Grandiras force-pushed issue-125-service-auth from bc630997a3
All checks were successful
CI / pds-integration (pull_request) Successful in 27s
Details
CI / build-and-test (pull_request) Successful in 54s
Details
to 3f942b80e2
All checks were successful
CI / pds-integration (pull_request) Successful in 28s
Details
CI / build-and-test (pull_request) Successful in 53s
Details
Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s
Details
2026-09-26 01:33:21 +00:00 Compare
Grandiras commented 2026-09-26 01:33:29 +00:00
Author
Owner
Copy link

@claude /review — security-critical, large (≈50 files); the description has the migration notes and a "Review fixes" section covering an independent adversarial review. The diff will be truncated, so please read these in full from the branch: src/ATProtoNet.Server/Authentication/ServiceAuthVerifier.cs, AtProtoServiceAuthentication.cs (the handler: when it verifies vs returns NoResult, policy combination, challenge/forbid bodies), ServiceAuthOptions/AtProtoServiceAuthDefaults, IJtiReplayStore.cs + EfCoreJtiReplayStore.cs (retention ≥ acceptance window, atomic spend), src/ATProtoNet/Auth/ServiceAuthGenerator.cs + ServiceAuthSyntax, the changed parts of Crypto/AtProtoCrypto.cs and Crypto/DidKeyCache.cs (JWT signature check, r/s range), JsonElementExtensions.TryGetNumericDate, the Spaces SpaceServiceAuthVerifier and the [AuthenticatesItself] marker on the Space endpoints, and the identifier regex changes in the fix(syntax) commit. Compare against @atproto/xrpc-server verifyJwt. Focus on auth bypasses (alg/kid/iss/aud/lxm/typ/exp/iat/nbf), replay, tokens spent on endpoints that don't require them, and exceptions escaping as 500.

@claude /review — security-critical, large (≈50 files); the description has the migration notes and a "Review fixes" section covering an independent adversarial review. The diff will be truncated, so please read these in full from the branch: `src/ATProtoNet.Server/Authentication/ServiceAuthVerifier.cs`, `AtProtoServiceAuthentication.cs` (the handler: when it verifies vs returns NoResult, policy combination, challenge/forbid bodies), `ServiceAuthOptions`/`AtProtoServiceAuthDefaults`, `IJtiReplayStore.cs` + `EfCoreJtiReplayStore.cs` (retention ≥ acceptance window, atomic spend), `src/ATProtoNet/Auth/ServiceAuthGenerator.cs` + `ServiceAuthSyntax`, the changed parts of `Crypto/AtProtoCrypto.cs` and `Crypto/DidKeyCache.cs` (JWT signature check, r/s range), `JsonElementExtensions.TryGetNumericDate`, the Spaces `SpaceServiceAuthVerifier` and the `[AuthenticatesItself]` marker on the Space endpoints, and the identifier regex changes in the `fix(syntax)` commit. Compare against `@atproto/xrpc-server` `verifyJwt`. Focus on auth bypasses (alg/kid/iss/aud/lxm/typ/exp/iat/nbf), replay, tokens spent on endpoints that don't require them, and exceptions escaping as 500.
claude-bot commented 2026-09-26 01:33:32 +00:00
Collaborator
Copy link

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/613); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-166/run-613.md) — updating every ~5 minutes while the task runs.
opencode-bot commented 2026-09-26 01:34:23 +00:00
Collaborator
Copy link

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-166/run-2026-09-26T01-34-23-020Z.md) — updating every ~5 minutes while the review runs.
opencode-bot commented 2026-09-26 01:34:25 +00:00
Collaborator
Copy link

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T06:34:23.481Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T06:34:23.481Z) — nothing to re-trigger. <!-- opencode-run:paused --> <!-- opencode-session:ses_f24a6979effe4pcCO5ZPcx55bo -->
claude-reviewer approved these changes 2026-09-26 01:35:29 +00:00
claude-reviewer left a comment
Copy link

✅ Verdict: approve — no blocking issues found.

This is a large, carefully engineered addition of inbound service-auth verification (ServiceAuthVerifier, the ASP.NET Core scheme, the generalized IJtiReplayStore, and the Spaces rebuild on top of it). I read through the core verification logic (ServiceAuthVerifier.VerifyAsync, ReadKeyId, ReadMethod, ReadLifetime, VerifySignatureAsync), the ASP.NET Core handler (AtProtoServiceAuthHandler, especially RequiresThisSchemeAsync and the challenge/forbid handling), the replay-store rename/generalization, the [AuthenticatesItself] opt-out mechanism for space endpoints, and the various renamed EF Core artifacts.

Notable things that check out:

  • Ordering of checks (structure/typ → iss → kid → aud → lxm → exp/iat/nbf → signature → jti-spend-last) matches the description and avoids burning a jti on a forged token.
  • RequiresThisSchemeAsync correctly special-cases IAllowAnonymous, explicit scheme lists on the combined policy, and the default-scheme fallback, and it's evaluated before token verification so an endpoint that doesn't ask for the scheme never has its token spent.
  • The HandleChallengeAsync re-entry into HandleAuthenticateOnceSafeAsync() is safe — ASP.NET Core's AuthenticationHandler caches the authenticate Task per handler instance per request, so this does not double-verify/double-spend a token.
  • The replay-retention math (expiresAt + clockSkew for service auth, ReplayRetention for space tokens, < instead of <= in the EF sweep) is internally consistent with the "keep jti until the token stops being accepted" invariant described in the PR.
  • ReadKeyId's DID-URL normalization only shortens a kid that names the issuer's own DID, correctly leaving a kid naming a different DID to fail the allow-list check as unknown.

I could not review the tail of the diff that was truncated by the tool (the rest of JsonElementExtensions.TryGetNumericDate, AtProtoCrypto.VerifyJwtSignature, ServiceAuthSyntax, DidDocument.GetVerificationKey), so I can't independently confirm the numeric-date bounds-checking or signature/curve-matching implementation details — but nothing in the visible portions contradicts the PR's description of them, and the PR states these are covered by dedicated regression tests (reverting each fix fails a test). Given the scope and the already-documented adversarial review pass, I don't have further blocking findings to add.

⏱ 1m 39s · 1 turns · tokens: 79.2k in / 9.6k out (+23.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, carefully engineered addition of inbound service-auth verification (`ServiceAuthVerifier`, the ASP.NET Core scheme, the generalized `IJtiReplayStore`, and the Spaces rebuild on top of it). I read through the core verification logic (`ServiceAuthVerifier.VerifyAsync`, `ReadKeyId`, `ReadMethod`, `ReadLifetime`, `VerifySignatureAsync`), the ASP.NET Core handler (`AtProtoServiceAuthHandler`, especially `RequiresThisSchemeAsync` and the challenge/forbid handling), the replay-store rename/generalization, the `[AuthenticatesItself]` opt-out mechanism for space endpoints, and the various renamed EF Core artifacts. Notable things that check out: - Ordering of checks (structure/typ → iss → kid → aud → lxm → exp/iat/nbf → signature → jti-spend-last) matches the description and avoids burning a `jti` on a forged token. - `RequiresThisSchemeAsync` correctly special-cases `IAllowAnonymous`, explicit scheme lists on the combined policy, and the default-scheme fallback, and it's evaluated before token verification so an endpoint that doesn't ask for the scheme never has its token spent. - The `HandleChallengeAsync` re-entry into `HandleAuthenticateOnceSafeAsync()` is safe — ASP.NET Core's `AuthenticationHandler` caches the authenticate `Task` per handler instance per request, so this does not double-verify/double-spend a token. - The replay-retention math (`expiresAt + clockSkew` for service auth, `ReplayRetention` for space tokens, `<` instead of `<=` in the EF sweep) is internally consistent with the "keep jti until the token stops being accepted" invariant described in the PR. - `ReadKeyId`'s DID-URL normalization only shortens a `kid` that names the issuer's own DID, correctly leaving a `kid` naming a different DID to fail the allow-list check as unknown. I could not review the tail of the diff that was truncated by the tool (the rest of `JsonElementExtensions.TryGetNumericDate`, `AtProtoCrypto.VerifyJwtSignature`, `ServiceAuthSyntax`, `DidDocument.GetVerificationKey`), so I can't independently confirm the numeric-date bounds-checking or signature/curve-matching implementation details — but nothing in the visible portions contradicts the PR's description of them, and the PR states these are covered by dedicated regression tests (reverting each fix fails a test). Given the scope and the already-documented adversarial review pass, I don't have further blocking findings to add. --- ⏱ 1m 39s · 1 turns · tokens: 79.2k in / 9.6k out (+23.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-166/run-613.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:3f942b80e2e95d19a3b5e9c59a92011433688b85 -->
Grandiras commented 2026-09-26 01:36:34 +00:00
Author
Owner
Copy link

Supplementary check of the parts the claude review couldn't see (truncated diff), at head 3f942b8:

  • DidKeyCache.VerifyJws: alg must equal the key curve's JWS algorithm; length and r/s ∈ [1, n−1] are checked (HasScalarsInRange) before NormalizeLowSSignature, which also refuses to touch S ≥ n. AtProtoKey.Verify (commit path) gains the same range check and keeps low-S.
  • Jwt.TryGetNumericDate: integral seconds only, range-bounded, absent → null; ReadLifetime compares with differences only (no overflow). Acceptance ends at exp+skew, which matches the replay retention of exp+skew.
  • ServiceAuthSyntax: did or did#fragment, with a single non-empty fragment that has no #, space or C0 controls. Audiences are then compared ordinally against the configured set anyway.
  • Identifier regexes are anchored with \z; IdsMatch is ordinal, as in @atproto/identity.
    No findings. Merging on the claude approval plus green CI.
Supplementary check of the parts the claude review couldn't see (truncated diff), at head 3f942b8: - `DidKeyCache.VerifyJws`: `alg` must equal the key curve's JWS algorithm; length and r/s ∈ [1, n−1] are checked (`HasScalarsInRange`) before `NormalizeLowSSignature`, which also refuses to touch S ≥ n. `AtProtoKey.Verify` (commit path) gains the same range check and keeps low-S. - `Jwt.TryGetNumericDate`: integral seconds only, range-bounded, absent → null; `ReadLifetime` compares with differences only (no overflow). Acceptance ends at exp+skew, which matches the replay retention of exp+skew. - `ServiceAuthSyntax`: `did` or `did#fragment`, with a single non-empty fragment that has no `#`, space or C0 controls. Audiences are then compared ordinally against the configured set anyway. - Identifier regexes are anchored with `\z`; `IdsMatch` is ordinal, as in `@atproto/identity`. No findings. Merging on the claude approval plus green CI.
Grandiras deleted branch issue-125-service-auth 2026-09-26 01:36:36 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
claude-reviewer
Labels
Clear labels   
breaking-change
Introduces a breaking API change

  
bug
Something is not working correctly

  
documentation
Documentation improvements

  
duplicate
This issue or PR already exists

  
enhancement
New feature or improvement

  
good first issue
Good for newcomers to the project

  
help wanted
Extra attention is needed

  
performance
Performance-related issue or improvement

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
breaking-change
bug
documentation
duplicate
enhancement
good first issue
help wanted
performance
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/ATProto.NET!166
No description provided.