Skip to content

Feat/aauth events implementation - #45

Open
dasiths wants to merge 73 commits into
mainfrom
feat/aauth-events-implementation
Open

Feat/aauth events implementation#45
dasiths wants to merge 73 commits into
mainfrom
feat/aauth-events-implementation

Conversation

@dasiths

@dasiths dasiths commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

This pull request introduces support for the standalone AAuth Events specification and its implementation across the repository. It adds the new AAuth.Events library, sample, and tests, updates documentation and instructions to handle the AAuth Events draft, and ensures the build, packaging, and solution files include all new components. The most important changes are grouped below.

AAuth Events support and implementation:

  • Added the new AAuth.Events library (src/AAuth.Events/AAuth.Events.csproj) and included it in the solution and build system, along with its corresponding tests (tests/AAuth.Events.Tests/AAuth.Events.Tests.csproj) and a sample agent (samples/EventAgent/EventAgent.csproj). ([[1]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-a9bb34433080ddb47f00a79ae377844a484525d36de716cf94bf1245b26534bbR27-R32), [[2]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-a9bb34433080ddb47f00a79ae377844a484525d36de716cf94bf1245b26534bbR5), [[3]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-551d1fcf87f78cc3bc18a7b332a4dc5d8773a512062df881c5aba28a6f5c48d7R42), [[4]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R19), [[5]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R76))
  • Added a detailed specification review for AAuth Events, documenting 13 upstream protocol issues and the SDK's handling/trade-offs for each. ([.agent/plans/2026-07-14-aauth-events/spec-review.mdR1-R254](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-c166e5f2af478b2ce60e18a3168666cc204b45dcc0623b4be9698b1682829d4bR1-R254))

Build, packaging, and workflow updates:

  • Updated the Makefile to add build targets, environment variables, and cache directory for the new Event Agent sample and library. ([[1]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R19), [[2]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R47), [[3]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R76))
  • Updated the GitHub Actions workflow to pack and publish the new AAuth.Events NuGet package. ([.github/workflows/publish.ymlR42](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-551d1fcf87f78cc3bc18a7b332a4dc5d8773a512062df881c5aba28a6f5c48d7R42))

Documentation and instructions:

  • Updated the spec update instructions to recognize, download, and track the standalone AAuth Events draft when present upstream, including handling for diffing, changelog, and snapshot auditing. ([[1]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783R45), [[2]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783L78-R91), [[3]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783R112-R114), [[4]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783L113-R128), [[5]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783L123-R138), [[6]](https://github.com/aauth-dev/dotnet-samples/pull/45/files#diff-ce28dc3ee215ff1d160cc2cfd473d09f1a38a5fe41be8c344b939ff5bc599783R160))

These changes ensure that the repository can track, implement, and test the evolving AAuth Events protocol alongside the main protocol and R3 drafts.

nedruk and others added 30 commits July 14, 2026 05:52
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 40fd3f5f-2525-470a-bad7-831cdaa6b7dc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 68b1ef25-c3f0-45c8-91da-3b652bf6572c
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: adc66cc6-0fd2-48f9-8987-22f9fc42aa67
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
nedruk and others added 24 commits July 15, 2026 09:37
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5937a85a-0316-4f90-9cde-190f63c22678
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 755c3573-12f3-4738-ac53-6a3bbc8d976d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 55c9a261-ada2-44d5-86d9-c8f71e31a848
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 55c9a261-ada2-44d5-86d9-c8f71e31a848
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 55c9a261-ada2-44d5-86d9-c8f71e31a848
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 55c9a261-ada2-44d5-86d9-c8f71e31a848
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 55c9a261-ada2-44d5-86d9-c8f71e31a848
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: bde8514e-5223-4a07-ad13-a444c8167f33
@dasiths

dasiths commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

@nedruk can we remove the samples changes from this PR. Let's focus on getting the events library in and then introduce the right samples for it once the draft settles around it.

image

dasiths added 2 commits July 29, 2026 12:20
…aft dates

- set draft-08 to 2026-06-24 and draft-09 to 2026-07-04 in both tracking files
- note upstream kramdown frontmatter is stale at 2026-06-17
- fix duplicated conjunction and stale per-revision example URL

📅 - Generated by Copilot
…net-Draft

Upstream leaves the kramdown `date =` frontmatter stale, which produced wrong
document dates in SPEC-VERSION.md and CHANGELOG.md. Adds an extraction snippet
and a carve-out for drafts not yet on the Datatracker.

📅 - Generated by Copilot

@dasiths dasiths left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: AAuth.Events package

Reviewed src/AAuth.Events/ (~4,900 LOC), the plan/research documents, and the vendored draft-09 Events spec. Samples are excluded per our discussion — they are being reverted so this branch ships only the AAuth.Events NuGet package.

What's solid

Worth stating, because the security scaffolding is largely right:

  • Check ordering is correct — cheap decode → URL policy → metadata → JWKS → JWT signature → HTTP signature → body digest. Nothing hits the network before cheap rejects.
  • Defence in depth on outbound URLsEventsJwtKeyResolver calls EnsureUrlAsync before both the metadata and JWKS fetches, independent of which HttpClient is injected.
  • FixedTimeEquals for digest comparison; bounded body read that checks the limit before writing; raw bytes preserved with no parse/reserialise.
  • Covered-component sequence compared exactly and order-sensitively.
  • Verified independently: no src/AAuth/ or src/AAuth.R3/ production file changed, and no bin//obj/ artifacts are committed. Both plan claims hold.
  • Spec line citations in research.md are accurate — I spot-checked 16 of them against the vendored draft and every one landed. That is unusually rigorous and made this review much faster.

Findings

Sev Issue Location
HIGH SSRF: DNS hostnames bypass the private-address check Discovery/DefaultEventsUrlPolicy.cs
HIGH Mandatory invented jti rejects spec-conformant peers Tokens/EventTokenClaims.cs
MEDIUM Caller-supplied handler silently re-enables redirects Discovery/EventsHttpClientFactory.cs
MEDIUM Loopback allowed by default Discovery/DefaultEventsUrlPolicy.cs
MEDIUM ReadAndVerifyAsync succeeds without verifying Http/EventsRequestBody.cs
MEDIUM Sync-over-async on the verification path Http/EventsHttpMessageVerifier.cs
MEDIUM Error bodies are draft-08 shaped, not RFC 9457 Resource/SubscriptionEndpointExtensions.cs
MEDIUM Error vocabulary leaks C# enum names, two casings Resource/SubscriptionEndpointExtensions.cs
MEDIUM AP endpoint emits no error body at all AgentProvider/EventEndpointExtensions.cs
MEDIUM C5 makes example-only covered components mandatory see below
LOW Incomplete IP ranges, Unicode base64 check, ASCII mangling, O(n) dedup inline
LOW Stale status line, broken link, missing upstream artifact research.md

Details are in the inline comments. Two items that do not anchor to a single line:

C5/D5 — example-only components made mandatory

research.md correctly notes the extra covered components "appear in examples, not normative prose", and D5 then requires them on verify. Same interop-strictness class as jti: a peer that signs only the four base components with a body is rejected.

Unlike jti there is a genuine security argument here — without content-digest the delivery body is unbound, which is precisely the RF2/RF3 problem. So I am not asking for it to be reversed, but I would like it ratified explicitly rather than inherited.

More useful: neither document answers the consolidated question — what does this SDK reject that the spec permits? That list currently has at least two entries (jti, covered components) and lives nowhere. Please add it to the package README so integrators hit it before a failed interop test does.

Error-format cluster

Findings 7–9 in the table are one coherent fix and the cheapest to land: pick RFC 9457 problem details, apply to both endpoints, and define a snake_case error vocabulary in AAuthEventsConstants.

Note on scope

The draft-08 core is not the cause of these. jti and the covered-component strictness both derive from the v09 Events draft itself. Separately, I confirmed signature-key 05→06 is purely additive (self-jwt), so Events' use of scheme=jwt is unaffected by core's lag — and event_endpoint / aa-subscribe+jwt / aa-event+jwt are all correctly composed inside the package rather than depending on core metadata. That part of the design was right.

Could you work through these? Happy to discuss any of the rulings, particularly jti — that one needs an owner decision rather than an implementer default.

!(uri.Scheme == Uri.UriSchemeHttp && uri.IsLoopback)))
return false;

if (IPAddress.TryParse(uri.Host.Trim('[', ']'), out var address) &&

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — SSRF: DNS hostnames bypass the private-address check entirely.

This guard only inspects the host when it parses as an IP literal. A hostname such as https://events.attacker.com/ that resolves to 169.254.169.254 (cloud metadata) or an RFC 1918 address passes unchecked. An attacker controls a hostname trivially; they would never use a literal.

Compounding it, the default trustCallback is null, and IsAllowedAsync treats null as allow — so the out-of-the-box posture permits this. There is also no IP pinning, so even a literal-checked host is rebindable between check and connect.

This conflicts with HTTP Signature Keys draft-06 §6.3 (normatively referenced by protocol draft-09), which requires rejecting private/loopback/link-local addresses and defending against DNS rebinding via IP pinning.

I realise C15/D11 deliberately scoped this to literals, so it is a design gap rather than a coding slip — but please reconsider the scope. Suggested fix: resolve the host, validate every resolved address, and connect to the pinned IP; or make the trust callback mandatory rather than optional.

Please address before this ships.

$"Outbound URL is not trusted: {uri}");
}

private static bool IsPrivateOrLinkLocal(IPAddress address)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — loopback is allowed by default; LOW — literal ranges are incomplete.

  1. The caller excludes loopback before reaching here, so 127.0.0.0/8 and ::1 are always permitted. A hostile AP can publish event_endpoint: https://127.0.0.1:<port>/ and induce the resource to POST attacker-shaped JSON at services co-located on the resource host. Reasonable for samples, wrong default for a shipped library — please make loopback opt-in (e.g. AllowLoopback = false by default).

  2. Missing ranges: 100.64.0.0/10 (CGNAT), 192.0.0.0/24, 198.18.0.0/15 (benchmarking), IPv4 multicast/broadcast 224.0.0.0/4 and 255.255.255.255, and IPv6 multicast ff00::/8.

HttpMessageHandler? innerHandler = null)
{
policy ??= new DefaultEventsUrlPolicy();
innerHandler ??= new HttpClientHandler { AllowAutoRedirect = false };

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — a caller-supplied handler silently re-enables redirects.

innerHandler ??= applies the hardening only when no handler is passed. Any caller supplying their own handler (common for proxy config or tests) gets AllowAutoRedirect = true, and redirects are then followed inside HttpClientHandler — so EventsUrlPolicyHandler never sees the redirect target and the URL policy is not applied to it.

D11 states redirects are disabled unconditionally. Suggested fix: if a caller supplies an HttpClientHandler, set AllowAutoRedirect = false on it (or reject a handler that has redirects enabled) rather than trusting the default.

{
var bytes = await ReadAsync(request, maxBytes, cancellationToken).ConfigureAwait(false);
var digest = GetSha256Digest(request);
if (digest is null)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — ReadAndVerifyAsync returns success without verifying anything when the digest header is absent.

When Content-Digest is missing this returns HasContentDigest: false and succeeds. EventsHttpMessageVerifier compensates with a separate Contains("Content-Digest") check afterwards, so the package's own path is safe — but this is public API whose name promises verification. An external consumer who calls it and ignores the flag gets unverified bytes and no error, on a security primitive.

Suggested fix: throw when the digest is absent, or rename to TryReadAndVerifyAsync and make the contract explicit in the XML docs.

throw InvalidDigest("sha-256 must be an RFC 9530 byte sequence.");
var encodedBytes = encoded[1..^1];
foreach (var c in encodedBytes)
if (!(char.IsLetterOrDigit(c) || c is '+' or '/' or '='))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — char.IsLetterOrDigit is Unicode-aware, so this is not a base64 charset check.

It accepts non-ASCII letters and digits (Arabic-Indic digits, accented letters, etc.). Not exploitable — Convert.FromBase64String rejects them a few lines later — but the validation does not do what it appears to. Please use an explicit ASCII base64 test (c is >= 'A' and <= 'Z' or >= 'a' and <= 'z' or >= '0' and <= '9' or '+' or '/' or '=').

var issuer = EventsClaimValidation.RequiredString(payload, AAuthEventsConstants.IssuerClaim);
var audience = EventsClaimValidation.RequiredString(payload, AAuthEventsConstants.AudienceClaim);
var eid = EventsClaimValidation.RequiredString(payload, AAuthEventsConstants.EventIdClaim);
var jti = EventsClaimValidation.RequiredString(payload, AAuthEventsConstants.TokenIdClaim);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — this requires a claim that does not exist in the specification, so conformant peers are rejected.

jti appears nowhere in draft-hardt-aauth-events.md — I grepped the whole vendored document. Requiring it here (and on the AP inbox path) means any spec-conformant third-party resource is rejected by this SDK.

This sits badly against two of the project's own stated rules:

  • the plan's headline principle, "Spec conformance is paramount";
  • research.md's role table, which sets the SDK boundary as "no invented wire fields".

The collision C23 defends against is real but narrow: two legitimate events on one subscription within the same second, identical iat/exp, deterministic EdDSA signature. Cheaper options that preserve interop were not taken:

  1. Emit jti, but tolerate its absence on verify — fall back to compact-token-hash dedup (i.e. today's C14 behaviour, with its narrow window). Keeps the benefit for our own resources, costs no interop.
  2. Key AP idempotency on token-hash + AP-local receipt sequence — needs no wire change at all.

I can see this was raised in internal review and consciously accepted with a README warning, which is good process. But it is the single decision that makes this SDK fail against a correct implementation, so it warrants an explicit owner ruling rather than an implementer default. Recommendation: keep emitting jti, relax verification to accept its absence.

}
catch (EventsVerificationException ex)
{
return Results.Json(new { error = ex.Error.Code.ToString(), error_description = ex.Error.Detail },

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — error bodies use the draft-08 shape, not draft-09 RFC 9457 problem details.

error_description is exactly what protocol draft-09 replaced with the RFC 9457 detail member, and the content type should be application/problem+json rather than plain JSON. This package was written against the v09 Events draft but ships v08-era error bodies (matching core's RevocationEndpoint, which has not migrated yet).

Worth fixing now rather than at migration time: when core moves to draft-09, error_description here is greppable and will be caught — but see my comment on EventEndpointExtensions.cs, where the absence of any body leaves nothing to grep, so that half would silently stay non-conformant.

Suggested fix: application/problem+json with { "error": ..., "detail": ... }.

statusCode: StatusCodes.Status400BadRequest);
return Results.Json(new { event_types = result.SelectedEventTypes }, statusCode: StatusCodes.Status200OK);
}
return Results.Json(new { error = result.Status.ToString().ToLowerInvariant(), error_description = result.Detail },

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — the wire error vocabulary is an accident of C# naming, and is internally inconsistent.

  • Lines 104 and 121 emit error = ex.Error.Code.ToString() → PascalCase enum identifiers on the wire: MalformedRequest, InvalidToken, UnknownKey.
  • This line emits .ToString().ToLowerInvariant()notfound, conflict.

Neither is snake_case, which is the convention for every AAuth error code in the protocol and in core (invalid_request, untrusted_revoker, mission_terminated). Two different casings within one file.

Consequence: renaming an internal enum member is a silent breaking wire change. AAuthEventsConstants is the designated home for wire names per the plan, but currently defines no error constants at all.

Suggested fix: declare an explicit snake_case error-code vocabulary in AAuthEventsConstants and map to it, rather than serialising enum names.

cancellationToken).ConfigureAwait(false);
}

private static Task WriteStatusAsync(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — the AP event endpoint returns no error body at all, unlike the registration endpoint.

Every failure path here writes a bare status code and returns. So within a single package there are two conventions: the registration endpoint returns { error, error_description } JSON, and this endpoint returns nothing. A client receives a bare 403 with no machine-readable reason.

This is also the half that will silently survive the draft-09 migration — there is no error_description string to grep for, so a migration sweep will not flag it.

Suggested fix: emit RFC 9457 problem details (application/problem+json with a required error member) consistently from both endpoints.

[src/AAuth.R3](../../../src/AAuth.R3) and
[.agent/plans/2026-07-02-r3-rich-resource-requests](../2026-07-02-r3-rich-resource-requests)
- Updated: 2026-07-15 (review follow-up)
- Status: ready for owner review; no implementation or commit has been made

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — documentation hygiene, three items.

  1. This status line is stale and misleading. It reads "ready for owner review; no implementation or commit has been made", but the work is complete, committed and pushed. Anyone reading top-down is misled immediately.

  2. Broken cross-reference. The > **Update (2026-07-15)** callout below attributes RF1–RF5 to spec-inconsistency-review.md; the actual file is spec-review.md. The link justifying five decisions is dead.

  3. Referenced artifact is not in the repo. implementation-log.md states the upstream jti proposal is "saved as files/aauth-events-jti-upstream-proposal.md in the session folder" — it exists in no branch. The one artifact needed to action the upstream fix is unretrievable; please commit it alongside the plan.

- consolidate issue #44 findings and prior migration rulings
- record draft deltas, pre-existing gaps, and contract debt
- include independent spec-grounded verification ledger

🔎 - Generated by Copilot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants