Skip to content

feat: CipherBox issues the identity token, and wallet login is a first login - #1273

Merged
FSM1 merged 17 commits into
mainfrom
claude/cipher-box-1253-r8ebt6-2
Aug 12, 2026
Merged

feat: CipherBox issues the identity token, and wallet login is a first login#1273
FSM1 merged 17 commits into
mainfrom
claude/cipher-box-1253-r8ebt6-2

Conversation

@FSM1

@FSM1 FSM1 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Implements ADR 0008 D1 and D2.

Closes #1256. Closes #1257. Closes #1258. Closes #1260. Part of #1253.

Supersedes #1272, which is closed unmerged — its two commits are carried here. There is no staging environment yet, so landing an intermediate fix to a code path this PR deletes bought nothing.

Why these ship together

The Core Kit derives its TSS key from the (verifier, verifierId) pair. Moving from Web3Auth's hosted verifier to a CipherBox custom verifier changes the derived key. If Google moved to the CipherBox verifier while email stayed on the email_passwordless sub-verifier, one person would land on two different accounts depending on which button they pressed. All three methods have to cross in one commit, so they do.

Identities created under the old Web3Auth-hosted verifier are not reachable after this change. That is fine because no accounts exist yet; there is deliberately no migration, dual-verifier fallback, or compatibility shim.

What changes

The API issues the identity token (#1257). A JWKS endpoint plus an RS256 mint. Each verified method produces a CipherBox JWT, and the Core Kit redeems it through loginWithJWT against CipherBox's own JWKS instead of loginWithOAuth.

The API owns passwordless email again (#1258). It issues the code, verifies it, and owns delivery behind a provider seam. The code is now collected in the app rather than in a provider window.

Wallet is a first-class first login, web only (#1260). WalletLoginButton no longer routes to the engine's Command::SiweLogin, which was refused with NotStarted and surfaced to the member as "engine not started". A SIWE signature now mints the same token as any other method and lands on the same Core Kit login and secret handoff, so a member with no prior session reaches a working vault.

The two client IDs stop being conflated (#1256). subVerifierDetails.clientId is the provider's client ID, but the app was passing the Web3Auth project client ID, so every Google sign-in ended at 401 invalid_client before Web3Auth was involved. VITE_GOOGLE_CLIENT_ID is declared, joins LOGIN_ENV and so the deploy gate, and loginEnv returns it — which the Google credential collection here consumes via main.tsxIdentityProviderLoginPage. The per-method sub-verifier client-ID map from that fix is deleted by this one: loginWithJWT takes no subVerifierDetails.

The subject mapping

A verified provider identity maps to a stable subject id through a new identity_subjects table. That id is the JWT sub and the verifierId passed to loginWithJWT.

It carries no user_id, deliberately. The account still materializes at POST /auth/login keyed by the derived publicKey, exactly as before — this table's only job is to yield a stable verifierId, so it cannot fork the account model. Keeping user_id out is also what leaves linking open later: linking a second method becomes pointing another provider identity at an existing subject id, authorized by an existing session the way siwe/link already works. No linking flow is built here, only left unforeclosed.

This is deliberately not v1's shape. v1 created a users row per provider identity with a placeholder publicKey (pending-core-kit-<id>) and used the user id as verifierId; that breaks the invariant that an account IS the derived key, and leaves junk accounts behind from abandoned login attempts.

Resolution is insert-then-read against the unique (kind, identifier_hash) index rather than an unguarded check-then-insert, so concurrent first logins for one identity yield one subject. Identifiers are stored SHA-256 hashed, following the auth_methods convention. No identity endpoint creates a users row.

Methods do not cross-link: signing in with Google and with email as the same person yields two accounts. That matches v1's documented behavior and is intentional.

Notes for review

  • /auth/siwe/login and /auth/siwe/link are untouched — the engine's Rust client and the contract suite call them. Under D2 siwe/login becomes largely redundant, since a wallet can now reach its vault as a first login rather than only authenticating an already-linked one. Worth deciding separately whether it survives; removing it here would be a contract break outside this PR's scope.
  • The identity endpoints are not in the Rust contract suite. The exchange happens before a login secret exists, so the TypeScript host calls them directly — the ADR explicitly rejects an unstarted engine command surface. They are covered by apps/api/src/auth/identity.http.itest.ts against real Postgres instead.
  • The JWKS caching hazard from v1 is handled. The signing key is required in every deployed profile, allowlisted exactly as buildJwtOptions allowlists the access-token secret — v1 only required it in production, so staging hit it. The public JWK is derived from the public key rather than by stripping private fields off the private JWK, so no private field can reach the JWKS by omission.
  • Google's audience check is enforced in every profile. An unconfigured dev profile gets a placeholder audience that no real token can match, so it fails closed rather than warn-and-skipping as v1 did.
  • The token carries no email claim. The verifierId is an opaque uuid, so Torus cannot link a subject to an address. The cost is that a page reload restores the Core Kit session but not the display email, so UserMenu shows [an0n] until the next explicit login. The login method survives a reload, read off the token's own method claim. Reversible by adding an email claim if the display matters more than the privacy — a deliberate trade, not an oversight.
  • New deployment configuration, documented in apps/api/.env.example: IDENTITY_JWT_PRIVATE_KEY (base64 PKCS8 PEM — multiline PEM does not survive a .env), GOOGLE_CLIENT_ID (the provider's, not the Web3Auth project's), MAIL_PROVIDER plus SENDGRID_API_KEY and MAIL_FROM_ADDRESS. The API refuses to boot in a deployed profile without the signing key or a mail provider. The Web3Auth dashboard also needs a custom verifier pointed at /auth/.well-known/jwks.json with sub as the verifier ID field.
  • blueprint/api.md gains the identity_subjects row in its data-model list, since that file is normative.

Verification

Ran green: pnpm --filter @cipherbox/api test, pnpm --filter @cipherbox/web test (tsc -b first), pnpm lint, pnpm lint:md, pnpm lint:tracker-refs, pnpm --filter @cipherbox/api typecheck, and openapi:generate (committed).

test:integration could not run locally — no Docker or Postgres in this environment, so all integration files fail on ECONNREFUSED 127.0.0.1:5432, pre-existing ones included. CI runs them.

Tests assert behavior, not source text. Among them: a token signed by a different key is refused; an expired code is refused, as is one CipherBox never issued; a code is single-use and attempt-capped; the JWKS serves no d/p/q/dp/dq/qi; a wallet signature mints a sub stable across repeat logins; the same identity logging in concurrently yields one subject row; identity endpoints leave users empty; and boot fails in a deployed profile without the signing key, without a mail provider, and without a Google client ID.

Needs human verification

No dev server or configured verifier exists in the build environment, so Puppeteer verification was not attempted. Against a real Web3Auth custom verifier and a real Google client:

  1. Google, email and wallet each sign in from a cold session and reach the same vault for the same provider identity.
  2. The same wallet reaches the same account on a second login, and a second wallet reaches a different one.
  3. A real code arrives by email from CipherBox and signs in; an expired one is refused.
  4. The Core Kit accepts the token against the verifier's JWKS fetch — the one step no local test can exercise.
  5. An API restart with IDENTITY_JWT_PRIVATE_KEY set does not break the next login, which is the v1 failure this guards against.

Note

Issue CipherBox identity tokens from the API and treat wallet login as a first-login flow

  • Adds IdentityController with endpoints for Google ID token exchange, email OTP send/verify, SIWE wallet signature, and a JWKS discovery route; all return short-lived RS256 identity tokens signed by IdentityTokenService.
  • Introduces EmailOtpService (6-digit uniform codes, single-use, attempt cap, per-address send rate limit) and GoogleOAuthService (jose-backed RS256 ID token verification) as the credential verification layer.
  • Adds IdentitySubjectService to resolve or create a stable identity_subjects row per (kind, identifierHash), with concurrency-safe insert-on-conflict; backed by a new DB migration.
  • Replaces the Web3Auth OAuth login path with loginWithJWT (verifier + verifierId + idToken); wallet login now performs a full identity-exchange round-trip via the API instead of the previous engine-facilitated SIWE path.
  • Splits the EmailLoginForm into a two-step send-then-verify flow; adds GoogleLoginButton backed by Google Identity Services (GIS), loaded once per document.
  • Risk: useAuth now requires an IdentityProvider ancestor with an exchange set, and loginWithGoogle now requires a Google ID token argument — both are breaking interface changes for callers.

Macroscope summarized 164f1c6.

Summary by CodeRabbit

  • New Features
    • Added Google, email verification-code, and wallet authentication options.
    • Added secure, single-use six-digit email codes with expiration, rate limits, and delivery support.
    • Added identity-token exchange and consistent session support across authentication methods.
    • Added Google Identity Services login integration with clearer loading and unavailable states.
  • Documentation
    • Updated setup guides and environment templates with Google OAuth, email delivery, and identity authentication requirements.
  • Tests
    • Expanded coverage for authentication flows, validation, security checks, error handling, and end-to-end scenarios.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

This change adds API-issued identity tokens and Google, email, and wallet authentication exchanges. The web client passes returned credentials to Core Kit JWT login. It also adds identity persistence, OTP delivery, configuration, OpenAPI definitions, UI flows, tests, and CI setup.

Changes

Identity authentication

Layer / File(s) Summary
API identity contracts and token foundation
apps/api/src/auth/dto/*, apps/api/src/auth/entities/*, apps/api/src/auth/services/identity-token.service.ts, apps/api/src/migrations/*
Adds identity DTOs, stable provider subjects, RSA JWT signing, JWKS output, and the identity-subject database table.
Provider verification and email delivery
apps/api/src/auth/services/google-oauth.service.ts, apps/api/src/auth/services/email-otp.service.ts, apps/api/src/auth/services/mail.provider.ts
Adds Google token verification, passwordless email codes, rate limits, replay protection, and logging or SendGrid delivery.
API identity exchange surface
apps/api/src/auth/identity.controller.ts, apps/api/src/auth/services/identity-exchange.service.ts, apps/api/openapi.json, apps/api/src/auth/auth.module.ts
Exposes JWKS, Google, email, and wallet endpoints and maps successful exchanges to identity-token responses.
Web identity exchange and Core Kit integration
apps/web/src/auth/identityExchange.ts, apps/web/src/auth/useAuth.ts, apps/web/src/auth/coreKit.ts, apps/web/src/auth/IdentityProvider.tsx
Exchanges provider credentials with the API, then redeems identity credentials through Core Kit JWT login.
Web login provider flows
apps/web/src/components/auth/*, apps/web/src/routes/LoginPage.tsx, apps/web/src/styles/login.css
Adds Google Identity Services, two-step email verification, and wallet first-login wiring with updated loading and error states.
Configuration, fixtures, and workflow support
.github/workflows/*, apps/web/.env.example, apps/web/src/test/*, README.md, docs/CONFIGURATION.md
Updates environment requirements, test fakes, documentation, and CI credentials for identity authentication.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LoginPage
  participant IdentityExchange
  participant IdentityController
  participant IdentityExchangeService
  participant IdentityTokenService
  participant CoreKit

  LoginPage->>IdentityExchange: submit Google, email, or wallet credential
  IdentityExchange->>IdentityController: POST identity exchange
  IdentityController->>IdentityExchangeService: verify credential and resolve subject
  IdentityExchangeService->>IdentityTokenService: sign identity claims
  IdentityTokenService-->>IdentityController: identity token and expiry
  IdentityController-->>IdentityExchange: identity grant
  IdentityExchange->>CoreKit: loginWithJWT(identity token)
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy the coding objectives for separated client IDs, API-issued JWTs, email OTP, and wallet first login [#1256] [#1257] [#1258] [#1260].
Out of Scope Changes check ✅ Passed The configuration, documentation, tests, migrations, API, and web changes directly support the linked identity-authentication objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes two primary changes: CipherBox-issued identity tokens and wallet authentication as a first-login flow.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/cipher-box-1253-r8ebt6-2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FSM1
FSM1 changed the base branch from claude/cipher-box-1253-r8ebt6 to main August 11, 2026 23:02
@FSM1
FSM1 marked this pull request as ready for review August 12, 2026 00:42

FSM1 commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

@FSM1 I will review pull request #1273.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

FSM1 commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

@FSM1 I will review pull request #1273.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@FSM1

FSM1 commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review please

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

@FSM1 I will perform a full review of pull request #1273.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/src/engine/config.test.ts (1)

100-123: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep Google configuration optional for email and wallet login.

Lines 100-123 and Lines 190-205 make VITE_GOOGLE_CLIENT_ID a deployment and loginEnv() prerequisite. createCoreKitSession() calls loginEnv() before method selection, so a missing Google client ID prevents Core Kit initialization and makes email and wallet login unavailable. This contradicts Lines 213-218, which define a missing Google client ID as an unavailable Google method only.

Remove VITE_GOOGLE_CLIENT_ID from the global Core Kit and deployment requirements. Keep it as a Google-button-specific configuration value. Add coverage that email and wallet login remain available without it.

Also applies to: 190-205, 208-218

🤖 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 `@apps/web/src/engine/config.test.ts` around lines 100 - 123, Remove
VITE_GOOGLE_CLIENT_ID from the global deployment validation and loginEnv()
prerequisite while retaining it for Google-specific method availability. Update
createCoreKitSession() so missing Google configuration does not block Core Kit
initialization or email and wallet login, and adjust the related tests to verify
those methods remain available without the Google client ID.
🧹 Nitpick comments (11)
apps/api/src/auth/dto/identity.dto.test.ts (1)

17-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: cover the forbidNonWhitelisted path.

The pipe is configured with whitelist: true and forbidNonWhitelisted: true, but no case sends an unexpected property. Add one so a later DTO change that drops a decorator cannot silently start accepting extra fields.

♻️ Proposed additional case
   it('applies the same trim on the verify request', async () => {
+  it('refuses a property the DTO does not declare', async () => {
+    await expect(
+      pipe.transform({ email: 'member@example.com', role: 'admin' }, body(EmailCodeRequestDto))
+    ).rejects.toBeInstanceOf(BadRequestException);
+  });
+
🤖 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 `@apps/api/src/auth/dto/identity.dto.test.ts` around lines 17 - 45, Add a test
in the EmailCodeRequestDto or EmailCodeVerifyRequestDto suite that passes an
unexpected property alongside valid fields to pipe.transform and asserts it
rejects with BadRequestException, covering the configured forbidNonWhitelisted
behavior.
apps/api/src/auth/services/email-otp.service.ts (1)

49-59: 🧹 Nitpick | 🔵 Trivial

Confirm the API runs as a single instance.

tracked holds issued codes, send budgets, and attempt budgets in process memory. With more than one API replica behind a load balancer, POST /auth/identity/email/verify-code can reach a replica that never issued the code, and the member receives 401. The per-address send cap is also enforced per replica, so the effective limit becomes 5 × replica count. ChallengeService shares the same constraint, so this may already be an accepted deployment property. If horizontal scaling is planned, move this state to a shared store, or pin these routes with sticky sessions.

🤖 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 `@apps/api/src/auth/services/email-otp.service.ts` around lines 49 - 59,
Confirm and document that the API deployment is single-instance for
EmailOtpService and the related ChallengeService in-memory state. If horizontal
scaling is required, replace the per-process tracked state with a shared store;
otherwise configure sticky sessions for the affected authentication routes and
preserve the existing code, send-budget, and attempt-budget behavior.
apps/api/src/auth/services/email-otp.service.test.ts (1)

78-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the attempt-cap boundary in this test.

Every rejection path in EmailOtpService.verify throws UnauthorizedException, so this test cannot tell "budget spent" from "incorrect code". verify decrements attemptsLeft and voids the code only when the value drops below zero, so after the five wrong guesses in this loop the code is still live and the sixth call is the one that reports "Too many attempts". Assert the message to fix that boundary, and add a case that a correct code still works after a smaller run of wrong guesses.

As per path instructions for **/*.{test,itest}.ts: "Focus on test coverage, edge cases, and test quality. Ensure tests are meaningful and not just for coverage metrics."

♻️ Proposed test tightening
     for (let attempt = 0; attempt < 5; attempt += 1) {
-      expect(() => service.verify(EMAIL, wrong)).toThrow(UnauthorizedException);
+      expect(() => service.verify(EMAIL, wrong)).toThrow(/Incorrect verification code/);
     }
     // The budget is spent, so even the right code no longer opens it.
-    expect(() => service.verify(EMAIL, code)).toThrow(UnauthorizedException);
+    expect(() => service.verify(EMAIL, code)).toThrow(/Too many attempts/);
+  });
+
+  it('keeps the code usable while attempts remain', async () => {
+    await service.send(EMAIL);
+    const code = lastCode();
+    const wrong = code === '000000' ? '111111' : '000000';
+
+    expect(() => service.verify(EMAIL, wrong)).toThrow(/Incorrect verification code/);
+    expect(() => service.verify(EMAIL, code)).not.toThrow();
   });
🤖 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 `@apps/api/src/auth/services/email-otp.service.test.ts` around lines 78 - 88,
Update the “voids the code after a run of wrong guesses” test to assert the
UnauthorizedException message, covering the boundary where the sixth
verification attempt—not the fifth—reports “Too many attempts.” Add a separate
case proving the correct code succeeds after fewer wrong guesses, using the
existing EmailOtp service test helpers.

Source: Path instructions

apps/api/src/auth/identity.http.itest.ts (1)

160-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise kid-based key selection in the JWKS test.

jose.importJWK takes the first key and an explicit algorithm, so the verification here never uses the kid in the token header. Web3Auth selects the key from the JWKS by kid. A mismatch between the header kid written by IdentityTokenService.sign and the kid published in the JWKS would break real verification while this suite stays green. Verify through createLocalJWKSet instead, and assert the token lifetime, so both halves of the D1 contract are covered.

♻️ Proposed verification through the JWK set
   it('verifies a minted token, and refuses one signed by anything else', async () => {
     const jwks = await request(http()).get('/auth/.well-known/jwks.json').expect(200);
-    const key = await jose.importJWK(jwks.body.keys[0], 'RS256');
+    const key = jose.createLocalJWKSet(jwks.body);
 
     const grant = await emailGrant(freshEmail());
     const { payload } = await jose.jwtVerify(grant.body.token, key, {
       issuer: 'cipherbox',
       audience: 'web3auth',
     });
     expect(payload.sub).toBe(grant.body.verifierId);
     expect(payload.method).toBe('email');
+    expect(payload.exp! - payload.iat!).toBe(300);
+    expect(new Date(grant.body.expiresAt).getTime()).toBe(payload.exp! * 1000);
🤖 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 `@apps/api/src/auth/identity.http.itest.ts` around lines 160 - 197, Update the
JWKS verification test around the minted token to use jose.createLocalJWKSet
with the complete response body instead of importing the first key via
jose.importJWK, allowing verification to select by the token header kid.
Preserve the issuer, audience, and payload assertions, and add an assertion that
the token expiration reflects the required lifetime.
apps/api/src/auth/auth.module.test.ts (1)

52-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the environment explicit and assert the new identity providers.

ignoreEnvFile: true leaves NODE_ENV to the ambient process. Two providers in this graph read it and fail closed: IdentityTokenService.onModuleInit throws without IDENTITY_JWT_PRIVATE_KEY, and buildMailProvider throws without MAIL_PROVIDER. Any runner that does not set NODE_ENV to development or test turns this suite red for an environmental reason instead of a graph defect. Supply the value through load so the test states its own preconditions.

The assertion also names only GoogleOAuthService. Fetch the providers this PR adds, so a later constructor change is attributed to the right service.

♻️ Proposed test hardening
     const builder = Test.createTestingModule({
       imports: [
-        ConfigModule.forRoot({ isGlobal: true, ignoreEnvFile: true }),
+        ConfigModule.forRoot({
+          isGlobal: true,
+          ignoreEnvFile: true,
+          load: [() => ({ NODE_ENV: 'test' })],
+        }),
         RuntimeModule,
         AuthModule,
       ],
     });
@@
     try {
       expect(moduleRef.get(GoogleOAuthService)).toBeInstanceOf(GoogleOAuthService);
+      expect(moduleRef.get(EmailOtpService)).toBeInstanceOf(EmailOtpService);
+      expect(moduleRef.get(IdentityExchangeService)).toBeInstanceOf(IdentityExchangeService);
+      expect(moduleRef.get(IdentityTokenService)).toBeInstanceOf(IdentityTokenService);
+      expect(moduleRef.get(IdentityController)).toBeInstanceOf(IdentityController);
     } finally {

Add the matching imports for EmailOtpService, IdentityExchangeService, IdentityTokenService, and IdentityController.

🤖 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 `@apps/api/src/auth/auth.module.test.ts` around lines 52 - 73, Make the
AuthModule dependency-graph test self-contained by configuring
ConfigModule.forRoot with an explicit test/development NODE_ENV and the required
identity JWT and mail-provider values through load, preventing ambient
environment dependence. Update the imports and assertions to retrieve
EmailOtpService, IdentityExchangeService, IdentityTokenService, and
IdentityController alongside GoogleOAuthService so each added
provider/controller is instantiated and attributed independently.
apps/api/src/auth/services/mail.provider.test.ts (1)

53-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Restore the fetch stub in afterEach.

Both tests call vi.unstubAllGlobals() as the last statement of the test body. If an assertion fails or the awaited call rejects, that line never runs and the stubbed fetch leaks into the following tests in this worker. Move the restore into an afterEach hook so it runs on failure too.

♻️ Proposed cleanup
 describe('SendGridMailProvider', () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
   it('addresses the message to the recipient and reports the code', async () => {
     const fetchMock = vi.fn().mockResolvedValue(new Response(null, { status: 202 }));
     vi.stubGlobal('fetch', fetchMock);
@@
     expect((init.headers as Record<string, string>).authorization).toBe('Bearer sg-key');
-
-    vi.unstubAllGlobals();
   });
 
   it('treats a refused send as a failure rather than reporting success', async () => {
     vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('detail', { status: 429 })));
@@
     ).rejects.toThrow(/status 429/);
-
-    vi.unstubAllGlobals();
   });
 });

Add afterEach to the vitest import.

🤖 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 `@apps/api/src/auth/services/mail.provider.test.ts` around lines 53 - 83, Move
global cleanup for the fetch stubs from the individual tests into an afterEach
hook, adding afterEach to the existing Vitest imports. Remove the
vi.unstubAllGlobals() calls from the tests so cleanup runs even when assertions
or awaited operations fail.
apps/api/src/auth/services/identity-token.service.ts (1)

8-9: 🧹 Nitpick | 🔵 Trivial

Plan for signing-key rotation.

KID is a fixed constant and the JWKS exposes exactly one key. A future rotation of IDENTITY_JWT_PRIVATE_KEY replaces the only published key under the same kid. Torus caches the JWKS per URL, which is the exact failure mode the comment on Line 50 describes. Consider deriving kid from a thumbprint of the public key and returning the previous key alongside the new one during an overlap window, so verification stays valid while caches refresh.

🤖 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 `@apps/api/src/auth/services/identity-token.service.ts` around lines 8 - 9,
Update the identity-token signing and JWKS key publication around KID and
ALGORITHM to derive each key ID from its public-key thumbprint instead of using
the fixed KID, and expose the previous public key alongside the current key
during a rotation overlap window. Preserve RS256 signing and ensure both
published keys remain verifiable while JWKS caches refresh.
apps/web/src/engine/config.ts (1)

126-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Read the Google variable through the named constant.

Line 116 uses env[GOOGLE_CLIENT_ID_ENV], and Line 126 repeats the literal VITE_GOOGLE_CLIENT_ID. The comment on Line 30 states the constant exists so the name cannot drift. Use the constant in both places. If you apply the fix above, this line goes away.

🤖 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 `@apps/web/src/engine/config.ts` at line 126, Update the Google client ID
lookup near configured in config.ts to use the existing GOOGLE_CLIENT_ID_ENV
constant instead of the literal VITE_GOOGLE_CLIENT_ID, and ensure both Google
variable reads consistently use that named constant.
apps/web/src/auth/useAuth.ts (1)

41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Confirm the string signature against the binary-data guideline.

loginWithWallet now takes signature: string instead of Uint8Array. The coding guidelines state: "Represent binary data with Vec<u8> in Rust and Uint8Array in TypeScript, not strings."

A 0x-prefixed hex signature is the practical JSON wire form, and wagmi returns that shape, so the change may be intentional. If it is, keep the hex form at the transport edge only, and document on this line that the value is a 0x-prefixed EIP-191 hex signature so callers cannot pass an arbitrary string. Otherwise accept Uint8Array here and encode inside identityExchange.

As per coding guidelines: "Represent binary data with Vec<u8> in Rust and Uint8Array in TypeScript, not strings."

🤖 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 `@apps/web/src/auth/useAuth.ts` at line 41, Clarify the `loginWithWallet`
signature contract: if the JSON transport requires wagmi’s hex representation,
retain `signature: string` but document it as a `0x`-prefixed EIP-191 hex
signature and ensure only that form reaches `identityExchange`; otherwise change
the API to accept `Uint8Array` and perform hex encoding within
`identityExchange`.

Source: Coding guidelines

apps/web/src/components/auth/EmailLoginForm.test.tsx (1)

52-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a case for a refused verification.

The suite covers a refused send at Lines 52-59. It does not cover a refused onVerify. EmailLoginForm swallows that rejection at Line 37 of apps/web/src/components/auth/EmailLoginForm.tsx, so the member must stay on the code step and be able to submit again. Add a case that rejects onVerify and then asserts that the code input is still present and that a second submit calls onVerify again.

💚 Proposed test to add
// A refused code must leave the member on the code step to try again.
it('stays on the code step when the verification is refused', async () => {
  const { onVerify } = renderForm({
    onVerify: () => Promise.reject(new Error('incorrect verification code')),
  });

  typeAddress('member@example.test');
  const code = await screen.findByTestId('email-code-input');
  fireEvent.change(code, { target: { value: '111111' } });
  fireEvent.click(screen.getByTestId('email-verify-button'));

  await waitFor(() => expect(onVerify).toHaveBeenCalledTimes(1));
  expect(screen.getByTestId('email-code-input')).toBeDefined();

  fireEvent.click(screen.getByTestId('email-verify-button'));
  await waitFor(() => expect(onVerify).toHaveBeenCalledTimes(2));
});
🤖 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 `@apps/web/src/components/auth/EmailLoginForm.test.tsx` around lines 52 - 59,
Add a test alongside the existing send-refusal case that configures renderForm’s
onVerify to reject, submits a valid-looking code, and verifies the code-step
input remains present after the rejection. Then submit again and assert the same
onVerify mock is called a second time, covering EmailLoginForm’s
refused-verification retry behavior.
apps/web/src/components/auth/GoogleLoginButton.tsx (1)

86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update deliver.current in a layout effect. A discarded render can otherwise expose an uncommitted callback to GIS. Do not replace it with useEffectEvent; GIS invokes this callback outside an Effect.

🤖 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 `@apps/web/src/components/auth/GoogleLoginButton.tsx` around lines 86 - 88,
Update the deliver ref assignment in GoogleLoginButton so deliver.current is
synchronized with onCredential inside a layout effect, preventing discarded
renders from exposing uncommitted callbacks while preserving GIS’s external
callback invocation.
🤖 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 `@apps/api/src/auth/identity.http.itest.ts`:
- Around line 53-58: Update the CI workflow job that runs `pnpm --filter
`@cipherbox/api` test:integration` against PostgreSQL so it is exposed as a
required status check rather than being marked optional. Preserve the existing
integration test command and database setup while removing the configuration
that allows this job to be non-blocking.

In `@apps/web/src/auth/coreKit.ts`:
- Around line 92-94: Update the session restore flow associated with CoreKit’s
email() and method() so signed-in email metadata is restored from the SDK user
information rather than only being assigned by login(). Preserve the email()
contract after reloads and add a regression test covering restored sessions
returning the user’s email.

In `@apps/web/src/components/auth/EmailLoginForm.tsx`:
- Around line 29-36: Update the successful onSendCode branch in the sentTo
transition to move focus to the newly rendered code input after
setSentTo(trimmed) completes. Reuse the code field’s existing ref or focus
mechanism, ensuring focus occurs only after a successful send and preserves the
current failure behavior.

In `@apps/web/src/components/auth/GoogleLoginButton.tsx`:
- Around line 90-116: Keep the Google target div rendered throughout the busy
state in the component around the target ref, and display the busy indicator as
an overlay instead of unmounting the target. Preserve the existing target
element so the clientId-dependent effect can retain the initialized button after
busy returns false. Add a test in GoogleLoginButton.test.tsx that rerenders busy
true then false and verifies the target remains present and initialized.
- Line 23: Update the staging CSP in the Caddyfile to allow the Google Identity
Services script origin used by GIS_SRC, including any required Google
authentication origins, while preserving the existing CSP directives.

In `@apps/web/src/engine/config.ts`:
- Around line 119-131: The loginEnv requirement incorrectly makes Google OAuth
configuration mandatory for all Core Kit sessions. In
apps/web/src/engine/config.ts:119-131, remove googleClientId from loginEnv’s
return type, requirement check, and missing-variable error while retaining it in
LOGIN_ENV; update apps/web/src/auth/coreKit.ts:170-170 to destructure only
web3AuthClientId and verifier; update apps/web/src/engine/config.test.ts:196 and
201 so missing or blank Google client IDs no longer cause loginEnv to throw.

In `@apps/web/src/test/authFakes.tsx`:
- Around line 95-106: Update fakeCoreKitSession’s method and email state to
start as null, then assign both from the credential inside login(). Preserve
options.email as an explicit override in the email accessor, while ensuring
wallet credentials return their own metadata after login instead of the default
Google method or test email.

In `@blueprint/api.md`:
- Around line 63-69: The complete data-model list must include the
identity_subjects table described near the auth data-model section. Update the
list around the existing completeness claim to add identity_subjects, preserving
the statement that no other tables exist.

In `@docs/CONFIGURATION.md`:
- Line 88: Update the VITE_ENVIRONMENT entry in the configuration documentation
to include ci among the accepted values, indicating that it is CI-only if
appropriate while preserving the existing local, staging, and production
descriptions.

---

Outside diff comments:
In `@apps/web/src/engine/config.test.ts`:
- Around line 100-123: Remove VITE_GOOGLE_CLIENT_ID from the global deployment
validation and loginEnv() prerequisite while retaining it for Google-specific
method availability. Update createCoreKitSession() so missing Google
configuration does not block Core Kit initialization or email and wallet login,
and adjust the related tests to verify those methods remain available without
the Google client ID.

---

Nitpick comments:
In `@apps/api/src/auth/auth.module.test.ts`:
- Around line 52-73: Make the AuthModule dependency-graph test self-contained by
configuring ConfigModule.forRoot with an explicit test/development NODE_ENV and
the required identity JWT and mail-provider values through load, preventing
ambient environment dependence. Update the imports and assertions to retrieve
EmailOtpService, IdentityExchangeService, IdentityTokenService, and
IdentityController alongside GoogleOAuthService so each added
provider/controller is instantiated and attributed independently.

In `@apps/api/src/auth/dto/identity.dto.test.ts`:
- Around line 17-45: Add a test in the EmailCodeRequestDto or
EmailCodeVerifyRequestDto suite that passes an unexpected property alongside
valid fields to pipe.transform and asserts it rejects with BadRequestException,
covering the configured forbidNonWhitelisted behavior.

In `@apps/api/src/auth/identity.http.itest.ts`:
- Around line 160-197: Update the JWKS verification test around the minted token
to use jose.createLocalJWKSet with the complete response body instead of
importing the first key via jose.importJWK, allowing verification to select by
the token header kid. Preserve the issuer, audience, and payload assertions, and
add an assertion that the token expiration reflects the required lifetime.

In `@apps/api/src/auth/services/email-otp.service.test.ts`:
- Around line 78-88: Update the “voids the code after a run of wrong guesses”
test to assert the UnauthorizedException message, covering the boundary where
the sixth verification attempt—not the fifth—reports “Too many attempts.” Add a
separate case proving the correct code succeeds after fewer wrong guesses, using
the existing EmailOtp service test helpers.

In `@apps/api/src/auth/services/email-otp.service.ts`:
- Around line 49-59: Confirm and document that the API deployment is
single-instance for EmailOtpService and the related ChallengeService in-memory
state. If horizontal scaling is required, replace the per-process tracked state
with a shared store; otherwise configure sticky sessions for the affected
authentication routes and preserve the existing code, send-budget, and
attempt-budget behavior.

In `@apps/api/src/auth/services/identity-token.service.ts`:
- Around line 8-9: Update the identity-token signing and JWKS key publication
around KID and ALGORITHM to derive each key ID from its public-key thumbprint
instead of using the fixed KID, and expose the previous public key alongside the
current key during a rotation overlap window. Preserve RS256 signing and ensure
both published keys remain verifiable while JWKS caches refresh.

In `@apps/api/src/auth/services/mail.provider.test.ts`:
- Around line 53-83: Move global cleanup for the fetch stubs from the individual
tests into an afterEach hook, adding afterEach to the existing Vitest imports.
Remove the vi.unstubAllGlobals() calls from the tests so cleanup runs even when
assertions or awaited operations fail.

In `@apps/web/src/auth/useAuth.ts`:
- Line 41: Clarify the `loginWithWallet` signature contract: if the JSON
transport requires wagmi’s hex representation, retain `signature: string` but
document it as a `0x`-prefixed EIP-191 hex signature and ensure only that form
reaches `identityExchange`; otherwise change the API to accept `Uint8Array` and
perform hex encoding within `identityExchange`.

In `@apps/web/src/components/auth/EmailLoginForm.test.tsx`:
- Around line 52-59: Add a test alongside the existing send-refusal case that
configures renderForm’s onVerify to reject, submits a valid-looking code, and
verifies the code-step input remains present after the rejection. Then submit
again and assert the same onVerify mock is called a second time, covering
EmailLoginForm’s refused-verification retry behavior.

In `@apps/web/src/components/auth/GoogleLoginButton.tsx`:
- Around line 86-88: Update the deliver ref assignment in GoogleLoginButton so
deliver.current is synchronized with onCredential inside a layout effect,
preventing discarded renders from exposing uncommitted callbacks while
preserving GIS’s external callback invocation.

In `@apps/web/src/engine/config.ts`:
- Line 126: Update the Google client ID lookup near configured in config.ts to
use the existing GOOGLE_CLIENT_ID_ENV constant instead of the literal
VITE_GOOGLE_CLIENT_ID, and ensure both Google variable reads consistently use
that named constant.
🪄 Autofix

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 Plus

Run ID: f1af8b6e-5204-46a4-b80a-e70f3571df12

📥 Commits

Reviewing files that changed from the base of the PR and between c2e4e9b and f8f1658.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
📒 Files selected for processing (50)
  • .github/workflows/ci.yml
  • .github/workflows/web-e2e.yml
  • README.md
  • apps/api/.env.example
  • apps/api/openapi.json
  • apps/api/package.json
  • apps/api/scripts/generate-openapi.ts
  • apps/api/src/auth/auth.module.test.ts
  • apps/api/src/auth/auth.module.ts
  • apps/api/src/auth/dto/identity.dto.test.ts
  • apps/api/src/auth/dto/identity.dto.ts
  • apps/api/src/auth/entities/identity-subject.entity.ts
  • apps/api/src/auth/identity.controller.ts
  • apps/api/src/auth/identity.http.itest.ts
  • apps/api/src/auth/services/email-otp.service.test.ts
  • apps/api/src/auth/services/email-otp.service.ts
  • apps/api/src/auth/services/google-oauth.service.test.ts
  • apps/api/src/auth/services/google-oauth.service.ts
  • apps/api/src/auth/services/identity-exchange.service.ts
  • apps/api/src/auth/services/identity-subject.service.test.ts
  • apps/api/src/auth/services/identity-subject.service.ts
  • apps/api/src/auth/services/identity-token.service.test.ts
  • apps/api/src/auth/services/identity-token.service.ts
  • apps/api/src/auth/services/mail.provider.test.ts
  • apps/api/src/auth/services/mail.provider.ts
  • apps/api/src/migrations/1784800000000-AddIdentitySubjects.ts
  • apps/api/src/testing/integration-db.ts
  • apps/web/.env.example
  • apps/web/src/auth/CoreKitProvider.test.tsx
  • apps/web/src/auth/IdentityProvider.tsx
  • apps/web/src/auth/coreKit.test.ts
  • apps/web/src/auth/coreKit.ts
  • apps/web/src/auth/identityExchange.test.ts
  • apps/web/src/auth/identityExchange.ts
  • apps/web/src/auth/useAuth.test.tsx
  • apps/web/src/auth/useAuth.ts
  • apps/web/src/components/auth/EmailLoginForm.test.tsx
  • apps/web/src/components/auth/EmailLoginForm.tsx
  • apps/web/src/components/auth/GoogleLoginButton.test.tsx
  • apps/web/src/components/auth/GoogleLoginButton.tsx
  • apps/web/src/components/auth/WalletLoginButton.tsx
  • apps/web/src/engine/config.test.ts
  • apps/web/src/engine/config.ts
  • apps/web/src/main.tsx
  • apps/web/src/routes/LoginPage.tsx
  • apps/web/src/styles/login.css
  • apps/web/src/test/authFakes.tsx
  • apps/web/src/vite-env.d.ts
  • blueprint/api.md
  • docs/CONFIGURATION.md

Comment on lines +53 to +58
describe('identity exchange HTTP flows (real Postgres)', () => {
let db: IntegrationDatabase;
let ctx: HttpIntegrationApp;
let mail: CapturingMailProvider;
let googleSigningKey: jose.CryptoKey;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm an integration script exists and a named CI job runs it with Postgres.
fd -t f 'package.json' -d 3 --exec jq -r '{name, scripts}' {}
rg -n -C6 'itest|integration' .github/workflows

Repository: FSM1/cipher-box

Length of output: 5240


🏁 Script executed:

#!/bin/bash
sed -n '374,426p' .github/workflows/ci.yml
printf '\n--- API integration config and suite references ---\n'
rg -n -C4 'vitest.integration|\\.itest\\.ts|DATABASE_URL|POSTGRES|postgres' apps/api package.json .github/workflows/ci.yml

Repository: FSM1/cipher-box

Length of output: 11600


Make API Integration (real Postgres) a required CI check.

The job runs pnpm --filter @cipherbox/api test:integration with PostgreSQL, but the workflow explicitly marks it as not required. Configure branch protection so this suite blocks merges.

🤖 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 `@apps/api/src/auth/identity.http.itest.ts` around lines 53 - 58, Update the CI
workflow job that runs `pnpm --filter `@cipherbox/api` test:integration` against
PostgreSQL so it is exposed as a required status check rather than being marked
optional. Preserve the existing integration test command and database setup
while removing the configuration that allows this job to be non-blocking.

Source: Coding guidelines

Comment on lines 92 to 94
email(): string | null {
return this.coreKit.getUserInfo().email ?? null;
return this.signedInEmail;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where the stored session email is rendered.
rg -nP -C3 '\bemail\b' apps/web/src/stores --type=ts
rg -nP -C3 'signedIn\(' apps/web/src --type=ts --type=tsx

Repository: FSM1/cipher-box

Length of output: 6377


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- coreKit symbols and implementation ---'
rg -n -C6 'signedInEmail|email\(\)|method\(\)|restore\(|handOff\(' apps/web/src/auth/coreKit.ts apps/web/src --glob '*.ts' --glob '*.tsx' | head -n 260

printf '%s\n' '--- candidate auth files ---'
fd -t f -i 'auth|corekit' apps/web/src | head -n 120

printf '%s\n' '--- relevant tests ---'
rg -n -C5 'restore|handOff|email\(\)|method\(\)|signedInEmail|session\.email|session\.method' apps/web/src --glob '*.test.ts' --glob '*.test.tsx' | head -n 320

Repository: FSM1/cipher-box

Length of output: 40584


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- coreKit implementation ---'
sed -n '1,220p' apps/web/src/auth/coreKit.ts

printf '%s\n' '--- auth handOff and restore callers ---'
rg -n -C8 'handOff|restore\(|session\.email|session\.method|signedIn\(' apps/web/src --glob '*.ts' --glob '*.tsx'

printf '%s\n' '--- related tests ---'
rg -n -C8 'restore|handOff|email\(\)|method\(\)|signedInEmail|sessionId' apps/web/src --glob '*.test.ts' --glob '*.test.tsx'

Repository: FSM1/cipher-box

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- identity credential and token claims ---'
sed -n '1,240p' apps/web/src/auth/identityExchange.ts

printf '%s\n' '--- restore and handoff tests ---'
rg -n -C12 'restor|hand.?off|email|method|isAuthenticated' apps/web/src/auth/useAuth.test.tsx apps/web/src/auth/CoreKitProvider.test.tsx apps/web/src/test/authFakes.tsx

printf '%s\n' '--- SDK user-info email references and project docs ---'
rg -n -i -C4 'getUserInfo|userInfo|email.*claim|claim.*email|email' apps/web blueprint apps/web/src/auth --glob '*.md' --glob '*.ts' --glob '*.tsx' | head -n 260

Repository: FSM1/cipher-box

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- exact identity exchange definitions ---'
rg -n -C12 'IdentityCredential|IdentityMethod|fromGoogle|fromEmail|email:' apps/web/src/auth/identityExchange.ts

printf '%s\n' '--- exact restore handoff tests ---'
rg -n -C15 'restore|handOff|email|method|isAuthenticated' apps/web/src/auth/useAuth.test.tsx apps/web/src/auth/CoreKitProvider.test.tsx apps/web/src/test/authFakes.tsx

printf '%s\n' '--- SDK user-info usage ---'
rg -n -i -C5 'getUserInfo|userInfo|email' apps/web/src/auth blueprint --glob '*.ts' --glob '*.tsx' --glob '*.md' | head -n 240

Repository: FSM1/cipher-box

Length of output: 50371


Preserve email metadata on session restore. signedInEmail is set only by login(), so reloads make email() return null while method() still restores from SDK user info. Use a durable non-storage source, or document that restored sessions have no email and update the auth state contract and UI. Add a restore regression test.

🤖 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 `@apps/web/src/auth/coreKit.ts` around lines 92 - 94, Update the session
restore flow associated with CoreKit’s email() and method() so signed-in email
metadata is restored from the SDK user information rather than only being
assigned by login(). Preserve the email() contract after reloads and add a
regression test covering restored sessions returning the user’s email.

Comment thread apps/web/src/components/auth/EmailLoginForm.tsx
Comment thread apps/web/src/components/auth/GoogleLoginButton.tsx
Comment thread apps/web/src/components/auth/GoogleLoginButton.tsx
Comment thread apps/web/src/engine/config.ts Outdated
Comment thread apps/web/src/test/authFakes.tsx Outdated
Comment thread blueprint/api.md
Comment thread docs/CONFIGURATION.md Outdated
@FSM1
FSM1 marked this pull request as draft August 12, 2026 11:31
claude added 13 commits August 12, 2026 11:50
`subVerifierDetails.clientId` is the provider's client ID, not the Web3Auth
project's, so every Google sign-in sent Google an identifier it has never
issued and got back `401 invalid_client`.

`loginEnv` now reads `VITE_GOOGLE_CLIENT_ID` alongside the Web3Auth pair and
returns them under names that cannot be swapped, and the session routes each
sub-verifier to its own registration: the Google connection takes the Google
Cloud OAuth client ID, the Torus-hosted email one the Web3Auth project's.

The variable was already set at every staging deploy site and read nowhere.
Adding it to `LOGIN_ENV` puts it behind the same build gate as the rest, so a
missing one names itself instead of surfacing a provider error.
Implements ADR 0008 D1 and D2 on the server. Each verified method now mints
a CipherBox JWT and the Core Kit logs in against a CipherBox custom verifier
over the API's own JWKS, rather than delegating to Web3Auth's hosted OAuth.

- JWKS endpoint plus an RS256 identity-token mint. The signing key is
  required in every deployed profile, allowlisted exactly as the access-token
  secret is: Torus caches the JWKS per URL, so a keypair regenerated on
  restart makes every later login fail verification behind a message that
  names neither the key nor the restart. The public JWK is derived from the
  public key rather than by stripping fields off the private one, so no
  private field can reach the JWKS by omission.
- Passwordless email returns to CipherBox: it issues the code, verifies it,
  and owns delivery through a configured provider seam. A deployed profile
  with no provider refuses to boot rather than failing at the first send.
  Codes are held in memory, hashed and salted, single-use, attempt-capped and
  expiry-checked, for the reason ChallengeService already documents.
- Wallet becomes a first-class first login: a SIWE signature mints the same
  token as any other method, so it reaches the same derived key.
- Google ID tokens are verified against Google's JWKS with a required
  audience, using the OAuth provider's client ID rather than the Web3Auth
  project's. The audience is enforced in every profile, not warn-and-skipped.

A verified provider identity maps to a stable subject id through a dedicated
identity_subjects table carrying no user_id: the account still materializes
at POST /auth/login against the derived key, so this cannot fork the account
model, and linking a method later is pointing a second provider identity at
an existing subject. Insert-then-read on the unique index, so concurrent
first logins for one identity yield one subject. No identity endpoint creates
a users row. /auth/siwe/login and /auth/siwe/link are untouched.
Implements the client half of ADR 0008 D1 and D2. The Core Kit now redeems a
CipherBox-issued token through loginWithJWT instead of driving Web3Auth's
hosted OAuth, so a method that authenticates against CipherBox first can
produce a login secret.

- The identity exchange is spoken over plain fetch against the API, not
  through the engine: it runs before a login secret exists, and the engine
  refuses every command until start. It sits behind a seam the login flow
  reads, so useAuth still touches no browser API and imports only react.
- Wallet login stops dead-ending. It no longer routes to the engine's SIWE
  login, which was refused with NotStarted and surfaced to the member as
  "engine not started"; it lands on the same mint, the same Core Kit login
  and the same secret handoff as Google, so a member with no prior session
  reaches a working vault. Web only.
- Google credential collection loads Google Identity Services and hands the
  API the ID token. The button Google renders is the one path that always
  presents, so the affordance cannot be clicked into nothing.
- The verification code is collected in the app now that CipherBox issues it,
  rather than in a provider window. The form advances only on a send that
  actually happened.
- The Torus-hosted email_passwordless sub-verifier is gone, and with it the
  client ID it was carried under.

The session tracks how it was established off the token's own method claim,
which the SDK parses into its user info and keeps across a restore, since an
opaque subject id carries no method the way a typeOfLogin did.
IdentitySubjectService carried its own copy of the SHA-256 identifier hash
that IdentityService already provides, so the two could drift apart while
keying the same rows. It now injects IdentityService and calls that one.

EmailOtpService.verify returns the normalized address it consumed, so the
exchange stops re-normalizing what the verifier already canonicalized, and
normalizeEmail is no longer exported.

The insert takes `.returning('id')`: on Postgres an ignored conflict returns
no row, so the winner answers from the insert and only a loser pays for the
re-read. Unit-tested on both paths, since the concurrency proof against real
Postgres only runs in CI.
…elper

The loader cast globalThis and re-checked the installed shape in two places,
once before injecting the script and once on load. Both now go through
installedGis, which returns the SDK or null.
`GoogleOAuthService` took its fake-JWKS seam as a TypeScript-optional second
constructor parameter. Nest reads every position from `design:paramtypes`,
where an interface type emits no injectable token, so booting the real module
graph died in `InstanceLoader` with UnknownDependenciesException and the API
never listened — taking down every check that boots it.

`@Optional()` restores the existing `keys ?? createRemoteJWKSet(...)` fallback
without changing the production default or removing the seam.

Every suite handed this provider in ready-made, so none of them compiled the
module Nest actually boots; `AuthModule` is now compiled and initialized in the
unit suite, which covers the other identity providers and both controllers too.
`EmailOtpService` keys an address by its trimmed, lowercased form, but the
validation pipe ran first and `@IsEmail` rejected any spelling carrying
surrounding space — so a member who typed a trailing space got a 400 naming
nothing they could act on, for an address the service would have accepted.

The transform is null-safe and leaves a non-string payload for `@IsEmail` to
refuse. The existing unit tests call the service directly and so never crossed
the pipe; the new ones drive it.
`EmailOtpService` caps sends per address over a 15-minute window and holds that
budget in memory for the app's whole life, while the suite truncates only the
database between tests. Seven tests sharing one literal address spent one
five-send budget between them, so the last two answered 429.

Each test now mints its own address, which keeps the cap under test intact
rather than configuring it away: the bucket is keyed by an address the test
chooses freely, unlike the per-IP auth bucket THROTTLE_AUTH_LIMIT exists for,
where a whole suite is forced to share one key.
main.tsx called loginEnv at module scope, so a build carrying no login
variables threw before the first render and served a blank page. A new
googleClientId reads that one variable as optional, and the Google button
presents the method as unavailable — naming the missing variable — instead
of offering an affordance it cannot serve.
The web e2e job builds a bundle that offers every login method the front
door asserts, so it gets a placeholder Google client ID. The contract job's
production-mode API refuses to boot without a mail provider, so it gets a
placeholder one; neither value is ever redeemed.
A deployed profile fails closed on the identity signing key and the Google
client ID as well as the mail provider, so the contract job's production
instance needs all three to reach a listening state. The signing key is
generated in the step rather than committed.
`loginEnv` required VITE_GOOGLE_CLIENT_ID and `createCoreKitSession` calls
it, so a build without the Google ID refused email and wallet logins too —
the same class of regression as the boot fix, one layer down. The variable
now gates only the method it configures; the deployed-build gate still
requires it.

Alongside, the Google button no longer unmounts its GIS target during a busy
cycle, which left the member with no button after a failed sign-in until a
reload, and the staging CSP now admits the GIS script origin so Google
sign-in can load at all.

- keep the GIS target mounted and overlay the busy state
- install the credential callback on commit rather than during render
- focus the code field once an email code is on its way
- report method and email from the redeemed credential in the auth fake
- verify identity tokens through the JWKS key set, exercising kid selection
- assert OTP refusal messages, the module's full provider graph, and
  forbidNonWhitelisted; unstub fetch in afterEach
@FSM1
FSM1 force-pushed the claude/cipher-box-1253-r8ebt6-2 branch from f8f1658 to b9202ae Compare August 12, 2026 11:54

FSM1 commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

All 21 review items dispositioned. Head is now b9202ae, rebased onto a7cf06f.

Fixed (15)

Every finding verified against the code before acting; three were genuine user-facing defects.

Majors

  • A missing VITE_GOOGLE_CLIENT_ID disabled every login method, not only Google. Correct, and the sharpest catch here. SESSION_ENV (the two Web3Auth values) is now split from LOGIN_ENV, so loginEnv no longer requires or returns the Google client ID while DEPLOY_ENV and the build gate stay exactly as they were. coreKit.ts needed no change — it already destructured only the two values it uses, so the throw was the entire bug.
  • The Google button did not survive a busy cycle. Correct, though the stated mechanism was slightly off in a way that matters for the test: React reuses the same div node across the ternary, so node identity survives and a toBe(target) assertion passes against the buggy code. What is actually lost is the node's children — GIS's injected markup — which the [clientId]-only effect never re-renders. The test now injects real button markup and asserts that survives, and it fails against the old code.
  • The staging CSP blocked Google Identity Services. Correct. script-src now allows https://accounts.google.com/gsi/client. frame-src was deliberately left undeclared: with no default-src, GIS's iframe already loads, and adding the directive would be a narrowing that risks breaking WalletConnect.

Others: identity_subjects added to the complete data-model list; ci documented for VITE_ENVIRONMENT (and two neighbouring rows corrected that the first fix falsified); test-fake credential fidelity; focus moved to the code field after a successful send; JWKS verification through createLocalJWKSet; afterEach stub restore; explicit NODE_ENV and broader provider assertions in the module-graph test; attempt-cap boundary messages; the named-constant read; useLayoutEffect for the GIS callback ref; a refused-verification case; and forbidNonWhitelisted coverage.

The JWKS one is worth calling out: the suite verified with importJWK(keys[0]) and an explicit algorithm, so it never exercised kid selection. Signing with a kid the JWKS does not publish passed 17/17 before and fails with JWKSNoMatchingKey now — a real gap between the tests and how Web3Auth actually verifies.

Not changed (6), with reasons

  • Email on session restore — deliberate and already in the PR body. The token carries no email claim precisely so Torus cannot link a subject to an address; the cost is [an0n] in UserMenu until the next explicit login. The login method does survive a reload, off the token's own claim.
  • In-memory OTP state — matches the existing ChallengeService precedent for the SIWE nonce store. Changing one and not the other would be worse than keeping both consistent; they should move together if they move.
  • kid rotation — real, but this PR introduces the signing key rather than managing its lifecycle. Worth its own issue. Verification now goes through createLocalJWKSet rather than an index-picked key, which is the precondition for testing any rotation scheme later.
  • Making API Integration (real Postgres) a required check — branch-protection configuration, not something this diff can set.
  • signature: string — the 0x-prefixed hex is the wire form wagmi returns and the API consumes verbatim; converting to Uint8Array here would decode and re-encode a value neither end treats as bytes. Documented on the declaration instead.
  • Docstring coverage 55.32% — CodeRabbit's default threshold, not a gate this repo sets.

On the inconclusive linked-issues check

The check asks for evidence that desktop hides wallet login. There is no desktop frontend to hide it in — apps/desktop is a Tauri skeleton with no TypeScript sources and no login surface at all. Wallet being web-only is currently enforced by that absence; desktop's own login surface, and the evidence for it, belong to the issue that builds it.

Verification

Web 327/327, API 227/227, integration 153/153 against real Postgres 16, typecheck, all three lint gates, openapi:generate with no diff — all re-run after the rebase. The first fix was also proved in a browser: with VITE_GOOGLE_CLIENT_ID unset and the Web3Auth values set, the page renders with email and wallet available and Google shown unavailable, zero page errors; a staging build without it still refuses, so the deploy gate is intact.

One pre-existing item noticed while fixing the data-model list: it also omits device_approvals, which the section above names. Left alone as outside this diff.


Generated by Claude Code

@FSM1
FSM1 marked this pull request as ready for review August 12, 2026 14:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@apps/web/src/components/auth/EmailLoginForm.test.tsx`:
- Around line 52-59: Update both tests in
apps/web/src/components/auth/EmailLoginForm.test.tsx (lines 52-59 and 73-90) to
use deferred promises for the send and verify callbacks, reject each promise
inside act after the callback begins, and await settlement before asserting.
Verify the send-rejection case keeps the email input visible; verify the
verify-rejection case keeps the code input visible and permits entering another
code.
🪄 Autofix

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 Plus

Run ID: 516c9d57-f3ce-43d6-afad-457004ba988e

📥 Commits

Reviewing files that changed from the base of the PR and between f8f1658 and b9202ae.

📒 Files selected for processing (19)
  • apps/api/src/auth/auth.module.test.ts
  • apps/api/src/auth/dto/identity.dto.test.ts
  • apps/api/src/auth/identity.http.itest.ts
  • apps/api/src/auth/services/email-otp.service.test.ts
  • apps/api/src/auth/services/mail.provider.test.ts
  • apps/web/src/auth/coreKit.test.ts
  • apps/web/src/auth/useAuth.test.tsx
  • apps/web/src/auth/useAuth.ts
  • apps/web/src/components/auth/EmailLoginForm.test.tsx
  • apps/web/src/components/auth/EmailLoginForm.tsx
  • apps/web/src/components/auth/GoogleLoginButton.test.tsx
  • apps/web/src/components/auth/GoogleLoginButton.tsx
  • apps/web/src/engine/config.test.ts
  • apps/web/src/engine/config.ts
  • apps/web/src/styles/login.css
  • apps/web/src/test/authFakes.tsx
  • blueprint/api.md
  • docker/Caddyfile
  • docs/CONFIGURATION.md
🚧 Files skipped from review as they are similar to previous changes (12)
  • docs/CONFIGURATION.md
  • blueprint/api.md
  • apps/api/src/auth/auth.module.test.ts
  • apps/api/src/auth/services/mail.provider.test.ts
  • apps/api/src/auth/services/email-otp.service.test.ts
  • apps/web/src/engine/config.test.ts
  • apps/web/src/styles/login.css
  • apps/web/src/auth/useAuth.test.tsx
  • apps/api/src/auth/identity.http.itest.ts
  • apps/web/src/components/auth/GoogleLoginButton.tsx
  • apps/web/src/auth/coreKit.test.ts
  • apps/web/src/test/authFakes.tsx

Comment thread apps/web/src/components/auth/EmailLoginForm.test.tsx
The step assertions ran after waitFor happened to drain the rejection, not
because the test made it settle. A deferred promise refused inside act ties
the assertion to the settlement it is about.
FSM1 and others added 3 commits August 12, 2026 18:10
The API refuses to boot in a deployed profile without a signing key, a
Google client ID, or a mail provider, but the generated .env.staging
carried neither MAIL_PROVIDER nor the SendGrid pair. Adds the three keys
and a preflight that fails the job when one is unset, since the deploy
has no health gate to catch a crash-looping container.

Entire-Checkpoint: 83a07f519199
The retirement row predates ADR 0008 D2, which gives the API a mail
surface again. SENDGRID_FROM_EMAIL still retires; MAIL_FROM_ADDRESS
replaces it.

Entire-Checkpoint: 168093cce9ef
The Identity and auth section names the table, so the list that claims to be
complete contradicted it.
@FSM1
FSM1 merged commit 69a8a72 into main Aug 12, 2026
36 checks passed
@FSM1
FSM1 deleted the claude/cipher-box-1253-r8ebt6-2 branch August 12, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants