feat(identity): verify-email session + PKCE callback to provisioning#155
Merged
Conversation
…provisioning Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe email verification endpoint now establishes a short-lived cookie session and returns typed success data; frontend PKCE callback consumption of the provisioning token is delayed until after successful code exchange; OpenAPI/TS types, test helpers, and tests are updated accordingly. ChangesEmail Verification with Cookie Session and Provisioning Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ments - callback: consume the post-verify provisioning token only after the code exchange succeeds, so an invalid callback no longer discards a valid token; redirect to /provisioning via location.assign so this slice compiles and degrades gracefully without the journey slice's typed route. - restore the explanatory comments in SignOut (token revoke, JTI blacklist, cookie clearing) that were inadvertently dropped. - fix import ordering flagged by Biome in api.ts and callback.lazy.tsx. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
main gained the OpenAPI-generated FE types (#165), legal/slug fields, and the typed /provisioning route after this branch forked. Resolution: - types.ts/api.ts: keep generated types + main's resend/provisioning helpers alongside this branch's verify-email session + PKCE helpers. - callback.lazy.tsx: adopt main's typed navigate to /provisioning, and consume the post-verify token only after a successful code exchange (drop the early consume so a failed callback keeps a valid token). - AuthHelper.cs: use main's TestRegistrationPayload (carries required legal versions). - identity-access README: record the verify-email session + post-verify PKCE→provisioning behavior in the implementation status. Verified: dotnet build Axis.sln 0/0, frontend npm run ci + 34 tests, doc-drift green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Axis.Api/Endpoints/AuthEndpoints.cs`:
- Around line 35-38: The endpoint in AuthEndpoints.cs currently uses
.Produces<object>() and returns an anonymous new { sessionEstablished = true },
which yields an opaque OpenAPI schema; define a concrete response DTO named
VerifyEmailSessionEstablishedDto in the Application layer (per repo guidelines),
change the endpoint signature to return/serialize that DTO and update the
OpenAPI annotation to .Produces<VerifyEmailSessionEstablishedDto>(), and replace
any anonymous response creation with new VerifyEmailSessionEstablishedDto {
SessionEstablished = true } (or equivalent property name) so the contract is
explicit and discoverable.
- Around line 118-129: The endpoint is orchestrating two mediator calls
(VerifyEmailCommand and GetUserTokenClaimsQuery) and calling
SignInPkceSessionAsync itself, which risks partial success if the second call
fails; instead collapse this flow into a single mediator operation that performs
verification, provisioning and returns the token claims (e.g., create a
VerifyEmailAndGetClaimsCommand/Query that returns UserTokenClaimsDto or a Result
wrapper), then replace the two mediator.Send calls and SignInPkceSessionAsync
usage with a single mediator.Send of that new operation and translate its Result
to ProblemDetails; ensure the new handler performs the verification and any DB
commits before fetching claims so the endpoint remains thin (route →
mediator.Send → ToProblemDetails) and use the existing VerifyEmailCommand and
GetUserTokenClaimsQuery logic within that handler or delegate to them
internally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a097e64-8b25-4f71-80b1-4aa109b1b8db
📒 Files selected for processing (10)
docs/use-cases/identity-access/README.mdfrontend/src/features/auth/api.tsfrontend/src/routes/callback.lazy.tsxsrc/Axis.Api/Endpoints/AuthEndpoints.cssrc/Modules/Identity/Axis.Identity.Application/Commands/VerifyEmail/VerifyEmailCommand.cssrc/Modules/Identity/Axis.Identity.Application/Commands/VerifyEmail/VerifyEmailHandler.cssrc/Modules/Identity/Axis.Identity.Application/Commands/VerifyEmail/VerifyEmailSuccessDto.cstests/Api/Axis.Api.Tests/Helpers/AuthHelper.cstests/Api/Axis.Api.Tests/Identity/AuthEndpointTests.cstests/Modules/Identity/Axis.Identity.Application.Tests/Commands/VerifyEmailHandlerTests.cs
Comment on lines
+118
to
+129
| Result<VerifyEmailSuccessDto> result = | ||
| await mediator.Send(new VerifyEmailCommand(request.Token), ct); | ||
| if (result.IsFailure) | ||
| return result.ToProblemDetails(); | ||
|
|
||
| Result<UserTokenClaimsDto> claimsResult = await mediator.Send( | ||
| new GetUserTokenClaimsQuery(result.Value.UserId, result.Value.OrganizationId), | ||
| ct); | ||
| if (claimsResult.IsFailure) | ||
| return claimsResult.ToProblemDetails(); | ||
|
|
||
| await SignInPkceSessionAsync(httpContext, claimsResult.Value); |
There was a problem hiding this comment.
Collapse the verify-email flow into a single application result.
The endpoint now orchestrates VerifyEmailCommand and GetUserTokenClaimsQuery itself. If the first call commits verification/provisioning and the second call fails, the client gets an error even though the one-time link has already been consumed. This also breaks the repo rule that Axis.Api endpoints stay as route → mediator.Send → ToProblemDetails.
As per coding guidelines, "Endpoint surface changes: keep Axis.Api endpoints thin (route → mediator.Send), translate failures to ProblemDetails (don’t embed logic in the endpoint)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Axis.Api/Endpoints/AuthEndpoints.cs` around lines 118 - 129, The endpoint
is orchestrating two mediator calls (VerifyEmailCommand and
GetUserTokenClaimsQuery) and calling SignInPkceSessionAsync itself, which risks
partial success if the second call fails; instead collapse this flow into a
single mediator operation that performs verification, provisioning and returns
the token claims (e.g., create a VerifyEmailAndGetClaimsCommand/Query that
returns UserTokenClaimsDto or a Result wrapper), then replace the two
mediator.Send calls and SignInPkceSessionAsync usage with a single mediator.Send
of that new operation and translate its Result to ProblemDetails; ensure the new
handler performs the verification and any DB commits before fetching claims so
the endpoint remains thin (route → mediator.Send → ToProblemDetails) and use the
existing VerifyEmailCommand and GetUserTokenClaimsQuery logic within that
handler or delegate to them internally.
…x casing Addresses the CodeRabbit review on the verify-email endpoint and a latent FE/BE casing bug: - Response DTO (CodeRabbit #1): replace `.Produces<object>()` + `new { sessionEstablished = true }` with VerifyEmailSessionEstablishedDto in the Application layer (repo rule: response DTOs live in Application). - Single application result (CodeRabbit #2): VerifyEmailHandler now gathers the sign-in permissions (before commit) and returns them on VerifyEmailSuccessDto, so the endpoint is one mediator.Send → sign in → return. Removes the second GetUserTokenClaims round-trip that could fail after the one-time link was already consumed. - Casing: regenerate openapi.json + api-types.ts (session_established), point VerifyEmailResponse at the generated schema, and read data?.session_established in useVerifyEmail — the post-verify PKCE branch was dead before (FE read camelCase off a snake_case wire). Verified: full dotnet test Axis.sln green (0 failed), frontend ci + tests, OpenApiDocumentTests green/idempotent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
phuongnse
added a commit
that referenced
this pull request
Jun 1, 2026
Two judgment-class findings CodeRabbit raised on #155 (response DTO must be an Application type, not object/anonymous; endpoints must not orchestrate multiple mediator.Send calls). Both are semi-deterministic and recurring → logged as candidate architecture fitness tests (not built yet, per the three-question rule). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 1, 2026
This was referenced Jun 5, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After email verification, the API establishes a short-lived session cookie and returns
{ sessionEstablished: true }. The SPA callback checks for a stored provisioning token and redirects to/provisioning?token=after PKCE instead of the dashboard.Linked spec
register-org — verify link activates account and signs user in.
Requirements
POST /api/auth/verify-email→ 200 + session cookie (was 204).completePostVerifyPkceFlow, callback provisioning redirect.Merge independence: Backend verify change is backward-compatible for clients that ignore the body. Callback provisioning redirect is a no-op until slice 1 (journey) stores the token via
completePostVerifyPkceFlow. Recommended merge order: slice 1 → slice 3.Summary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests