Skip to content

Conversation

@Zafar7645
Copy link
Owner

@Zafar7645 Zafar7645 commented Dec 24, 2025

This PR implements the frontend authentication UI and connects it to the NestJS backend, completing the requirements for Issue #3.

Key Changes:

  • UI Components: Created Register and Login standalone components using Angular Reactive Forms.
  • Validation: Implemented strict form validation using shared constants (Regex patterns for email and password).
  • Backend Integration:
    • Created AuthService to handle HTTP requests.
    • Enabled CORS in the backend (main.ts) to allow requests from localhost:4200.
  • Architecture:
    • Moved DTOs to a new shared library (libs/shared-dtos) for type safety across frontend and backend.
    • Updated tsconfig.json paths to support shared libraries.
  • Testing: Added comprehensive unit tests (Jasmine/Karma) for:
    • RegisterComponent
    • LoginComponent
    • AuthService (using provideHttpClientTesting)

Closes: #3

Summary by CodeRabbit

  • New Features

    • Added authentication system with login and registration pages, including real-time form validation with password strength and email format checks.
    • Enabled automatic browser opening when starting the frontend application.
  • Bug Fixes

    • Configured CORS to allow frontend communication with backend.
  • Chores

    • Updated dependencies and build configurations for improved development workflows.

✏️ Tip: You can customize this high-level summary in your review settings.

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.
@coderabbitai
Copy link

coderabbitai bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Configuration & Build Tooling
.github/workflows/lint.yml, apps/backend/eslint.config.mjs, apps/backend/nest-cli.json, apps/frontend/eslint.config.js
Updated GitHub Actions permissions, migrated TypeScript ESLint from deprecated projectService to project-based discovery, added webpack builder to NestJS config, configured ESLint parserOptions for frontend TypeScript project files.
Backend Dependencies & Configuration
apps/backend/package.json, apps/backend/tsconfig.json
Replaced bcrypt with bcryptjs, added Jest moduleNameMapper entries for shared lib imports (@shared/validation/*, @shared/dtos/*), extended TypeScript path aliases for shared libraries.
Backend Auth Service & Controllers (Import Updates)
apps/backend/src/auth/auth.controller.ts, apps/backend/src/auth/auth.controller.spec.ts, apps/backend/src/auth/auth.service.ts, apps/backend/src/auth/auth.service.spec.ts, apps/backend/src/main.ts
Updated DTO import paths from local aliases to shared library paths (@shared/dtos/...), switched password hashing dependency from bcrypt to bcryptjs in auth service, added CORS configuration to application bootstrap.
Backend DTO Files (Deleted)
apps/backend/src/auth/dto/register-user.dto.ts, apps/backend/src/auth/dto/access-token.dto.ts
Removed RegisterUserDto and AccessTokenDto classes; these DTOs are now centralized in shared libraries.
Shared DTO Library
libs/shared-dtos/src/auth/access-token.dto.ts, libs/shared-dtos/src/auth/login-user.dto.ts, libs/shared-dtos/src/auth/register-user.dto.ts, libs/shared-dtos/src/user/user-response.dto.ts
Created new AccessTokenDto, updated LoginUserDto and UserResponseDto with non-null assertions, introduced RegisterUserDto with class-validator decorators for name, email, and password validation.
Shared Validation Library
libs/shared-validation/src/email.constants.ts, libs/shared-validation/src/password.constants.ts
Added EMAIL_REGEX_STRING constant and PASSWORD_REGEX_STRING with PASSWORD_VALIDATION_MESSAGE for form validation rules.
Frontend Dependencies & Core Configuration
apps/frontend/package.json, apps/frontend/tsconfig.json, apps/frontend/src/app/app.config.ts
Added class-validator and class-transformer dependencies, configured auto-open browser on start, added baseUrl and path aliases including shared library mappings, injected provideHttpClient into app config.
Frontend Routing & Components
apps/frontend/src/app/app.routes.ts, apps/frontend/src/app/app.html, apps/frontend/src/app/app.ts, apps/frontend/src/app/app.spec.ts
Replaced empty routes with lazy-loaded auth module route, removed placeholder UI template, simplified App component by removing signal-based title.
Frontend Auth Module & Routing
apps/frontend/src/app/auth/auth-module.ts, apps/frontend/src/app/auth/auth-routing-module.ts, apps/frontend/src/app/auth/services/auth.ts, apps/frontend/src/app/auth/services/auth.spec.ts
Created AuthModule and AuthRoutingModule with child routes for login/register, implemented Auth service with register and login methods consuming backend API endpoints.
Frontend Login Component
apps/frontend/src/app/auth/components/login/login.ts, apps/frontend/src/app/auth/components/login/login.html, apps/frontend/src/app/auth/components/login/login.spec.ts
Built Login component with reactive form (email, password), client-side validation, error message display, onSubmit handler calling Auth.login, comprehensive unit tests covering form validation and API interactions.
Frontend Register Component
apps/frontend/src/app/auth/components/register/register.ts, apps/frontend/src/app/auth/components/register/register.html, apps/frontend/src/app/auth/components/register/register.spec.ts
Built Register component with reactive form (name, email, password), decorator-based validation, error handling with default fallback message, onSubmit calls Auth.register with form data, complete test suite for validation and registration flow.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested labels

feature, frontend, backend

Poem

🐰 Hops of joy across the auth walls,
Forms now validate when user calls,
Shared DTOs in libraries dance,
Login, register—give them a chance!
From backend to frontend, the data will flow,
A secure auth journey, now watch it grow! 🔐✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Out of Scope Changes check ❓ Inconclusive Several backend changes appear tangential to the UI forms objective: bcrypt→bcryptjs swap, webpack builder config, ESLint parser upgrades, and CI permissions—these may support the frontend but their necessity is unclear. Clarify whether backend refactoring (bcryptjs, webpack, ESLint configs, CI) is required for Issue #3 or constitutes separate scope; consider splitting into separate PRs if independent of UI forms feature.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(ui): [#3] Build registration and login forms' clearly summarizes the main change: building registration and login UI forms for Issue #3, which is the primary objective of this changeset.
Linked Issues check ✅ Passed The PR successfully implements all coding-related requirements from Issue #3: AuthModule with /login and /register routes, registration form with validation, login form, AuthService for API calls, error message display, and shared DTOs/validation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/3-user-forms

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@@ -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

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
.

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.

Suggested changeset 1
libs/shared-validation/src/email.constants.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/libs/shared-validation/src/email.constants.ts b/libs/shared-validation/src/email.constants.ts
--- a/libs/shared-validation/src/email.constants.ts
+++ b/libs/shared-validation/src/email.constants.ts
@@ -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,}$';
EOF
@@ -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,}$';
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

@coderabbitai coderabbitai bot left a 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 --open flag is a convenience, but consider team preference.

Automatically opening the browser on npm start can 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 plainToClass utility from class-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.register was 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.login was 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.log statement 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

📥 Commits

Reviewing files that changed from the base of the PR and between 89b7d34 and 7f03a38.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (38)
  • .github/workflows/lint.yml
  • apps/backend/eslint.config.mjs
  • apps/backend/nest-cli.json
  • apps/backend/package.json
  • apps/backend/src/auth/auth.controller.spec.ts
  • apps/backend/src/auth/auth.controller.ts
  • apps/backend/src/auth/auth.service.spec.ts
  • apps/backend/src/auth/auth.service.ts
  • apps/backend/src/auth/dto/access-token.dto.ts
  • apps/backend/src/auth/dto/register-user.dto.ts
  • apps/backend/src/main.ts
  • apps/backend/tsconfig.json
  • apps/frontend/eslint.config.js
  • apps/frontend/package.json
  • apps/frontend/src/app/app.config.ts
  • apps/frontend/src/app/app.html
  • apps/frontend/src/app/app.routes.ts
  • apps/frontend/src/app/app.spec.ts
  • apps/frontend/src/app/app.ts
  • apps/frontend/src/app/auth/auth-module.ts
  • apps/frontend/src/app/auth/auth-routing-module.ts
  • apps/frontend/src/app/auth/components/login/login.css
  • apps/frontend/src/app/auth/components/login/login.html
  • apps/frontend/src/app/auth/components/login/login.spec.ts
  • apps/frontend/src/app/auth/components/login/login.ts
  • apps/frontend/src/app/auth/components/register/register.css
  • apps/frontend/src/app/auth/components/register/register.html
  • apps/frontend/src/app/auth/components/register/register.spec.ts
  • apps/frontend/src/app/auth/components/register/register.ts
  • apps/frontend/src/app/auth/services/auth.spec.ts
  • apps/frontend/src/app/auth/services/auth.ts
  • apps/frontend/tsconfig.json
  • libs/shared-dtos/src/auth/access-token.dto.ts
  • libs/shared-dtos/src/auth/login-user.dto.ts
  • libs/shared-dtos/src/auth/register-user.dto.ts
  • libs/shared-dtos/src/user/user-response.dto.ts
  • libs/shared-validation/src/email.constants.ts
  • libs/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.ts
  • apps/backend/src/auth/auth.service.ts
  • apps/backend/src/auth/auth.service.spec.ts
  • apps/backend/src/auth/auth.controller.spec.ts
  • apps/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 permissions with contents: read is 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/js and other packages that are not found. Run npm install to 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 in tsconfig.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 parserOptions properly 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 projectService to project: true correctly aligns with typescript-eslint 8.x's recommended approach. The configuration properly pairs project: true with tsconfigRootDir: import.meta.dirname, and the tsconfig.json includes all required paths (@/*, @shared/*) to support both src and 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: readonly enforces immutability for response data.

Using readonly properties 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 TestingModule and Test symbols 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 readonly modifier correctly prevents mutation, and the access_token property 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 empty declarations array is intentional and correct.

The Login and Register components are standalone (indicated by their @Component decorators with the imports array). Standalone components should not be declared in the NgModule's declarations array. Instead, they are imported directly in the AuthRoutingModule and 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/validation and @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';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find package.json to check bcryptjs version
find . -name "package.json" -type f | head -20

Repository: 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 -30

Repository: 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 bcrypt

Repository: 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 -5

Repository: 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 -100

Repository: 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.

Comment on lines +14 to +18
app.enableCors({
origin: 'http://localhost:4200',
methods: 'GET,HEAD,PUT,PATCH,POST,DELETE',
credentials: true,
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.

Comment on lines +32 to +33
"class-transformer": "^0.5.1",
"class-validator": "^0.14.2",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 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

  1. npm / jsDelivr package metadata showing version 0.5.1. [1]
  2. Snyk package page (latest version 0.5.1; no known security issues on latest). [2]
  3. GitLab Advisory / CVE-2020-7637 (details, affected versions <0.3.1, fixed in 0.3.1). [3]
  4. 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

  1. jsDelivr/npm package info for class-validator (v0.14.3). [turn1search4]
  2. Snyk / package page showing latest version and security status (class-validator latest = 0.14.3). [turn1search1]
  3. Snyk vulnerability notes advising upgrade to >=0.14.0 to fix an improper input validation bypass. [turn1search3]
  4. Snyk package vulnerability list / changelog entries referencing 0.14.3 as latest. [turn0search6]
  5. 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:


Consider upgrading class-validator to 0.14.3 and verify transitive dependencies.

  • class-transformer@0.5.1 is current and secure (no known vulnerabilities).
  • class-validator@0.14.2 is secure but patch version 0.14.3 is available; upgrade recommended for consistency.
  • Verify that the lockfile (package-lock.json, yarn.lock, or pnpm-lock.yaml) pulls validator.js@≥13.15.22 to 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.

Comment on lines +2 to +3
<label for="email">Email</label>
<input id="name" type="email" formControlName="email" />
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
<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', () => {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Comment on lines +99 to +119
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.');
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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).

Comment on lines +52 to +55
if (err.error && err.error.message) {
this.errorMessage = Array.isArray(err.error.errorMessage)
? err.error.message[0]
: err.error.message;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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', () => {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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';
Copy link

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.

Comment on lines +1 to +2
export const EMAIL_REGEX_STRING: string =
'^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feat(ui): Build registration and login forms

2 participants