refactor: moves user registraction logic under user controller#1796
refactor: moves user registraction logic under user controller#1796
Conversation
WalkthroughAdds user registration and current-user endpoints and schemas, implements UserService with upsert/anonymous migration and wallet transfer, passes client context from AuthInterceptor, updates repository utilities, and adjusts web Auth0 callback/profile flows, DI, logout, and onboarding storage key usage. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Router
participant UC as UserController
participant US as UserService
participant UR as UserRepository
participant AW as UserWalletRepository
participant AN as Analytics/Logger
Client->>API: POST /v1/register-user (wantedUsername, email, ...)
API->>UC: registerUser(data)
UC->>US: registerUser(data)
US->>UR: upsert(user details)
alt anonymousUserId provided
US->>UR: load anonymous user
US->>AW: tryToTransferWallet(anon -> new)
end
US->>AN: track/log registration
US-->>UC: user subset
UC-->>API: { data: user }
API-->>Client: 200 JSON
sequenceDiagram
actor Client
participant API as API Router
participant UC as UserController
Client->>API: GET /v1/user/me
API->>UC: getCurrentUser()
UC-->>API: { data: user }
API-->>Client: 200 JSON
sequenceDiagram
participant Browser
participant Next as Next.js Auth API
participant Auth0 as Auth0
participant MainAPI as Mainnet API
Browser->>Next: /api/auth/callback
Next->>Auth0: handleCallback (afterCallback)
Note right of Next: afterCallback calls /v1/register-user with Authorization (and optional x-anonymous-authorization)
Next->>MainAPI: POST /v1/register-user
MainAPI-->>Next: { data: user }
Next->>Auth0: finalize session (merge user)
Auth0-->>Browser: redirect with session
Browser->>Next: /api/auth/me (profile)
Next->>MainAPI: GET /v1/user/me
MainAPI-->>Next: { data: user }
Next-->>Browser: session with merged user
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
stalniy
force-pushed
the
refactor/user-register
branch
2 times, most recently
from
b355236 to
0db2ba4
Compare
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (30.76%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1796 +/- ##
==========================================
- Coverage 42.47% 42.27% -0.21%
==========================================
Files 934 933 -1
Lines 26288 26242 -46
Branches 6899 6909 +10
==========================================
- Hits 11167 11094 -73
- Misses 13961 13987 +26
- Partials 1160 1161 +1
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (4)
apps/api/src/routers/userRouter.ts (1)
68-68: Remove the any cast from container.resolve to comply with TS guidelines.Avoid any. Let tsyringe infer the type or provide a proper generic.
- const anonymousUserId = await container.resolve<any>(AuthTokenService).getValidUserId(anonymousBearer); + const authTokenService = container.resolve(AuthTokenService); + const anonymousUserId = await authTokenService.getValidUserId(anonymousBearer);apps/api/src/auth/services/auth.interceptor.spec.ts (1)
95-101: Inline the setup input type per test guidelines.The coding guidelines require an inline type for setup’s single parameter and no shared interface. Inline the type and drop the interface.
- function setup(input?: SetupInput) { + function setup(input?: { user?: UserOutput; apiKey?: string }) { ... - interface SetupInput { - user?: UserOutput; - apiKey?: string; - }Also applies to: 172-175
apps/api/test/services/wallet-testing.service.ts (1)
125-141: Handle non-2xx response and surface errors in test helperBetter fail fast in tests with a clear message if registration fails. This avoids hard-to-trace JSON parsing or shape errors.
Apply this diff to add error handling:
- const userResponse = await this.app.request(`/v1/registerUser`, { + const userResponse = await this.app.request(`/v1/registerUser`, { method: "POST", headers: new Headers({ "Content-Type": "application/json", Authorization: `Bearer ${access_token}` }), body: JSON.stringify({ wantedUsername: decoded.nickname, email: decoded.email, emailVerified: decoded.email_verified, subscribedToNewsletter: false }) }); - const body = await userResponse.json(); - - return { user: body.data, token: access_token }; + if (!userResponse.ok) { + const text = await userResponse.text(); + throw new Error(`registerUser failed: ${userResponse.status} ${userResponse.statusText} - ${text}`); + } + const body = await userResponse.json(); + return { user: body.data, token: access_token };apps/api/src/user/controllers/user/user.controller.ts (1)
47-54: Unify return type of getByIdThe union Promise<{ data: UserSchema } | GetUserResponseOutput> needlessly complicates the contract and docs. The method returns { data: user } anyway.
- async getById({ id }: GetUserParams): Promise<{ data: UserSchema } | GetUserResponseOutput> { + async getById({ id }: GetUserParams): Promise<{ data: UserSchema }> {Ensure route/OpenAPI for this endpoint also reflects the full UserSchema if that's what you return.
🧹 Nitpick comments (15)
apps/deploy-web/src/components/layout/AccountMenu.tsx (1)
17-17: Optional: use DI-provided urlService for all links for consistencyYou’ve introduced useServices; consider replacing direct UrlService.* calls with urlService.* across this component to standardize URL construction via DI.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
179-185: Optional: prefer typed mocks for urlServiceConsider using jest-mock-extended to strictly type the mock urlService (onboarding, signup) and avoid casting through unknown.
Example:
import { mockDeep } from "jest-mock-extended"; const mockUrlService = mockDeep<typeof UrlService>(); mockUrlService.onboarding.mockReturnValue("/onboarding"); mockUrlService.signup.mockReturnValue("/signup");apps/api/src/routers/userRouter.ts (1)
42-44: Deprecation notice is good; consider adding runtime deprecation headers on the endpoint.To assist client migration, add Deprecation/Sunset/Link headers in the handler to provide machine-readable hints.
Example diff within the same handler:
userRequiredRouter.post("/tokenInfo", async c => { + // Mark endpoint as deprecated for clients + c.header("Deprecation", "true"); + // Optional: RFC 8594 Sunset header with a date + // c.header("Sunset", new Date("2025-12-31").toUTCString()); + // Optional: provide successor link + c.header("Link", '</v1/registerUser>; rel="successor-version"'); const userId = getCurrentUserId(c);apps/api/src/user/repositories/user/user.repository.ts (1)
128-134: Tighten override signature and visibility for toInput.Make it an override of BaseRepository.toInput with the correct visibility and return type to ensure type safety across inherited calls.
- toInput(payload: Partial<UserInput>): Partial<UserInput> { + protected override toInput(payload: Partial<UserInput>): Partial<typeof this.table.$inferInsert> { if (!payload.lastUserAgent) return payload; return { ...payload, lastUserAgent: payload.lastUserAgent.slice(0, userAgentMaxLength) }; }apps/api/src/core/repositories/base.repository.ts (1)
226-228: Guard looks good; optionally broaden to handle drivers that don’t throw PostgresError.If the runtime uses pg (DatabaseError) or rethrows plain objects, the current check will miss 23505. Consider a fallback shape check without using any.
-export function isUniqueViolation(error: unknown): error is PostgresError { - return error instanceof PostgresError && error.code === "23505"; -} +export function isUniqueViolation(error: unknown): error is PostgresError { + if (error instanceof PostgresError) return error.code === "23505"; + // Fallback: accept objects that expose a 23505 code + return typeof error === "object" && error !== null && "code" in error && (error as { code?: unknown }).code === "23505"; +}apps/api/src/user/routes/get-current-user/get-current-user.router.ts (1)
8-25: Document 401 for unauthenticated accessgetCurrentUser is protected; include 401 in OpenAPI to reflect reality.
responses: { description: "Returns the logged in user", body: { content: { "application/json": { schema: z.object({ data: UserSchema }) } } } }, + 401: { + description: "Unauthorized" + } }apps/api/src/user/schemas/user.schema.ts (1)
24-38: Value/type name collision on UserSchema may confuse consumersYou export both a value (Zod schema) and a type with the same name UserSchema. TS allows it, but it’s easy to import the wrong symbol and accidentally pull in the value instead of the type (or vice versa).
Consider:
- Keep Zod as UserSchema (value) and export the type as User (preferred), or
- Rename Zod to UserSchemaZ and keep type as UserSchema.
Additionally, consider adding .openapi() hints on fields if you want richer docs (e.g., email format).
apps/api/src/user/routes/register-user/register-user.router.ts (1)
22-48: API naming consistencyMost endpoints follow kebab-case or resource-oriented paths (/v1/user/me, /v1/anonymous-users). /v1/registerUser deviates.
Consider adding an alias (keeping current path for backward compatibility):
- POST /v1/users/register
- or POST /v1/users
Also, document error responses (e.g., 400 validation error, 401 unauthorized, 409 username conflict).
Example:
responses: { description: "Returns the registered user", body: { content: { "application/json": { schema: registerUserResponseSchema } } } - } + }, + 400: { description: "Invalid input" }, + 401: { description: "Unauthorized" }, + 409: { description: "Username conflict" } }apps/api/src/user/services/user/user.service.spec.ts (2)
155-159: Avoid dynamic RegExp in tests; prefer startsWith for clarityMinor nit: new RegExp(
^${conflictingUsername}) is overkill and flagged by static analysis. startsWith is simpler and avoids regex footguns.- expect(newUsers[0].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[0].username.startsWith(conflictingUsername)).toBe(true); @@ - expect(newUsers[1].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[1].username.startsWith(conflictingUsername)).toBe(true);
265-271: Setup helper: allow overrides via a single param to match test guidelinesTests follow the setup pattern and use jest-mock-extended correctly. To align fully with the stated guidelines, accept a single param with inline type for overrides.
- function setup() { - const analyticsService = mock<AnalyticsService>(); - const logger = mock<LoggerService>(); - const service = new UserService(container.resolve(UserRepository), analyticsService, container.resolve(UserWalletRepository), logger); - - return { service, analyticsService, logger }; - } + function setup(params: { analyticsService?: AnalyticsService; logger?: LoggerService } = {}) { + const analyticsService = params.analyticsService ?? mock<AnalyticsService>(); + const logger = params.logger ?? mock<LoggerService>(); + const service = new UserService(container.resolve(UserRepository), analyticsService, container.resolve(UserWalletRepository), logger); + return { service, analyticsService, logger }; + }apps/api/src/user/services/user/user.service.ts (2)
65-68: Align analytics event names and payloads.Logs use ANONYMOUS_USER_REGISTERED/USER_REGISTERED, analytics tracks "user_registered". Consider unifying the event name across both and include useful properties (e.g., isAnonymous) for consistent observability.
- const event = isAnonymous ? "ANONYMOUS_USER_REGISTERED" : "USER_REGISTERED"; - this.logger.info({ event, id: user.id, userId: user.userId }); - this.analyticsService.track(user.id, "user_registered"); + const event = isAnonymous ? "ANONYMOUS_USER_REGISTERED" : "USER_REGISTERED"; + this.logger.info({ event, id: user.id, userId: user.userId }); + this.analyticsService.track(event, { userId: user.userId, isAnonymous });Confirm AnalyticsService.track signature; adjust accordingly if it expects (userId, eventName, props).
72-85: Remove self-referential return type assertion for clarity.Using
as Awaited<ReturnType<this["registerUser"]>>inside registerUser is a code smell and harms readability. Prefer an explicit DTO type.Define a reusable type and use it as the return type, then remove the assertion:
// near the top of the file export type RegisterUserResult = { id: string; userId: string; username: string; email: string; emailVerified: boolean; stripeCustomerId: string | null; bio: string | null; subscribedToNewsletter: boolean; youtubeUsername: string | null; twitterUsername: string | null; githubUsername: string | null; }; // signature async registerUser(data: RegisterUserInput): Promise<RegisterUserResult> { // ... return { id, userId, username, email, emailVerified, stripeCustomerId, bio, subscribedToNewsletter, youtubeUsername, twitterUsername, githubUsername }; }apps/deploy-web/src/pages/api/auth/[...auth0].ts (3)
37-78: Harden registration flow in afterCallback.
- subscribedToNewsletter should accept boolean or string values from user_metadata.
- Minor: consider using the existing request interceptor for anonymous header to avoid duplicating header logic here.
- subscribedToNewsletter: user_metadata?.subscribedToNewsletter === "true" + subscribedToNewsletter: + user_metadata?.subscribedToNewsletter === true || + user_metadata?.subscribedToNewsletter === "true"Confirm whether user_metadata.subscribedToNewsletter is consistently a string in your Auth0 tenant; if so, we can keep the original comparison.
53-64: Consider using relative URL with consoleApiHttpClient’s baseURL.Since consoleApiHttpClient can be configured with baseURL, you can simplify and avoid coupling to apiUrlService here. Also, to avoid leaking headers object shape, pass headers as a plain object literal.
- const userSettings = await services.consoleApiHttpClient.post<{ data: UserSettings }>( - `${services.apiUrlService.getBaseApiUrlFor("mainnet")}/v1/registerUser`, + const userSettings = await services.consoleApiHttpClient.post<{ data: UserSettings }>( + "/v1/registerUser", { wantedUsername: session.user.nickname, email: session.user.email, emailVerified: session.user.email_verified, subscribedToNewsletter: user_metadata?.subscribedToNewsletter === "true" }, { - headers: headers.toJSON() + headers: { ...headers.toJSON() } } );If consoleApiHttpClient is intentionally created without baseURL on the server, ignore this suggestion.
110-110: Session merge is fine; consider normalizing property names.Merging server-returned keys directly into session.user is fine as long as UserSettings keys don’t collide with Auth0’s reserved keys. If there’s any risk, namespace them under session.user.profile.
Validate that id/username/etc. don’t shadow claims required by @auth0/nextjs-auth0.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (20)
apps/api/src/app.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.spec.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.ts(3 hunks)apps/api/src/core/repositories/base.repository.ts(2 hunks)apps/api/src/routers/userRouter.ts(1 hunks)apps/api/src/user/controllers/user/user.controller.ts(4 hunks)apps/api/src/user/repositories/user/user.repository.ts(4 hunks)apps/api/src/user/routes/get-current-user/get-current-user.router.ts(1 hunks)apps/api/src/user/routes/index.ts(1 hunks)apps/api/src/user/routes/register-user/register-user.router.ts(1 hunks)apps/api/src/user/schemas/user.schema.ts(1 hunks)apps/api/src/user/services/user/user.service.spec.ts(1 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(2 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(7 hunks)apps/deploy-web/src/pages/api/auth/[...auth0].ts(4 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(1 hunks)apps/deploy-web/src/services/auth/auth.service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/routers/userRouter.tsapps/api/test/services/wallet-testing.service.tsapps/api/src/user/routes/get-current-user/get-current-user.router.tsapps/api/src/core/repositories/base.repository.tsapps/deploy-web/src/components/layout/AccountMenu.tsxapps/deploy-web/src/services/auth/auth.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/api/src/app.tsapps/api/src/user/repositories/user/user.repository.tsapps/api/src/user/routes/register-user/register-user.router.tsapps/api/src/user/schemas/user.schema.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/api/src/user/services/user/user.service.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/user/routes/index.tsapps/api/src/user/controllers/user/user.controller.tsapps/api/src/auth/services/auth.interceptor.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/api/src/auth/services/auth.interceptor.tsapps/deploy-web/src/pages/api/auth/[...auth0].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/routers/userRouter.tsapps/api/test/services/wallet-testing.service.tsapps/api/src/user/routes/get-current-user/get-current-user.router.tsapps/api/src/core/repositories/base.repository.tsapps/deploy-web/src/components/layout/AccountMenu.tsxapps/deploy-web/src/services/auth/auth.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/api/src/app.tsapps/api/src/user/repositories/user/user.repository.tsapps/api/src/user/routes/register-user/register-user.router.tsapps/api/src/user/schemas/user.schema.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/api/src/user/services/user/user.service.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/user/routes/index.tsapps/api/src/user/controllers/user/user.controller.tsapps/api/src/auth/services/auth.interceptor.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/api/src/auth/services/auth.interceptor.tsapps/deploy-web/src/pages/api/auth/[...auth0].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/src/auth/services/auth.interceptor.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.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/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧠 Learnings (1)
📚 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/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧬 Code Graph Analysis (12)
apps/api/src/user/routes/get-current-user/get-current-user.router.ts (3)
apps/api/src/user/schemas/user.schema.ts (1)
GetUserResponseOutputSchema(14-20)apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/user/controllers/user/user.controller.ts (1)
getCurrentUser(81-85)
apps/deploy-web/src/components/layout/AccountMenu.tsx (2)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)apps/deploy-web/src/components/shared/CustomDropdownLinkItem.tsx (1)
CustomDropdownLinkItem(12-21)
apps/deploy-web/src/services/auth/auth.service.ts (1)
apps/deploy-web/src/config/auth.config.ts (1)
ANONYMOUS_USER_TOKEN_KEY(2-2)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
apps/deploy-web/src/services/auth/auth.service.ts (1)
AuthService(7-30)
apps/api/src/app.ts (2)
apps/api/src/user/routes/register-user/register-user.router.ts (1)
registerUserRouter(49-52)apps/api/src/user/routes/get-current-user/get-current-user.router.ts (1)
getCurrentUserRouter(26-28)
apps/api/src/user/repositories/user/user.repository.ts (1)
apps/api/src/user/model-schemas/user/user.schema.ts (1)
userAgentMaxLength(6-6)
apps/api/src/user/routes/register-user/register-user.router.ts (5)
apps/api/src/user/services/user/user.service.ts (2)
RegisterUserInput(139-149)registerUser(19-85)apps/api/src/user/schemas/user.schema.ts (2)
UserSchema(24-36)UserSchema(38-38)apps/api/src/dashboard/routes/leases-duration/leases-duration.router.ts (1)
route(15-37)apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/user/controllers/user/user.controller.ts (1)
registerUser(60-78)
apps/api/src/user/services/user/user.service.ts (4)
apps/api/src/user/repositories/user/user.repository.ts (1)
UserOutput(11-13)apps/deploy-web/src/services/analytics/analytics.service.ts (1)
AnalyticsService(141-266)apps/api/src/user/routes/register-user/register-user.router.ts (1)
RegisterUserInput(15-15)apps/api/src/core/repositories/base.repository.ts (1)
isUniqueViolation(226-228)
apps/api/src/user/services/user/user.service.spec.ts (2)
apps/api/src/user/routes/register-user/register-user.router.ts (1)
RegisterUserInput(15-15)apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)
apps/api/src/user/controllers/user/user.controller.ts (6)
apps/api/src/user/routes/get-anonymous-user/get-anonymous-user.router.ts (1)
GetUserParams(11-11)apps/api/src/user/schemas/user.schema.ts (3)
UserSchema(24-36)UserSchema(38-38)GetUserResponseOutput(22-22)apps/api/src/user/routes/register-user/register-user.router.ts (2)
RegisterUserInput(15-15)RegisterUserResponse(20-20)apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)apps/notifications/src/interfaces/rest/services/auth/auth.service.ts (1)
userId(18-25)apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)
apps/api/src/auth/services/auth.interceptor.ts (1)
apps/api/src/user/controllers/user/user.controller.ts (1)
httpContext(31-33)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (3)
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
services(22-54)apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(22-32)apps/deploy-web/src/types/user.ts (1)
UserSettings(5-16)
🪛 GitHub Actions: Deploy Web CI
apps/deploy-web/src/services/auth/auth.service.ts
[error] 456-456: Cannot find module '../storage/keys' from 'src/services/auth/auth.service.ts'. This error occurred during running 'npm run test:cov'. Ensure the module exists at src/services/storage/keys.ts and that the relative import path is correct.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
[error] 9-9: Type error: Cannot find module '@src/services/storage/keys' or its corresponding type declarations. (import at OnboardingContainer.tsx:9:37). Build step 'npm run build -w apps/deploy-web' failed due to this error.
🪛 GitHub Check: validate / validate-app
apps/api/src/user/routes/register-user/register-user.router.ts
[failure] 6-6:
Dependency cycle detected
apps/api/src/user/controllers/user/user.controller.ts
[failure] 11-11:
Dependency cycle detected
🪛 ast-grep (0.38.6)
apps/api/src/user/services/user/user.service.spec.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Actions: API CI
apps/api/src/user/controllers/user/user.controller.ts
[error] 11-11: ESLint: Dependency cycle detected (import-x/no-cycle).
🔇 Additional comments (16)
apps/deploy-web/src/components/layout/AccountMenu.tsx (1)
31-31: LGTM: logout flow now goes through AuthServiceUsing authService from DI and delegating logout is consistent with the new architecture.
Also applies to: 103-103
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
47-47: LGTM: inject urlService into AuthServiceConstructor wiring matches the new AuthService signature and keeps URL concerns inside DI.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
47-47: LGTM: urlService + ONBOARDING_STEP_KEY integration
- Reads/writes remove hard-coded key in favor of ONBOARDING_STEP_KEY.
- Signup/onboarding URLs now flow via urlService from DI.
Also applies to: 128-131, 50-50, 67-67, 97-97, 117-117
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
188-191: LGTM: tests updated to mock useServices.urlServiceMocks align with the new DI shape; no jest.mock() used, setup stays at the bottom per guidelines.
Also applies to: 199-200
apps/api/src/user/routes/index.ts (1)
3-4: LGTM — re-exports align with new endpoints.No issues.
apps/api/src/auth/services/auth.interceptor.spec.ts (1)
8-8: LGTM — ExecutionContextService import for test context is appropriate.apps/api/src/app.ts (1)
76-76: Routers wired correctly; OpenAPI inclusion looks goodImporting and registering createAnonymousUserRouter/getAnonymousUserRouter/registerUserRouter/getCurrentUserRouter is consistent with the routing pattern and middleware order. No issues found.
Also applies to: 126-127
apps/api/src/user/routes/get-current-user/get-current-user.router.ts (1)
26-28: Handler is fineResolves controller via DI and returns JSON 200. Good.
apps/api/src/user/routes/register-user/register-user.router.ts (1)
49-52: Handler looks goodValidates body via zod-openapi and delegates to controller. Good.
apps/api/src/auth/services/auth.interceptor.ts (2)
29-38: ExecutionContextService injection: LGTMOrder in app.ts initializes RequestContextInterceptor before AuthInterceptor, so HTTP_CONTEXT should be available. No issues.
117-122: Mark activity with context: LGTMThrottle logic preserved; passing ip and fingerprint is safe with optional chaining. Ensure UserRepository.markAsActive signature accepts these optional fields.
Would you like me to scan repository for markAsActive signature to ensure it matches { throttleTimeSeconds: number; ip?: string; fingerprint?: string }?
apps/api/src/user/controllers/user/user.controller.ts (1)
80-85: getCurrentUser: LGTMAuthorization asserted; returns current user wrapped in { data }. Align route schema accordingly (see router comment).
apps/api/src/user/services/user/user.service.ts (2)
61-63: Double-check wallet transfer condition.Transfer only occurs when isAnonymous is false. Given the flow, when the anonymous record was successfully upgraded (isAnonymous === true), upsert should target the same row and no transfer is needed. Please verify this holds across all DB states to avoid missing a required transfer.
If there are cases where upsert creates a new row despite a prior anonymous update, we should drop the isAnonymous guard and rely solely on id inequality.
139-149: Rename service input type and align client-context fieldsThere’s a name collision between the router’s
RegisterUserInput(inapps/api/src/user/routes/register-user/register-user.router.ts) and the service’sRegisterUserInput(inapps/api/src/user/services/user/user.service.ts), and the service currently requiresip,userAgent, andfingerprintas non-optional strings even thoughctxmay not always supply them. To address both issues:• In
apps/api/src/user/services/user/user.service.ts(around lines 139–149), rename the interface and make the context fields optional if your TRPC context can be undefined:- export interface RegisterUserInput { + // avoid colliding with router’s type + export interface RegisterUserServiceInput { anonymousUserId?: string; userId: string; wantedUsername: string; email: string; emailVerified: boolean; subscribedToNewsletter: boolean; - ip: string; - userAgent: string; - fingerprint: string; + ip?: string; + userAgent?: string; + fingerprint?: string; }• Update the
registerUsermethod signature to acceptRegisterUserServiceInputand adjust all its callers.
• Verify your TRPC context type—ifctx.ip,ctx.userAgent, orctx.fingerprintare guaranteed to be defined, you can keep them required; otherwise leave them optional or enforce defaults.apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
47-51: Propagating anonymous token header: ensure we’re forwarding only the anonymous token.req.headers.authorization may sometimes hold non-anonymous credentials. If there’s any chance of mixing tokens, add a simple guard to ensure the header contains an anonymous token (e.g., a distinct prefix) before forwarding.
If the anonymous token is indistinguishable, consider passing it via a dedicated cookie or header from the browser flow to avoid confusion.
103-104: Good switch to GET /v1/user/me.Fetching the current user profile via a dedicated endpoint is clearer and avoids overloading token endpoints. LGTM.
| responses: { | ||
| 200: { | ||
| description: "Returns the logged in user", | ||
| body: { | ||
| content: { | ||
| "application/json": { | ||
| schema: GetUserResponseOutputSchema | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Response schema mismatch: route documents only {data: {id}} but controller returns full user
Controller returns { data: UserSchema }. The route currently points to GetUserResponseOutputSchema with just id, which will produce incorrect OpenAPI.
Use UserSchema in the response:
-import { GetUserResponseOutputSchema } from "@src/user/schemas/user.schema";
+import { z } from "zod";
+import { UserSchema } from "@src/user/schemas/user.schema";
@@
responses: {
description: "Returns the logged in user",
- body: {
+ body: {
content: {
"application/json": {
- schema: GetUserResponseOutputSchema
+ schema: z.object({ data: UserSchema })
}
}
}
}
}📝 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.
| responses: { | |
| 200: { | |
| description: "Returns the logged in user", | |
| body: { | |
| content: { | |
| "application/json": { | |
| schema: GetUserResponseOutputSchema | |
| } | |
| } | |
| } | |
| } | |
| // Replace this import: | |
| -import { GetUserResponseOutputSchema } from "@src/user/schemas/user.schema"; | |
| // With these: | |
| +import { z } from "zod"; | |
| +import { UserSchema } from "@src/user/schemas/user.schema"; | |
| // … elsewhere in this file … | |
| responses: { | |
| 200: { | |
| description: "Returns the logged in user", | |
| body: { | |
| content: { | |
| "application/json": { | |
| // Use the full UserSchema instead of only {id} | |
| - schema: GetUserResponseOutputSchema | |
| + schema: z.object({ data: UserSchema }) | |
| } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/user/routes/get-current-user/get-current-user.router.ts around
lines 13 to 23, the OpenAPI response uses GetUserResponseOutputSchema which only
documents { data: { id } } but the controller returns the full user object;
update the response schema to use the full UserSchema (or the exported schema
that represents the complete user response) so the route documentation matches
the controller output, and ensure the proper import/alias for UserSchema is
present and referenced in the responses section.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
Show resolved
Hide resolved
stalniy
force-pushed
the
refactor/user-register
branch
from
0db2ba4 to
632a08c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (2)
134-138: Avoidanyin test types; use minimal concrete types.This violates the guideline to never use
anyin TS. Define the shapes actually used by the component.Apply:
- input: { - paymentMethods?: any[]; - user?: any; - windowLocation?: Partial<Location>; - windowHistory?: Partial<History>; - } = {} + input: { + paymentMethods?: Array<{ id: string; type: string }>; + user?: { emailVerified?: boolean; stripeCustomerId?: string }; + windowLocation?: Partial<Location>; + windowHistory?: Partial<History>; + } = {}
179-185: Drop double assertion and spreading real service into a mock.
as unknown asbypasses type safety, and spreading UrlService risks pulling in real behavior. Provide a minimal, typed mock instead.Use a precise type:
- const mockUrlService = { - ...UrlService, - onboarding: jest.fn(), - signup: jest.fn() - } as unknown as typeof UrlService; + const mockUrlService: Pick<typeof UrlService, "onboarding" | "signup"> = { + onboarding: jest.fn(), + signup: jest.fn() + };apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
50-56: Validate saved step using numeric members; avoid enum “/2” heuristic.Object.keys(enum).length/2 relies on TS enum emit details and is brittle. Prefer counting numeric values explicitly.
- if (savedStep) { - const step = parseInt(savedStep, 10); - if (step >= 0 && step < Object.keys(OnboardingStepIndex).length / 2) { - setCurrentStep(step); - } - } + if (savedStep) { + const step = parseInt(savedStep, 10); + const stepCount = Object.values(OnboardingStepIndex).filter(v => typeof v === "number").length; + if (!Number.isNaN(step) && step >= 0 && step < stepCount) { + setCurrentStep(step); + } + }
♻️ Duplicate comments (1)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
9-9: Import path fix resolves prior pipeline failure.The ONBOARDING_STEP_KEY import now points to an existing module, addressing the earlier “Cannot find module '@src/services/storage/keys'” build error.
🧹 Nitpick comments (7)
apps/api/src/user/services/user/user.service.spec.ts (5)
16-16: Fix test name: it contradicts the input (userId is provided).The test title says “no userId is provided” but the payload includes a userId (and the type requires it). Update the description for clarity.
Apply this diff:
-it("registers a new user if no anonymousUserId and no userId is provided", async () => { +it("registers a new user when anonymousUserId is not provided", async () => {
64-70: Strengthen assertion to prove we updated the same anonymous user recordYou already reload by id and compare objects, but explicitly asserting the id didn’t change eliminates ambiguity and guards against accidental “create new” flows.
Apply this diff:
- const user = await service.registerUser(input); - const reloadedUser = await container.resolve(UserRepository).findById(anonymousUser.id); + const user = await service.registerUser(input); + expect(user.id).toBe(anonymousUser.id); + const reloadedUser = await container.resolve(UserRepository).findById(anonymousUser.id);
154-158: Avoid dynamic RegExp; use startsWith for clearer intent and to quiet static analysisEven though the variable is a constant in this test, replacing the regex with startsWith improves readability and avoids ReDoS lint warnings.
Apply this diff:
- expect(newUsers[0].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[0].username.startsWith(conflictingUsername)).toBe(true); - expect(newUsers[1].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[1].username.startsWith(conflictingUsername)).toBe(true);
188-190: Assert wallet record identity did not change during transferAdd an assertion that the same wallet row was moved (id unchanged). This better validates a transfer vs. re-creation.
Apply this diff:
const updatedWallet = await container.resolve(UserWalletRepository).findById(wallet.id); + expect(updatedWallet?.id).toBe(wallet.id); expect(updatedWallet?.userId).toBe(newUser.id);
265-271: Align setup() with our test guideline to accept a single, inline-typed parameterOur spec guideline prefers a setup function that accepts a single parameter with inline type definition (no return type annotation). This enables per-test overrides without shared state.
Apply this diff:
- function setup() { - const analyticsService = mock<AnalyticsService>(); - const logger = mock<LoggerService>(); - const service = new UserService(container.resolve(UserRepository), analyticsService, container.resolve(UserWalletRepository), logger); - - return { service, analyticsService, logger }; - } + function setup( + { analyticsService = mock<AnalyticsService>(), logger = mock<LoggerService>() }: { + analyticsService?: AnalyticsService; + logger?: LoggerService; + } = {} + ) { + const service = new UserService( + container.resolve(UserRepository), + analyticsService, + container.resolve(UserWalletRepository), + logger + ); + return { service, analyticsService, logger }; + }apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
171-177: Prefer typed mocks via jest-mock-extended for stronger guarantees.Guideline: use jest-mock-extended in tests. It improves type inference for mocks like analyticsService/urlService and prevents accidental API drift.
Outside the selected lines, add:
import { mock } from "jest-mock-extended";Then replace ad-hoc mocks:
const mockAnalyticsService = mock<{ track: (event: string, props?: Record<string, unknown>) => void }>(); const mockUrlService = mock<Pick<typeof UrlService, "onboarding" | "signup">>(); mockUrlService.onboarding.mockReturnValue("/onboarding"); mockUrlService.signup.mockReturnValue("/signup");And keep:
- const mockUseServices = jest.fn().mockReturnValue({ + const mockUseServices = jest.fn().mockReturnValue({ analyticsService: mockAnalyticsService, urlService: mockUrlService });Also applies to: 188-191
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
89-95: DRY the step name mapping used in analytics.stepNames is duplicated in two places, risking drift. Hoist to a constant.
Apply locally:
- const stepNames = ["free_trial", "signup", "email_verification", "payment_method", "welcome"]; analyticsService.track("onboarding_step_started", { category: "onboarding", - step: stepNames[step], + step: STEP_NAMES[step], step_index: step });- const stepNames = ["free_trial", "signup", "email_verification", "payment_method", "welcome"]; analyticsService.track("onboarding_step_completed", { category: "onboarding", - step: stepNames[step], + step: STEP_NAMES[step], step_index: step });Outside the selected ranges, add near the enum:
const STEP_NAMES = ["free_trial", "signup", "email_verification", "payment_method", "welcome"] as const;Also applies to: 104-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
apps/api/src/app.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.spec.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.ts(3 hunks)apps/api/src/core/repositories/base.repository.ts(2 hunks)apps/api/src/routers/userRouter.ts(1 hunks)apps/api/src/user/controllers/user/user.controller.ts(4 hunks)apps/api/src/user/repositories/user/user.repository.ts(4 hunks)apps/api/src/user/routes/get-current-user/get-current-user.router.ts(1 hunks)apps/api/src/user/routes/index.ts(1 hunks)apps/api/src/user/routes/register-user/register-user.router.ts(1 hunks)apps/api/src/user/schemas/user.schema.ts(1 hunks)apps/api/src/user/services/user/user.service.spec.ts(1 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(2 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(7 hunks)apps/deploy-web/src/pages/api/auth/[...auth0].ts(4 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(1 hunks)apps/deploy-web/src/services/auth/auth.service.ts(2 hunks)apps/deploy-web/src/services/storage/keys.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/api/src/user/routes/get-current-user/get-current-user.router.ts
- apps/api/src/routers/userRouter.ts
- apps/api/src/core/repositories/base.repository.ts
- apps/api/src/auth/services/auth.interceptor.ts
- apps/api/src/auth/services/auth.interceptor.spec.ts
- apps/deploy-web/src/services/app-di-container/app-di-container.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/deploy-web/src/services/auth/auth.service.ts
- apps/api/src/user/schemas/user.schema.ts
- apps/api/src/app.ts
- apps/api/src/user/repositories/user/user.repository.ts
- apps/api/src/user/controllers/user/user.controller.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/src/user/routes/index.ts
- apps/api/src/user/routes/register-user/register-user.router.ts
- apps/deploy-web/src/pages/api/auth/[...auth0].ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/services/storage/keys.tsapps/api/src/user/services/user/user.service.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
**/*.{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/services/storage/keys.tsapps/api/src/user/services/user/user.service.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
**/*.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/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.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/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧠 Learnings (1)
📚 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/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧬 Code Graph Analysis (2)
apps/api/src/user/services/user/user.service.spec.ts (1)
apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
apps/deploy-web/src/services/storage/keys.ts (1)
ONBOARDING_STEP_KEY(1-1)
🪛 ast-grep (0.38.6)
apps/api/src/user/services/user/user.service.spec.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (5)
apps/api/src/user/services/user/user.service.spec.ts (1)
1-6: Good functional test harness and mocking strategySide-effect test setup + jest-mock-extended mocks align with our testing guidelines. Container-resolved repositories enable realistic DB-level coverage.
apps/deploy-web/src/services/storage/keys.ts (1)
1-1: No stray “onboardingStep” references found
The centralizedONBOARDING_STEP_KEYis the sole source of truth—no hard-coded occurrences remain outsideapps/deploy-web/src/services/storage/keys.ts.apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
142-169: Ensure global window overrides are always restored.setup() mutates window.location/history but most tests don’t call cleanup(), risking cross-test leakage.
Either:
- Call cleanup() at the end of every test, or
- Modify setup() to wrap render and auto-restore in a finally block on unmount, or
- Have setup() return a dispose() and use try/finally in tests.
Example (test usage):
const { cleanup, child } = setup(); try { // assertions } finally { cleanup(); }Also applies to: 207-216
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
169-175: Reassess disabling the “Payment Method” step when none exist.If the step is disabled when paymentMethods.length === 0, users may not be able to reach the UI needed to add the first method. Confirm UX: often the step is enabled but its “Next/Continue” is gated until a method is added.
Option if needed:
- isDisabled: paymentMethods.length === 0 + isDisabled: falseand rely on the existing guard in handleStepChange that blocks advancing to WELCOME when no payment methods exist.
116-119: Clear persisted state, then navigate: good.Removing ONBOARDING_STEP_KEY before redirect avoids resuming onboarding after completion.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
Show resolved
Hide resolved
stalniy
force-pushed
the
refactor/user-register
branch
from
632a08c to
13852f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/api/src/user/controllers/user/user.controller.ts (1)
48-54: Simplify return type; avoid union and router-dependent typesgetById always returns { data }, so the union with GetUserResponseOutput is unnecessary and pulls in a router-dependent type. Inline the param type to avoid the router import.
- async getById({ id }: GetUserParams): Promise<{ data: UserSchema } | GetUserResponseOutput> { + async getById({ id }: { id: string }): Promise<{ data: UserSchema }> { const user = await this.userRepository.accessibleBy(this.authService.ability, "read").findById(id); assert(user, 404); return { data: user }; }
♻️ Duplicate comments (3)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
128-131: Use URL API to build signup URL robustly (handles absolute/relative paths and existing query strings)String concatenation breaks if signup() already includes query params or onboarding() returns an absolute URL. Use URL/URLSearchParams to avoid edge cases.
- const returnUrl = `${window.location.origin}${urlService.onboarding(true)}`; - const signupUrl = `${urlService.signup()}?returnTo=${encodeURIComponent(returnUrl)}`; - window.location.href = signupUrl; + const onboardingPath = urlService.onboarding(true); + const returnUrl = new URL(onboardingPath, window.location.origin).toString(); + const signup = new URL(urlService.signup(), window.location.origin); + signup.searchParams.set("returnTo", returnUrl); + window.location.href = signup.toString();apps/api/src/user/controllers/user/user.controller.ts (2)
10-13: Break the import cycle: controller should not import router typesImporting RegisterUserInput/RegisterUserResponse and GetUserParams from routers introduces a dependency cycle (router imports controller). Use service types or inline shapes, and return a structural response.
-import type { GetUserParams } from "@src/user/routes/get-anonymous-user/get-anonymous-user.router"; -import type { RegisterUserInput, RegisterUserResponse } from "@src/user/routes/register-user/register-user.router"; -import { AnonymousUserResponseOutput, GetUserResponseOutput, UserSchema } from "@src/user/schemas/user.schema"; +import type { RegisterUserInput as ServiceRegisterUserInput } from "@src/user/services/user/user.service"; +import { AnonymousUserResponseOutput, UserSchema } from "@src/user/schemas/user.schema";
60-78: Decouple from router types and validate anonymous token explicitly
- Use the service input type (or a narrowed pick) instead of router types to avoid cycles.
- If the anonymous auth header is present but invalid, return 401 to prevent silently treating it as “no anonymous user”.
- async registerUser(data: RegisterUserInput): Promise<RegisterUserResponse> { + async registerUser( + data: Pick<ServiceRegisterUserInput, "wantedUsername" | "email" | "emailVerified" | "subscribedToNewsletter"> + ): Promise<{ data: UserSchema }> { const { req, env, var: httpVars } = this.httpContext; const [userId, anonymousUserId] = await Promise.all([ this.userAuthTokenService.getValidUserId(req.header("authorization") || "", env), this.anonymousUserAuthService.getValidUserId(req.header("x-anonymous-authorization") || "") ]); + // If an anonymous token was provided but failed validation, reject the request + if (req.header("x-anonymous-authorization")) { + assert(anonymousUserId, 401, "Invalid anonymous user token"); + } const user = await this.userService.registerUser({ anonymousUserId, userId, wantedUsername: data.wantedUsername, email: data.email, emailVerified: data.emailVerified, subscribedToNewsletter: !!data.subscribedToNewsletter, ip: httpVars.clientInfo?.ip, userAgent: httpVars.clientInfo?.userAgent, fingerprint: httpVars.clientInfo?.fingerprint }); return { data: user }; }
🧹 Nitpick comments (6)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (4)
49-56: Harden localStorage read and enum bounds checkAccess to localStorage may throw (e.g., blocked/disabled). Also, rely on numeric enum members instead of Object.keys(...)/2 for bounds.
Apply this diff:
useEffect(() => { - const savedStep = localStorage.getItem(ONBOARDING_STEP_KEY); - if (savedStep) { - const step = parseInt(savedStep, 10); - if (step >= 0 && step < Object.keys(OnboardingStepIndex).length / 2) { - setCurrentStep(step); - } - } + let savedStep: string | null = null; + try { + savedStep = localStorage.getItem(ONBOARDING_STEP_KEY); + } catch {} + if (savedStep) { + const step = Number.parseInt(savedStep, 10); + const maxIndex = Object.values(OnboardingStepIndex).filter(v => typeof v === "number").length; + if (Number.isFinite(step) && step >= 0 && step < maxIndex) { + setCurrentStep(step as OnboardingStepIndex); + } + }
65-72: Guard localStorage.setItem when persisting EMAIL_VERIFICATION stepAvoid potential runtime errors if storage is unavailable.
- localStorage.setItem(ONBOARDING_STEP_KEY, OnboardingStepIndex.EMAIL_VERIFICATION.toString()); + try { + localStorage.setItem(ONBOARDING_STEP_KEY, OnboardingStepIndex.EMAIL_VERIFICATION.toString()); + } catch {}
96-98: Guard localStorage.setItem on step changeSame rationale as above; prevents hard failures in restricted environments.
- localStorage.setItem(ONBOARDING_STEP_KEY, step.toString()); + try { + localStorage.setItem(ONBOARDING_STEP_KEY, step.toString()); + } catch {}
116-119: Guard localStorage.removeItem on completionMinor hardening; keeps completion flow resilient.
- localStorage.removeItem(ONBOARDING_STEP_KEY); + try { + localStorage.removeItem(ONBOARDING_STEP_KEY); + } catch {}apps/api/src/user/services/user/user.service.spec.ts (1)
156-158: Avoid dynamic RegExp in tests; prefer startsWith for clarity and safetyConstructing RegExp from variable input is unnecessary here and flagged by static analysis. Use startsWith to assert the prefix.
- expect(newUsers[0].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[0].username.startsWith(conflictingUsername)).toBe(true); @@ - expect(newUsers[1].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[1].username.startsWith(conflictingUsername)).toBe(true);apps/api/src/auth/services/auth.interceptor.ts (1)
117-122: Avoid magic string for HTTP_CONTEXT and revisit error/PII handlingSuggestions:
- Replace the "HTTP_CONTEXT" magic string with a shared typed token/const to avoid typos and improve discoverability.
- If the DB write in markAsActive fails, consider setting the lastUserActivityCache only after a successful write to prevent long silences; or explicitly decide to prefer reduced DB load over guaranteed persistence and document the trade-off.
- Ensure ip and fingerprint usage aligns with your privacy policy (storage duration, hashing/anonymization of fingerprint if needed).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
apps/api/src/app.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.spec.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.ts(3 hunks)apps/api/src/core/repositories/base.repository.ts(2 hunks)apps/api/src/routers/userRouter.ts(1 hunks)apps/api/src/user/controllers/user/user.controller.ts(4 hunks)apps/api/src/user/repositories/user/user.repository.ts(4 hunks)apps/api/src/user/routes/get-current-user/get-current-user.router.ts(1 hunks)apps/api/src/user/routes/index.ts(1 hunks)apps/api/src/user/routes/register-user/register-user.router.ts(1 hunks)apps/api/src/user/schemas/user.schema.ts(1 hunks)apps/api/src/user/services/user/user.service.spec.ts(1 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(2 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(7 hunks)apps/deploy-web/src/pages/api/auth/[...auth0].ts(4 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(1 hunks)apps/deploy-web/src/services/auth/auth.service.ts(2 hunks)apps/deploy-web/src/services/storage/keys.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/api/src/user/schemas/user.schema.ts
- apps/api/src/routers/userRouter.ts
- apps/deploy-web/src/services/storage/keys.ts
- apps/api/src/user/routes/get-current-user/get-current-user.router.ts
- apps/deploy-web/src/services/app-di-container/app-di-container.ts
- apps/api/src/app.ts
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/api/src/user/routes/register-user/register-user.router.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/src/auth/services/auth.interceptor.spec.ts
- apps/api/src/core/repositories/base.repository.ts
- apps/deploy-web/src/services/auth/auth.service.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
- apps/deploy-web/src/pages/api/auth/[...auth0].ts
- apps/api/src/user/repositories/user/user.repository.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/user/routes/index.tsapps/api/src/auth/services/auth.interceptor.tsapps/api/src/user/controllers/user/user.controller.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/api/src/user/services/user/user.service.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/user/routes/index.tsapps/api/src/auth/services/auth.interceptor.tsapps/api/src/user/controllers/user/user.controller.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/api/src/user/services/user/user.service.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/src/user/services/user/user.service.spec.ts
🪛 ast-grep (0.38.6)
apps/api/src/user/services/user/user.service.spec.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (7)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (3)
9-9: Using a shared storage key constant — LGTMImporting ONBOARDING_STEP_KEY centralizes the key and avoids string duplication.
33-38: Expose useRouter via DEPENDENCIES for testability — good changeKeeping router acquisition behind DEPENDENCIES aligns with your DI pattern and simplifies unit testing.
47-47: Switch to urlService from useServices — aligned with DI changesConsistent with the updated ServicesProvider pattern and improves composability.
apps/api/src/user/routes/index.ts (1)
3-4: LGTM: route exports addedThe new route re-exports look correct and maintain the pattern used in this barrel file.
apps/api/src/auth/services/auth.interceptor.ts (3)
36-38: Constructor injection updated — LGTMAdding ExecutionContextService to the interceptor’s constructor is consistent with its usage below.
117-122: Pass HTTP context (ip/fingerprint) to userRepository.markAsActive — LGTMGood use of ExecutionContextService and optional chaining to safely enrich activity tracking with client info.
11-11: ExecutionContextService DI registration and HTTP_CONTEXT setup confirmed
- apps/api/src/core/services/execution-context/execution-context.service.ts
• @singleton() decorator is applied on the ExecutionContextService class.- apps/api/src/core/services/request-context-interceptor/request-context.interceptor.ts (lines 15–18)
• executionContextService.runWithContext wraps each request and calls set("HTTP_CONTEXT", c) before next().The HTTP_CONTEXT key will therefore be populated on each request prior to AuthInterceptor execution. No further action required.
stalniy
force-pushed
the
refactor/user-register
branch
from
13852f7 to
36cdea4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
24-35: Sanitize and validate returnTo to prevent open-redirects and bad decode inputsThe from query param is decoded without validation and forwarded to Auth0 as returnTo. Malformed encodings can throw, and unvalidated absolute/external URLs can enable open-redirects. Prefer a safe, relative path-only fallback.
Apply this diff:
- const returnUrl = decodeURIComponent((req.query.from as string) ?? "/"); + const from = Array.isArray(req.query.from) ? req.query.from[0] : req.query.from; + let returnTo = "/"; + if (typeof from === "string") { + try { + const decoded = decodeURIComponent(from); + // only allow same-origin relative paths + if (decoded.startsWith("/") && !decoded.startsWith("//")) { + returnTo = decoded; + } + } catch { + // ignore invalid encoding and fall back to "/" + } + } @@ - await handleLogin(req, res, { - returnTo: returnUrl, + await handleLogin(req, res, { + returnTo, // Reduce the scope to minimize session data authorizationParams: { scope: "openid profile email" } });
118-127: Avoidanyin catch and use safe property access for status/messageReplace error: any with unknown and derive status/message via narrow type checks.
- } catch (error: any) { - let severity: SeverityLevel = "error"; - if (error?.status && error.status >= 400 && error.status < 500) { - severity = "warning"; - res.status(400).send({ message: error.message }); - } else { - res.status(503).send({ message: "An unexpected error occurred. Please try again later." }); - } - services.errorHandler.reportError({ severity, error, tags: { category: "auth0", event: "AUTH_PROFILE_ERROR" } }); - } + } catch (error: unknown) { + let severity: SeverityLevel = "error"; + const status = typeof (error as { status?: unknown })?.status === "number" ? (error as { status: number }).status : undefined; + if (status && status >= 400 && status < 500) { + severity = "warning"; + const message = + typeof (error as { message?: unknown })?.message === "string" + ? (error as { message: string }).message + : "Bad Request"; + res.status(400).send({ message }); + } else { + res.status(503).send({ message: "An unexpected error occurred. Please try again later." }); + } + services.errorHandler.reportError({ severity, error, tags: { category: "auth0", event: "AUTH_PROFILE_ERROR" } }); + }
♻️ Duplicate comments (3)
apps/api/src/user/repositories/user/user.repository.ts (2)
96-106: Normalize update path, avoid writing undefineds, and set updated_at in upsertUpdate currently uses raw data for set:, bypassing toInput (e.g., user agent truncation) and may write undefineds.
[ suggest_essential_refactor ]
Apply this diff:async upsert(data: UserInput): Promise<UserOutput> { - const [item] = await this.cursor + const insertChanges = this.toInput(data); + const updateChanges = Object.fromEntries(Object.entries(insertChanges).filter(([, v]) => v !== undefined)) as PgUpdateSetSource< + typeof this.table + >; + const [item] = await this.cursor .insert(this.table) - .values(this.toInput(data)) + .values(insertChanges) .onConflictDoUpdate({ target: [this.table.userId], - set: data + set: { + ...updateChanges, + updated_at: sql`now()` + } }) .returning(); return this.toOutput(item); }Additionally, consider normalizing in create() as well to ensure lastUserAgent is consistently truncated:
// outside selected lines; apply near line 31 const [item] = await this.cursor.insert(this.table).values(this.toInput(input)).returning();
62-71: Throttle condition ignores NULL lastActiveAt; first updates will never mark activeRows with lastActiveAt = NULL won’t match lt(NULL, …). Include an isNull branch.
Apply this diff:
-import { and, desc, eq, isNull, lt, lte, SQL, sql } from "drizzle-orm"; +import { and, desc, eq, isNull, lt, lte, or, SQL, sql } from "drizzle-orm"; @@ .where( and( // keep new line eq(this.table.id, id), - lt(this.table.lastActiveAt, sql`now() - make_interval(secs => ${options.throttleTimeSeconds})`) + or( + isNull(this.table.lastActiveAt), + lt(this.table.lastActiveAt, sql`now() - make_interval(secs => ${options.throttleTimeSeconds})`) + ) ) );apps/api/src/user/services/user/user.service.spec.ts (1)
265-271: Align setup() with testing guideline: accept a single parameter with inline typePer repo rules for *.spec.ts: setup should accept a single parameter with inline type to allow overrides.
[ suggest_essential_refactor ]
- function setup() { - const analyticsService = mock<AnalyticsService>(); - const logger = mock<LoggerService>(); + function setup(overrides: { analyticsService?: AnalyticsService; logger?: LoggerService } = {}) { + const analyticsService = overrides.analyticsService ?? mock<AnalyticsService>(); + const logger = overrides.logger ?? mock<LoggerService>(); const service = new UserService(container.resolve(UserRepository), analyticsService, container.resolve(UserWalletRepository), logger); return { service, analyticsService, logger }; }
🧹 Nitpick comments (6)
apps/deploy-web/src/services/auth/auth.service.ts (3)
4-4: Prefer path alias over relative path for storage keys importUse the configured @src alias (as used elsewhere) to avoid brittle relative paths and keep imports consistent across the codebase.
-import { ONBOARDING_STEP_KEY } from "../storage/keys"; +import { ONBOARDING_STEP_KEY } from "@src/services/storage/keys";
27-27: Consider replace() to avoid back navigation into authenticated stateUsing replace prevents the previous page from being reachable via the back button after logout, which is often desired.
- window.location.href = this.urlService.logout(); + window.location.replace(this.urlService.logout());
31-32: Export LogoutUrlService for DI typing reuseExporting the interface allows other modules (e.g., DI container) to type the provider explicitly without duplicating the shape.
-interface LogoutUrlService { +export interface LogoutUrlService { logout(): string; }apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
132-141: Remove explicitanyfrom NextApiResponseDon’t use
any. NextApiResponse’s default generic is sufficient here.-function rewriteLocalRedirect(res: NextApiResponse<any>, config: Pick<ServerEnvConfig, "AUTH0_REDIRECT_BASE_URL" | "AUTH0_ISSUER_BASE_URL">) { +function rewriteLocalRedirect(res: NextApiResponse, config: Pick<ServerEnvConfig, "AUTH0_REDIRECT_BASE_URL" | "AUTH0_ISSUER_BASE_URL">) { const redirect = res.redirect; - res.redirect = function rewriteLocalRedirect(urlOrStatus: string | number, maybeUrl?: string): NextApiResponse<any> { + res.redirect = function rewriteLocalRedirect(urlOrStatus: string | number, maybeUrl?: string): NextApiResponse { const code = typeof urlOrStatus === "string" ? 302 : urlOrStatus; const inputUrl = typeof urlOrStatus === "string" ? urlOrStatus : maybeUrl; const rewritten = config.AUTH0_REDIRECT_BASE_URL ? inputUrl!.replace(config.AUTH0_ISSUER_BASE_URL, config.AUTH0_REDIRECT_BASE_URL || "") : inputUrl!; return redirect.apply(this, [code, rewritten]); }; }
1-1: Update the header comment to reflect .ts fileTiny mismatch can be confusing during debugging and code search.
-// pages/api/auth/[...auth0].js +// pages/api/auth/[...auth0].tsapps/api/src/user/services/user/user.service.spec.ts (1)
156-158: Avoid variable-driven RegExp in tests; use startsWith insteadUsing a dynamic RegExp is unnecessary and can trigger ReDoS linters. startsWith is simpler and precise.
- expect(newUsers[0].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[0].username?.startsWith(conflictingUsername)).toBe(true); @@ - expect(newUsers[1].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[1].username?.startsWith(conflictingUsername)).toBe(true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
apps/api/src/app.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.spec.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.ts(3 hunks)apps/api/src/core/repositories/base.repository.ts(2 hunks)apps/api/src/routers/userRouter.ts(1 hunks)apps/api/src/user/controllers/user/user.controller.ts(4 hunks)apps/api/src/user/repositories/user/user.repository.ts(4 hunks)apps/api/src/user/routes/get-current-user/get-current-user.router.ts(1 hunks)apps/api/src/user/routes/index.ts(1 hunks)apps/api/src/user/routes/register-user/register-user.router.ts(1 hunks)apps/api/src/user/schemas/user.schema.ts(1 hunks)apps/api/src/user/services/user/user.service.spec.ts(1 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(2 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(7 hunks)apps/deploy-web/src/pages/api/auth/[...auth0].ts(4 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(1 hunks)apps/deploy-web/src/services/auth/auth.service.ts(2 hunks)apps/deploy-web/src/services/storage/keys.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/api/src/routers/userRouter.ts
- apps/api/src/user/routes/get-current-user/get-current-user.router.ts
- apps/api/src/user/controllers/user/user.controller.ts
- apps/deploy-web/src/services/storage/keys.ts
- apps/api/src/user/schemas/user.schema.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/src/user/routes/index.ts
- apps/api/src/user/routes/register-user/register-user.router.ts
- apps/api/src/auth/services/auth.interceptor.spec.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
- apps/api/src/app.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
- apps/deploy-web/src/services/app-di-container/app-di-container.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/core/repositories/base.repository.tsapps/api/src/auth/services/auth.interceptor.tsapps/deploy-web/src/services/auth/auth.service.tsapps/api/src/user/repositories/user/user.repository.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/api/src/user/services/user/user.service.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/core/repositories/base.repository.tsapps/api/src/auth/services/auth.interceptor.tsapps/deploy-web/src/services/auth/auth.service.tsapps/api/src/user/repositories/user/user.repository.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/api/src/user/services/user/user.service.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/src/user/services/user/user.service.spec.ts
🧠 Learnings (5)
📚 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} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/src/user/services/user/user.service.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/src/user/services/user/user.service.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} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/user/services/user/user.service.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} : Don't specify return type of `setup` function
Applied to files:
apps/api/src/user/services/user/user.service.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} : Don't use shared state in `setup` function
Applied to files:
apps/api/src/user/services/user/user.service.spec.ts
🧬 Code Graph Analysis (5)
apps/api/src/auth/services/auth.interceptor.ts (2)
apps/api/src/user/controllers/user/user.controller.ts (1)
httpContext(31-33)apps/notifications/src/interfaces/rest/services/auth/auth.service.ts (1)
userId(18-25)
apps/deploy-web/src/services/auth/auth.service.ts (2)
apps/deploy-web/src/config/auth.config.ts (1)
ANONYMOUS_USER_TOKEN_KEY(2-2)apps/deploy-web/src/services/storage/keys.ts (1)
ONBOARDING_STEP_KEY(1-1)
apps/api/src/user/repositories/user/user.repository.ts (1)
apps/api/src/user/model-schemas/user/user.schema.ts (1)
userAgentMaxLength(6-6)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (4)
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
services(22-54)apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(22-32)apps/deploy-web/src/types/user.ts (1)
UserSettings(5-16)packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)
apps/api/src/user/services/user/user.service.spec.ts (3)
apps/api/src/user/routes/register-user/register-user.router.ts (1)
RegisterUserInput(15-15)apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)apps/deploy-web/src/services/analytics/analytics.service.ts (1)
AnalyticsService(141-266)
🪛 ast-grep (0.38.6)
apps/api/src/user/services/user/user.service.spec.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate (apps/stats-web) / should-validate-unsafe / should-validate
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (7)
apps/deploy-web/src/services/auth/auth.service.ts (2)
7-7: Good decoupling via interface for UrlService dependencySwitching to a minimal LogoutUrlService interface resolves the previous typeof misuse and improves testability.
23-26: Logout: correct SSR guard and cleanup-before-redirect orderThe window check prevents SSR issues, and clearing storage before navigation ensures cleanup is not skipped.
apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)
74-77: Type the caught error as unknownPer coding guidelines for TS files: avoid any/implicit any. Explicitly type error as unknown.
[ suggest_essential_refactor ]
- } catch (error) { + } catch (error: unknown) { services.errorHandler.reportError({ error, tags: { category: "auth0", event: "AUTH_CALLBACK_ERROR" } }); throw error; }apps/api/src/core/repositories/base.repository.ts (1)
226-228: Useful and correct unique-violation type guardThe guard is precise and leverages the Postgres error code 23505. Good addition for upsert/collision handling.
apps/api/src/auth/services/auth.interceptor.ts (2)
36-38: DI: ExecutionContextService injection is appropriateWiring ExecutionContextService into the interceptor aligns with controller usage patterns and enables passing contextual client info.
117-123: Propagate client context (ip/fingerprint) to markAsActiveGood use of optional chaining with a throttled update. Complements the repository’s new signature.
apps/api/src/user/repositories/user/user.repository.ts (1)
128-134: Input normalization for lastUserAgent is correctTruncation at userAgentMaxLength is guarded and minimal. LGTM.
stalniy
force-pushed
the
refactor/user-register
branch
from
36cdea4 to
7d186cf
Compare
stalniy
force-pushed
the
refactor/user-register
branch
from
7d186cf to
9a4b605
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (3)
24-31: Harden returnTo handling: decoding safety and open-redirect guardProtect against invalid percent-encoding and restrict to internal paths to avoid open redirects.
- const returnUrl = decodeURIComponent((req.query.from as string) ?? "/"); + const rawFrom = Array.isArray(req.query.from) ? req.query.from[0] : req.query.from; + let safeReturnUrl = "/"; + if (typeof rawFrom === "string") { + try { + const decoded = decodeURIComponent(rawFrom); + // only allow relative internal paths + safeReturnUrl = decoded.startsWith("/") ? decoded : "/"; + } catch { + safeReturnUrl = "/"; + } + } @@ - await handleLogin(req, res, { - returnTo: returnUrl, + await handleLogin(req, res, { + returnTo: safeReturnUrl, // Reduce the scope to minimize session data authorizationParams: { scope: "openid profile email" } });
118-127: Disallowanyin catch; useunknownand narrow safelyPer guidelines, avoid
any. Narrow the error via type checks.- } catch (error: any) { - let severity: SeverityLevel = "error"; - if (error?.status && error.status >= 400 && error.status < 500) { - severity = "warning"; - res.status(400).send({ message: error.message }); - } else { - res.status(503).send({ message: "An unexpected error occurred. Please try again later." }); - } - services.errorHandler.reportError({ severity, error, tags: { category: "auth0", event: "AUTH_PROFILE_ERROR" } }); + } catch (unknownError: unknown) { + let severity: SeverityLevel = "error"; + const hasStatus = (e: unknown): e is { status: number; message?: string } => + typeof e === "object" && e !== null && "status" in e && typeof (e as { status: unknown }).status === "number"; + if (hasStatus(unknownError) && unknownError.status >= 400 && unknownError.status < 500) { + severity = "warning"; + res.status(400).send({ message: unknownError.message ?? "Bad Request" }); + } else { + res.status(503).send({ message: "An unexpected error occurred. Please try again later." }); + } + services.errorHandler.reportError({ + severity, + error: unknownError, + tags: { category: "auth0", event: "AUTH_PROFILE_ERROR" } + }); }
29-35: Ensure Auth0 audience is passed to handleLogin for an access tokenIt looks like you have AUTH0_AUDIENCE defined in your env schemas, but you’re not passing it into
handleLogin. Without anaudienceparam, Auth0 won’t issue an API access token andsession.accessTokenwill remainundefined.• File:
apps/deploy-web/src/pages/api/auth/[...auth0].ts(around lines 29–35)
• Add theaudiencefromprocess.env.AUTH0_AUDIENCEintoauthorizationParamsProposed diff:
await handleLogin(req, res, { returnTo: returnUrl, // Reduce the scope to minimize session data authorizationParams: { - scope: "openid profile email" + scope: "openid profile email", + audience: process.env.AUTH0_AUDIENCE } });• Confirm that
AUTH0_AUDIENCEis set in your runtime environment (and matches the value inapps/deploy-web/src/config/env-config.schema.ts).
♻️ Duplicate comments (5)
apps/deploy-web/src/services/auth/auth.service.ts (1)
7-7: Good decoupling: interface-based dependency instead of typeof value importSwitching to a minimal
LogoutUrlServiceinterface resolves the prior misuse oftypeof UrlServicewith a type-only import and avoids a value import dependency.apps/api/src/user/repositories/user/user.repository.ts (2)
47-71: Fix throttle condition: lastActiveAt NULL rows never update (regression).If lastActiveAt can be NULL (new/legacy users), lt(NULL, …) evaluates to NULL and the row is not matched. Add an OR isNull(lastActiveAt) to allow the first update to go through.
Apply this diff within the where clause:
.where( - and( - // keep new line - eq(this.table.id, id), - lt(this.table.lastActiveAt, sql`now() - make_interval(secs => ${options.throttleTimeSeconds})`) - ) + and( + // keep new line + eq(this.table.id, id), + or( + isNull(this.table.lastActiveAt), + lt(this.table.lastActiveAt, sql`now() - make_interval(secs => ${options.throttleTimeSeconds})`) + ) + ) );Additionally, ensure
oris imported:-import { and, desc, eq, isNull, lt, lte, SQL, sql } from "drizzle-orm"; +import { and, desc, eq, isNull, lt, lte, or, SQL, sql } from "drizzle-orm";
96-106: Normalize and filter update payload in upsert; avoid raw data on update.Insert path uses toInput but update path uses raw data. This bypasses normalization (e.g., lastUserAgent truncation) and may write undefineds. Use a single normalized object for both paths and filter undefineds.
async upsert(data: UserInput): Promise<UserOutput> { - const [item] = await this.cursor + const insertChanges = this.toInput(data); + const updateChanges = Object.fromEntries( + Object.entries(insertChanges).filter(([, v]) => v !== undefined) + ) as PgUpdateSetSource<typeof this.table>; + + const [item] = await this.cursor .insert(this.table) - .values(this.toInput(data)) + .values(insertChanges) .onConflictDoUpdate({ target: [this.table.userId], - set: data + set: updateChanges }) .returning(); return this.toOutput(item); }apps/api/src/user/services/user/user.service.spec.ts (1)
265-271: Align setup() with testing guideline: accept a single parameter with inline type.Supports dependency overrides per test and follows project rules for *.spec.ts files.
- function setup() { - const analyticsService = mock<AnalyticsService>(); - const logger = mock<LoggerService>(); + function setup(overrides: { analyticsService?: AnalyticsService; logger?: LoggerService } = {}) { + const analyticsService = overrides.analyticsService ?? mock<AnalyticsService>(); + const logger = overrides.logger ?? mock<LoggerService>(); const service = new UserService(container.resolve(UserRepository), analyticsService, container.resolve(UserWalletRepository), logger); return { service, analyticsService, logger }; }apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)
47-51: Forward the correct anonymous header and validate Bearer formatThis still reads req.headers.authorization; the browser sets x-anonymous-authorization. Forwarding the wrong header risks leaking unrelated auth headers. Validate it’s a Bearer token before forwarding.
Apply this diff:
- const anonymousAuthorization = req.headers.authorization; - - if (anonymousAuthorization) { - headers.set("x-anonymous-authorization", anonymousAuthorization); - } + const anonymousAuthorizationHeader = + (req.headers["x-anonymous-authorization"] as string | string[] | undefined) ?? req.headers.authorization; + const anonymousAuthorization = Array.isArray(anonymousAuthorizationHeader) + ? anonymousAuthorizationHeader[0] + : anonymousAuthorizationHeader; + if (typeof anonymousAuthorization === "string" && /^Bearer\s+\S+$/i.test(anonymousAuthorization)) { + headers.set("x-anonymous-authorization", anonymousAuthorization); + }
🧹 Nitpick comments (8)
apps/deploy-web/src/services/auth/auth.service.ts (3)
4-4: Prefer path alias for consistency and resilienceUse the existing @src alias (already used on Line 3) to avoid fragile relative paths and keep imports consistent across the codebase.
-import { ONBOARDING_STEP_KEY } from "../storage/keys"; +import { ONBOARDING_STEP_KEY } from "@src/services/storage/keys";
27-27: Use location.replace to prevent back navigation after logoutReplace location assignment with replace() to avoid adding a history entry that can lead users back into a stale authenticated page.
- window.location.href = this.urlService.logout(); + window.location.replace(this.urlService.logout());
31-32: Consider exporting the LogoutUrlService interfaceExporting enables strongly-typed DI wiring and easier mocking in tests across modules. If you want to keep it internal, ignore.
-interface LogoutUrlService { +export interface LogoutUrlService { logout(): string; }apps/api/src/user/services/user/user.service.spec.ts (2)
16-16: Fix test title to match behavior.The test provides a userId, so the description is misleading.
- it("registers a new user if no anonymousUserId and no userId is provided", async () => { + it("registers a new user when no anonymousUserId is provided", async () => {
155-158: Avoid dynamic RegExp in tests; use startsWith for clarity and speed.Reduces complexity and silences static analysis warnings about variable-based regex.
- expect(newUsers[0].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[0].username.startsWith(conflictingUsername)).toBe(true); @@ - expect(newUsers[1].username).toMatch(new RegExp(`^${conflictingUsername}`)); + expect(newUsers[1].username.startsWith(conflictingUsername)).toBe(true);apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
42-42: Prefer camelCase and add a precise type for metadataFollow TS naming conventions and avoid implicitly any-typed values by typing the metadata shape.
- const user_metadata = session.user["https://console.akash.network/user_metadata"]; + const userMetadata = + session.user["https://console.akash.network/user_metadata"] as + | { subscribedToNewsletter?: string } + | undefined; @@ - subscribedToNewsletter: user_metadata?.subscribedToNewsletter === "true" + subscribedToNewsletter: userMetadata?.subscribedToNewsletter === "true"Also applies to: 59-60
1-1: Fix header comment to reflect TypeScript fileThe file is .ts, not .js.
-// pages/api/auth/[...auth0].js +// pages/api/auth/[...auth0].tsapps/api/src/user/controllers/user/user.controller.ts (1)
80-85: Optional: Drop redundant assert under @Protected
@Protectedalready enforces authentication. The additionalassert(this.authService.currentUser, 401)is likely redundant. If you keep it for belt-and-suspenders, it’s fine; otherwise, remove for clarity.@Protected([{ action: "read", subject: "User" }]) async getCurrentUser(): Promise<{ data: UserSchema }> { - assert(this.authService.currentUser, 401); - return { data: this.authService.currentUser as UserSchema }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
apps/api/src/app.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.spec.ts(2 hunks)apps/api/src/auth/services/auth.interceptor.ts(3 hunks)apps/api/src/core/repositories/base.repository.ts(2 hunks)apps/api/src/routers/userRouter.ts(1 hunks)apps/api/src/user/controllers/user/user.controller.ts(4 hunks)apps/api/src/user/repositories/user/user.repository.ts(4 hunks)apps/api/src/user/routes/get-current-user/get-current-user.router.ts(1 hunks)apps/api/src/user/routes/index.ts(1 hunks)apps/api/src/user/routes/register-user/register-user.router.ts(1 hunks)apps/api/src/user/schemas/user.schema.ts(1 hunks)apps/api/src/user/services/user/user.service.spec.ts(1 hunks)apps/api/src/user/services/user/user.service.ts(1 hunks)apps/api/test/services/wallet-testing.service.ts(2 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(7 hunks)apps/deploy-web/src/pages/api/auth/[...auth0].ts(4 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(1 hunks)apps/deploy-web/src/services/auth/auth.service.ts(2 hunks)apps/deploy-web/src/services/storage/keys.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/api/src/user/routes/index.ts
- apps/deploy-web/src/services/storage/keys.ts
- apps/api/src/routers/userRouter.ts
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/api/src/core/repositories/base.repository.ts
- apps/api/src/user/routes/register-user/register-user.router.ts
- apps/api/src/user/routes/get-current-user/get-current-user.router.ts
- apps/deploy-web/src/services/app-di-container/app-di-container.ts
- apps/api/src/user/schemas/user.schema.ts
- apps/api/src/auth/services/auth.interceptor.ts
- apps/api/src/app.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
- apps/api/src/auth/services/auth.interceptor.spec.ts
- apps/api/src/user/services/user/user.service.ts
- apps/api/test/services/wallet-testing.service.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧰 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/deploy-web/src/services/auth/auth.service.tsapps/api/src/user/controllers/user/user.controller.tsapps/api/src/user/repositories/user/user.repository.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/api/src/user/services/user/user.service.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/deploy-web/src/services/auth/auth.service.tsapps/api/src/user/controllers/user/user.controller.tsapps/api/src/user/repositories/user/user.repository.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/api/src/user/services/user/user.service.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/src/user/services/user/user.service.spec.ts
🧠 Learnings (5)
📚 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} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/src/user/services/user/user.service.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/src/user/services/user/user.service.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} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/user/services/user/user.service.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} : Don't specify return type of `setup` function
Applied to files:
apps/api/src/user/services/user/user.service.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} : Don't use shared state in `setup` function
Applied to files:
apps/api/src/user/services/user/user.service.spec.ts
🧬 Code Graph Analysis (5)
apps/deploy-web/src/services/auth/auth.service.ts (2)
apps/deploy-web/src/config/auth.config.ts (1)
ANONYMOUS_USER_TOKEN_KEY(2-2)apps/deploy-web/src/services/storage/keys.ts (1)
ONBOARDING_STEP_KEY(1-1)
apps/api/src/user/controllers/user/user.controller.ts (6)
apps/api/src/user/routes/get-anonymous-user/get-anonymous-user.router.ts (1)
GetUserParams(11-11)apps/api/src/user/schemas/user.schema.ts (3)
UserSchema(24-36)UserSchema(38-38)GetUserResponseOutput(22-22)apps/api/src/user/routes/register-user/register-user.router.ts (2)
RegisterUserInput(15-15)RegisterUserResponse(20-20)apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)apps/notifications/src/interfaces/rest/services/auth/auth.service.ts (1)
userId(18-25)apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)
apps/api/src/user/repositories/user/user.repository.ts (1)
apps/api/src/user/model-schemas/user/user.schema.ts (1)
userAgentMaxLength(6-6)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(22-32)apps/deploy-web/src/types/user.ts (1)
UserSettings(5-17)
apps/api/src/user/services/user/user.service.spec.ts (3)
apps/api/src/user/routes/register-user/register-user.router.ts (1)
RegisterUserInput(15-15)apps/api/src/user/services/user/user.service.ts (1)
RegisterUserInput(139-149)apps/deploy-web/src/services/analytics/analytics.service.ts (2)
analyticsService(274-291)AnalyticsService(141-266)
🪛 ast-grep (0.38.6)
apps/api/src/user/services/user/user.service.spec.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${conflictingUsername})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (6)
apps/deploy-web/src/services/auth/auth.service.ts (1)
23-26: Correct logout order and SSR guardClearing storage before navigation and guarding for non-browser environments is the right sequence.
apps/api/src/user/repositories/user/user.repository.ts (2)
80-88: Confirm whether NULL lastActiveAt should be treated as stale.paginateStaleAnonymousUsers excludes NULL lastActiveAt rows; if “never active” anonymous users should be considered stale, include isNull(lastActiveAt) in the filter.
Proposed adjustment (if desired):
- const clauses = [isNull(this.table.userId), lte(this.table.lastActiveAt, subDays(new Date(), inactivityInDays))]; + const clauses = [ + isNull(this.table.userId), + or(isNull(this.table.lastActiveAt), lte(this.table.lastActiveAt, subDays(new Date(), inactivityInDays))) + ];Also add
orto the drizzle import if not already present.
128-134: LGTM: input normalization bounds user agent length.Truncating lastUserAgent to userAgentMaxLength is correct and minimizes DB/storage issues.
apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)
53-67: Nice integration of register-user and user/me flowsRegistering on callback and merging user settings, then refetching and merging on profile, is cohesive and aligns with the new controller endpoints.
Also applies to: 103-111
apps/api/src/user/controllers/user/user.controller.ts (2)
7-7: LGTM: Importing UserAuthTokenService is appropriateThis aligns with the new flow where user tokens are validated at the controller level. No issues here.
26-29: LGTM: DI additions look correctInjecting ExecutionContextService, UserService, and UserAuthTokenService is consistent with the new responsibilities of the controller. No issues detected.
2 participants
Why
To make logic more transparent and add tests on db level to cover different edge cases. Ref: #1735
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation