fix: sync email_verified from auth0 on demand#1880
fix: sync email_verified from auth0 on demand#1880ygrishajev merged 10 commits intoakash-network:mainfrom
Conversation
WalkthroughAdds a verify-email feature: new POST /v1/verify-email route, request/response schemas, controller endpoint enforcing a "verify-email" permission, UserService sync with Auth0 via injected ManagementClient, client SDK + web hooks/pages, tests, seeders, and a new AutoButton UI component. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant W as Web: VerifyEmailPage
participant SDK as SDK: AuthHttpService
participant API as API (/v1/verify-email)
participant C as AuthController
participant AB as AbilityService
participant US as UserService
participant A0 as Auth0Service
participant DB as UserRepository
U->>W: open /user/verify-email?email=...
W->>SDK: verifyEmail(email)
SDK->>API: POST /v1/verify-email { data: { email } } (withCredentials)
API->>C: validate request
C->>AB: enforce "verify-email" on User(email)
AB-->>C: allow / deny
alt allowed
C->>US: syncEmailVerified({ email })
US->>A0: getUserByEmail(email)
alt auth0 user found
A0-->>US: user (email_verified)
US->>DB: updateBy({ email }, { emailVerified })
DB-->>US: updated user
US-->>C: { emailVerified }
C-->>API: 200 { data: { emailVerified } }
else not found
A0-->>US: null
US-->>C: throws 404
C-->>API: 404
end
else forbidden
C-->>API: 403
end
note right of A0: ManagementClient provided via singleton DI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
apps/api/src/services/db/userDataService.ts (1)
224-236: Detect no-op updates and log/act on missing user rowCurrently, failures to match a row are silent. Capture the affected row count to avoid masking bugs and aid observability.
-export async function setEmailVerified(userId: string, emailVerified: boolean) { - await UserSetting.update( +export async function setEmailVerified(userId: string, emailVerified: boolean) { + const [affected] = await UserSetting.update( { emailVerified }, { where: { userId } } ); + if (affected === 0) { + logger.warn({ event: "SET_EMAIL_VERIFIED_NO_MATCH", userId }); + } }apps/api/src/auth/services/auth0/auth0.service.ts (1)
1-1: Use type-only import to avoid bundling typesImport the Auth0 user type as a type-only import to keep runtime bundle lean.
-import { GetUsers200ResponseOneOfInner, ManagementClient } from "auth0"; +import type { GetUsers200ResponseOneOfInner } from "auth0"; +import { ManagementClient } from "auth0";apps/api/src/auth/controllers/auth/auth.controller.ts (1)
16-24: Optional: Protect the sync endpointAdd
@Protected()for consistency withsendVerificationEmailand to avoid unnecessary Auth0 calls from unauthenticated hits.- async sendVerificationEmail({ data: { userId } }: SendVerificationEmailRequestInput) { + async sendVerificationEmail({ data: { userId } }: SendVerificationEmailRequestInput) { // unchanged... } + @Protected() async syncEmailVerified({ email }: SyncEmailVerifiedInput) { // unchanged... }apps/api/src/auth/routes/email-verified/email-verified.router.ts (2)
10-12: Tighten email validationValidate email format and normalize.
-const EmailVerifiedQuerySchema = z.object({ - email: z.string() -}); +const EmailVerifiedQuerySchema = z.object({ + email: z.string().email().trim().toLowerCase() +});
25-27: Fix 301/302 mismatchDocs say 301, handler sends 302. Align to 302 to avoid confusion.
- responses: { - 301: { - description: "Redirects to the front page" - } - } + responses: { + 302: { + description: "Redirects to the front page" + } + }apps/api/test/functional/auth0.spec.ts (3)
56-87: Move setup() to bottom of the root describeGuidelines require setup at the bottom of the root describe. Hoist it out of the nested describe.
- function setup(input: { emailVerified: boolean }) { - const user = UserSeeder.create({ - emailVerified: !input.emailVerified, - userId: faker.string.uuid() - }); - const executionContextService = container.resolve(ExecutionContextService); - jest.spyOn(executionContextService, "get").mockImplementation(key => { - if (key === "CURRENT_USER") { - return user; - } - if (key === "ABILITY") { - return {} as any; - } - return undefined; - }); - const auth0Service = container.resolve(Auth0Service); - jest.spyOn(auth0Service, "getUserByEmail").mockResolvedValue({ - user_id: user.userId, - email_verified: input.emailVerified - } as GetUsers200ResponseOneOfInner); - const userRepository = container.resolve(UserRepository); - jest.spyOn(userRepository, "findById").mockResolvedValue(user); - jest.spyOn(userRepository, "findByUserId").mockResolvedValue(user); - jest.spyOn(UserSetting, "update"); - return { user }; - }Place this at the bottom of the root describe (after all inner describes):
function setup(input: { emailVerified: boolean }) { const user = UserSeeder.create({ emailVerified: !input.emailVerified, userId: faker.string.uuid() }); const executionContextService = container.resolve(ExecutionContextService); jest.spyOn(executionContextService, "get").mockImplementation(key => { if (key === "CURRENT_USER") return user; if (key === "ABILITY") return {} as unknown as AppAbility; // import proper type return undefined; }); const auth0Service = container.resolve(Auth0Service); jest.spyOn(auth0Service, "getUserByEmail").mockResolvedValue({ user_id: user.userId, email_verified: input.emailVerified } as GetUsers200ResponseOneOfInner); const userRepository = container.resolve(UserRepository); jest.spyOn(userRepository, "findById").mockResolvedValue(user); jest.spyOn(userRepository, "findByUserId").mockResolvedValue(user); jest.spyOn(UserSetting, "update"); return { user }; }
20-41: Add negative/validation testsOnce the router uses openapi(route, ...), add tests for:
- Missing email → 400
- Invalid email → 400
I can add these scenarios after you switch to openapi().
63-71: Use proper CASL Ability type instead ofany
Replace theanycast with theAbilitytype from@casl/ability:- if (key === "ABILITY") { - return {} as any; - } + if (key === "ABILITY") { + // Return a minimal stub of CASL Ability + return {} as unknown as Ability; + }Add at top of
auth0.spec.ts:import type { Ability } from "@casl/ability";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/routes/email-verified/email-verified.router.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/services/db/userDataService.ts(1 hunks)apps/api/test/functional/auth0.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/services/db/userDataService.tsapps/api/src/auth/services/auth0/auth0.service.tsapps/api/src/rest-app.tsapps/api/src/auth/routes/email-verified/email-verified.router.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.tsapps/api/test/functional/auth0.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/services/db/userDataService.tsapps/api/src/auth/services/auth0/auth0.service.tsapps/api/src/rest-app.tsapps/api/src/auth/routes/email-verified/email-verified.router.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.tsapps/api/test/functional/auth0.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/auth0.spec.ts
🧬 Code graph analysis (5)
apps/api/src/rest-app.ts (1)
apps/api/src/auth/routes/email-verified/email-verified.router.ts (1)
emailVerifiedRouter(8-8)
apps/api/src/auth/routes/email-verified/email-verified.router.ts (2)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/dashboard/routes/leases-duration/leases-duration.router.ts (1)
route(15-37)
apps/api/src/auth/controllers/auth/auth.controller.ts (3)
apps/api/src/auth/routes/email-verified/email-verified.router.ts (1)
EmailVerifiedQuery(14-14)apps/api/src/auth/services/auth.service.ts (2)
currentUser(12-14)currentUser(16-19)apps/api/src/services/db/userDataService.ts (1)
setEmailVerified(225-236)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)
apps/api/test/functional/auth0.spec.ts (1)
apps/api/test/seeders/user.seeder.ts (1)
UserSeeder(5-45)
🪛 GitHub Check: validate / validate-app
apps/api/src/auth/routes/email-verified/email-verified.router.ts
[failure] 5-5:
Dependency cycle detected
apps/api/src/auth/controllers/auth/auth.controller.ts
[failure] 4-4:
Dependency cycle detected
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-build
🔇 Additional comments (6)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
22-29: LGTM: Safe handling of empty resultsEarly-return on empty array and selecting the first user is reasonable for this flow.
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
8-9: LGTM: Early export is cleanerHoisting the router export improves discoverability and avoids duplicate exports.
apps/api/src/rest-app.ts (2)
12-12: LGTM: Router wiringImport looks correct and scoped.
126-127: LGTM: OpenAPI handler registrationAdding
emailVerifiedRouterto the handlers array is consistent with existing pattern.apps/api/test/functional/auth0.spec.ts (2)
21-41: Happy-path assertions look goodCovers true/false flows and redirect semantics.
43-54: Not-found behavior coveredGood check that no update occurs when Auth0 returns null.
apps/api/src/auth/routes/email-verified/email-verified.router.ts
Outdated
Show resolved
Hide resolved
apps/api/src/auth/routes/email-verified/email-verified.router.ts
Outdated
Show resolved
Hide resolved
apps/api/src/auth/routes/email-verified/email-verified.router.ts
Outdated
Show resolved
Hide resolved
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
8b7980a to
de3a8cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/auth/http-schemas/email-verified.schema.ts (1)
3-7: HTTP semantics: avoid mutating state via GET.
If the associated route triggers a DB update ("sync emailVerified"), using GET is risky (caching/prefetch may cause unintended writes). Prefer POST/PUT/PATCH for the sync action, or split into:
- GET /v1/email-verified (read-only)
- POST /v1/email-verified/sync (mutating)
Do you want me to open a follow-up to switch the route method and update tests?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/email-verified.schema.ts(1 hunks)apps/api/src/auth/routes/email-verified/email-verified.router.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/services/db/userDataService.ts(1 hunks)apps/api/test/functional/auth0.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/src/rest-app.ts
- apps/api/src/services/db/userDataService.ts
- apps/api/src/auth/controllers/auth/auth.controller.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
- apps/api/test/functional/auth0.spec.ts
- apps/api/src/auth/routes/email-verified/email-verified.router.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/auth/http-schemas/email-verified.schema.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/auth/http-schemas/email-verified.schema.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/api/src/auth/http-schemas/email-verified.schema.ts (3)
1-1: LGTM on Zod import.
No issues with the import.
7-7: Type inference looks good.
Usingz.infer<typeof EmailVerifiedQuerySchema>avoidsanyand stays in sync with the schema.
3-5: Tighten EmailVerifiedQuerySchema in apps/api/src/auth/http-schemas/email-verified.schema.ts-export const EmailVerifiedQuerySchema = z.object({ - email: z.string() -}); +export const EmailVerifiedQuerySchema = z + .object({ + email: z.string().trim().toLowerCase().email().max(254), + }) + .strict();
- Confirm Auth0’s email lookup is case-insensitive; if it isn’t, remove or adjust the
.toLowerCase()transform.- Update callers in apps/api/src/auth/routes/email-verified/email-verified.router.ts (line 16) and apps/api/src/auth/controllers/auth/auth.controller.ts (line 5) to handle the normalized email.
apps/api/src/auth/routes/email-verified/email-verified.router.ts
Outdated
Show resolved
Hide resolved
|
Next step will be to implement the page and then we can update the email template? |
There was a problem hiding this comment.
one of the point of using setup function is that jest hooks aren't needed as every test case gets it's own clean state
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
de3a8cf to
7b304b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
apps/api/src/user/services/user/user.service.ts (1)
95-111: Visibility change: confirm intended surface area and guard against misuse.Making upsertUser public is fine for AuthController, but it broadens the API to mutate sensitive fields (emailVerified, lastIp, etc.). Consider marking it @internal and ensure only trusted callers (controllers/services) use it. If external exposure is possible, add an allowlist for updatable fields at the repository layer.
apps/deploy-web/src/queries/useVerifyEmailQuery.ts (1)
8-11: Remove or relocate unrelated export.USE_API_KEYS_DEPENDENCIES appears unrelated to email verification and drags in extra imports. Drop it or move to the relevant module to avoid dead exports.
-export const USE_API_KEYS_DEPENDENCIES = { - useUser, - useWallet -};Also remove now-unused imports of useUser/useWallet at the top.
apps/deploy-web/src/pages/user/verify-email/index.tsx (1)
33-36: Optional: handle missing email query param explicitly.If email is absent, render a clear message to guide the user.
<h3 className="text-2xl"> {isVerifying && "Just a moment while we finish verifying your email."} {isVerified === true && "Your email was verified. You can continue using the application."} - {isVerified === false && "Your email was not verified. Please try again."} + {isVerified === false && "Your email was not verified. Please try again."} + {!isVerifying && isVerified === null && !email && "Missing email in the link. Please open the verification link from your email again."} </h3>apps/api/src/auth/http-schemas/verify-email.schema.ts (2)
3-5: Normalize email to avoid case-mismatch frictionConsider normalizing here so controller checks don’t 401 on case differences.
-export const VerifyEmailParamsSchema = z.object({ - email: z.string().email({ message: "Email is invalid." }) -}); +export const VerifyEmailParamsSchema = z.object({ + email: z + .string() + .trim() + .transform(v => v.toLowerCase()) + .email({ message: "Email is invalid." }) +});
9-15: Avoid server/SDK schema driftVerifyEmailResponseSchema is duplicated in the SDK. Prefer single source (e.g., generate SDK types from OpenAPI or re-export server schema) to prevent drift.
apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
19-26: Fix response description and document auth errorsThe 200 description is incorrect. Also document 401/404 (or 403/404) outcomes for clearer API contracts.
responses: { - description: "Returns a created wallet", + description: "Returns email verification status", content: { "application/json": { schema: VerifyEmailResponseSchema } } - } + }, + 401: { description: "Unauthorized" }, + 404: { description: "Auth0 user not found" } }apps/api/src/auth/controllers/auth/auth.controller.ts (1)
29-35: Compare emails case-insensitively and use clearer status codesPrevent false 401 on case-only differences and prefer 403 for mismatch vs. 404 for “not found.”
- async syncEmailVerified({ email }: VerifyEmailParams) { + async syncEmailVerified({ email }: VerifyEmailParams) { const { currentUser } = this.authService; - assert(currentUser.userId && currentUser?.email === email, 401); + const targetEmail = email.trim().toLowerCase(); + const currentEmail = currentUser.email?.trim().toLowerCase(); + assert(currentUser.userId && currentEmail === targetEmail, 403); - const auth0User = await this.auth0.getUserByEmail(email); - assert(auth0User, 401); + const auth0User = await this.auth0.getUserByEmail(targetEmail); + assert(auth0User, 404);apps/api/test/functional/verify-email.spec.ts (6)
22-28: URL-encode the email path paramRaw emails in path segments can contain reserved characters. Safer to encode.
Apply this diff:
- const response = await app.request(`/v1/verify-email/${user.email}`, { + const response = await app.request(`/v1/verify-email/${encodeURIComponent(user.email)}`, { @@ - const response = await app.request(`/v1/verify-email/${user.email}`, { + const response = await app.request(`/v1/verify-email/${encodeURIComponent(user.email)}`, { @@ - const response = await app.request(`/v1/verify-email/${user.email}`, { + const response = await app.request(`/v1/verify-email/${encodeURIComponent(user.email)}`, { @@ - const response = await app.request(`/v1/verify-email/not-${user.email}`, { method: "POST" }); + const response = await app.request(`/v1/verify-email/not-${encodeURIComponent(user.email)}`, { method: "POST" }); @@ - const response = await app.request(`/v1/verify-email/not-${user.email}`, { + const response = await app.request(`/v1/verify-email/not-${encodeURIComponent(user.email)}`, {Also applies to: 45-51, 70-76, 84-85, 92-98
18-19: Restore mocks after each test to prevent leakageEnsure spies/mocks from setup don’t bleed across tests.
Apply this diff:
describe("POST /v1/verify-email", () => { + afterEach(() => { + jest.restoreAllMocks(); + });
78-79: Assert no local mutation when Auth0 user is not foundStrengthen the negative test by verifying that persistence isn’t touched.
Apply this diff:
expect(response.status).toBe(401); + expect(UserSetting.update).not.toHaveBeenCalled();
114-116: Provide a real or minimal ability instead of casting an empty objectCasting
{}toMongoAbilitycan hide runtime errors if the controller checks permissions.Option A (preferred): use the real AbilityService
import { AbilityService } from "@src/auth/services/ability/ability.service"; // ... if (key === "ABILITY") { return container.resolve(AbilityService).getAbilityFor("REGULAR_ANONYMOUS_USER", user); }Option B (minimal stub for tests):
- if (key === "ABILITY") { - return {} as MongoAbility; - } + if (key === "ABILITY") { + return { can: () => true, cannot: () => false } as unknown as MongoAbility; + }
103-155: Place the setup function at the bottom of the root describeGuidelines require the
setupfunction at the bottom of the rootdescribeblock. Move it one scope up (outside the innerdescribe) and keep it as the last item.Apply this localized removal and re-add at the bottom of the root describe:
- async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { - // ... (function body unchanged) - }Add below the inner
describe(but before the rootdescribecloses):async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { const walletTestingService = new WalletTestingService(app); const { user, token } = await walletTestingService.createRegisteredUser({ email_verified: input.emailVerified }); const executionContextService = container.resolve(ExecutionContextService); jest.spyOn(executionContextService, "get").mockImplementation(key => { if (key === "CURRENT_USER") return user; if (key === "ABILITY") return { can: () => true, cannot: () => false } as unknown as MongoAbility; if (key === "HTTP_CONTEXT") return { req: { header: (name: string) => (name === "authorization" ? `Bearer ${token}` : name === "x-anonymous-authorization" ? "" : undefined) }, env: {}, var: { clientInfo: { ip: "127.0.0.1", userAgent: "test-agent", fingerprint: "test-fingerprint" } } } as AppContext; return undefined; }); const auth0Service = container.resolve(Auth0Service); jest.spyOn(auth0Service, "getUserByEmail").mockResolvedValue( input.auth0UserFound === false ? null : ({ user_id: user.userId, email_verified: input.emailVerified } as GetUsers200ResponseOneOfInner) ); jest.spyOn(container.resolve(UserAuthTokenService), "getValidUserId").mockResolvedValue(user.userId); const userRepository = container.resolve(UserRepository); jest.spyOn(userRepository, "findById").mockResolvedValue(user); jest.spyOn(userRepository, "findByUserId").mockResolvedValue(user); jest.spyOn(UserSetting, "update"); return { user, token }; }
18-18: Clarify describe title to reflect the route shapeMinor: The block title says
POST /v1/verify-email, but tests hit/v1/verify-email/:email. Consider updating for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(1 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/rest-app.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/auth/routes/verify-email/verify-email.router.tsapps/deploy-web/src/pages/user/verify-email/index.tsxpackages/http-sdk/src/auth/auth-http.types.tspackages/http-sdk/src/auth/index.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/user/services/user/user.service.tspackages/http-sdk/src/auth/auth-http.service.tspackages/http-sdk/src/index.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/deploy-web/src/queries/useVerifyEmailQuery.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/auth/routes/verify-email/verify-email.router.tsapps/deploy-web/src/pages/user/verify-email/index.tsxpackages/http-sdk/src/auth/auth-http.types.tspackages/http-sdk/src/auth/index.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/user/services/user/user.service.tspackages/http-sdk/src/auth/auth-http.service.tspackages/http-sdk/src/index.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/deploy-web/src/queries/useVerifyEmailQuery.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/test/functional/verify-email.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
🧬 Code graph analysis (8)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (3)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/auth/http-schemas/verify-email.schema.ts (2)
VerifyEmailParamsSchema(3-5)VerifyEmailResponseSchema(9-13)packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(14-16)
apps/deploy-web/src/pages/user/verify-email/index.tsx (4)
packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(14-16)apps/deploy-web/src/queries/useVerifyEmailQuery.ts (1)
useVerifyEmail(13-26)apps/provider-console/src/components/layout/Layout.tsx (1)
Layout(22-43)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)
packages/http-sdk/src/auth/auth-http.types.ts (1)
apps/api/src/auth/http-schemas/verify-email.schema.ts (2)
VerifyEmailResponseSchema(9-13)VerifyEmailResponse(15-15)
apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
packages/http-sdk/src/auth/auth-http.types.ts (2)
VerifyEmailResponseSchema(3-7)VerifyEmailResponse(9-9)
apps/api/src/user/services/user/user.service.ts (1)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)
apps/api/test/functional/verify-email.spec.ts (2)
packages/database/dbSchemas/user/index.ts (1)
UserSetting(3-3)apps/api/test/services/wallet-testing.service.ts (1)
WalletTestingService(15-156)
apps/api/src/auth/controllers/auth/auth.controller.ts (4)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
singleton(6-30)apps/api/src/user/services/user/user.service.ts (1)
singleton(11-122)apps/api/src/auth/services/auth.service.ts (3)
Protected(39-55)currentUser(12-14)currentUser(16-19)apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
VerifyEmailParams(7-7)
apps/deploy-web/src/queries/useVerifyEmailQuery.ts (2)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)packages/http-sdk/src/auth/auth-http.types.ts (1)
VerifyEmailResponse(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (5)
packages/http-sdk/src/auth/auth-http.types.ts (1)
1-9: LGTM — schema and types align with server.The zod schema mirrors the API and keeps client/server in sync.
packages/http-sdk/src/index.ts (1)
12-12: Barrel export safe—no direct deep imports found
Repo-wide search forauth-http.serviceimports returned no matches, so no compatibility re-export is needed.packages/http-sdk/src/auth/index.ts (1)
1-2: LGTMBarrel export is clean and aligns the public surface.
apps/api/test/functional/verify-email.spec.ts (2)
19-40: Positive-path coverage looks goodHappy path assertions validate HTTP status, response shape, and DB state flip.
Also applies to: 42-63
65-79: Consider response semantics for “Auth0 user not found”401 (unauthorized) conflates auth with existence. 404 or 422 may better convey “email not found in Auth0.” If 401 is intentional for defensive ambiguity, keep as-is.
There was a problem hiding this comment.
🛠️ Refactor suggestion
Coerce Auth0’s email_verified to boolean and avoid unnecessary writes
Guard against undefined/null and skip DB write when unchanged.
- await this.userService.upsertUser({
- userId: currentUser.userId,
- emailVerified: auth0User.email_verified
- });
+ const emailVerified = !!auth0User.email_verified;
+ if (currentUser.emailVerified !== emailVerified) {
+ await this.userService.upsertUser({
+ userId: currentUser.userId,
+ emailVerified
+ });
+ }
return {
data: {
- emailVerified: auth0User.email_verified
+ emailVerified
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await this.userService.upsertUser({ | |
| userId: currentUser.userId, | |
| emailVerified: auth0User.email_verified | |
| }); | |
| return { | |
| data: { | |
| emailVerified: auth0User.email_verified | |
| } | |
| }; | |
| const emailVerified = !!auth0User.email_verified; | |
| if (currentUser.emailVerified !== emailVerified) { | |
| await this.userService.upsertUser({ | |
| userId: currentUser.userId, | |
| emailVerified | |
| }); | |
| } | |
| return { | |
| data: { | |
| emailVerified | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/api/src/auth/controllers/auth/auth.controller.ts around lines 36 to 45,
coerce auth0User.email_verified to a strict boolean and avoid an unnecessary DB
write: compute const emailVerified = Boolean(auth0User.email_verified), compare
it to the current value (use currentUser.emailVerified if available or fetch the
user’s current record), and only call this.userService.upsertUser({ userId:
currentUser.userId, emailVerified }) when the value has changed; always return
the coerced boolean in the response.
| export function useVerifyEmail( | ||
| options: { | ||
| onSuccess?: (isVerified: boolean) => void; | ||
| } = {} | ||
| ) { | ||
| const { auth } = useServices(); | ||
|
|
||
| return useMutation<VerifyEmailResponse, Error, string>({ | ||
| mutationFn: (email: string) => auth.verifyEmail(email), | ||
| onSuccess: response => { | ||
| options.onSuccess?.(response.data.emailVerified); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate errors to callers (so the page can show failure state).
Expose onError and wire it to useMutation; keep data typing intact.
-export function useVerifyEmail(
- options: {
- onSuccess?: (isVerified: boolean) => void;
- } = {}
-) {
+export function useVerifyEmail(
+ options: {
+ onSuccess?: (isVerified: boolean) => void;
+ onError?: (error: Error) => void;
+ } = {}
+) {
const { auth } = useServices();
return useMutation<VerifyEmailResponse, Error, string>({
mutationFn: (email: string) => auth.verifyEmail(email),
onSuccess: response => {
options.onSuccess?.(response.data.emailVerified);
- }
+ },
+ onError: error => {
+ options.onError?.(error);
+ }
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useVerifyEmail( | |
| options: { | |
| onSuccess?: (isVerified: boolean) => void; | |
| } = {} | |
| ) { | |
| const { auth } = useServices(); | |
| return useMutation<VerifyEmailResponse, Error, string>({ | |
| mutationFn: (email: string) => auth.verifyEmail(email), | |
| onSuccess: response => { | |
| options.onSuccess?.(response.data.emailVerified); | |
| } | |
| }); | |
| } | |
| export function useVerifyEmail( | |
| options: { | |
| onSuccess?: (isVerified: boolean) => void; | |
| onError?: (error: Error) => void; | |
| } = {} | |
| ) { | |
| const { auth } = useServices(); | |
| return useMutation<VerifyEmailResponse, Error, string>({ | |
| mutationFn: (email: string) => auth.verifyEmail(email), | |
| onSuccess: response => { | |
| options.onSuccess?.(response.data.emailVerified); | |
| }, | |
| onError: error => { | |
| options.onError?.(error); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/deploy-web/src/queries/useVerifyEmailQuery.ts around lines 13 to 26, the
hook currently accepts only an onSuccess callback and does not propagate errors
to callers; add an optional onError?: (error: Error) => void to the options
param and pass it into the useMutation config (i.e., include onError:
options.onError) while preserving the existing generics <VerifyEmailResponse,
Error, string> and current mutationFn and onSuccess wiring so callers receive
failures and can show error state.
| async verifyEmail(email: string) { | ||
| return this.post(`/v1/verify-email/${email}`, {}, { withCredentials: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Encode email path param and type the response (prevents breakage for '+'/'@' and enforces schema).
Use encodeURIComponent for the path segment and return a typed payload.
- async verifyEmail(email: string) {
- return this.post(`/v1/verify-email/${email}`, {}, { withCredentials: true });
- }
+ async verifyEmail(email: string) {
+ return this.post<VerifyEmailResponse>(`/v1/verify-email/${encodeURIComponent(email)}`, {}, { withCredentials: true });
+ }Add the missing import (outside the selected range):
import type { VerifyEmailResponse } from "./auth-http.types";🤖 Prompt for AI Agents
In packages/http-sdk/src/auth/auth-http.service.ts around lines 14 to 16, the
verifyEmail method currently interpolates the raw email into the URL and returns
an untyped response; update it to encode the email path segment using
encodeURIComponent(email) to avoid breaking on '+' or '@', and annotate the
method to return the proper typed payload (VerifyEmailResponse) so callers get
schema enforcement; also add the missing import at the top: import type {
VerifyEmailResponse } from "./auth-http.types".
7b304b5 to
6d9a4cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/auth/controllers/auth/auth.controller.ts (1)
36-48: Coerce email_verified to boolean and skip no-op writesAuth0’s field can be undefined; write only on change and always return a boolean.
- const auth0User = await this.auth0.getUserByEmail(targetEmail); - assert(auth0User, 404); - - await this.userService.upsertUser({ - userId: currentUser.userId, - emailVerified: auth0User.email_verified - }); - - return { - data: { - emailVerified: auth0User.email_verified - } - }; + const auth0User = await this.auth0.getUserByEmail(targetEmail); + assert(auth0User, 404); + + const emailVerified = !!auth0User.email_verified; + if (currentUser.emailVerified !== emailVerified) { + await this.userService.upsertUser({ + userId: currentUser.userId, + emailVerified + }); + } + + return { + data: { emailVerified } + };
🧹 Nitpick comments (3)
packages/http-sdk/src/auth/auth-http.service.ts (1)
14-16: Return a typed payload from post()Annotate the response to enforce the schema at call sites.
+import type { VerifyEmailResponse } from "./auth-http.types"; ... - async verifyEmail(email: string) { - return this.post("/v1/verify-email", { data: { email } }, { withCredentials: true }); - } + async verifyEmail(email: string) { + return this.post<VerifyEmailResponse>("/v1/verify-email", { data: { email } }, { withCredentials: true }); + }apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
3-7: Confirm zod’s .toLowerCase() availability; otherwise use preprocessSome Zod versions lack string().toLowerCase(). If not supported, switch to preprocess to avoid runtime/type errors.
-export const VerifyEmailRequestSchema = z.object({ - data: z.object({ - email: z.string().trim().toLowerCase().email({ message: "Email is invalid." }) - }) -}); +export const VerifyEmailRequestSchema = z.object({ + data: z.object({ + email: z.preprocess( + v => (typeof v === "string" ? v.trim().toLowerCase() : v), + z.string().email({ message: "Email is invalid." }) + ) + }) +});apps/api/src/auth/controllers/auth/auth.controller.ts (1)
34-34: Nit: 403 vs 401 on email mismatch401 is acceptable, but 403 better signals “authenticated but forbidden.”
- assert(currentUser.userId && currentEmail === targetEmail, 401); + assert(currentUser.userId && currentEmail === targetEmail, 403);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(1 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/api/src/auth/services/auth0/auth0.service.ts
- packages/http-sdk/src/auth/auth-http.types.ts
- packages/http-sdk/src/index.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/src/rest-app.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/http-sdk/src/auth/auth-http.service.tspackages/http-sdk/src/auth/index.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/auth/routes/verify-email/verify-email.router.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/http-sdk/src/auth/auth-http.service.tspackages/http-sdk/src/auth/index.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/auth/routes/verify-email/verify-email.router.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
🧬 Code graph analysis (3)
apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
packages/http-sdk/src/auth/auth-http.types.ts (2)
VerifyEmailResponseSchema(3-7)VerifyEmailResponse(9-9)
apps/api/src/auth/controllers/auth/auth.controller.ts (4)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
singleton(6-30)apps/api/src/user/services/user/user.service.ts (1)
singleton(11-122)apps/api/src/auth/services/auth.service.ts (3)
Protected(39-55)currentUser(12-14)currentUser(16-19)apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
VerifyEmailRequest(9-9)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (3)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/auth/http-schemas/verify-email.schema.ts (2)
VerifyEmailRequestSchema(3-7)VerifyEmailResponseSchema(11-15)packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(14-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
packages/http-sdk/src/auth/index.ts (1)
1-2: Barrel export looks goodRe-exports are correct and minimal.
apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
11-15: Response schema is clear and minimalShape matches the controller response and client expectations.
apps/api/src/auth/controllers/auth/auth.controller.ts (1)
32-35: Good: bind sync to the authenticated user’s emailPrevents cross-account tampering.
| verifyEmailRouter.post(route.path, async function verifyEmail(c) { | ||
| const data = VerifyEmailRequestSchema.parse(await c.req.json()); | ||
|
|
||
| const result = await container.resolve(AuthController).syncEmailVerified(data); | ||
| return c.json(result, 200); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Register the handler via openapi() so it’s included in the spec and benefits from route-level validation
Using post() bypasses OpenAPI registration; prefer openapi(route, handler).
-verifyEmailRouter.post(route.path, async function verifyEmail(c) {
- const data = VerifyEmailRequestSchema.parse(await c.req.json());
- const result = await container.resolve(AuthController).syncEmailVerified(data);
- return c.json(result, 200);
-});
+verifyEmailRouter.openapi(route, async function verifyEmail(c) {
+ const data = VerifyEmailRequestSchema.parse(await c.req.json());
+ const result = await container.resolve(AuthController).syncEmailVerified(data);
+ return c.json(result, 200);
+});Optional: if you use Hono’s validated payloads, replace parse() with const data = c.req.valid("json");.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| verifyEmailRouter.post(route.path, async function verifyEmail(c) { | |
| const data = VerifyEmailRequestSchema.parse(await c.req.json()); | |
| const result = await container.resolve(AuthController).syncEmailVerified(data); | |
| return c.json(result, 200); | |
| }); | |
| verifyEmailRouter.openapi(route, async function verifyEmail(c) { | |
| const data = VerifyEmailRequestSchema.parse(await c.req.json()); | |
| const result = await container.resolve(AuthController).syncEmailVerified(data); | |
| return c.json(result, 200); | |
| }); |
🤖 Prompt for AI Agents
In apps/api/src/auth/routes/verify-email/verify-email.router.ts around lines 39
to 44, the route is registered with post() which bypasses OpenAPI registration
and route-level validation; replace the post(route.path, async function...) call
with openapi(route, async function verifyEmail(c) { ... }) so the handler is
included in the OpenAPI spec and benefits from route-level validation, and
inside the handler remove manual parsing and instead use Hono’s validated
payload (const data = c.req.valid("json");) or keep
VerifyEmailRequestSchema.parse only if you don't enable Hono validation; ensure
you import/use openapi from your router utilities and retain the call to
container.resolve(AuthController).syncEmailVerified(data) and return
c.json(result, 200).
6d9a4cf to
d95465e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/services/ability/ability.service.ts (1)
55-58: Tighten getAbilityFor user parameter type to include id and email
Use a context type matching the actualuserobjects (withid/{ userId }to catch missing properties at compile time.- getAbilityFor(role: Role, user: { userId: string | null }) { + type CurrentUserCtx = { id?: string | null; userId?: string | null; email?: string | null }; + getAbilityFor(role: Role, user: CurrentUserCtx) {
♻️ Duplicate comments (1)
apps/api/test/functional/verify-email.spec.ts (1)
73-77: Remove duplicate spy; drive 404 via setup flagYou’re re-spying on
Auth0Service.getUserByEmailinside the test, while it’s already spied insetup, which can cause flaky tests and “already spied” errors.Apply this diff to the 404 test:
- const { user, token } = await setup({ emailVerified: false }); - const auth0Service = container.resolve(Auth0Service); - jest.spyOn(auth0Service, "getUserByEmail").mockResolvedValue(null); + const { user, token } = await setup({ emailVerified: false, auth0UserFound: false });And update
setupaccordingly:- async function setup(input: { emailVerified: boolean }) { + async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { @@ - jest.spyOn(auth0Service, "getUserByEmail").mockImplementation(async (email: string) => { - if (email.toLowerCase() === user.email.toLowerCase()) { - return Promise.resolve({ - user_id: user.userId, - email_verified: input.emailVerified - } as GetUsers200ResponseOneOfInner); - } - - return Promise.resolve(null); - }); + jest.spyOn(auth0Service, "getUserByEmail").mockImplementation(async (email: string) => { + if (input.auth0UserFound === false) return null; + if (email.toLowerCase() === user.email.toLowerCase()) { + return { + user_id: user.userId, + email_verified: input.emailVerified + } as GetUsers200ResponseOneOfInner; + } + return null; + });
🧹 Nitpick comments (7)
apps/api/test/services/wallet-testing.service.ts (1)
93-99: Normalize provided claims.email as wellLowercasing generated emails is good. Also normalize when
claims.emailis supplied to avoid mixed-case test data discrepancies.- email: claims.email ?? faker.internet.email().toLowerCase(), + email: (claims.email ?? faker.internet.email()).toLowerCase(),apps/api/test/functional/verify-email.spec.ts (6)
128-156: Place setup at the bottom of the root describePer our spec test guideline,
setupmust live at the bottom of the rootdescribe(not inside a nesteddescribe). Move it out of the inner block for consistency and to avoid scope surprises.Apply this removal here:
- async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { - const walletTestingService = new WalletTestingService(app); - const { user, token } = await walletTestingService.createRegisteredUser({ - email_verified: input.emailVerified - }); - const auth0Service = container.resolve(Auth0Service); - jest.spyOn(auth0Service, "getUserByEmail").mockImplementation(async (email: string) => { - if (input.auth0UserFound === false) return null; - if (email.toLowerCase() === user.email.toLowerCase()) { - return { - user_id: user.userId, - email_verified: input.emailVerified - } as GetUsers200ResponseOneOfInner; - } - return null; - }); - const userAuthTokenService = container.resolve(UserAuthTokenService); - jest.spyOn(userAuthTokenService, "getValidUserId").mockResolvedValue(user.userId); - const userRepository = container.resolve(UserRepository); - jest.spyOn(userRepository, "findById").mockResolvedValue(user); - jest.spyOn(userRepository, "findByUserId").mockResolvedValue(user); - jest.spyOn(UserSetting, "update"); - return { user, token }; - }Then add it just before the closing
});of the root describe:async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { const walletTestingService = new WalletTestingService(app); const { user, token } = await walletTestingService.createRegisteredUser({ email_verified: input.emailVerified }); const auth0Service = container.resolve(Auth0Service); jest.spyOn(auth0Service, "getUserByEmail").mockImplementation(async (email: string) => { if (input.auth0UserFound === false) return null; if (email.toLowerCase() === user.email.toLowerCase()) { return { user_id: user.userId, email_verified: input.emailVerified } as GetUsers200ResponseOneOfInner; } return null; }); const userAuthTokenService = container.resolve(UserAuthTokenService); jest.spyOn(userAuthTokenService, "getValidUserId").mockResolvedValue(user.userId); const userRepository = container.resolve(UserRepository); jest.spyOn(userRepository, "findById").mockResolvedValue(user); jest.spyOn(userRepository, "findByUserId").mockResolvedValue(user); return { user, token }; }
153-153: Remove unused spy on UserSetting.updateThis spy isn’t asserted and doesn’t alter behavior. Drop it to reduce noise.
- jest.spyOn(UserSetting, "update");
41-43: Assert existence before field assertionsIf
findOnereturnsnull, the current assertion fails with a vague message. Add an existence check first.- const userSetting = await UserSetting.findOne({ where: { userId: user.userId } }); + const userSetting = await UserSetting.findOne({ where: { userId: user.userId } }); + expect(userSetting).toBeTruthy();Also applies to: 69-71
20-31: DRY the request constructionThe POST body/headers are duplicated. Extract a small helper (e.g.,
postVerifyEmail(token, email)) to reduce repetition and future drift.Also applies to: 48-59, 97-104, 112-123
13-14: Restore mocks after each testPrevents spy leakage across tests and simplifies local overrides.
jest.setTimeout(30000); + +afterEach(() => { + jest.restoreAllMocks(); +});
15-17: Consider adding GET callback coverage (if implemented)Auth0 email verification redirects typically hit a GET callback. If you added a GET route (separate from this POST sync), add a functional test to cover that flow as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(2 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(1 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/api/src/rest-app.ts
- apps/api/src/auth/http-schemas/verify-email.schema.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
- packages/http-sdk/src/auth/auth-http.types.ts
- packages/http-sdk/src/auth/index.ts
- packages/http-sdk/src/auth/auth-http.service.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- apps/api/src/auth/routes/verify-email/verify-email.router.ts
- packages/http-sdk/src/index.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/auth/services/ability/ability.service.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/test/services/wallet-testing.service.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/controllers/auth/auth.controller.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/auth/services/ability/ability.service.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/test/services/wallet-testing.service.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/controllers/auth/auth.controller.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/user/services/user/user.service.spec.tsapps/api/test/functional/verify-email.spec.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Applied to files:
apps/api/test/functional/verify-email.spec.ts
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/test/functional/verify-email.spec.ts
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/test/functional/verify-email.spec.ts
🧬 Code graph analysis (2)
apps/api/test/functional/verify-email.spec.ts (1)
apps/api/test/services/wallet-testing.service.ts (1)
WalletTestingService(15-156)
apps/api/src/auth/controllers/auth/auth.controller.ts (3)
apps/api/src/user/services/user/user.service.ts (1)
singleton(13-142)apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
VerifyEmailRequest(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/api/src/auth/services/ability/ability.service.ts (1)
20-21: New "verify-email" rule looks correctPermission ties to the caller’s own email, matching controller checks. LGTM.
Also applies to: 26-27, 36-37
apps/api/src/user/services/user/user.service.spec.ts (1)
14-14: Constructor update and mock injection look goodAdapts tests to the new
Auth0Servicedependency usingjest-mock-extendedand keeps thesetupat the bottom. LGTM.Also applies to: 311-313
apps/api/src/auth/controllers/auth/auth.controller.ts (1)
27-37: Controller flow is sound; rely on service for strictnessAuth check + delegation is clean. With the proposed service guard (404 when local user absent) and boolean coercion, this endpoint remains idempotent and safe.
Run the functional tests for
/v1/verify-emailto confirm 200/404 paths and that the response payload contains a booleandata.emailVerified.
| async syncEmailVerified({ email }: { email: string }) { | ||
| const auth0User = await this.auth0.getUserByEmail(email); | ||
| assert(auth0User, 404); | ||
|
|
||
| return await this.userRepository.updateBy( | ||
| { | ||
| }, | ||
| { | ||
| emailVerified: auth0User.email_verified | ||
| }, | ||
| { | ||
| returning: true | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden syncEmailVerified: boolean coercion, normalization, and 404 when local user not found
Avoid potential undefined destructuring in the controller and prevent no-op updates.
- async syncEmailVerified({ email }: { email: string }) {
- const auth0User = await this.auth0.getUserByEmail(email);
- assert(auth0User, 404);
-
- return await this.userRepository.updateBy(
- {
- email
- },
- {
- emailVerified: auth0User.email_verified
- },
- {
- returning: true
- }
- );
- }
+ async syncEmailVerified({ email }: { email: string }) {
+ const normalizedEmail = email.trim().toLowerCase();
+ const auth0User = await this.auth0.getUserByEmail(normalizedEmail);
+ assert(auth0User, 404);
+
+ const emailVerified = !!auth0User.email_verified;
+ const updated = await this.userRepository.updateBy(
+ { email: normalizedEmail },
+ { emailVerified },
+ { returning: true }
+ );
+ assert(updated, 404);
+ return updated;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async syncEmailVerified({ email }: { email: string }) { | |
| const auth0User = await this.auth0.getUserByEmail(email); | |
| assert(auth0User, 404); | |
| return await this.userRepository.updateBy( | |
| { | |
| }, | |
| { | |
| emailVerified: auth0User.email_verified | |
| }, | |
| { | |
| returning: true | |
| } | |
| ); | |
| } | |
| async syncEmailVerified({ email }: { email: string }) { | |
| const normalizedEmail = email.trim().toLowerCase(); | |
| const auth0User = await this.auth0.getUserByEmail(normalizedEmail); | |
| assert(auth0User, 404); | |
| const emailVerified = !!auth0User.email_verified; | |
| const updated = await this.userRepository.updateBy( | |
| { email: normalizedEmail }, | |
| { emailVerified }, | |
| { returning: true } | |
| ); | |
| assert(updated, 404); | |
| return updated; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/user/services/user/user.service.ts around lines 126-141, the
syncEmailVerified flow should first normalize auth0User.email_verified to a
strict boolean, ensure a local user exists before attempting update (return 404
if not), and avoid no-op updates that can lead to undefined destructuring in
callers: fetch the local user by email and assert it exists, compute const
emailVerified = Boolean(auth0User?.email_verified), compare against the local
user's current emailVerified and if unchanged return the existing user (or the
expected shape), otherwise call updateBy to persist the new boolean and return
the updated user; make sure to return a consistent user object shape so callers
don't destructure undefined.
There was a problem hiding this comment.
thanks for the changes @jzsfkzm! i think it's essential to update the type here so it's not misused accidentally https://github.com/akash-network/console/pull/1880/files#diff-98e087b6c1c3c2f322904604e83ce6cb3632af0db4438fc354d08945d9428d37R55 - should also have optional email
There was a problem hiding this comment.
I don't think any mocks above are needed (besides 3rd party auth0). App should function normally without tweaks
There was a problem hiding this comment.
shouldn't we just navigate and show some success message?
There was a problem hiding this comment.
I agree, we could add a little coutndown "5 seconds until being redirected" and keep the continue button to fast forward
There was a problem hiding this comment.
Added such a timeout, now it will redirect automatically.
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
d95465e to
a34da89
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/src/auth/services/auth0/auth0.service.spec.ts (2)
100-141: Move setup() to bottom of the root describe to match test guidelines.Project guideline: use a setup function at the bottom of the root describe, not nested.
- Hoist
function setup(...) { ... }to the end of the outerdescribe(Auth0Service.name, ...).- Keep it stateless and return the constructed SUT + mocks.
111-117: Type-narrow the mock client to avoid unsafe casts.Prefer an explicit minimal interface over
unknown as ManagementClientto keep types honest.type UsersByEmailClient = Pick<ManagementClient, "usersByEmail">; const mockManagementClient: UsersByEmailClient = { usersByEmail: { getByEmail: mockGetByEmail as any } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(2 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(1 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/api/src/auth/controllers/auth/auth.controller.ts
- apps/api/src/rest-app.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/api/src/auth/http-schemas/verify-email.schema.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- packages/http-sdk/src/auth/auth-http.types.ts
- packages/http-sdk/src/auth/index.ts
- apps/api/src/user/services/user/user.service.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/api/src/user/services/user/user.service.spec.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/auth/routes/verify-email/verify-email.router.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/http-sdk/src/index.tsapps/api/src/auth/services/auth0/auth0.service.tspackages/http-sdk/src/auth/auth-http.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/http-sdk/src/index.tsapps/api/src/auth/services/auth0/auth0.service.tspackages/http-sdk/src/auth/auth-http.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/auth/services/auth0/auth0.service.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: Validate local packages
🔇 Additional comments (2)
packages/http-sdk/src/index.ts (1)
12-12: Confirm external deep imports
No internal deep imports of@akash-network/http-sdk/auth/auth-http.servicewere found in the monorepo. Ensure no external consumers rely on that deep path—if they do, this change should trigger a minor (not patch) version bump.packages/http-sdk/src/auth/auth-http.service.ts (1)
14-16: Nice move away from path params.Body payload removes encoding pitfalls for '+' and '@' that would've required encodeURIComponent in the URL.
| it("should handle empty email string", async () => { | ||
| const email = ""; | ||
|
|
||
| const { auth0Service, mockGetByEmail } = setup({ | ||
| mockUsers: [] | ||
| }); | ||
|
|
||
| const result = await auth0Service.getUserByEmail(email); | ||
|
|
||
| expect(result).toBeNull(); | ||
| expect(mockGetByEmail).toHaveBeenCalledWith({ email: "" }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adjust empty-email test to expect an early return (no Auth0 call).
If we short-circuit on empty input, assert no external call.
- const result = await auth0Service.getUserByEmail(email);
-
- expect(result).toBeNull();
- expect(mockGetByEmail).toHaveBeenCalledWith({ email: "" });
+ const result = await auth0Service.getUserByEmail(email);
+ expect(result).toBeNull();
+ expect(mockGetByEmail).not.toHaveBeenCalled();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should handle empty email string", async () => { | |
| const email = ""; | |
| const { auth0Service, mockGetByEmail } = setup({ | |
| mockUsers: [] | |
| }); | |
| const result = await auth0Service.getUserByEmail(email); | |
| expect(result).toBeNull(); | |
| expect(mockGetByEmail).toHaveBeenCalledWith({ email: "" }); | |
| }); | |
| it("should handle empty email string", async () => { | |
| const email = ""; | |
| const { auth0Service, mockGetByEmail } = setup({ | |
| mockUsers: [] | |
| }); | |
| const result = await auth0Service.getUserByEmail(email); | |
| expect(result).toBeNull(); | |
| expect(mockGetByEmail).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
In apps/api/src/auth/services/auth0/auth0.service.spec.ts around lines 87 to 98,
the test currently expects getUserByEmail to call Auth0 even when given an empty
email; update the test to assert that the service short-circuits on empty input
by keeping the result expectation (toBeNull) but changing the mock assertion to
expect mockGetByEmail not to have been called (i.e., assert no external Auth0
call was made for empty email).
| @@ -0,0 +1,9 @@ | |||
| import { z } from "zod"; | |||
|
|
|||
| export const VerifyEmailResponseSchema = z.object({ | |||
There was a problem hiding this comment.
why is schema needed here? perhaps just type would be enough?
There was a problem hiding this comment.
We can add the Akash Console logo and big icon (check or X or spinner) to make it prettier and more interactive.
Hm, that's right, types should be the same, but shouldn't it rather be accepting the same UserOutput we're passing when using this method? - getAbilityFor(role: Role, user: { userId: string | null }) {
+ getAbilityFor(role: Role, user: UserOutput) { |
this can work too |
There was a problem hiding this comment.
none of these mocks (except 3rd part auth0) is really needed i think. it should be possible to create a user and it would work without them
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
a34da89 to
04eb061
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
34-38: Fix OpenAPI response description
Change the 200 response description from “Returns a created wallet” to “Resends a verification email”:responses: { - 200: { description: "Returns a created wallet", content: {} } + 200: { description: "Resends a verification email", content: {} } }Optional: for a more RESTful design, switch to a 204 status and update the handler to
return c.text("", 204)instead.
♻️ Duplicate comments (3)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
39-44: Register via openapi() and use c.req.valid("json") for schema-validated inputUsing post() bypasses OpenAPI registration and route-level validation; switch to openapi() and leverage validated payloads.
-verifyEmailRouter.post(route.path, async function verifyEmail(c) { - const data = VerifyEmailRequestSchema.parse(await c.req.json()); - - const result = await container.resolve(AuthController).syncEmailVerified(data); - return c.json(result, 200); -}); +verifyEmailRouter.openapi(route, async function verifyEmail(c) { + const data = c.req.valid("json"); + const result = await container.resolve(AuthController).syncEmailVerified(data); + return c.json(result, 200); +});apps/api/src/auth/services/ability/ability.service.ts (2)
27-27: Same concern as above for REGULAR_USER.
Guard for absent/unnormalized email to prevent"undefined"conditions and case mismatches.
37-37: Same concern as above for REGULAR_PAYING_USER.
Ensure email is present and normalized before ability checks.
🧹 Nitpick comments (3)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
41-44: Name the handler consistently with the route intentMinor readability nit: rename the handler to reflect “sendVerificationEmail”.
-sendVerificationEmailRouter.openapi(route, async function verifyEmail(c) { +sendVerificationEmailRouter.openapi(route, async function sendVerificationEmail(c) {apps/api/src/auth/routes/verify-email/verify-email.router.ts (2)
10-37: Add an operationId for clearer, stable client generationImproves OpenAPI readability and SDK codegen stability.
const route = createRoute({ method: "post", path: "/v1/verify-email", summary: "Checks if the email is verified", + operationId: "verifyEmail", tags: ["Users"],
11-13: Confirm intent: POST is for “on-demand sync”, not the Auth0 browser callbackPOST here is fine for server-triggered sync. Ensure the separate GET callback endpoint (for Auth0’s redirect after verification) exists and is wired, per our convention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(4 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(2 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/http-sdk/src/auth/index.ts
- apps/api/src/auth/http-schemas/verify-email.schema.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- apps/api/src/auth/services/auth0/auth0.service.spec.ts
- apps/api/src/user/services/user/user.service.spec.ts
- packages/http-sdk/src/index.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
- apps/api/src/auth/controllers/auth/auth.controller.ts
- packages/http-sdk/src/auth/auth-http.types.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/api/test/services/wallet-testing.service.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/src/rest-app.ts
- packages/http-sdk/src/auth/auth-http.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/auth/routes/verify-email/verify-email.router.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/auth/routes/verify-email/verify-email.router.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.tsapps/api/src/auth/services/ability/ability.service.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-28T10:40:13.595Z
Learnt from: stalniy
PR: akash-network/console#1480
File: apps/deploy-web/src/hooks/useFlag.tsx:0-0
Timestamp: 2025-07-28T10:40:13.595Z
Learning: In the Akash Network Console project, backend-specific feature flags are intentional and acceptable. The frontend FeatureFlag union type should only include flags that are actually used by the frontend, not all flags defined in the backend. Backend can have internal feature flags for backend-only functionality without requiring frontend synchronization.
Applied to files:
apps/api/src/auth/services/ability/ability.service.ts
🧬 Code graph analysis (3)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (3)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/auth/http-schemas/verify-email.schema.ts (2)
VerifyEmailRequestSchema(3-7)VerifyEmailResponseSchema(11-15)packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(15-17)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)
apps/api/src/auth/services/ability/ability.service.ts (1)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
8-9: LGTM: early router instantiation is correctExporting the router right after imports is fine and avoids TDZ-style surprises.
apps/api/src/auth/services/ability/ability.service.ts (2)
56-59: Verified no remaining{ userId }shape; all callers now pass fullUserOutput.
21-21: Guard against missing or unnormalized email in verify-email rules (ability.service.ts:21,27,37)
- Only add the
{ action: "verify-email", … }rule whenuser.emailis non-null/undefined- Lowercase
user.emailwhen interpolating into the rule to avoid case-sensitive mismatches- (Optional) rename the action to
sync-email-verifiedto distinguish it from Auth0’s verification callback flow
|
|
||
| import { FeatureFlags, FeatureFlagValue } from "@src/core/services/feature-flags/feature-flags"; | ||
| import { FeatureFlagsService } from "@src/core/services/feature-flags/feature-flags.service"; | ||
| import type { UserOutput } from "@src/user/repositories"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Using UserOutput here is the right alignment with rule templates.
Good change to match ${user.id} / ${user.email} usage.
Run to ensure the type is re-exported and no shadow exports linger:
🏁 Script executed:
#!/bin/bash
rg -nP 'export\s+type\s+UserOutput' apps/api/src/user/repositories -C1Length of output: 332
🏁 Script executed:
#!/bin/bash
# Check for re-exports of UserOutput in index files under user/repositories
find apps/api/src/user/repositories -type f -name index.ts -print | while read f; do
echo "---- ${f} ----"
rg -nP 'export.*UserOutput' -C3 "${f}" || echo "no matches"
doneLength of output: 299
Export UserOutput in the repository index
Add the following to apps/api/src/user/repositories/index.ts so that @src/user/repositories correctly exposes the type:
export type { UserOutput } from "./user/user.repository";🤖 Prompt for AI Agents
In apps/api/src/auth/services/ability/ability.service.ts around line 8, the
imported type UserOutput from "@src/user/repositories" is not exported by that
module; open apps/api/src/user/repositories/index.ts and add an export type line
to re-export UserOutput from the concrete repository (export type { UserOutput }
from "./user/user.repository";) so the import in ability.service.ts resolves
correctly.
04eb061 to
ad72dbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/rest-app.ts (1)
12-12: Confirm verify-email route semantics (Auth0 callback is GET) and auth bypass as neededUsing learnings noted on September 2, 2025: Auth0 verification callbacks hit a GET endpoint due to browser redirect. If this new router is the callback, it should expose a public GET handler; if it’s an on-demand sync initiated by the UI, POST is fine—ensure a separate public GET callback exists and that AuthInterceptor allows anonymous access where required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(4 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(2 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/http-sdk/src/auth/index.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/api/src/auth/services/auth0/auth0.service.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/api/src/user/services/user/user.service.spec.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/api/src/auth/services/ability/ability.service.ts
- apps/api/src/auth/routes/verify-email/verify-email.router.ts
- apps/api/src/auth/controllers/auth/auth.controller.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/api/src/auth/services/auth0/auth0.service.spec.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- apps/api/src/auth/http-schemas/verify-email.schema.ts
- packages/http-sdk/src/auth/auth-http.service.ts
- packages/http-sdk/src/index.ts
- packages/http-sdk/src/auth/auth-http.types.ts
- apps/api/src/user/services/user/user.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/rest-app.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/rest-app.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
🧬 Code graph analysis (1)
apps/api/src/rest-app.ts (1)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
verifyEmailRouter(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (2)
apps/api/src/rest-app.ts (2)
126-127: Registration + OpenAPI wiring looks goodRouter is grouped next to sendVerificationEmailRouter and will appear in docs. LGTM.
126-127: No legacy “email-verified” routes remain Confirmed no occurrences ofemail-verifiedoremailVerifiedRouterin the codebase.
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
ad72dbc to
6b5d19e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/http-sdk/src/auth/auth-http.types.ts (1)
1-9: Do we need to ship the runtime Zod schema in the SDK, or is a type-only export sufficient?Echoing the earlier question: exporting the schema pulls zod into consumer bundles if they import this symbol; a type-only export avoids that.
Two options:
- If schema isn’t required in the SDK, simplify to type-only to minimize bundle size:
-import { z } from "zod"; - -export const VerifyEmailResponseSchema = z.object({ - data: z.object({ - emailVerified: z.boolean() - }) -}); - -export type VerifyEmailResponse = z.infer<typeof VerifyEmailResponseSchema>; +export type VerifyEmailResponse = { + data: { emailVerified: boolean }; +};
- If keeping the schema is desired for client-side parsing, consider relocating it to a separate module path (e.g., auth/verify-email.schema.ts) and only re-export the type from your package root so tree-shaking can drop zod for type-only consumers. I can draft the re-exports if you want.
apps/api/src/auth/services/ability/ability.service.ts (1)
8-8: VerifyUserOutputis re-exported from@src/user/repositories.Import path looks good, but ensure the package re-exports
UserOutputfrom its index to avoid type-resolution drift.Run:
#!/bin/bash # Expect to see a type re-export of UserOutput in repositories' index rg -nP 'export\s+type\s+{?\s*UserOutput\s*}?' apps/api/src/user/repositories -g '!*dist/**' -C2If missing, add in apps/api/src/user/repositories/index.ts:
export type { UserOutput } from "./user/user.repository";
🧹 Nitpick comments (4)
packages/http-sdk/src/auth/auth-http.types.ts (2)
3-7: Consider making the schema strict to catch unexpected fields.Failing fast on unknown keys helps detect server drift early.
Apply this diff:
-export const VerifyEmailResponseSchema = z.object({ - data: z.object({ - emailVerified: z.boolean() - }) -}); +export const VerifyEmailResponseSchema = z + .object({ + data: z + .object({ + emailVerified: z.boolean(), + }) + .strict(), + }) + .strict();
3-7: Naming/semantics check: “verify-email” vs “sync email_verified”.Per prior learning, Auth0’s email verification callback is typically a GET redirect endpoint, while this PR mentions an on-demand sync (often POST). Ensure these flows aren’t conflated and naming makes the intent obvious to callers.
If this schema is for the on-demand sync, consider a more explicit name (e.g., SyncEmailVerifiedResponseSchema) and keep the callback flow separate.
apps/api/src/user/services/user/user.service.spec.ts (1)
311-313: Extract and expose the Auth0Service mock in setup
Defineconst auth0Service = mock<Auth0Service>(), pass it into yourUserServiceconstructor call, and return it from the setup helper so you can assert on its calls (e.g. ensure it’s not called inregisterUserand stub/verify it insyncEmailVerified).apps/api/src/auth/services/ability/ability.service.ts (1)
21-21: Optional: Rename action to reduce confusion with Auth0’s email verification redirect.Consider
sync-email-verifiedorsyncEmailVerifiedto disambiguate this server-side sync from the Auth0 GET verification callback flow.Also applies to: 27-27, 37-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(4 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(2 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/api/src/rest-app.ts
- packages/http-sdk/src/index.ts
- apps/api/src/auth/http-schemas/verify-email.schema.ts
- apps/api/src/auth/controllers/auth/auth.controller.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- packages/http-sdk/src/auth/index.ts
- apps/api/src/auth/services/auth0/auth0.service.spec.ts
- apps/api/src/auth/routes/verify-email/verify-email.router.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- packages/http-sdk/src/auth/auth-http.service.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/user/services/user/user.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/http-sdk/src/auth/auth-http.types.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/http-sdk/src/auth/auth-http.types.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/user/services/user/user.service.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-28T10:40:13.595Z
Learnt from: stalniy
PR: akash-network/console#1480
File: apps/deploy-web/src/hooks/useFlag.tsx:0-0
Timestamp: 2025-07-28T10:40:13.595Z
Learning: In the Akash Network Console project, backend-specific feature flags are intentional and acceptable. The frontend FeatureFlag union type should only include flags that are actually used by the frontend, not all flags defined in the backend. Backend can have internal feature flags for backend-only functionality without requiring frontend synchronization.
Applied to files:
apps/api/src/auth/services/ability/ability.service.ts
🧬 Code graph analysis (1)
apps/api/src/auth/services/ability/ability.service.ts (1)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (4)
packages/http-sdk/src/auth/auth-http.types.ts (1)
3-9: LGTM: schema and exported type match the intended response envelope.The nested data.emailVerified boolean is clear and consistent, and the exported type will flow nicely to consumers.
apps/api/src/user/services/user/user.service.spec.ts (1)
14-14: Type-only import is correct.Using
import type { Auth0Service } ...avoids runtime deps and circulars. No action needed.apps/api/src/auth/services/ability/ability.service.ts (2)
56-56: LGTM: Signature now acceptsUserOutput, matching rule templates (user.id,user.email).
21-21: Verifyability.canusage andlodash.templateSettingsconfiguration
No calls toability.can("verify-email", "User", …)were detected inapps/api/src/auth/services/ability/ability.service.tsor elsewhere in the API, and no customtemplateSettings.interpolateoverride was found for parsing${…}placeholders. Manually confirm that everyability.can("verify-email", "User", { email })invocation passes the subject object with an exactcompileRulesis configured withtemplateSettings.interpolate = /\$\{([\s\S]+?)\}/g(or equivalent) so that values are JSON-safe and null/undefined are handled correctly.
| getAbilityFor(role: Role, user: UserOutput) { | ||
| const compiledRules = this.compileRules(); | ||
| return this.toAbility(compiledRules[role]({ user })); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Coerce nullable fields before templating to avoid "undefined"/"null" string values.
Passing a sanitized user context prevents accidental stringification of nullish values and reduces template fragility.
Apply:
- getAbilityFor(role: Role, user: UserOutput) {
- const compiledRules = this.compileRules();
- return this.toAbility(compiledRules[role]({ user }));
- }
+ getAbilityFor(role: Role, user: UserOutput) {
+ const compiledRules = this.compileRules();
+ // Prevent null/undefined from becoming literal strings in JSON
+ const userCtx: UserOutput & { email?: string } = {
+ ...user,
+ // Coerce to empty string; if you prefer to skip the rule entirely when no email,
+ // handle that in RULES/template or controller.
+ email: (user as any)?.email ?? ""
+ };
+ return this.toAbility(compiledRules[role]({ user: userCtx }));
+ }Committable suggestion skipped: line range outside the PR's diff.
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
6b5d19e to
1d56621
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/api/src/auth/services/ability/ability.service.ts (2)
8-8: EnsureUserOutputis re-exported from@src/user/repositories.This import relies on an index re-export. Without it, TS will fail to resolve the type.
Apply outside this file:
// apps/api/src/user/repositories/index.ts +export type { UserOutput } from "./user/user.repository";Run to verify:
#!/bin/bash fd -a index.ts apps/api/src/user/repositories | while read f; do echo "== $f ==" rg -nP 'export\s+type\s+UserOutput\b' -C2 "$f" || echo "Missing re-export" done
56-59: Sanitize the user context before templating to keep JSON valid and rules reliable.Coerce
- getAbilityFor(role: Role, user: UserOutput) { - const compiledRules = this.compileRules(); - return this.toAbility(compiledRules[role]({ user })); - } + getAbilityFor(role: Role, user: UserOutput) { + const compiledRules = this.compileRules(); + const userCtx = { + ...user, + email: typeof user.email === "string" ? user.email.trim() : "" + // if DB stores emails lowercased, consider: .toLowerCase() + }; + return this.toAbility(compiledRules[role]({ user: userCtx })); + }apps/api/src/user/services/user/user.service.ts (1)
126-141: Normalize email, coerce boolean, and 404 when local user not foundPrevents mismatches due to casing/whitespace, avoids undefined email_verified, and eliminates downstream destructuring of undefined. Mirrors earlier feedback.
- async syncEmailVerified({ email }: { email: string }) { - const auth0User = await this.auth0.getUserByEmail(email); - assert(auth0User, 404); - - return await this.userRepository.updateBy( - { - email - }, - { - emailVerified: auth0User.email_verified - }, - { - returning: true - } - ); - } + async syncEmailVerified({ email }: { email: string }) { + const normalizedEmail = email.trim().toLowerCase(); + const auth0User = await this.auth0.getUserByEmail(normalizedEmail); + assert(auth0User, 404); + + const emailVerified = !!auth0User.email_verified; + const updated = await this.userRepository.updateBy( + { email: normalizedEmail }, + { emailVerified }, + { returning: true } + ); + assert(updated, 404); + return updated; + }apps/api/src/auth/services/auth0/auth0.service.spec.ts (1)
87-98: Short-circuit on empty email: don’t call Auth0Aligns behavior with the intent to avoid unnecessary external calls.
- it("should handle empty email string", async () => { + it("should short-circuit on empty email string", async () => { const email = ""; const { auth0Service, mockGetByEmail } = setup({ mockUsers: [] }); const result = await auth0Service.getUserByEmail(email); expect(result).toBeNull(); - expect(mockGetByEmail).toHaveBeenCalledWith({ email: "" }); + expect(mockGetByEmail).not.toHaveBeenCalled(); });
🧹 Nitpick comments (5)
apps/api/src/auth/services/ability/ability.service.ts (1)
21-21: Guard against null/undefined email inverify-emailconditions.If
user.emailis absent, the template may produce empty or literal "undefined"/"null" strings in conditions, breaking authorization for legitimate requests. Prefer sanitizing the user context before templating (see next comment).Also applies to: 27-27, 37-37
apps/api/src/auth/http-schemas/verify-email.schema.ts (3)
3-7: Make request schema strict; confirm transform order before email() validation
- Add .strict() to both objects to reject unknown keys instead of silently stripping them.
- Confirm Zod applies trim/toLowerCase before email() validation; if not, use z.preprocess to normalize first.
-export const VerifyEmailRequestSchema = z.object({ - data: z.object({ - email: z.string().trim().toLowerCase().email({ message: "Email is invalid." }) - }) -}); +export const VerifyEmailRequestSchema = z + .object({ + data: z + .object({ + email: z.string().trim().toLowerCase().email({ message: "Email is invalid." }) + }) + .strict() + }) + .strict();
3-7: Naming/flow clarity: don’t conflate POST sync with Auth0 redirect callbackPer your team learning, Auth0 email verification redirects should hit a GET callback. Ensure this POST schema/route is only for on-demand sync and is not used as the redirect target; consider a clearer path like POST /v1/users/email-verified:sync to avoid confusion.
11-15: Make response schema strictPrevents accidental extra fields from leaking into the response.
-export const VerifyEmailResponseSchema = z.object({ - data: z.object({ - emailVerified: z.boolean() - }) -}); +export const VerifyEmailResponseSchema = z + .object({ + data: z + .object({ + emailVerified: z.boolean() + }) + .strict() + }) + .strict();apps/api/src/auth/services/auth0/auth0.service.spec.ts (1)
100-141: Move setup() to bottom of the root describe blockMatches repo testing guidelines and keeps test structure consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(4 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(2 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(2 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/http-sdk/src/auth/auth-http.types.ts
- packages/http-sdk/src/auth/index.ts
- apps/api/test/functional/verify-email.spec.ts
- apps/api/src/auth/controllers/auth/auth.controller.ts
- apps/api/src/rest-app.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
- apps/deploy-web/src/queries/useVerifyEmailQuery.ts
- apps/deploy-web/src/pages/user/verify-email/index.tsx
- apps/api/src/user/services/user/user.service.spec.ts
- apps/api/src/auth/services/auth0/auth0.service.ts
- packages/http-sdk/src/auth/auth-http.service.ts
- packages/http-sdk/src/index.ts
- apps/api/src/auth/routes/verify-email/verify-email.router.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/src/auth/services/ability/ability.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/auth/services/auth0/auth0.service.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-28T10:40:13.595Z
Learnt from: stalniy
PR: akash-network/console#1480
File: apps/deploy-web/src/hooks/useFlag.tsx:0-0
Timestamp: 2025-07-28T10:40:13.595Z
Learning: In the Akash Network Console project, backend-specific feature flags are intentional and acceptable. The frontend FeatureFlag union type should only include flags that are actually used by the frontend, not all flags defined in the backend. Backend can have internal feature flags for backend-only functionality without requiring frontend synchronization.
Applied to files:
apps/api/src/auth/services/ability/ability.service.ts
🧬 Code graph analysis (1)
apps/api/src/auth/services/ability/ability.service.ts (1)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
jzsfkzm
force-pushed
the
bugfixes/1863-sync-email-verified
branch
from
b418cb5 to
81745b3
Compare
There was a problem hiding this comment.
this is marked a singleton higher so the container should make sure this would be a singleton.
basically this should work:
import { ManagementClient } from "auth0";
import { container } from "tsyringe";
import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service";
export const AUTH0_MANAGEMENT_CLIENT = "AUTH0_MANAGEMENT_CLIENT";
container.register(AUTH0_MANAGEMENT_CLIENT, {
useFactory: c => {
const config = c.resolve(AuthConfigService);
return new ManagementClient({
domain: config.get("AUTH0_M2M_DOMAIN"),
clientId: config.get("AUTH0_M2M_CLIENT_ID"),
clientSecret: config.get("AUTH0_M2M_SECRET")
});
}
});
class Foo () {
constructor(@inject(AUTH0_MANAGEMENT_CLIENT) private readonly managementClient: ManagementClient) {}
}or
import { ManagementClient } from "auth0";
import { container } from "tsyringe";
import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service";
container.register(ManagementClient, {
useFactory: c => {
const config = c.resolve(AuthConfigService);
return new ManagementClient({
domain: config.get("AUTH0_M2M_DOMAIN"),
clientId: config.get("AUTH0_M2M_CLIENT_ID"),
clientSecret: config.get("AUTH0_M2M_SECRET")
});
}
});
// above needs to be imported somewhere in the root of the app
class Foo () {
constructor(private readonly managementClient: ManagementClient) {}
}There was a problem hiding this comment.
Simplified it a lot now, but the instance injected still has to be a singleton - otherwise the instance used in production and the one we get from DI in the test will be different instances, so when we mock the latter, the prior will just go unmocked, and cause errors, and tests will break this way.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
34-37: Fix incorrect OpenAPI response description (copy/paste).The route resends a verification email; “Returns a created wallet” is misleading.
responses: { - description: "Returns a created wallet", + description: "Verification email re-sent", content: {} } }
♻️ Duplicate comments (6)
apps/deploy-web/src/queries/useVerifyEmailQuery.ts (2)
7-11: Surface the actual Error to callers.Pass the error object so UIs can render details or retry logic.
export function useVerifyEmail( options: { onSuccess?: (isVerified: boolean) => void; - onError?: () => void; + onError?: (error: Error) => void; } = {} ) {
14-22: Propagate error and add a mutation key (helps devtools and caching semantics).- return useMutation<VerifyEmailResponse, Error, string>({ + return useMutation<VerifyEmailResponse, Error, string>({ + mutationKey: ["verifyEmail"], mutationFn: (email: string) => auth.verifyEmail(email), onSuccess: response => { options.onSuccess?.(response.data.emailVerified); }, - onError: _error => { - options.onError?.(); + onError: error => { + options.onError?.(error); } });apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
40-45: Register via openapi() and use validated payloads.Using post() omits OpenAPI registration and route-level validation; wire with openapi(route, …) and use c.req.valid("json").
Apply:
-verifyEmailRouter.post(route.path, async function verifyEmail(c) { - const data = VerifyEmailRequestSchema.parse(await c.req.json()); +verifyEmailRouter.openapi(route, async function verifyEmail(c) { + const data = c.req.valid("json"); const result = await container.resolve(AuthController).syncEmailVerified(data); return c.json(result, 200); });apps/api/src/auth/services/auth0/auth0.service.spec.ts (1)
65-76: Short-circuit empty email; don’t call Auth0.Prefer returning null without hitting the API when email is empty; update the test accordingly.
- expect(result).toBeNull(); - expect(mockGetByEmail).toHaveBeenCalledWith({ email: "" }); + expect(result).toBeNull(); + expect(mockGetByEmail).not.toHaveBeenCalled();Outside this file (service), add:
// in Auth0Service.getUserByEmail if (!email?.trim()) return null;apps/api/test/functional/verify-email.spec.ts (1)
70-89: Drive 404 via setup flag; avoid spying at two layers.Let setup control the Auth0 response (none vs. some) and drop the per-test spy.
- it("throws 404 when user is not found in auth0", async () => { - const { user, token } = await setup({ emailVerified: false }); - const auth0Service = container.resolve(Auth0Service); - jest.spyOn(auth0Service, "getUserByEmail").mockResolvedValue(null); + it("throws 404 when user is not found in auth0", async () => { + const { user, token } = await setup({ emailVerified: false, auth0UserFound: false }); @@ - async function setup(input: { emailVerified: boolean }) { + async function setup(input: { emailVerified: boolean; auth0UserFound?: boolean }) { @@ - .mockImplementation(async ({ email }: GetUsersByEmailRequest): Promise<ApiResponse<GetUsers200ResponseOneOfInner[]>> => { + .mockImplementation(async ({ email }: GetUsersByEmailRequest): Promise<ApiResponse<GetUsers200ResponseOneOfInner[]>> => { if (email.toLowerCase() === user.email.toLowerCase()) { - return Promise.resolve({ - data: [ - { - user_id: user.userId, - email_verified: input.emailVerified - } - ] - } as ApiResponse<GetUsers200ResponseOneOfInner[]>); + return Promise.resolve({ + data: input.auth0UserFound === false + ? [] + : [{ + user_id: user.userId, + email_verified: input.emailVerified + }] + } as ApiResponse<GetUsers200ResponseOneOfInner[]>); }Also applies to: 126-154
apps/api/src/user/services/user/user.service.ts (1)
126-141: Normalize email, coerce boolean, assert local update exists, and add explicit return typePrevents case-mismatch no-ops and undefined returns when no local user row matches. Also makes the method contract explicit.
- async syncEmailVerified({ email }: { email: string }) { - const auth0User = await this.auth0.getUserByEmail(email); - assert(auth0User, 404); - - return await this.userRepository.updateBy( - { - email - }, - { - emailVerified: auth0User.email_verified - }, - { - returning: true - } - ); - } + async syncEmailVerified({ email }: { email: string }): Promise<UserOutput> { + const normalizedEmail = email.trim().toLowerCase(); + const auth0User = await this.auth0.getUserByEmail(normalizedEmail); + assert(auth0User, 404); + + const emailVerified = !!auth0User.email_verified; + const updated = await this.userRepository.updateBy( + { email: normalizedEmail }, + { emailVerified }, + { returning: true } + ); + assert(updated, 404); + return updated; + }
🧹 Nitpick comments (14)
apps/api/test/services/wallet-testing.service.ts (1)
93-95: Normalize provided emails too to avoid case-sensitivity bugs.Lowercase the explicit claims.email as well to keep test data and lookups consistent.
- email: claims.email ?? faker.internet.email().toLowerCase(), + email: (claims.email ?? faker.internet.email()).toLowerCase(),apps/api/src/auth/services/ability/ability.service.ts (1)
21-21: Avoid templating undefined emails into rules.When user.email is missing, the template injects "undefined". Sanitize the user context before templating.
getAbilityFor(role: Role, user: UserOutput) { const compiledRules = this.compileRules(); - return this.toAbility(compiledRules[role]({ user })); + const userCtx = { ...user, email: user.email ?? "" }; + return this.toAbility(compiledRules[role]({ user: userCtx })); }Also applies to: 27-27, 37-37
apps/api/src/rest-app.ts (1)
12-12: Optional: consider a more explicit route name.To avoid confusion with Auth0’s GET email-verification callback (redirect), consider naming the POST as “sync-email-verified”.
packages/http-sdk/src/auth/auth-http.service.ts (2)
15-17: Add explicit return type for public API stability.Lock the method’s return type to avoid accidental widening in future edits.
- async verifyEmail(email: string) { + async verifyEmail(email: string): Promise<VerifyEmailResponse> { return this.extractData(await this.post<VerifyEmailResponse>("/v1/verify-email", { data: { email } }, { withCredentials: true })); }
11-13: Consider error handling/CSRF posture (withCredentials).Since this call sends cookies, ensure the API route is protected against CSRF (e.g., SameSite=strict/lax and/or CSRF token).
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (2)
41-43: Rename handler for clarity (matches route).Minor naming nit to avoid confusion during tracing.
-sendVerificationEmailRouter.openapi(route, async function verifyEmail(c) { +sendVerificationEmailRouter.openapi(route, async function sendVerificationEmail(c) {
18-21: Consider 204 No Content for side-effect-only action.Semantics might fit better since no body is returned.
- method: "post", + method: "post", path: "/v1/send-verification-email", @@ - return c.text("", 200) as never; + return c.body(null, 204) as never;apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
26-37: Add 403 to OpenAPI responses.The controller enforces ability checks and tests assert 403; expose it in the spec.
responses: { @@ - 401: { description: "Unauthorized" }, + 401: { description: "Unauthorized" }, + 403: { description: "Forbidden" }, }apps/api/src/auth/services/auth0/auth0.service.spec.ts (2)
79-87: Remove unused AuthConfigService mock.It’s created but never used; drop it to reduce noise.
- const mockAuthConfig = mock<AuthConfigService>(); - mockAuthConfig.get.mockImplementation((key: string) => { - const config = { - AUTH0_M2M_DOMAIN: "test-domain.auth0.com", - AUTH0_M2M_CLIENT_ID: "test-client-id", - AUTH0_M2M_SECRET: "test-client-secret" - }; - return config[key as keyof typeof config]; - });
10-12: Place setup() at the bottom of the root describe.House style: define setup at the end of the root describe block, not inside nested describes.
Also applies to: 78-112
packages/ui/components/auto-button.tsx (1)
18-21: Clear the timer on unmount to avoid stray navigations.Without cleanup, the scheduled onClick may fire after unmount.
useEffect(() => { setStarted(true); - setTimeout(onClick, timeout); - }, [onClick, timeout]); + const id = window.setTimeout(onClick, timeout); + return () => window.clearTimeout(id); + }, [onClick, timeout]);apps/api/test/seeders/auth0-user.seeder.ts (1)
7-8: Normalize faker email to lowercase for consistency.Aligns with backend comparisons and schema normalization.
- email = faker.internet.email(), + email = faker.internet.email().toLowerCase(),apps/api/src/user/services/user/user.service.ts (2)
126-129: Avoid email-enumeration via distinct 404s; ensure authZ binds email to callerIf the route is callable by end users, 404 for unknown Auth0 user leaks existence. Derive email from the authenticated principal instead of accepting arbitrary input, or map not-found to a generic 204/200 with no body at the controller layer.
Can you confirm the controller requires authentication and uses the current user’s email (ignoring the request body)?
1-1: Optional: use a typed assertion helper for TS narrowing
http-assertdoesn’t narrow types. A small helper improves type safety at call sites.Add once (e.g., core/utils/assert.ts):
export function assertPresent<T>(value: T | null | undefined, status = 404): asserts value is T { assert(value, status); }Then replace
assert(auth0User, 404);withassertPresent(auth0User, 404);to getauth0Usernarrowed to non-null.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
apps/api/src/auth/controllers/auth/auth.controller.ts(2 hunks)apps/api/src/auth/http-schemas/verify-email.schema.ts(1 hunks)apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts(1 hunks)apps/api/src/auth/routes/verify-email/verify-email.router.ts(1 hunks)apps/api/src/auth/services/ability/ability.service.ts(4 hunks)apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(1 hunks)apps/api/src/core/providers/auth.provider.ts(1 hunks)apps/api/src/core/providers/index.ts(1 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/src/user/services/user/user.service.spec.ts(2 hunks)apps/api/src/user/services/user/user.service.ts(3 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/seeders/auth0-user.seeder.ts(1 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.ts(1 hunks)packages/http-sdk/src/auth/auth-http.service.ts(2 hunks)packages/http-sdk/src/auth/auth-http.types.ts(1 hunks)packages/http-sdk/src/auth/index.ts(1 hunks)packages/http-sdk/src/index.ts(1 hunks)packages/ui/components/auto-button.css(1 hunks)packages/ui/components/auto-button.tsx(1 hunks)packages/ui/components/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/ui/components/index.tsxpackages/http-sdk/src/auth/index.tsapps/deploy-web/src/pages/user/verify-email/index.tsxapps/api/test/seeders/index.tspackages/http-sdk/src/auth/auth-http.service.tspackages/ui/components/auto-button.tsxapps/api/test/services/wallet-testing.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/test/seeders/auth0-user.seeder.tsapps/api/src/core/providers/auth.provider.tsapps/deploy-web/src/queries/useVerifyEmailQuery.tsapps/api/src/auth/services/auth0/auth0.service.tspackages/http-sdk/src/index.tspackages/http-sdk/src/auth/auth-http.types.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/rest-app.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/auth/routes/verify-email/verify-email.router.tsapps/api/src/core/providers/index.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/services/ability/ability.service.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/ui/components/index.tsxpackages/http-sdk/src/auth/index.tsapps/deploy-web/src/pages/user/verify-email/index.tsxapps/api/test/seeders/index.tspackages/http-sdk/src/auth/auth-http.service.tspackages/ui/components/auto-button.tsxapps/api/test/services/wallet-testing.service.tsapps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/test/seeders/auth0-user.seeder.tsapps/api/src/core/providers/auth.provider.tsapps/deploy-web/src/queries/useVerifyEmailQuery.tsapps/api/src/auth/services/auth0/auth0.service.tspackages/http-sdk/src/index.tspackages/http-sdk/src/auth/auth-http.types.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/auth/controllers/auth/auth.controller.tsapps/api/src/rest-app.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/auth/http-schemas/verify-email.schema.tsapps/api/src/auth/routes/verify-email/verify-email.router.tsapps/api/src/core/providers/index.tsapps/api/src/user/services/user/user.service.tsapps/api/src/auth/services/ability/ability.service.tsapps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/auth/services/auth0/auth0.service.spec.tsapps/api/test/functional/verify-email.spec.tsapps/api/src/user/services/user/user.service.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-28T10:40:13.595Z
Learnt from: stalniy
PR: akash-network/console#1480
File: apps/deploy-web/src/hooks/useFlag.tsx:0-0
Timestamp: 2025-07-28T10:40:13.595Z
Learning: In the Akash Network Console project, backend-specific feature flags are intentional and acceptable. The frontend FeatureFlag union type should only include flags that are actually used by the frontend, not all flags defined in the backend. Backend can have internal feature flags for backend-only functionality without requiring frontend synchronization.
Applied to files:
apps/api/src/auth/services/ability/ability.service.ts
🧬 Code graph analysis (12)
apps/deploy-web/src/pages/user/verify-email/index.tsx (4)
packages/ui/components/auto-button.tsx (1)
AutoButton(15-34)packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(15-17)apps/deploy-web/src/queries/useVerifyEmailQuery.ts (1)
useVerifyEmail(6-23)apps/deploy-web/src/components/layout/Layout.tsx (1)
Loading(151-162)
packages/http-sdk/src/auth/auth-http.service.ts (1)
packages/http-sdk/src/auth/auth-http.types.ts (1)
VerifyEmailResponse(9-9)
packages/ui/components/auto-button.tsx (1)
apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)
apps/api/src/auth/services/auth0/auth0.service.spec.ts (1)
apps/api/test/seeders/auth0-user.seeder.ts (1)
Auth0UserSeeder(4-22)
apps/api/src/core/providers/auth.provider.ts (1)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
singleton(4-20)
apps/deploy-web/src/queries/useVerifyEmailQuery.ts (2)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)packages/http-sdk/src/auth/auth-http.types.ts (1)
VerifyEmailResponse(9-9)
apps/api/test/functional/verify-email.spec.ts (1)
apps/api/test/services/wallet-testing.service.ts (1)
WalletTestingService(15-156)
apps/api/src/auth/controllers/auth/auth.controller.ts (5)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
singleton(4-20)apps/api/src/auth/services/ability/ability.service.ts (1)
singleton(12-83)apps/api/src/user/services/user/user.service.ts (1)
singleton(13-142)apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
VerifyEmailRequest(9-9)
apps/api/src/rest-app.ts (1)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (1)
verifyEmailRouter(9-9)
apps/api/src/auth/routes/verify-email/verify-email.router.ts (4)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
VerifyEmailRequestSchema(3-7)packages/http-sdk/src/auth/auth-http.types.ts (1)
VerifyEmailResponseSchema(3-7)packages/http-sdk/src/auth/auth-http.service.ts (1)
verifyEmail(15-17)
apps/api/src/auth/services/ability/ability.service.ts (1)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)
apps/api/src/auth/routes/send-verification-email/send-verification-email.router.ts (1)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (13)
apps/api/test/seeders/index.ts (1)
2-2: LGTM — exposing the new Auth0 user seeder.Makes the seeder discoverable for tests.
apps/api/src/auth/services/ability/ability.service.ts (1)
8-8: Good: align AbilityService with actual user shape.Importing UserOutput and updating getAbilityFor signature removes the previous mismatch with rule templates.
Also applies to: 56-59
packages/ui/components/index.tsx (1)
37-38: LGTM — public surfacing of AutoButton.Keeps component imports consistent via the central index.
apps/api/src/rest-app.ts (1)
12-12: LGTM — route registered and included in OpenAPI.Import and registration order look correct next to sendVerificationEmailRouter.
Also applies to: 126-127
apps/api/src/core/providers/index.ts (1)
1-1: LGTM — provider side-effect import and re-export.Ensures the ManagementClient is registered and available for DI.
Also applies to: 6-6
packages/http-sdk/src/index.ts (1)
12-12: Barrel export for auth looks good.Public surface stays consistent; no issues spotted.
apps/api/src/core/providers/auth.provider.ts (1)
24-26: Singleton wiring via factory is fine.Factory returns a cached instance; DI will reuse it across the app.
apps/api/src/user/services/user/user.service.spec.ts (2)
14-14: Type-only import for Auth0Service is appropriate.Keeps runtime clean while enabling strong typing.
311-313: Good use of jest-mock-extended for new dependency.Constructor wiring matches the updated signature.
packages/http-sdk/src/auth/auth-http.types.ts (1)
1-9: Retain Zod schema—it's used at runtime
VerifyEmailResponseSchema is imported in apps/api/src/auth/routes/verify-email/verify-email.router.ts (line 31) and passed into createRoute for runtime validation; removing it would break the API.Likely an incorrect or invalid review comment.
apps/api/src/auth/http-schemas/verify-email.schema.ts (1)
3-7: LGTM. Schema is succinct and correct.Trim + lowercase + email validation covers the key edge cases.
apps/api/src/auth/controllers/auth/auth.controller.ts (1)
27-37: LGTM. Authorization boundary + service delegation is clean.Permission check is explicit; controller returns a minimal response; business logic is in UserService.
apps/api/src/user/services/user/user.service.ts (1)
5-5: LGTM: Auth0Service integration importImport looks correct and consistent with the new dependency.
| async getUserByEmail(email: string): Promise<GetUsers200ResponseOneOfInner | null> { | ||
| const { data: users } = await this.managementClient.usersByEmail.getByEmail({ email }); | ||
| if (users.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return users[0]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Short-circuit invalid emails and normalize before querying Auth0.
Prevents unnecessary 400s and ensures consistent lookups.
- async getUserByEmail(email: string): Promise<GetUsers200ResponseOneOfInner | null> {
- const { data: users } = await this.managementClient.usersByEmail.getByEmail({ email });
+ async getUserByEmail(email: string): Promise<GetUsers200ResponseOneOfInner | null> {
+ const normalized = email?.trim().toLowerCase();
+ if (!normalized) return null;
+ const { data: users } = await this.managementClient.usersByEmail.getByEmail({ email: normalized });
if (users.length === 0) {
return null;
}
return users[0];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getUserByEmail(email: string): Promise<GetUsers200ResponseOneOfInner | null> { | |
| const { data: users } = await this.managementClient.usersByEmail.getByEmail({ email }); | |
| if (users.length === 0) { | |
| return null; | |
| } | |
| return users[0]; | |
| } | |
| async getUserByEmail(email: string): Promise<GetUsers200ResponseOneOfInner | null> { | |
| const normalized = email?.trim().toLowerCase(); | |
| if (!normalized) return null; | |
| const { data: users } = await this.managementClient.usersByEmail.getByEmail({ email: normalized }); | |
| if (users.length === 0) { | |
| return null; | |
| } | |
| return users[0]; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/auth/services/auth0/auth0.service.ts around lines 12 to 19,
short-circuit invalid emails and normalize before calling Auth0: validate the
input email (e.g., simple RFC-like regex or existing validator lib), trim
whitespace and lowercase it, return null immediately for empty or invalid emails
to avoid a 400 from Auth0, and only then call
this.managementClient.usersByEmail.getByEmail with the normalized email so
lookups are consistent.
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fail fast on missing Auth0 M2M config.
Throw early if any required config is absent to avoid latent runtime errors.
static getInstance(): ManagementClient {
if (!ManagementClientFactory.instance) {
const authConfig = container.resolve(AuthConfigService);
+ const domain = authConfig.get("AUTH0_M2M_DOMAIN");
+ const clientId = authConfig.get("AUTH0_M2M_CLIENT_ID");
+ const clientSecret = authConfig.get("AUTH0_M2M_SECRET");
+ if (!domain || !clientId || !clientSecret) {
+ throw new Error("Missing Auth0 M2M configuration (domain/clientId/clientSecret).");
+ }
ManagementClientFactory.instance = new ManagementClient({
- domain: authConfig.get("AUTH0_M2M_DOMAIN"),
- clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"),
- clientSecret: authConfig.get("AUTH0_M2M_SECRET")
+ domain,
+ clientId,
+ clientSecret
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static getInstance(): ManagementClient { | |
| if (!ManagementClientFactory.instance) { | |
| const authConfig = container.resolve(AuthConfigService); | |
| ManagementClientFactory.instance = new ManagementClient({ | |
| domain: authConfig.get("AUTH0_M2M_DOMAIN"), | |
| clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), | |
| clientSecret: authConfig.get("AUTH0_M2M_SECRET") | |
| }); | |
| static getInstance(): ManagementClient { | |
| if (!ManagementClientFactory.instance) { | |
| const authConfig = container.resolve(AuthConfigService); | |
| const domain = authConfig.get("AUTH0_M2M_DOMAIN"); | |
| const clientId = authConfig.get("AUTH0_M2M_CLIENT_ID"); | |
| const clientSecret = authConfig.get("AUTH0_M2M_SECRET"); | |
| if (!domain || !clientId || !clientSecret) { | |
| throw new Error("Missing Auth0 M2M configuration (domain/clientId/clientSecret)."); | |
| } | |
| ManagementClientFactory.instance = new ManagementClient({ | |
| domain, | |
| clientId, | |
| clientSecret | |
| }); | |
| } | |
| return ManagementClientFactory.instance; | |
| } |
| private readonly notificationService: NotificationService, | ||
| private readonly auth0: Auth0Service |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
DI wiring check for Auth0Service
New constructor dep requires container wiring. Ensure Auth0Service is decorated (@singleton/@Injectable) or registered, and tests updated to provide it.
🏁 Script executed:
#!/bin/bash
# Verify DI wiring for Auth0Service
rg -n --type=ts -C2 $'@singleton|@injectable' | rg -n 'Auth0Service' || true
rg -n --type=ts -C3 'registerSingleton\\(|container\\.register\\(|container\\.registerInstance\\(' | rg -n 'Auth0Service' || true
# Sanity: ensure no manual `new Auth0Service()` calls
rg -n --type=ts -C2 'new\\s+Auth0Service\\(' || trueLength of output: 381
🏁 Script executed:
#!/bin/bash
# 1. Check for decorator on Auth0Service
rg -n --type=ts -C2 '@(singleton|injectable)' | rg -n 'Auth0Service' || echo 'no decorator found'
# 2. Check for DI container registration of Auth0Service
rg -n --type=ts -C3 'registerSingleton\(|container\.register\(|registerInstance\(' | rg -n 'Auth0Service' || echo 'no registration found'
# 3. Check for manual instantiation of Auth0Service
rg -n --type=ts -C2 'new\s+Auth0Service\(' || echo 'no manual instantiation found'Length of output: 492
Register and decorate Auth0Service for DI
Add @singleton() or @injectable() above the Auth0Service class in apps/api/src/auth/services/auth0/auth0.service.ts, register it in your DI container setup (e.g. container.registerSingleton(Auth0Service)), and update both the auth0.service.spec.ts and any user.service specs to inject via the container (e.g. container.resolve(Auth0Service)) instead of calling new Auth0Service(...).
🤖 Prompt for AI Agents
In apps/api/src/user/services/user/user.service.ts around lines 20-21, the
Auth0Service is being constructed directly in tests rather than provided via the
DI container; open apps/api/src/auth/services/auth0/auth0.service.ts and add the
proper DI decorator (@singleton() or @injectable()) to the Auth0Service class,
export it if not already exported, then register it in your DI bootstrap (e.g.
container.registerSingleton(Auth0Service) or container.register(Auth0Service, {
useClass: Auth0Service })). After registering, update auth0.service.spec.ts and
any user.service specs to stop calling new Auth0Service(...) and instead obtain
instances from the container (e.g. container.resolve(Auth0Service)) or inject
the service via the container in tests so the user.service receives Auth0Service
through DI.
| export * from "./auth-http.service"; | ||
| export * from "./auth-http.types"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid re-exporting runtime schema to consumers.
Re-exporting the types module pulls the zod schema into the public surface; prefer type-only re-exports to keep web bundles lean.
-export * from "./auth-http.service";
-export * from "./auth-http.types";
+export * from "./auth-http.service";
+export type { VerifyEmailResponse } from "./auth-http.types";Run to confirm no external imports depend on VerifyEmailResponseSchema:
🏁 Script executed:
#!/bin/bash
rg -nP '\bVerifyEmailResponseSchema\b' -g '!**/node_modules/**' -C2Length of output: 148
🏁 Script executed:
#!/bin/bash
# Search for any references to the VerifyEmailResponseSchema across the repo
rg -nP '\bVerifyEmailResponseSchema\b' -C2Length of output: 1375
Preserve runtime schema export until consumer imports are updated
apps/api/src/auth/routes/verify-email/verify-email.router.ts imports VerifyEmailResponseSchema from the SDK index; removing its re-export will break this consumer—either update that import to reference auth-http.types directly or defer removing the schema export.
🤖 Prompt for AI Agents
In packages/http-sdk/src/auth/index.ts lines 1-2, the SDK currently re-exports
runtime schema(s) (e.g., VerifyEmailResponseSchema) that apps/api still imports
from the SDK index; removing that re-export will break the consumer. Either (A)
revert/keep the schema export in this index until the consumer import is
updated, or (B) update the consumer at
apps/api/src/auth/routes/verify-email/verify-email.router.ts to import
VerifyEmailResponseSchema directly from
packages/http-sdk/src/auth/auth-http.types (change the import path there), then
safely remove the schema re-export from the SDK index.
| .auto-button { | ||
| overflow: hidden; | ||
| z-index: 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Anchor the absolutely positioned overlay.
Without a positioning context, ::before may position relative to an ancestor and misalign.
.auto-button {
+ position: relative;
overflow: hidden;
z-index: 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .auto-button { | |
| overflow: hidden; | |
| z-index: 0; | |
| } | |
| .auto-button { | |
| position: relative; | |
| overflow: hidden; | |
| z-index: 0; | |
| } |
🤖 Prompt for AI Agents
In packages/ui/components/auto-button.css around lines 1 to 4, the .auto-button
rule lacks a positioning context so any absolutely positioned ::before overlay
can end up positioned relative to an ancestor; add position: relative to the
.auto-button selector to anchor the ::before/::after overlay (keeping overflow:
hidden and z-index as-is) so the pseudo-element aligns correctly within the
button.
| .auto-button::before { | ||
| content: ""; | ||
| position: absolute; | ||
| left: 0; | ||
| top: 36px; | ||
| height: 100%; | ||
| width: 0; | ||
| background: rgba(0, 0, 0, 0.25); | ||
| z-index: -1; | ||
| transition: width var(--timeout, 5s) linear; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix overlay positioning and interaction.
The overlay starts at top: 36px; likely unintended. Also make it non-interactive.
.auto-button::before {
content: "";
position: absolute;
left: 0;
- top: 36px;
+ top: 0;
height: 100%;
width: 0;
background: rgba(0, 0, 0, 0.25);
z-index: -1;
transition: width var(--timeout, 5s) linear;
+ pointer-events: none;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .auto-button::before { | |
| content: ""; | |
| position: absolute; | |
| left: 0; | |
| top: 36px; | |
| height: 100%; | |
| width: 0; | |
| background: rgba(0, 0, 0, 0.25); | |
| z-index: -1; | |
| transition: width var(--timeout, 5s) linear; | |
| } | |
| .auto-button::before { | |
| content: ""; | |
| position: absolute; | |
| left: 0; | |
| top: 0; | |
| height: 100%; | |
| width: 0; | |
| background: rgba(0, 0, 0, 0.25); | |
| z-index: -1; | |
| transition: width var(--timeout, 5s) linear; | |
| pointer-events: none; | |
| } |
🤖 Prompt for AI Agents
In packages/ui/components/auto-button.css around lines 6 to 16, the overlay
pseudo-element is positioned at top: 36px (so it doesn't cover the button) and
can capture pointer events; change the overlay to span the full element by
setting top: 0 (or use inset: 0), left: 0, width: 0 (keep animation) and height:
100%, add pointer-events: none to make it non-interactive, and ensure the host
element has position: relative so the absolute ::before is correctly anchored;
keep z-index so it stays behind content and retain the transition for width.
81745b3 to
08ff6e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/core/providers/auth.provider.ts (1)
10-17: Fail fast on missing Auth0 M2M config.Avoid lazy, hard-to-diagnose failures by validating required vars before constructing the client.
Apply:
static getInstance(): ManagementClient { if (!ManagementClientFactory.instance) { const authConfig = container.resolve(AuthConfigService); + const domain = authConfig.get("AUTH0_M2M_DOMAIN"); + const clientId = authConfig.get("AUTH0_M2M_CLIENT_ID"); + const clientSecret = authConfig.get("AUTH0_M2M_SECRET"); + if (!domain || !clientId || !clientSecret) { + throw new Error("Missing Auth0 M2M configuration (domain/clientId/clientSecret)."); + } ManagementClientFactory.instance = new ManagementClient({ - domain: authConfig.get("AUTH0_M2M_DOMAIN"), - clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), - clientSecret: authConfig.get("AUTH0_M2M_SECRET") + domain, + clientId, + clientSecret }); }
🧹 Nitpick comments (3)
apps/api/src/core/providers/auth.provider.ts (2)
6-7: @singleton on the factory is redundant.The class isn’t resolved via DI; a static cache controls instancing. Remove the decorator or switch to a DI-managed singleton.
-@singleton() class ManagementClientFactory {
24-26: Register explicitly as a DI singleton for clarity and testability.Make the lifetime explicit (or register an instance) to avoid confusion and ease mocking.
Option A — lifecycle singleton:
-import { container, singleton } from "tsyringe"; +import { container, singleton, Lifecycle } from "tsyringe"; ... -container.register(ManagementClient, { - useFactory: () => ManagementClientFactory.getInstance() -}); +container.register( + ManagementClient, + { useFactory: () => ManagementClientFactory.getInstance() }, + { lifecycle: Lifecycle.Singleton } +);Option B — drop the factory and register with DI only:
-import { container, singleton } from "tsyringe"; +import { container, Lifecycle } from "tsyringe"; -import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service"; +import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service"; - -@singleton() -class ManagementClientFactory { - ... -} - -container.register(ManagementClient, { - useFactory: () => ManagementClientFactory.getInstance() -}); +container.register( + ManagementClient, + { + useFactory: c => { + const cfg = c.resolve(AuthConfigService); + const domain = cfg.get("AUTH0_M2M_DOMAIN"); + const clientId = cfg.get("AUTH0_M2M_CLIENT_ID"); + const clientSecret = cfg.get("AUTH0_M2M_SECRET"); + if (!domain || !clientId || !clientSecret) { + throw new Error("Missing Auth0 M2M configuration (domain/clientId/clientSecret)."); + } + return new ManagementClient({ domain, clientId, clientSecret }); + } + }, + { lifecycle: Lifecycle.Singleton } +);apps/deploy-web/src/pages/user/verify-email/index.tsx (1)
19-21: Prefer Next.js router over window.location for client navigation.Keeps navigation SPA-friendly and testable.
-import React, { useCallback, useState } from "react"; +import React, { useCallback, useState } from "react"; +import { useRouter } from "next/navigation"; ... -function VerificationResult({ isVerified }: VerificationResultProps) { - const gotoHome = useCallback(() => { - window.location.href = UrlService.home(); - }, []); +function VerificationResult({ isVerified }: VerificationResultProps) { + const router = useRouter(); + const gotoHome = useCallback(() => { + router.push(UrlService.home()); + }, [router]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/api/src/auth/services/auth0/auth0.service.spec.ts(1 hunks)apps/api/src/auth/services/auth0/auth0.service.ts(1 hunks)apps/api/src/core/providers/auth.provider.ts(1 hunks)apps/api/src/core/providers/index.ts(1 hunks)apps/api/test/functional/verify-email.spec.ts(1 hunks)apps/api/test/seeders/auth0-user.seeder.ts(1 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/deploy-web/src/pages/user/verify-email/index.tsx(1 hunks)packages/ui/components/auto-button.css(1 hunks)packages/ui/components/auto-button.tsx(1 hunks)packages/ui/components/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/api/test/seeders/index.ts
- packages/ui/components/auto-button.css
- apps/api/test/seeders/auth0-user.seeder.ts
- packages/ui/components/index.tsx
- apps/api/src/auth/services/auth0/auth0.service.ts
- apps/api/src/auth/services/auth0/auth0.service.spec.ts
- packages/ui/components/auto-button.tsx
- apps/api/test/functional/verify-email.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/core/providers/auth.provider.tsapps/deploy-web/src/pages/user/verify-email/index.tsxapps/api/src/core/providers/index.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/core/providers/auth.provider.tsapps/deploy-web/src/pages/user/verify-email/index.tsxapps/api/src/core/providers/index.ts
🧬 Code graph analysis (2)
apps/api/src/core/providers/auth.provider.ts (1)
apps/api/src/auth/services/auth0/auth0.service.ts (1)
singleton(4-20)
apps/deploy-web/src/pages/user/verify-email/index.tsx (3)
packages/ui/components/auto-button.tsx (1)
AutoButton(16-35)apps/deploy-web/src/queries/useVerifyEmailQuery.ts (1)
useVerifyEmail(6-23)apps/deploy-web/src/components/layout/Layout.tsx (1)
Loading(151-162)
🔇 Additional comments (2)
apps/api/src/core/providers/index.ts (1)
1-6: Auth provider wiring looks good.Side-effect import ensures DI registration; barrel export exposes tokens for consumers. No issues.
apps/api/src/core/providers/auth.provider.ts (1)
1-26: ManagementClient constructor options are correct
TheManagementClientusesdomain,clientId, andclientSecret, matching the Auth0 Node.js SDK v4.x constructor signature. (github.com)
| {isVerifying ? ( | ||
| <Loading text="Just a moment while we finish verifying your email." /> | ||
| ) : ( | ||
| <> | ||
| <VerificationResult isVerified={isVerified === true} /> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix initial false-negative render and handle missing email param.
Gate on isVerified === null and show a specific state when email is absent to avoid flashing the error UI.
- {isVerifying ? (
- <Loading text="Just a moment while we finish verifying your email." />
- ) : (
- <>
- <VerificationResult isVerified={isVerified === true} />
- </>
- )}
+ {!email ? (
+ <VerificationResult isVerified={false} />
+ ) : isVerifying || isVerified === null ? (
+ <Loading text="Just a moment while we finish verifying your email." />
+ ) : (
+ <VerificationResult isVerified={isVerified === true} />
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isVerifying ? ( | |
| <Loading text="Just a moment while we finish verifying your email." /> | |
| ) : ( | |
| <> | |
| <VerificationResult isVerified={isVerified === true} /> | |
| </> | |
| )} | |
| {!email ? ( | |
| <VerificationResult isVerified={false} /> | |
| ) : isVerifying || isVerified === null ? ( | |
| <Loading text="Just a moment while we finish verifying your email." /> | |
| ) : ( | |
| <VerificationResult isVerified={isVerified === true} /> | |
| )} |
🤖 Prompt for AI Agents
In apps/deploy-web/src/pages/user/verify-email/index.tsx around lines 66 to 72,
the component currently renders a false-negative error flash because it shows
VerificationResult when isVerified is falsey; change the rendering logic to
treat isVerified === null as the "pending" state (show Loading) so the component
only renders VerificationResult after isVerified is true or false explicitly,
and add a guard: if the email query param is missing render a specific "missing
email" UI/message instead of VerificationResult to avoid confusing error
displays. Ensure comparisons use strict equality (isVerified === null / === true
/ === false) and short-circuit rendering accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/core/providers/auth.provider.ts (1)
13-17: Fail fast on missing Auth0 M2M config.Throw early to avoid late runtime failures and clearer boot-time diagnostics.
- managementClientInstance = new ManagementClient({ - domain: authConfig.get("AUTH0_M2M_DOMAIN"), - clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), - clientSecret: authConfig.get("AUTH0_M2M_SECRET") - }); + const domain = authConfig.get("AUTH0_M2M_DOMAIN"); + const clientId = authConfig.get("AUTH0_M2M_CLIENT_ID"); + const clientSecret = authConfig.get("AUTH0_M2M_SECRET"); + if (!domain || !clientId || !clientSecret) { + throw new Error("Missing Auth0 M2M configuration (domain/clientId/clientSecret)."); + } + return new ManagementClient({ domain, clientId, clientSecret });
🧹 Nitpick comments (10)
apps/api/src/core/providers/auth.provider.ts (3)
8-22: Use tsyringe’s Singleton lifecycle and the factory’s container param; drop manual cache.Cleaner, deterministic DI and better testability. Avoids global container lookups and redundant factory invocations.
-import { container } from "tsyringe"; +import { container, Lifecycle } from "tsyringe"; -import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service"; +import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service"; -let managementClientInstance: ManagementClient | null = null; - -container.register(ManagementClient, { - useFactory: () => { - if (!managementClientInstance) { - const authConfig = container.resolve(AuthConfigService); - - managementClientInstance = new ManagementClient({ - domain: authConfig.get("AUTH0_M2M_DOMAIN"), - clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), - clientSecret: authConfig.get("AUTH0_M2M_SECRET") - }); - } - - return managementClientInstance; - } -}); +container.register( + ManagementClient, + { + useFactory: (c) => { + const authConfig = c.resolve(AuthConfigService); + return new ManagementClient({ + domain: authConfig.get("AUTH0_M2M_DOMAIN"), + clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), + clientSecret: authConfig.get("AUTH0_M2M_SECRET") + }); + } + }, + { lifecycle: Lifecycle.Singleton } +);Also applies to: 6-6
14-16: Normalize Auth0 domain (host only).Guard against configs including protocol (e.g., https://tenant.auth0.com), which some SDK versions reject.
- return new ManagementClient({ domain, clientId, clientSecret }); + const normalizedDomain = domain.replace(/^https?:\/\//, ""); + return new ManagementClient({ domain: normalizedDomain, clientId, clientSecret });Please confirm your AUTH0_M2M_DOMAIN format matches SDK expectations.
8-22: Consider a named injection token for stability.Using a string token (e.g., AUTH0_MANAGEMENT_CLIENT) decouples DI from the concrete class symbol and avoids issues with type-only imports or future SDK swaps. Low impact now; adopt if you foresee SDK changes.
+export const AUTH0_MANAGEMENT_CLIENT = "AUTH0_MANAGEMENT_CLIENT"; -container.register( - ManagementClient, +container.register( + AUTH0_MANAGEMENT_CLIENT, { useFactory: (c) => { /* ... */ } }, { lifecycle: Lifecycle.Singleton } );Consumers would inject with @Inject(AUTH0_MANAGEMENT_CLIENT).
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx (7)
17-19: Prefer per-testsetup()overbeforeEach(repo guideline)Guidelines require a
setupfunction instead ofbeforeEach. Re-initialize mocks insetupto avoid shared state and call it from each test.Apply this to remove the hook-level
beforeEach:- beforeEach(() => { - jest.clearAllMocks(); - });Add a local setup helper at the bottom of the file:
// place at the bottom of this describe block function setup(params: { onSuccess?: (v: boolean) => void; onError?: () => void } = {}) { const mockAuth = mock<AuthHttpService>(); const services: ServicesProviderProps["services"] = { auth: () => mockAuth }; const { result } = setupQuery(() => useVerifyEmail(params), { services }); return { result, mockAuth }; }Then, in tests:
const { result, mockAuth } = setup(/* optional callbacks */);
34-40: UsemutateAsyncto simplify (no act/waitFor needed here)Awaiting
mutateAsyncmakes the assertion synchronous and reduces flakiness.- await act(async () => { - result.current.mutate("test@example.com"); - }); - - await waitFor(() => { - expect(mockAuthService.verifyEmail).toHaveBeenCalledWith("test@example.com"); - }); + await result.current.mutateAsync("test@example.com"); + expect(mockAuthService.verifyEmail).toHaveBeenCalledWith("test@example.com");
57-63: Flatten success flow withmutateAsync- await act(async () => { - result.current.mutate("test@example.com"); - }); - - await waitFor(() => { - expect(onSuccessCallback).toHaveBeenCalledWith(true); - }); + await result.current.mutateAsync("test@example.com"); + expect(onSuccessCallback).toHaveBeenCalledWith(true);
80-86: Same here:mutateAsyncsimplifies the non-verified case- await act(async () => { - result.current.mutate("test@example.com"); - }); - - await waitFor(() => { - expect(onSuccessCallback).toHaveBeenCalledWith(false); - }); + await result.current.mutateAsync("test@example.com"); + expect(onSuccessCallback).toHaveBeenCalledWith(false);
98-104: Assert rejection and callback without waitFor- await act(async () => { - result.current.mutate("test@example.com"); - }); - - await waitFor(() => { - expect(onErrorCallback).toHaveBeenCalled(); - }); + await expect(result.current.mutateAsync("test@example.com")).rejects.toThrow("Verification failed"); + expect(onErrorCallback).toHaveBeenCalled();
8-9: Drop unused imports aftermutateAsyncrefactorAfter switching to
mutateAsync,act/waitForshould be removable.-import { act, waitFor } from "@testing-library/react"; +// (imports can be removed if no longer used)
22-27: Tighten mock typing to catch SDK/response-shape drift
Type the mocked return using the real method’s return type so tests break ifverifyEmailchanges.Add near the imports:
type VerifyEmailReturn = Awaited<ReturnType<AuthHttpService["verifyEmail"]>>;In each test mock:
- const mockResponse = { - data: { emailVerified: true } - }; + const mockResponse: VerifyEmailReturn = { data: { emailVerified: true } };(and the same for the
falsecase)Optionally confirm SDK/hook shapes haven’t drifted:
# Show AuthHttpService.verifyEmail signature rg -nC2 'class\s+AuthHttpService' packages/http-sdk/src/auth/auth-http.service.ts rg -nC3 'verifyEmail\(' packages/http-sdk/src/auth/auth-http.service.ts rg -nC3 'export\s+type\s+VerifyEmailResponse' -g '!**/dist/**' # Confirm hook return type rg -nC3 'useVerifyEmail\(' apps/deploy-web/src/queries/useVerifyEmailQuery.tsAlso apply to the mocks at lines 44–49 and 67–72.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/core/providers/auth.provider.ts(1 hunks)apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsxapps/api/src/core/providers/auth.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsxapps/api/src/core/providers/auth.provider.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1880
File: apps/api/src/auth/routes/email-verified/email-verified.router.ts:11-12
Timestamp: 2025-09-02T07:58:39.756Z
Learning: Auth0 email verification callbacks use GET endpoints because Auth0 redirects the user's browser to a "redirect to" URL after email verification. This follows standard OAuth/OIDC callback patterns where external auth providers redirect users back to the application, requiring GET endpoints to handle browser navigation.
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx
📚 Learning: 2025-07-11T10:46:43.711Z
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Applied to files:
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx
🧬 Code graph analysis (1)
apps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx (2)
apps/deploy-web/src/queries/useVerifyEmailQuery.ts (1)
useVerifyEmail(6-23)packages/http-sdk/src/auth/auth-http.service.ts (1)
AuthHttpService(6-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/api/src/core/providers/auth.provider.ts (2)
8-22: Testability note: ensure DI reset between tests.Singletons can leak across test cases. If you use Jest or similar, call container.reset() in afterEach/afterAll, or register this provider in a test child container.
8-22: Set HTTP timeoutDuration and retry options on ManagementClientUse
ManagementClientOptionssupported by auth0 v4.x to harden network calls. For example:- managementClientInstance = new ManagementClient({ - domain: authConfig.get("AUTH0_M2M_DOMAIN"), - clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), - clientSecret: authConfig.get("AUTH0_M2M_SECRET") - }); + managementClientInstance = new ManagementClient({ + domain: authConfig.get("AUTH0_M2M_DOMAIN"), + clientId: authConfig.get("AUTH0_M2M_CLIENT_ID"), + clientSecret: authConfig.get("AUTH0_M2M_SECRET"), + timeoutDuration: 15000, // ms before aborting requests + retry: { enabled: true, maxRetries: 3 } // configure retry attempts + });
timeoutDuration(default 10 000 ms) sets the global HTTP timeout andretry(default enabled with 5 retries) controls retry logic (auth0.github.ioapps/deploy-web/src/queries/useVerifyEmailQuery.spec.tsx (1)
10-16: LGTM: solid hook tests and DI via ServicesProviderGood coverage of success/false/error paths. Typed mocks via jest-mock-extended and injecting AuthHttpService through ServicesProvider look clean.
|
|
||
| import { AuthConfigService } from "@src/auth/services/auth-config/auth-config.service"; | ||
|
|
||
| let managementClientInstance: ManagementClient | null = null; |
There was a problem hiding this comment.
non-blocking: why is this important to make it a singleton?
if it is needed indeed, found this helper https://github.com/Microsoft/tsyringe?tab=readme-ov-file#instancecachingfactory
3 participants
refs #1863
Summary by CodeRabbit
New Features
Security
Integration
Tests