rob/PlaceMark
Watch
1
0
Fork
You've already forked PlaceMark
 0
Code Issues Pull requests Projects Releases Packages 3 Wiki Activity Actions

Log why the OIDC callback refused an exchanged code #214

Merged
rob merged 1 commit from log-oidc-exchange-failure into main 2026-09-25 06:25:23 +00:00
Conversation 1 Commits 1 Files changed 2 +54 -2
Claude commented 2026-09-25 06:08:11 +00:00
Collaborator
Copy link

A real sign-in against Authentik got a 200 from the token endpoint and then a 401 from the callback, with nothing in the log saying why. The catch in GetCallback swallowed the OidcExchangeFailedException whose message and inner exception carry the reason. This logs it at Warning under the endpoint's own category before rethrowing the deliberately blank 401. The response is unchanged.

Tested with a new OidcEndpointsTests case that drives the refused-code branch and asserts the record. Watched it fail with the log call removed. Full suite green on SDK 10.0.100.

A real sign-in against Authentik got a 200 from the token endpoint and then a 401 from the callback, with nothing in the log saying why. The catch in `GetCallback` swallowed the `OidcExchangeFailedException` whose message and inner exception carry the reason. This logs it at Warning under the endpoint's own category before rethrowing the deliberately blank 401. The response is unchanged. Tested with a new `OidcEndpointsTests` case that drives the refused-code branch and asserts the record. Watched it fail with the log call removed. Full suite green on SDK 10.0.100.
Log why the OIDC callback refused an exchanged code
All checks were successful
CI / build (pull_request) Successful in 6m24s
Details
CI / container-images (pull_request) Successful in 2s
Details
CI / e2e (pull_request) Successful in 5m7s
Details
844a9a47de 
The callback answers every failure with the same 401 by design (ADR-0032),
but the catch that collapsed an OidcExchangeFailedException into it never
logged the exception, so a real refusal after a successful token exchange
left nothing to diagnose from.
Claude reviewed 2026-09-25 06:10:28 +00:00
Claude left a comment
Copy link

Verdict: mergeable

Nothing to change. The catch still throws OidcFlowInvalidException, so the 401 and its body are unchanged (the existing identical-refusal test covers that). Every OidcExchangeFailedException message in OidcProviderClient is a fixed string naming the step; none carries the code, token, verifier or secret, and ShowPII is not enabled anywhere, so the validator's inner exception stays redacted. The log follows the _logCategory + ILoggerFactory shape in AuthenticationEndpoints. Deleting the two log lines from the catch turns the new test red on ShouldHaveSingleItem.

Verdict: mergeable Nothing to change. The catch still throws `OidcFlowInvalidException`, so the 401 and its body are unchanged (the existing identical-refusal test covers that). Every `OidcExchangeFailedException` message in `OidcProviderClient` is a fixed string naming the step; none carries the code, token, verifier or secret, and `ShowPII` is not enabled anywhere, so the validator's inner exception stays redacted. The log follows the `_logCategory` + `ILoggerFactory` shape in `AuthenticationEndpoints`. Deleting the two log lines from the catch turns the new test red on `ShouldHaveSingleItem`.
rob merged commit 32785eca02 into main 2026-09-25 06:25:23 +00:00
rob deleted branch log-oidc-exchange-failure 2026-09-25 06:25:23 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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
rob/PlaceMark!214
No description provided.