-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ui): [#3] Build registration and login forms #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit implements a new shared library (`libs/shared-validation`) and configures the entire monorepo to support it. This was required to share the password validation regex between the frontend and backend. Changes include: - Adding the `@shared/validation` path alias to both the backend and frontend `tsconfig.json` files. - Updating the backend's Jest config (`moduleNameMapper`) to resolve the alias during tests. - Swapping `bcrypt` for `bcryptjs` (a pure-JS library) to resolve a build conflict with Webpack. - Configuring the backend's `nest-cli.json` to use the `webpack` builder, which is required to bundle the aliased `libs` folder for production.
…dundant validations
📝 WalkthroughWalkthroughThis PR implements user authentication UI with registration and login forms, refactors auth DTOs into shared libraries, switches password hashing from bcrypt to bcryptjs, adds CORS configuration to the backend, and updates ESLint/TypeScript configurations across frontend and backend applications. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Frontend as Frontend App
participant AuthService as Auth Service
participant Backend as Backend API
participant DB as Database
User->>Frontend: Enter credentials & submit form
Frontend->>Frontend: Validate form (client-side)
alt Form Invalid
Frontend->>User: Display validation errors
else Form Valid
Frontend->>AuthService: login(credentials) or register(data)
AuthService->>Backend: POST /auth/login or /auth/register
Backend->>Backend: Validate DTO & business logic
alt Valid Request
Backend->>DB: Check/create user
DB-->>Backend: User found/created
Backend->>Backend: Hash password (bcryptjs)
Backend-->>AuthService: Return AccessTokenDto or UserResponseDto
AuthService->>Frontend: Observable response
Frontend->>Frontend: Store token in localStorage
Frontend->>User: Navigate to dashboard / Show success
else Invalid Request
Backend-->>AuthService: Error response
AuthService->>Frontend: Error Observable
Frontend->>User: Display error message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| @@ -0,0 +1,2 @@ | |||
| export const EMAIL_REGEX_STRING: string = | |||
| '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'; | |||
Check failure
Code scanning / CodeQL
Useless regular-expression character escape High
regular expression
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a
regular expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, when writing a regular expression inside a string literal, you must escape characters that are special in regex twice: once for the string literal and once for the regex. In TypeScript/JavaScript strings, \. becomes just ., so to get a literal \. into the regex you must write '\\.' in the string.
To fix this specific instance without changing functionality, update the EMAIL_REGEX_STRING constant so that the domain dot is represented as '\\.' in the string. That way, when the string is passed to new RegExp(EMAIL_REGEX_STRING), the resulting regex will contain \. and will match a literal dot in the domain instead of any character. No additional methods or imports are needed; this is a one-character change (adding an extra backslash) within libs/shared-validation/src/email.constants.ts, on the line where EMAIL_REGEX_STRING is defined.
-
Copy modified line R2
| @@ -1,2 +1,2 @@ | ||
| export const EMAIL_REGEX_STRING: string = | ||
| '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'; | ||
| '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (9)
apps/frontend/src/app/app.config.ts (1)
9-9: LGTM! HttpClient provider correctly configured.The addition of
provideHttpClient()is necessary for the AuthService to make HTTP requests to the backend API. This follows the modern Angular standalone API pattern.Optional: Consider HTTP interceptor for JWT token management.
While the basic HTTP client configuration is complete, authentication flows typically benefit from an HTTP interceptor to automatically attach JWT tokens to outgoing requests. If not already implemented elsewhere, consider adding:
provideHttpClient( withInterceptors([authTokenInterceptor]) )This would centralize token management rather than handling it in each service method. However, if this is planned for future work or handled differently, feel free to disregard.
Also applies to: 15-16
apps/frontend/package.json (1)
6-6: The--openflag is a convenience, but consider team preference.Automatically opening the browser on
npm startcan be helpful during development, but some developers prefer to control when the browser opens manually. Consider whether this aligns with your team's workflow preferences.apps/backend/src/auth/auth.service.ts (1)
48-52: Consider using class-transformer for DTO mapping.While the manual object construction is correct, you could leverage the
plainToClassutility fromclass-transformer(already in dependencies) for more maintainable DTO mapping:import { plainToClass } from 'class-transformer'; const response = plainToClass(UserResponseDto, savedUser, { excludeExtraneousValues: true });This provides better type safety and makes it easier to add transformations or exclusions later.
apps/frontend/src/app/app.routes.ts (1)
3-8: LGTM: Proper lazy loading implementation.The lazy-loaded route configuration follows Angular best practices and will improve initial bundle size.
Consider adding a default route and wildcard route for better user experience:
export const routes: Routes = [ { path: '', redirectTo: '/auth/login', pathMatch: 'full' }, { path: 'auth', loadChildren: () => import('./auth/auth-module').then((m) => m.AuthModule), }, { path: '**', redirectTo: '/auth/login' } ];apps/frontend/src/app/auth/components/login/login.ts (1)
49-49: Remove console.log from production code.The console.log statement should be removed or replaced with a proper logging service for production environments.
🔎 Proposed fix
this.errorMessage = null; localStorage.setItem('access_token', response.access_token); - console.log('Login successful!', response); },apps/backend/package.json (1)
29-29: bcryptjs 3.0.3 is the latest stable version; no critical vulnerabilities in the package itself.Version 3.0.3 (released November 2, 2025) is confirmed as the latest stable release. The package has no known direct security vulnerabilities. However, note that bcryptjs follows the bcrypt specification and truncates passwords longer than 72 bytes—ensure your application validates or enforces a maximum password length before hashing to avoid security assumptions.
Regarding the linting pipeline failure mentioned: no evidence of actual linting errors was provided. Verify the pipeline output and address any specific linting issues if they exist.
apps/frontend/src/app/auth/components/register/register.spec.ts (1)
63-78: Add assertion to verify service is not called on invalid form.The test should also verify that
authService.registerwas not called when the form is invalid.🔎 Proposed enhancement
// Assert expect(component.registerForm.invalid).toBeTrue(); expect(component.registerForm.markAllAsTouched).toHaveBeenCalled(); + expect(authService.register).not.toHaveBeenCalled(); });apps/frontend/src/app/auth/components/login/login.spec.ts (1)
63-77: Add assertion to verify service is not called on invalid form.Similar to the register test, this should also verify that
authService.loginwas not called when the form is invalid.🔎 Proposed enhancement
// Assert expect(component.loginForm.invalid).toBeTrue(); expect(component.loginForm.markAllAsTouched).toHaveBeenCalled(); + expect(authService.login).not.toHaveBeenCalled(); });apps/frontend/src/app/auth/components/register/register.ts (1)
50-54: Consider removing console.log and adding navigation.The
console.logstatement should be removed or replaced with proper logging for production. Additionally, consider whether successful registration should navigate the user (e.g., to login page or auto-login and redirect to dashboard).🔎 Suggested improvements
next: (response) => { this.errorMessage = null; - console.log('Registration successful!', response); + // TODO: Navigate to login page or dashboard after successful registration },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
.github/workflows/lint.ymlapps/backend/eslint.config.mjsapps/backend/nest-cli.jsonapps/backend/package.jsonapps/backend/src/auth/auth.controller.spec.tsapps/backend/src/auth/auth.controller.tsapps/backend/src/auth/auth.service.spec.tsapps/backend/src/auth/auth.service.tsapps/backend/src/auth/dto/access-token.dto.tsapps/backend/src/auth/dto/register-user.dto.tsapps/backend/src/main.tsapps/backend/tsconfig.jsonapps/frontend/eslint.config.jsapps/frontend/package.jsonapps/frontend/src/app/app.config.tsapps/frontend/src/app/app.htmlapps/frontend/src/app/app.routes.tsapps/frontend/src/app/app.spec.tsapps/frontend/src/app/app.tsapps/frontend/src/app/auth/auth-module.tsapps/frontend/src/app/auth/auth-routing-module.tsapps/frontend/src/app/auth/components/login/login.cssapps/frontend/src/app/auth/components/login/login.htmlapps/frontend/src/app/auth/components/login/login.spec.tsapps/frontend/src/app/auth/components/login/login.tsapps/frontend/src/app/auth/components/register/register.cssapps/frontend/src/app/auth/components/register/register.htmlapps/frontend/src/app/auth/components/register/register.spec.tsapps/frontend/src/app/auth/components/register/register.tsapps/frontend/src/app/auth/services/auth.spec.tsapps/frontend/src/app/auth/services/auth.tsapps/frontend/tsconfig.jsonlibs/shared-dtos/src/auth/access-token.dto.tslibs/shared-dtos/src/auth/login-user.dto.tslibs/shared-dtos/src/auth/register-user.dto.tslibs/shared-dtos/src/user/user-response.dto.tslibs/shared-validation/src/email.constants.tslibs/shared-validation/src/password.constants.ts
💤 Files with no reviewable changes (3)
- apps/backend/src/auth/dto/access-token.dto.ts
- apps/backend/src/auth/dto/register-user.dto.ts
- apps/frontend/src/app/app.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-19T16:44:42.665Z
Learnt from: Zafar7645
Repo: Zafar7645/syncup PR: 12
File: apps/backend/src/users/users.service.spec.ts:15-17
Timestamp: 2025-10-19T16:44:42.665Z
Learning: In the syncup repository (apps/backend/src/users/users.service.ts), the UsersService is scaffolding for future implementation and currently contains no methods or business logic. Basic "should be defined" smoke tests are sufficient for empty scaffolding services.
Applied to files:
apps/frontend/src/app/auth/services/auth.tsapps/backend/src/auth/auth.service.tsapps/backend/src/auth/auth.service.spec.tsapps/backend/src/auth/auth.controller.spec.tsapps/frontend/src/app/auth/services/auth.spec.ts
🧬 Code graph analysis (9)
apps/frontend/src/app/auth/services/auth.ts (4)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)libs/shared-dtos/src/auth/login-user.dto.ts (1)
LoginUserDto(3-13)libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
apps/frontend/src/app/auth/components/register/register.ts (4)
apps/frontend/src/app/auth/components/login/login.ts (1)
Component(12-62)libs/shared-validation/src/email.constants.ts (1)
EMAIL_REGEX_STRING(1-2)libs/shared-validation/src/password.constants.ts (2)
PASSWORD_REGEX_STRING(1-2)PASSWORD_VALIDATION_MESSAGE(4-5)libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
libs/shared-validation/src/password.constants.ts (2)
PASSWORD_REGEX_STRING(1-2)PASSWORD_VALIDATION_MESSAGE(4-5)
apps/frontend/src/app/auth/auth-routing-module.ts (2)
apps/frontend/src/app/app.routes.ts (1)
routes(3-8)apps/frontend/src/app/auth/auth-module.ts (1)
NgModule(7-11)
apps/frontend/src/app/auth/auth-module.ts (1)
apps/frontend/src/app/auth/auth-routing-module.ts (1)
NgModule(22-26)
apps/frontend/src/app/auth/components/login/login.spec.ts (1)
libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
apps/frontend/src/app/auth/components/register/register.spec.ts (1)
libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)
apps/frontend/src/app/app.config.ts (1)
apps/frontend/src/app/app.routes.ts (1)
routes(3-8)
apps/frontend/src/app/auth/services/auth.spec.ts (4)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)libs/shared-dtos/src/auth/login-user.dto.ts (1)
LoginUserDto(3-13)libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
🪛 ast-grep (0.40.3)
apps/frontend/src/app/auth/components/register/register.ts
[warning] 23-23: 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(EMAIL_REGEX_STRING)
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] 24-24: 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(PASSWORD_REGEX_STRING)
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)
libs/shared-dtos/src/auth/register-user.dto.ts
[warning] 16-16: 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(PASSWORD_REGEX_STRING)
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)
apps/frontend/src/app/auth/components/login/login.ts
[warning] 22-22: 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(EMAIL_REGEX_STRING)
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] 23-23: 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(PASSWORD_REGEX_STRING)
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] 47-47: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem('access_token', response.access_token)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
🪛 GitHub Actions: Linting Check
apps/backend/src/auth/auth.service.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/eslint.config.mjs
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.service.spec.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/package.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.controller.spec.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/main.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.controller.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/tsconfig.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/nest-cli.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
🪛 GitHub Check: CodeQL
libs/shared-validation/src/email.constants.ts
[failure] 2-2: Useless regular-expression character escape
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
🔇 Additional comments (32)
.github/workflows/lint.yml (1)
3-4: Security improvement: Explicit minimal permissions correctly applied.Adding explicit
permissionswithcontents: readis a best practice that reduces the attack surface by adhering to the principle of least privilege. This is appropriate for a read-only linting workflow.apps/backend/src/main.ts (1)
1-1: Install ESLint dependencies to resolve the module resolution error.The pipeline is failing because ESLint dependencies are missing, not due to code linting issues. The eslint.config.mjs requires
@eslint/jsand other packages that are not found. Runnpm installto install all dependencies before attempting to lint.Likely an incorrect or invalid review comment.
apps/frontend/src/app/app.spec.ts (2)
2-2: LGTM: Path alias usage is correct.The import now uses the
@/path alias configured intsconfig.json, promoting consistent imports across the frontend codebase.
11-15: LGTM: Test suite correctly reduced to match component changes.The removal of the title rendering test aligns with the simplification of the App component (removal of the title signal property).
apps/frontend/tsconfig.json (1)
17-22: LGTM: Path aliases correctly configured for shared libraries.The baseUrl and paths configuration properly enables imports from shared validation and DTO libraries, supporting the monorepo structure.
apps/backend/tsconfig.json (2)
18-21: LGTM: Path aliases correctly mirror frontend configuration.The shared library aliases are properly configured for consistent cross-project imports.
32-32: [Your rewritten review comment text here]
[Exactly ONE classification tag]apps/backend/nest-cli.json (1)
6-6: LGTM: Webpack builder enables path alias resolution.Adding the webpack builder is the correct approach for handling TypeScript path aliases (e.g.,
@shared/*) in NestJS builds.apps/frontend/src/app/app.ts (1)
1-10: LGTM: App component correctly simplified to routing shell.Removing the title signal and simplifying the component to an empty class aligns with the architectural shift toward lazy-loaded feature modules (e.g., auth module) and routing-driven content.
apps/frontend/eslint.config.js (1)
34-39: LGTM: TypeScript project references correctly configured for type-aware linting.The
parserOptionsproperly reference both app and spec TypeScript configurations, enabling type-aware ESLint rules across the frontend codebase.apps/backend/eslint.config.mjs (1)
23-23: Using standard project-based TypeScript discovery with proper configuration.The switch from
projectServicetoproject: truecorrectly aligns with typescript-eslint 8.x's recommended approach. The configuration properly pairsproject: truewithtsconfigRootDir: import.meta.dirname, and thetsconfig.jsonincludes all required paths (@/*,@shared/*) to support bothsrcand shared library files. This setup is compatible with the import resolver configuration at line 36. The ESLint pipeline integration is properly configured for type-checking against the project.libs/shared-dtos/src/auth/login-user.dto.ts (1)
7-7: LGTM: Definite assignment assertions improve type safety.The definite assignment assertions are appropriate here since the class-validator decorators (
@IsNotEmpty,@IsEmail,@IsString) guarantee these properties will be populated during validation.Also applies to: 12-12
libs/shared-dtos/src/user/user-response.dto.ts (1)
6-8: Excellent:readonlyenforces immutability for response data.Using
readonlyproperties for response DTOs prevents accidental mutation and clearly signals these are read-only data structures returned from the API.apps/backend/src/auth/auth.service.spec.ts (3)
4-4: LGTM: Shared DTO imports align with monorepo architecture.The migration to
@shared/dtos/*path aliases correctly centralizes DTOs for reuse across frontend and backend.Also applies to: 11-12
89-283: Well-structured tests with comprehensive coverage.The test suite effectively covers:
- Happy path registration with transaction handling
- Duplicate email conflict detection
- Generic error rollback
- Salt rounds configuration edge cases
1-1: No linting error detected on this import statement.The
TestingModuleandTestsymbols imported on line 1 are both actively used in the file (e.g., line 54:const module: TestingModule = await Test.createTestingModule(...)). The import statement complies with the ESLint configuration and should not block the pipeline.apps/backend/src/auth/auth.service.ts (1)
6-7: LGTM: Consistent migration to shared DTOs and bcryptjs.The import updates correctly reference shared DTOs and align with the bcryptjs migration tested in the spec file.
Also applies to: 10-10, 14-15
libs/shared-dtos/src/auth/access-token.dto.ts (1)
1-3: LGTM: Simple and appropriate token response DTO.The
readonlymodifier correctly prevents mutation, and theaccess_tokenproperty name follows standard JWT response conventions.apps/backend/src/auth/auth.controller.ts (1)
2-2: LGTM: Import updates align with shared DTO architecture.The controller correctly references shared DTOs via the
@shared/dtos/*path aliases.Also applies to: 4-6
apps/frontend/src/app/auth/auth-module.ts (1)
7-10: The emptydeclarationsarray is intentional and correct.The
LoginandRegistercomponents are standalone (indicated by their@Componentdecorators with theimportsarray). Standalone components should not be declared in theNgModule'sdeclarationsarray. Instead, they are imported directly in theAuthRoutingModuleand used in its route definitions, which is the proper pattern for standalone components in Angular. No changes needed.libs/shared-validation/src/password.constants.ts (1)
1-5: LGTM!The password validation constants are well-defined and provide clear, reusable validation rules across the codebase. The regex pattern correctly enforces strong password requirements, and the accompanying message clearly communicates these requirements to users.
apps/frontend/src/app/auth/auth-routing-module.ts (1)
1-26: LGTM!The routing module is correctly implemented with appropriate routes for login and registration, and a sensible default redirect to the login page.
apps/frontend/src/app/auth/components/register/register.html (1)
1-35: LGTM!The registration form template is well-structured with clear validation feedback and appropriate form controls. The error messages provide helpful guidance to users.
apps/frontend/src/app/auth/components/login/login.ts (1)
48-48: Note: JWT stored in localStorage per acceptance criteria.While the acceptance criteria specify storing the JWT in localStorage, be aware this exposes the token to XSS attacks. Consider implementing httpOnly cookies or more secure storage mechanisms in the future for production deployments.
apps/backend/package.json (1)
84-86: LGTM!The Jest moduleNameMapper configuration correctly maps the shared library paths, enabling tests to resolve imports from
@shared/validationand@shared/dtos.apps/frontend/src/app/auth/components/register/register.spec.ts (1)
1-60: LGTM! Comprehensive test setup and validation coverage.The test setup properly mocks the Auth service, and the validation tests thoroughly cover required fields, email pattern, and password pattern validation with both valid and invalid cases.
apps/frontend/src/app/auth/components/login/login.spec.ts (1)
1-60: LGTM! Well-structured test setup and validation coverage.The test configuration and validation tests properly cover all required scenarios for the Login component.
apps/frontend/src/app/auth/components/register/register.ts (3)
19-27: LGTM! Proper use of shared validation constants.The form setup correctly uses shared validation constants for email and password patterns. The static analysis ReDoS warnings can be safely ignored as these regex patterns are constants (not user input) and are relatively simple.
31-41: LGTM! Standard form control getters.The getters provide convenient access to form controls for validation feedback in the template.
55-64: LGTM! Proper error handling with fallback.The error handling correctly extracts the server error message (handling both string and array formats) and provides a sensible default message when no server error is available.
libs/shared-dtos/src/auth/register-user.dto.ts (1)
1-21: LGTM! Well-structured DTO with proper validation.The RegisterUserDto correctly uses class-validator decorators and shared validation constants to ensure consistent validation rules across frontend and backend. The static analysis ReDoS warning can be safely ignored as PASSWORD_REGEX_STRING is a constant with a simple, safe pattern.
apps/backend/src/auth/auth.controller.spec.ts (1)
4-7: [rewritten comment]
[classification tag]
| import { RegisterUserDto } from '@shared/dtos/auth/register-user.dto'; | ||
| import { User } from '@/users/user.entity'; | ||
| import * as bcrypt from 'bcrypt'; | ||
| import * as bcrypt from 'bcryptjs'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find package.json to check bcryptjs version
find . -name "package.json" -type f | head -20Repository: Zafar7645/syncup
Length of output: 131
🏁 Script executed:
# Read the spec file to understand context
cat -n apps/backend/src/auth/auth.service.spec.ts | head -30Repository: Zafar7645/syncup
Length of output: 1231
🌐 Web query:
bcryptjs security vulnerability maintenance status 2024 2025
💡 Result:
Short summary — status (2024–2025)
-
bcryptjs itself: No public direct CVE for bcryptjs in 2024–2025 and security scanners (Snyk) report “no known security issues” for the latest bcryptjs package and classify its maintenance as healthy (recent commits/releases). [1][2]
-
Known behavior to be aware of: bcrypt (and bcryptjs) only uses the first 72 bytes of input — this is an inherent algorithm limitation (not a new bug). Several downstream advisories in 2025 surfaced because apps using bcryptjs (e.g., Strapi) did not enforce or document a max-password length, leading to a medium-severity advisory (CVE-2025-25298) about silent truncation in affected Strapi versions; the fix was to enforce/handle the 72‑byte limit in the application. [3][4][5]
What to do
- Treat bcryptjs as actively maintained but continue to: (a) enforce a max password length (≤72 bytes) or explicitly truncate before hashing, (b) keep bcryptjs and dependent packages up to date, and (c) monitor advisories for downstream integrations (e.g., frameworks that use bcryptjs). [1][3][4]
Sources
- Snyk package page — bcryptjs (maintenance/health). [1]
- Snyk/npm health data for bcrypt/bcryptjs. [2]
- NVD / Tenable / dbugs coverage of CVE-2025-25298 (Strapi using bcryptjs and 72‑byte truncation). [3][4][5]
🌐 Web query:
bcryptjs vs bcrypt performance comparison production use
💡 Result:
Short answer: use native bcrypt in production when you can — it's noticeably faster (~20–30%+) and offloads work to libuv worker threads so it doesn't block the event loop. Use bcryptjs only when native compilation / native bindings are impossible (browser, some edge runtimes or constrained CI/builds) or you need a zero-dependency JS implementation; expect slower hashing and avoid synchronous calls (they block). [1][2][3][4]
Notes (very brief):
- Performance: native bcrypt (C++ binding) wins in benchmarks vs bcryptjs (pure JS) — typically faster by ~20–30%. [3][4]
- Non-blocking behavior: bcrypt (native) runs in libuv threadpool for async calls; bcryptjs is pure JS so async still runs on the main thread unless you use worker threads — avoid hashSync. [3][5]
- Production recommendation: prefer native bcrypt or (better) Argon2 for new projects; use bcryptjs only for environments where native modules are not viable. [1][2]
Sources:
[1] npm bcryptjs package readme / notes on performance.
[2] bcrypt vs bcryptjs comparison articles and package comparisons.
[3] Benchmarks / blog posts and community benchmarks showing native bcrypt speed advantage.
[4] Practical guides / tutorials advising native bcrypt and warning about sync blocking.
🏁 Script executed:
# Check bcryptjs version in backend package.json
cat apps/backend/package.json | grep -A 2 -B 2 bcryptRepository: Zafar7645/syncup
Length of output: 400
🏁 Script executed:
# Check if the actual service implementation (not spec) uses bcryptjs
find apps/backend/src/auth -name "auth.service.ts" | head -5Repository: Zafar7645/syncup
Length of output: 97
🏁 Script executed:
# Read the actual auth service to see how bcryptjs is used in production
cat -n apps/backend/src/auth/auth.service.ts | head -100Repository: Zafar7645/syncup
Length of output: 3664
Remove security concern; performance trade-off is acceptable for async password hashing.
bcryptjs 3.0.3 is actively maintained and has no known security vulnerabilities. However, it is ~20–30% slower than native bcrypt. Since the service correctly uses async methods (bcrypt.hash() and bcrypt.compare()), it avoids blocking the event loop. For password hashing operations—which are intentionally computationally expensive—this performance difference is acceptable for most applications.
If production performance is a critical concern, consider evaluating native bcrypt (requires compiler toolchain) or Argon2 (better security properties) for future improvements.
🤖 Prompt for AI Agents
In apps/backend/src/auth/auth.service.spec.ts around line 6, the import of
bcryptjs is flagged as a security concern; keep the import as-is (import * as
bcrypt from 'bcryptjs') because the code uses async bcrypt.hash/compare and
bcryptjs has no known vulnerabilities; update the file by adding a short inline
comment next to the import noting that async methods are used so the event loop
is not blocked and that native bcrypt/Argon2 can be considered later if
production performance becomes critical.
| app.enableCors({ | ||
| origin: 'http://localhost:4200', | ||
| methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', | ||
| credentials: true, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded origin will break production deployments.
The CORS origin is hardcoded to http://localhost:4200, which will prevent the frontend from making requests in production or staging environments.
🔎 Recommended fix: Use environment variables
app.enableCors({
- origin: 'http://localhost:4200',
+ origin: process.env.FRONTEND_URL ?? 'http://localhost:4200',
methods: 'GET,HEAD,PUT,PATCH,POST,DELETE',
credentials: true,
});Add FRONTEND_URL to your .env files for each environment (production, staging, etc.).
Optional: Since the PR mentions storing JWT in localStorage, the credentials: true flag may not be strictly necessary (it's primarily for cookies/auth headers). However, keeping it doesn't cause issues and provides flexibility for future auth patterns.
📝 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.
| app.enableCors({ | |
| origin: 'http://localhost:4200', | |
| methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', | |
| credentials: true, | |
| }); | |
| app.enableCors({ | |
| origin: process.env.FRONTEND_URL ?? 'http://localhost:4200', | |
| methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', | |
| credentials: true, | |
| }); |
🤖 Prompt for AI Agents
In apps/backend/src/main.ts around lines 14 to 18, the CORS origin is hardcoded
to 'http://localhost:4200' which will break non-local environments; replace the
literal with an environment-driven value (e.g. process.env.FRONTEND_URL or via
your config service) and provide a sensible default/fallback for local dev, and
if needed support multiple origins by parsing a comma-separated env var into an
array or using a regex; keep credentials: true if you want cookie support but
it's optional for token-in-localStorage flows.
| "class-transformer": "^0.5.1", | ||
| "class-validator": "^0.14.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
class-transformer npm latest version security advisories December 2025
💡 Result:
Summary (as of Dec 24, 2025)
- Latest official class-transformer release on npm: 0.5.1. [1][2]
- Known security advisory: CVE-2020-7637 — Prototype Pollution in classToPlainFromExist affecting versions < 0.3.1; fixed in 0.3.1. [3][4]
- Current status: no known vulnerabilities affecting the latest 0.5.1 (Snyk reports no direct vulnerabilities for 0.5.1). [2]
Sources
- npm / jsDelivr package metadata showing version 0.5.1. [1]
- Snyk package page (latest version 0.5.1; no known security issues on latest). [2]
- GitLab Advisory / CVE-2020-7637 (details, affected versions <0.3.1, fixed in 0.3.1). [3]
- NVD / third‑party vulnerability summaries (CVE-2020-7637 writeups). [4]
If you want, I can: (a) show which of your projects depend on vulnerable versions, (b) provide an npm update/patch plan, or (c) fetch the exact npm package page / changelog for 0.5.1.
🌐 Web query:
class-validator npm latest version security advisories December 2025
💡 Result:
Summary (as of Dec 24, 2025)
- Latest class-validator release: 0.14.3. [1][2]
- Direct security status: no known vulnerabilities in 0.14.3; earlier versions (<0.14.0) are affected by an improper-input-validation issue and should be upgraded. [3][4]
- Related dependency advisory: validator.js (used by class-validator) had CVE-2025-12758 (fixed in validator >= 13.15.22); ensure transitive dependency is up-to-date. [5]
Recommended action
- Upgrade to class-validator@0.14.3 and confirm your lockfile (package-lock / yarn.lock / pnpm-lock) picks a patched validator.js (>=13.15.22). Re-scan with your SCA tooling (Snyk/OSS-Scanner/etc.). [3][5]
Sources
- jsDelivr/npm package info for class-validator (v0.14.3). [turn1search4]
- Snyk / package page showing latest version and security status (class-validator latest = 0.14.3). [turn1search1]
- Snyk vulnerability notes advising upgrade to >=0.14.0 to fix an improper input validation bypass. [turn1search3]
- Snyk package vulnerability list / changelog entries referencing 0.14.3 as latest. [turn0search6]
- GitLab / advisory for validator.js CVE-2025-12758 (fixed in 13.15.22). [turn0search3]
🌐 Web query:
class-transformer 0.5.1 CVE security vulnerabilities
💡 Result:
Short answer: class-transformer v0.5.1 has no publicly listed CVE affecting that version. The known CVE (CVE‑2020‑7637 / prototype pollution) affects versions < 0.3.1 and was fixed in 0.3.1 — later releases including 0.5.1 are not listed as vulnerable. [1][2][3]
Sources:
- Snyk vulnerability entry (CVE-2020-7637) — affected <0.3.1, fixed in 0.3.1. [1]
- Snyk package page for class-transformer — no direct vulnerabilities for latest (0.5.1). [2]
- GitLab/NPM advisory summarizing the prototype-pollution issue and fixed versions. [3]
If you want, I can search again for any newer advisories (post‑Dec 24, 2025) or scan your project dependency tree for transitive issues.
🌐 Web query:
class-validator 0.14.2 CVE security vulnerabilities
💡 Result:
Short answer: There are no known CVEs published that affect class-validator version 0.14.2. [1]
Notes and sources:
- Snyk’s database shows no direct vulnerabilities for class-validator@0.14.2 and marks 0.14.3 as the latest non‑vulnerable version. [1]
- A previously published issue (CVE-2019-18413) describes an input‑validation bypass that was fixed by upgrading to 0.14.0 or later — i.e., versions >=0.14.0 (including 0.14.2) are not vulnerable to that CVE. [2]
- Official package release info (npm / release history) showing 0.14.x releases. [3]
Sources:
[1] Snyk — class-validator@0.14.2 vulnerabilities. (security.snyk.io)
[2] Vulert / CVE-2019-18413 (disclosure & mitigation notes). (vulert.com)
[3] npm / release history for class-validator (0.14.x). (npmjs.com)
If you want, I can (a) scan your project for indirect vulnerabilities that affect class-validator via dependencies, or (b) monitor for any newly published advisories.
Citations:
- 1: https://security.snyk.io/package/npm/class-validator/0.14.2?utm_source=openai
- 2: https://vulert.com/vuln-db/CVE-2019-18413?utm_source=openai
- 3: https://www.npmjs.com/package/class-validator?utm_source=openai
Consider upgrading class-validator to 0.14.3 and verify transitive dependencies.
class-transformer@0.5.1is current and secure (no known vulnerabilities).class-validator@0.14.2is secure but patch version 0.14.3 is available; upgrade recommended for consistency.- Verify that the lockfile (
package-lock.json,yarn.lock, orpnpm-lock.yaml) pullsvalidator.js@≥13.15.22to address CVE-2025-12758 in the transitive dependency chain.
🤖 Prompt for AI Agents
In apps/frontend/package.json around lines 32 to 33, update the class-validator
dependency from 0.14.2 to 0.14.3 and then regenerate the lockfile (npm install /
yarn install / pnpm install) to ensure transitive dependencies are refreshed;
after installing, inspect the resulting lockfile to confirm validator.js
resolves to >=13.15.22 (or patch it via resolutions/overrides if your package
manager supports it), run your test suite, and commit the updated package.json
and lockfile.
| <label for="email">Email</label> | ||
| <input id="name" type="email" formControlName="email" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix mismatched id attribute.
The label's for attribute references "email", but the input's id is set to "name". This breaks the label-input association, preventing users from clicking the label to focus the input field, which is an accessibility issue.
🔎 Proposed fix
<label for="email">Email</label>
- <input id="name" type="email" formControlName="email" />
+ <input id="email" type="email" formControlName="email" />📝 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.
| <label for="email">Email</label> | |
| <input id="name" type="email" formControlName="email" /> | |
| <label for="email">Email</label> | |
| <input id="email" type="email" formControlName="email" /> |
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/components/login/login.html around lines 2 to 3,
the label's for attribute is "email" but the input id is "name", breaking the
label-input association; change the input's id to "email" so it matches the
label's for attribute (or alternatively change the label's for to "name" if
intended), ensuring the formControlName remains "email" and preserving
accessibility.
| }); | ||
|
|
||
| describe('onSubmit', () => { | ||
| it('should call markAllAsTouched on registerForm and return when form is invalid', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix test description to reference loginForm instead of registerForm.
The test description incorrectly references "registerForm" when it should say "loginForm".
🔎 Proposed fix
- it('should call markAllAsTouched on registerForm and return when form is invalid', () => {
+ it('should call markAllAsTouched on loginForm and return when form is invalid', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should call markAllAsTouched on registerForm and return when form is invalid', () => { | |
| it('should call markAllAsTouched on loginForm and return when form is invalid', () => { |
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/components/login/login.spec.ts around line 63, the
test description incorrectly mentions "registerForm" instead of "loginForm";
update the it(...) description string to reference "loginForm" (e.g., "should
call markAllAsTouched on loginForm and return when form is invalid") so it
accurately reflects the test behavior.
| it('should set the error message received from server on failure', () => { | ||
| // Arrange | ||
| const mockUserData = { | ||
| email: 'email@test.com', | ||
| password: 'TestPassword@123', | ||
| }; | ||
| component.loginForm.setValue(mockUserData); | ||
| const errorResponse = { | ||
| error: { | ||
| message: 'Unable to complete registration.', | ||
| }, | ||
| }; | ||
| authService.login.and.returnValue(throwError(() => errorResponse)); | ||
|
|
||
| // Act | ||
| component.onSubmit(); | ||
|
|
||
| // Assert | ||
| expect(authService.login).toHaveBeenCalledWith(mockUserData); | ||
| expect(component.errorMessage).toBe('Unable to complete registration.'); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error message to be login-specific.
The error message references "registration" but this is a login test. It should use a login-specific error message.
🔎 Proposed fix
const errorResponse = {
error: {
- message: 'Unable to complete registration.',
+ message: 'Invalid credentials.',
},
};
authService.login.and.returnValue(throwError(() => errorResponse));
// Act
component.onSubmit();
// Assert
expect(authService.login).toHaveBeenCalledWith(mockUserData);
- expect(component.errorMessage).toBe('Unable to complete registration.');
+ expect(component.errorMessage).toBe('Invalid credentials.');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should set the error message received from server on failure', () => { | |
| // Arrange | |
| const mockUserData = { | |
| email: 'email@test.com', | |
| password: 'TestPassword@123', | |
| }; | |
| component.loginForm.setValue(mockUserData); | |
| const errorResponse = { | |
| error: { | |
| message: 'Unable to complete registration.', | |
| }, | |
| }; | |
| authService.login.and.returnValue(throwError(() => errorResponse)); | |
| // Act | |
| component.onSubmit(); | |
| // Assert | |
| expect(authService.login).toHaveBeenCalledWith(mockUserData); | |
| expect(component.errorMessage).toBe('Unable to complete registration.'); | |
| }); | |
| it('should set the error message received from server on failure', () => { | |
| // Arrange | |
| const mockUserData = { | |
| email: 'email@test.com', | |
| password: 'TestPassword@123', | |
| }; | |
| component.loginForm.setValue(mockUserData); | |
| const errorResponse = { | |
| error: { | |
| message: 'Invalid credentials.', | |
| }, | |
| }; | |
| authService.login.and.returnValue(throwError(() => errorResponse)); | |
| // Act | |
| component.onSubmit(); | |
| // Assert | |
| expect(authService.login).toHaveBeenCalledWith(mockUserData); | |
| expect(component.errorMessage).toBe('Invalid credentials.'); | |
| }); |
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/components/login/login.spec.ts around lines 99 to
119, the test's mocked server error message incorrectly mentions "registration"
for a login test; update the mocked errorResponse.message to a login-specific
message (for example "Unable to complete login.") and update the assertion to
expect that login-specific message (and optionally update the test description
to reflect it's a login failure).
| if (err.error && err.error.message) { | ||
| this.errorMessage = Array.isArray(err.error.errorMessage) | ||
| ? err.error.message[0] | ||
| : err.error.message; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error property mismatch.
Line 53 checks for err.error.errorMessage, but Line 54 accesses err.error.message[0]. This property name mismatch will cause the error handling to fail when the condition on Line 53 is true.
🔎 Proposed fix
error: (err) => {
if (err.error && err.error.message) {
- this.errorMessage = Array.isArray(err.error.errorMessage)
+ this.errorMessage = Array.isArray(err.error.message)
? err.error.message[0]
: err.error.message;
} else {📝 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.
| if (err.error && err.error.message) { | |
| this.errorMessage = Array.isArray(err.error.errorMessage) | |
| ? err.error.message[0] | |
| : err.error.message; | |
| if (err.error && err.error.message) { | |
| this.errorMessage = Array.isArray(err.error.message) | |
| ? err.error.message[0] | |
| : err.error.message; |
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/components/login/login.ts around lines 52 to 55,
the condition checks Array.isArray(err.error.errorMessage) but the assignment
reads from err.error.message, causing a property mismatch; update the check and
assignment to use the same property (either consistently use err.error.message
or consistently use err.error.errorMessage). Specifically, if the backend
returns message as an array, change the condition to
Array.isArray(err.error.message) and set this.errorMessage to
err.error.message[0] (otherwise err.error.message), or alternatively use
err.error.errorMessage in both places if that is the canonical field.
| }); | ||
|
|
||
| describe('login', () => { | ||
| it('should POST to /login an return acces token', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in test description.
The test description contains typos: "an return acces token" should be "and return access token".
🔎 Proposed fix
- it('should POST to /login an return acces token', () => {
+ it('should POST to /login and return access token', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should POST to /login an return acces token', () => { | |
| it('should POST to /login and return access token', () => { |
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/services/auth.spec.ts around line 60, the test
description string contains typos ("should POST to /login an return acces
token"); update the description to read "should POST to /login and return access
token" so it is grammatically correct and spells "access" correctly.
| }) | ||
| export class Auth { | ||
| private httpClient = inject(HttpClient); | ||
| private apiUrl = 'http://localhost:3000/auth'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Use environment configuration for API URL.
The API URL is hardcoded to localhost, which will cause issues in different environments (development, staging, production). Move this to Angular environment configuration files.
🔎 Proposed fix
Create or update apps/frontend/src/environments/environment.ts:
export const environment = {
production: false,
apiUrl: 'http://localhost:3000'
};Then update the service:
+import { environment } from '@/environments/environment';
+
@Injectable({
providedIn: 'root',
})
export class Auth {
private httpClient = inject(HttpClient);
- private apiUrl = 'http://localhost:3000/auth';
+ private apiUrl = `${environment.apiUrl}/auth`;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/frontend/src/app/auth/services/auth.ts around line 14 the API URL is
hardcoded to 'http://localhost:3000/auth'; move this into Angular environment
configuration: add apiUrl (e.g., 'http://localhost:3000') to
apps/frontend/src/environments/environment.ts and
apps/frontend/src/environments/environment.prod.ts (or update existing), import
the environment in the service, replace the hardcoded string with
environment.apiUrl + '/auth', and ensure imports/paths are correct and the
service uses the environment value for all auth requests.
| export const EMAIL_REGEX_STRING: string = | ||
| '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix regex to properly escape dots for literal matching.
The email regex has a critical validation flaw. In JavaScript string literals, \. becomes just . after string parsing, which in regex matches any character, not a literal dot. This allows invalid emails like test@example/com to pass validation.
🔎 Proposed fix
export const EMAIL_REGEX_STRING: string =
- '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$';
+ '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$';Or use a regex literal with proper escaping if consumed as a pattern object elsewhere:
export const EMAIL_REGEX = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 2-2: Useless regular-expression character escape
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
🤖 Prompt for AI Agents
In libs/shared-validation/src/email.constants.ts around lines 1 to 2, the string
regex uses a single backslash which is lost in the JS string literal so `\.`
becomes `.` (matching any char) and allows invalid emails; fix by either
doubling the backslash so the string contains `\\.` (i.e.
'^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$') or replace the exported
value with a RegExp literal that contains an escaped dot (e.g. export the
pattern as a RegExp) so the dot matches a literal period.
This PR implements the frontend authentication UI and connects it to the NestJS backend, completing the requirements for Issue #3.
Key Changes:
Closes: #3
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.