Skip to content

Lais v2.1#117

Open
gouravmore wants to merge 37 commits intomainfrom
lais-v2.1
Open

Lais v2.1#117
gouravmore wants to merge 37 commits intomainfrom
lais-v2.1

Conversation

@gouravmore
Copy link
Copy Markdown
Member

@gouravmore gouravmore commented Jun 20, 2025

Summary by CodeRabbit

  • New Features

    • Introduced JWT-based authentication, securing all content and collection endpoints.
    • Added support for CEFR level filtering in content and collection management.
    • Enhanced CORS configuration to restrict allowed origins based on environment settings.
    • Implemented additional security headers for all HTTP responses.
  • Improvements

    • Expanded and refined English content level configurations for more granular complexity filtering.
    • Improved handling of reading complexity for Tamil language content.
    • Enhanced content search and filtering capabilities, including better duplicate handling and more flexible queries.
    • Improved Swagger API documentation formatting for better readability.
    • Added CEFR level parameter support across multiple content service methods.
    • Refined pagination and content retrieval methods with improved parameter handling and formatting.
  • Bug Fixes

    • Enforced pagination limits to prevent out-of-range values.
  • Chores

    • Updated dependencies to include authentication and security packages.
    • Updated workflow to trigger deployments from a new branch.
  • Style

    • Standardized formatting and indentation across multiple configuration and service files for consistency.

DevendraPPatil and others added 30 commits April 30, 2025 14:59
Fix :1.security policy added 2.Formating chnages done
Feat: 1.Session expired feature added
Feat: password added for the redis
Task : Change the complexity values with new Complexity for hi
Added new CEFR_level changes (for M4 to M9 content) #242170
Fix : cors origin added for the specific apis
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jun 20, 2025

Walkthrough

This update introduces JWT-based authentication using NestJS modules and applies it to key controllers, adds new authentication dependencies, and enhances CORS and security headers. It expands language and complexity configurations, adds CEFR level support in schemas and services, and refines query logic for content and collections. Formatting improvements are applied throughout.

Changes

File(s) Change Summary
.github/workflows/Dev.yml Changed workflow trigger branch from "test" to "-v2.0" for push events.
package.json Added dependencies for JWT, Passport, CORS, and related authentication/security packages.
src/app-cluster.service.ts, src/app.controller.ts Reformatted code and improved indentation; no logic changes.
src/app.module.ts Imported and integrated new AuthModule into the main application module.
src/auth/auth.guard.ts, src/auth/auth.module.ts Added new JWT authentication guard and module, exporting guard and JWT service.
src/config/commonConfig.ts Added readingComplexity property for Tamil reading complexity levels.
src/config/language/en.ts Expanded and restructured English content level configuration with detailed syllable/word constraints per type/level.
src/config/language/hi.ts, src/config/language/kn.ts, src/config/language/ta.ts, src/config/language/te.ts Reformatted code: changed quote style, added commas, adjusted indentation, added trailing newlines.
src/controllers/collection.controller.ts, src/controllers/content.controller.ts Applied @UseGuards(JwtAuthGuard) for JWT protection; improved Swagger schema formatting; added CEFR level handling.
src/main.ts Replaced CORS wildcard with environment-based origins using cors package; added security headers via response hook.
src/schemas/collection.schema.ts, src/schemas/content.schema.ts Added optional CEFR_level property to level_complexity objects; improved formatting.
src/services/collection.service.ts Refactored to support optional CEFR_level parameter in competency collection queries.
src/services/content.service.ts Enhanced search/filter/mechanics methods to support CEFR level, improved query logic, and code formatting.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Controller
  participant JwtAuthGuard
  participant JwtService
  participant ExternalAuthAPI
  participant Service

  Client->>Controller: HTTP Request (with JWT)
  Controller->>JwtAuthGuard: Guard checks request
  JwtAuthGuard->>JwtService: Decrypts & verifies JWT
  JwtAuthGuard->>ExternalAuthAPI: Check user login status
  ExternalAuthAPI-->>JwtAuthGuard: Login status response
  JwtAuthGuard-->>Controller: Allow or deny request
  Controller->>Service: Proceeds with business logic (if allowed)
  Service-->>Controller: Response
  Controller-->>Client: HTTP Response
Loading

Possibly related PRs

  • Merge branch 'all-1.1-prod' #37: Modifies the GitHub Actions workflow trigger branch, similar to this PR's workflow trigger branch change.
  • Lais 1.0 #96: Also changes the workflow trigger branch in the same workflow file, indicating related CI/CD trigger configuration.
  • 0.1 vapt fixes #94: Adds JWT authentication dependencies, modules, and guards, closely related to the authentication features introduced here.

Poem

In burrows deep, with code anew,
JWT guards now hop right through.
CORS is trimmed, and headers tight,
CEFR levels shine so bright.
With every hop, complexity grows,
Secure and swift, the service flows!
🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-06-24T13_40_35_682Z-debug-0.log


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e78f0c and a65d564.

📒 Files selected for processing (1)
  • src/main.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main.ts
✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

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

🔭 Outside diff range comments (1)
src/services/content.service.ts (1)

325-1555: Consider refactoring the search method to reduce complexity.

The search method is over 1200 lines with deeply nested conditions and repeated patterns. This makes it difficult to maintain and test.

Consider breaking down the search logic into smaller, focused methods:

  1. Extract token processing logic
  2. Create separate methods for each fallback strategy
  3. Consolidate duplicate aggregation pipeline patterns
  4. Consider using a strategy pattern for language-specific search logic

This would improve readability, testability, and reduce the chance of bugs.

🧹 Nitpick comments (9)
package.json (2)

35-35: Move type definitions to devDependencies.

Type definitions should be placed in devDependencies rather than dependencies since they're only needed during development and build time.

-    "@types/passport-jwt": "^4.0.1",

Add to devDependencies section:

     "@types/express": "^4.17.17",
     "@types/jest": "^29.5.2",
     "@types/node": "^20.3.1",
+    "@types/passport-jwt": "^4.0.1",
     "@types/supertest": "^2.0.12",

44-45: Consider potential redundancy between JWT libraries.

Both jose and jsonwebtoken provide JWT functionality. Consider whether both are necessary or if one can be removed to reduce bundle size.

#!/bin/bash
# Description: Check usage of jose and jsonwebtoken libraries to determine if both are needed

# Search for jose usage
echo "=== JOSE usage ==="
rg -A 3 "import.*jose|require.*jose" 

# Search for jsonwebtoken usage  
echo "=== jsonwebtoken usage ==="
rg -A 3 "import.*jsonwebtoken|require.*jsonwebtoken"
src/controllers/collection.controller.ts (3)

164-164: Fix typo in method name.

The method name has a typo that affects code readability.

-  async fatchAll(@Res() response: FastifyReply) {
+  async fetchAll(@Res() response: FastifyReply) {

235-237: Fix typo in method name.

The method name has a typo that affects code readability.

-  async fatchByLanguage(
+  async fetchByLanguage(

151-151: Fix typos in API response descriptions.

Multiple instances of "retrive" should be "retrieve" in the API documentation.

Update all occurrences:

  • Line 151: "Error while retrieve the data from collection"
  • Line 221: "Error while retrieve the data from collection"
  • Line 289: "Error while retrieve the data from collection"
  • Line 374: "Error while retrieve the data from collection"

Also applies to: 221-221, 289-289, 374-374

src/main.ts (1)

38-38: Fix typos in comments.

-  // Cors aalowd for the specific url
+  // CORS allowed for specific URLs
-  // content-security-policys added
+  // Content Security Policy headers added

Also applies to: 53-53

src/auth/auth.guard.ts (1)

17-20: Remove unnecessary empty line in constructor.

 constructor(
   private jwtService: JwtService
- ) { }
+ ) {}
src/controllers/content.controller.ts (1)

1041-1116: Consider refactoring complex getContent logic for maintainability.

The nested conditionals and multiple service calls make this method difficult to follow. Consider extracting the story mode logic into a separate method.

The method has multiple responsibilities:

  1. Handling story mode with competency collections
  2. Fetching content with various filters
  3. Processing mechanics data

Consider breaking this into smaller, focused methods like getStoryModeContent(), getFilteredContent(), and processMechanicsData() for better readability and testability.

src/services/content.service.ts (1)

676-676: Replace delete operator with undefined assignment for better performance.

The delete operator can impact performance in V8 engine.

-delete query.contentSourceData.$elemMatch['$or'];
+query.contentSourceData.$elemMatch['$or'] = undefined;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf5b86c and 54ea0e5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (20)
  • .github/workflows/Dev.yml (1 hunks)
  • package.json (1 hunks)
  • src/app-cluster.service.ts (1 hunks)
  • src/app.controller.ts (1 hunks)
  • src/app.module.ts (2 hunks)
  • src/auth/auth.guard.ts (1 hunks)
  • src/auth/auth.module.ts (1 hunks)
  • src/config/commonConfig.ts (1 hunks)
  • src/config/language/en.ts (1 hunks)
  • src/config/language/hi.ts (1 hunks)
  • src/config/language/kn.ts (1 hunks)
  • src/config/language/ta.ts (1 hunks)
  • src/config/language/te.ts (1 hunks)
  • src/controllers/collection.controller.ts (15 hunks)
  • src/controllers/content.controller.ts (32 hunks)
  • src/main.ts (3 hunks)
  • src/schemas/collection.schema.ts (1 hunks)
  • src/schemas/content.schema.ts (9 hunks)
  • src/services/collection.service.ts (2 hunks)
  • src/services/content.service.ts (43 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/auth/auth.module.ts (1)
src/app.module.ts (1)
  • Module (15-45)
src/schemas/content.schema.ts (1)
src/schemas/collection.schema.ts (1)
  • Schema (6-104)
🪛 Biome (1.9.4)
src/app-cluster.service.ts

[error] 9-9: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

src/services/content.service.ts

[error] 623-623: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)


[error] 676-676: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 1084-1084: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)


[error] 1087-1087: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)


[error] 1227-1227: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)


[error] 1230-1230: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🔇 Additional comments (21)
.github/workflows/Dev.yml (1)

5-5: Verify the branch name format.

The branch name -v2.0 starts with a hyphen, which is unusual and could potentially cause issues with git commands or CI/CD systems. Branch names typically follow patterns like v2.0, release/v2.0, or feature/v2.0.

Please confirm this is the intended branch name format.

src/config/language/kn.ts (1)

2-7: LGTM - Consistent formatting improvements.

The formatting changes (single quotes, trailing commas, proper newline ending) improve code consistency and align with common JavaScript/TypeScript style guides.

src/config/language/hi.ts (1)

2-7: LGTM - Consistent formatting improvements.

The formatting changes align with the improvements made to other language configuration files, maintaining consistency across the codebase.

src/app-cluster.service.ts (1)

10-22: LGTM - Good formatting improvements.

The indentation fixes and semicolon addition improve code consistency and follow best practices.

src/config/commonConfig.ts (2)

2-167: LGTM - Comprehensive content level and complexity configurations.

The addition of detailed contentLevel and complexity arrays provides robust support for Tamil language content classification. The MongoDB-style query conditions are well-structured and cover various content types (Word, Sentence, Paragraph) with appropriate complexity criteria.


168-223: LGTM - New reading complexity configuration enhances content filtering.

The new readingComplexity array adds CEFR-level support (C0-C4) which aligns with the PR objectives. The complexity ranges appear logically progressive and well-defined for Tamil language content.

Ensure that the services consuming this configuration properly handle all the complexity levels and edge cases.

package.json (1)

28-28: LGTM! JWT authentication dependencies added correctly.

The addition of JWT and authentication packages (@nestjs/jwt, @nestjs/passport, passport-jwt, jsonwebtoken, jose, cors) properly supports the authentication module implementation described in the summary.

Also applies to: 31-31, 35-35, 39-39, 44-47

src/app.controller.ts (1)

14-21: LGTM! Formatting improvement applied.

The indentation formatting of the checkHealth method has been improved for better code readability while maintaining the same functionality.

src/schemas/collection.schema.ts (1)

93-93: LGTM! CEFR level support added correctly.

The optional CEFR_level field is properly typed and integrated into the level_complexity object, supporting the new content filtering capabilities mentioned in the summary.

src/config/language/ta.ts (1)

2-7: LGTM! Code formatting improvements applied.

The formatting changes (single quotes, trailing commas, and final newline) improve code consistency and follow standard JavaScript/TypeScript conventions.

src/app.module.ts (1)

13-13: LGTM! AuthModule integration added correctly.

The AuthModule is properly imported and added to the module imports, enabling JWT authentication throughout the application as described in the summary.

#!/bin/bash
# Description: Verify AuthModule exists and is properly configured

# Check if AuthModule file exists
echo "=== Checking AuthModule file ==="
fd "auth.module.ts" --exec cat {}

# Check for JwtAuthGuard usage in controllers
echo -e "\n=== Checking JwtAuthGuard usage ==="
rg -A 3 "JwtAuthGuard|UseGuards.*Jwt"

Also applies to: 40-40

src/config/language/te.ts (1)

1-8: LGTM! Formatting improvements are consistent.

The formatting changes align with the style updates applied to other language configuration files.

src/controllers/collection.controller.ts (1)

29-29: Good implementation of JWT authentication.

Applying JwtAuthGuard at the controller level properly secures all endpoints.

src/config/language/en.ts (1)

3-93: Well-structured content level configuration.

The expanded configuration provides excellent granularity for content filtering with clear progression across levels L1-L6 and appropriate use of MongoDB query operators.

src/main.ts (1)

60-60: Verify the restrictive Content Security Policy.

The CSP default-src 'self' is very restrictive and may break functionality if your application needs to load external resources or use inline scripts/styles.

Please verify that this CSP doesn't break any application functionality, especially:

  • External API calls
  • Third-party scripts or stylesheets
  • Inline scripts or styles
  • WebSocket connections
src/schemas/content.schema.ts (1)

4-10: LGTM! Schema changes properly implement CEFR level support.

The addition of the optional CEFR_level property to the level_complexity object is well-structured and follows the existing schema patterns. The import statement reformatting also improves readability.

Also applies to: 78-78

src/services/collection.service.ts (1)

69-96: To confirm the expected type for that fourth argument, let’s inspect the pagination method signature in src/services/content.service.ts:

#!/bin/bash
# Show the declaration of pagination in ContentService
rg -n 'pagination' -A5 -B5 src/services/content.service.ts
src/controllers/content.controller.ts (2)

34-34: JWT authentication properly applied at controller level.

Good security practice to apply authentication at the controller level, ensuring all endpoints are protected.


507-512: Good input validation for pagination limits.

The clamping logic ensures reasonable boundaries for the limit parameter, preventing potential performance issues.

src/services/content.service.ts (2)

335-336: CEFR_level filtering properly implemented across search methods.

The conditional CEFR_level filtering using spread operators is consistently applied. Good use of default empty array parameters.

Also applies to: 1124-1126, 1340-1342


1967-2184: Good implementation of split queries for mechanics content.

The logic to split queries across multiple level competencies and handle remainders is well thought out. The fallback strategies ensure content is always returned.

console.log(`Cluster server started on ${process.pid}`)
callback();
}
static clusterize(callback: Function): void {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace generic Function type with specific function signature.

The static analysis tool correctly identifies that using Function as a type parameter is too generic and can be a source of bugs. Consider defining a more specific type for the callback parameter.

-  static clusterize(callback: Function): void {
+  static clusterize(callback: () => void): void {
📝 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
static clusterize(callback: Function): void {
static clusterize(callback: () => void): void {
🧰 Tools
🪛 Biome (1.9.4)

[error] 9-9: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

🤖 Prompt for AI Agents
In src/app-cluster.service.ts at line 9, the callback parameter is typed as the
generic Function type, which is too broad and can lead to bugs. Replace the
generic Function type with a more specific function signature that matches the
expected parameters and return type of the callback. Define an explicit type or
interface for the callback if needed to ensure type safety and clarity.

Comment on lines +13 to +14
providers: [JwtAuthGuard, JwtService],
exports: [JwtAuthGuard, JwtService],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove JwtService from providers array to avoid conflicts.

JwtService is already provided by JwtModule.register(). Including it in the providers array may cause dependency injection issues.

Apply this diff to fix the module configuration:

-  providers: [JwtAuthGuard, JwtService],
-  exports: [JwtAuthGuard, JwtService],
+  providers: [JwtAuthGuard],
+  exports: [JwtAuthGuard, JwtService],
📝 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
providers: [JwtAuthGuard, JwtService],
exports: [JwtAuthGuard, JwtService],
providers: [JwtAuthGuard],
exports: [JwtAuthGuard, JwtService],
🤖 Prompt for AI Agents
In src/auth/auth.module.ts at lines 13 to 14, remove JwtService from the
providers array because it is already provided by JwtModule.register(). This
will prevent dependency injection conflicts. Keep JwtAuthGuard in providers and
JwtService only in exports if needed.

imports: [
PassportModule,
JwtModule.register({
secret: process.env.JOSE_SECRET,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate the JWT secret configuration.

Ensure that JOSE_SECRET environment variable is properly set to avoid security vulnerabilities.

Consider adding validation or a fallback:

-      secret: process.env.JOSE_SECRET,
+      secret: process.env.JOSE_SECRET || (() => { throw new Error('JOSE_SECRET must be defined'); })(),
📝 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
secret: process.env.JOSE_SECRET,
secret: process.env.JOSE_SECRET || (() => { throw new Error('JOSE_SECRET must be defined'); })(),
🤖 Prompt for AI Agents
In src/auth/auth.module.ts at line 10, the JWT secret is assigned directly from
the environment variable JOSE_SECRET without validation. Add a check to ensure
JOSE_SECRET is defined and not empty before using it. If it is missing, throw an
error or provide a secure fallback to prevent the application from running with
an undefined or insecure secret.

src/main.ts Outdated
Comment on lines 21 to 24
app.enableCors({
origin: ['*'],
methods: ['GET', 'POST', 'HEAD', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'],
credentials: false,
credentials: false
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate CORS configuration.

CORS is being configured twice - via enableCors() and the cors middleware. This may cause conflicts.

Remove the enableCors() call since the custom cors middleware provides more control:

-  app.enableCors({
-    methods: ['GET', 'POST', 'HEAD', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'],
-    credentials: false
-  });

Also applies to: 40-51

🤖 Prompt for AI Agents
In src/main.ts at lines 21 to 24 and also lines 40 to 51, there is a duplicate
CORS configuration using both app.enableCors() and a custom cors middleware.
Remove the app.enableCors() calls entirely to avoid conflicts, keeping only the
custom cors middleware which offers more control over CORS settings.

Comment on lines +31 to +32
const secret_key = process.env.JOSE_SECRET || '';
const hash = createHash('sha256').update(secret_key).digest();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate environment variables to prevent runtime errors.

Missing environment variables will cause runtime failures. Add validation at guard initialization.

Would you like me to generate a solution that validates required environment variables in the constructor?

Also applies to: 43-45

🤖 Prompt for AI Agents
In src/auth/auth.guard.ts around lines 31-32 and 43-45, the code uses
environment variables without validating their presence, which can cause runtime
errors if they are missing. Add validation in the guard's constructor to check
if the required environment variables like JOSE_SECRET are set; if not, throw an
error or handle it gracefully to prevent runtime failures. This ensures the
guard only initializes when all necessary environment variables are properly
configured.

Comment on lines +66 to +80
const url = process.env.ALL_ORC_SERVICE_URL;
const response = await axios.post(url, {
user_id: user_id,
});

return {
isLoggedIn: response.data?.result?.isLoggedIn ?? false
};
} catch (error: any) {
console.error('Error calling token-status API:', error?.response?.data || error.message);
return {
isLoggedIn: false,
};
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add timeout and retry logic for external API reliability.

The external API call lacks timeout configuration and retry logic, which could cause hanging requests or transient failures.

 async checkTokenStatus(user_id: any): Promise<{ isLoggedIn: boolean }> {
   try {
     const url = process.env.ALL_ORC_SERVICE_URL;
-    const response = await axios.post(url, {
-      user_id: user_id,
-    });
+    const response = await axios.post(
+      url, 
+      { user_id: user_id },
+      { 
+        timeout: 5000, // 5 second timeout
+        headers: { 'Content-Type': 'application/json' }
+      }
+    );
 
     return {
       isLoggedIn: response.data?.result?.isLoggedIn ?? false
     };
📝 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
const url = process.env.ALL_ORC_SERVICE_URL;
const response = await axios.post(url, {
user_id: user_id,
});
return {
isLoggedIn: response.data?.result?.isLoggedIn ?? false
};
} catch (error: any) {
console.error('Error calling token-status API:', error?.response?.data || error.message);
return {
isLoggedIn: false,
};
}
}
async checkTokenStatus(user_id: any): Promise<{ isLoggedIn: boolean }> {
try {
const url = process.env.ALL_ORC_SERVICE_URL;
- const response = await axios.post(url, {
- user_id: user_id,
- });
+ const response = await axios.post(
+ url,
+ { user_id },
+ {
+ timeout: 5000, // 5 second timeout
+ headers: { 'Content-Type': 'application/json' }
+ }
+ );
return {
isLoggedIn: response.data?.result?.isLoggedIn ?? false
};
} catch (error: any) {
console.error('Error calling token-status API:', error?.response?.data || error.message);
return {
isLoggedIn: false,
};
}
}
🤖 Prompt for AI Agents
In src/auth/auth.guard.ts around lines 66 to 80, the axios POST request to the
external API lacks timeout and retry logic, risking hanging or failing calls.
Add a timeout option to the axios request configuration to limit wait time, and
implement retry logic that attempts the request multiple times with delays
between attempts in case of transient errors. This will improve reliability by
preventing indefinite waits and handling temporary failures gracefully.

Comment on lines +58 to +60
} catch (err) {
throw new UnauthorizedException('Invalid or expired token');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling specificity for better debugging.

The generic error message makes it difficult to diagnose authentication failures. Consider logging the actual error and returning more specific messages for different failure scenarios.

-    } catch (err) {
-      throw new UnauthorizedException('Invalid or expired token');
+    } catch (err) {
+      console.error('JWT verification failed:', err.message);
+      if (err.code === 'ERR_JWT_EXPIRED') {
+        throw new UnauthorizedException('Token has expired');
+      } else if (err.code === 'ERR_JWS_INVALID') {
+        throw new UnauthorizedException('Invalid token signature');
+      }
+      throw new UnauthorizedException('Token verification failed');
     }
📝 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
} catch (err) {
throw new UnauthorizedException('Invalid or expired token');
}
} catch (err) {
console.error('JWT verification failed:', err.message);
if (err.code === 'ERR_JWT_EXPIRED') {
throw new UnauthorizedException('Token has expired');
} else if (err.code === 'ERR_JWS_INVALID') {
throw new UnauthorizedException('Invalid token signature');
}
throw new UnauthorizedException('Token verification failed');
}
🤖 Prompt for AI Agents
In src/auth/auth.guard.ts around lines 58 to 60, the catch block throws a
generic UnauthorizedException without logging the actual error or
differentiating failure reasons. Modify the catch block to log the caught error
for debugging purposes and refine the exception messages to reflect specific
failure scenarios such as token expiration or invalid token format, improving
error handling clarity.

contentType.toLocaleLowerCase() !== 'paragraph'
) {
mileStoneQuery = mileStoneQuery.filter((mileStoneQueryEle) => {
return !mileStoneQueryEle.hasOwnProperty('totalOrthoComplexity');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace hasOwnProperty with Object.hasOwn for better safety.

Using Object.hasOwn() is safer as it doesn't have issues with objects that override the hasOwnProperty method.

-return !mileStoneQueryEle.hasOwnProperty('totalOrthoComplexity');
+return !Object.hasOwn(mileStoneQueryEle, 'totalOrthoComplexity');

Apply similar changes to lines 1084, 1087, 1227, and 1230.

Also applies to: 1084-1084, 1087-1087, 1227-1227, 1230-1230

🧰 Tools
🪛 Biome (1.9.4)

[error] 623-623: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🤖 Prompt for AI Agents
In src/services/content.service.ts at lines 623, 1084, 1087, 1227, and 1230,
replace all uses of hasOwnProperty with Object.hasOwn for safer property checks.
Specifically, change expressions like
mileStoneQueryEle.hasOwnProperty('totalOrthoComplexity') to
Object.hasOwn(mileStoneQueryEle, 'totalOrthoComplexity'). This avoids issues if
the object overrides hasOwnProperty.

Copy link
Copy Markdown

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

♻️ Duplicate comments (5)
src/auth/auth.guard.ts (5)

31-32: Validate environment variables to prevent runtime errors.

Missing environment variables will cause runtime failures. Add validation at guard initialization.

This issue was already flagged in previous reviews for the same lines.


43-45: Validate environment variables to prevent runtime errors.

Missing environment variables will cause runtime failures. Add validation at guard initialization.

This issue was already flagged in previous reviews for the same lines.


58-60: Improve error handling specificity for better debugging.

The generic error message makes it difficult to diagnose authentication failures.

This issue was already flagged in previous reviews for these exact lines.


66-69: Add timeout and retry logic for external API reliability.

The external API call lacks timeout configuration and retry logic, which could cause hanging requests or transient failures.

This issue was already flagged in previous reviews for the axios call implementation.


66-66: Validate environment variable for external service URL.

The ALL_ORC_SERVICE_URL environment variable is used without validation, which could cause runtime errors if missing.

  async checkTokenStatus(user_id: any): Promise<{ token: string }> {
    try {
      const url = process.env.ALL_ORC_SERVICE_URL;
+     if (!url) {
+       throw new Error('ALL_ORC_SERVICE_URL environment variable is not configured');
+     }
      const response = await axios.post(url, {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54ea0e5 and 3e78f0c.

📒 Files selected for processing (1)
  • src/auth/auth.guard.ts (1 hunks)
🔇 Additional comments (1)
src/auth/auth.guard.ts (1)

64-64: Inconsistent API response handling.

The method signature and AI summary mention different return properties. The summary references isLoggedIn but the actual implementation returns token.

Ensure the implementation aligns with the intended API contract and update documentation accordingly.

Likely an incorrect or invalid review comment.

if (!authHeader) {
throw new UnauthorizedException('Authorization header missing');
}
const token = authHeader.split(' ')[1];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for authorization header format.

The code assumes the authorization header follows the "Bearer " format without validation, which could cause runtime errors if the header is malformed.

-    const token = authHeader.split(' ')[1];
+    const parts = authHeader.split(' ');
+    if (parts.length !== 2 || parts[0] !== 'Bearer') {
+      throw new UnauthorizedException('Invalid authorization header format');
+    }
+    const token = parts[1];
📝 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
const token = authHeader.split(' ')[1];
const parts = authHeader.split(' ');
if (parts.length !== 2 || parts[0] !== 'Bearer') {
throw new UnauthorizedException('Invalid authorization header format');
}
const token = parts[1];
🤖 Prompt for AI Agents
In src/auth/auth.guard.ts at line 27, the code assumes the authorization header
is in the "Bearer <token>" format without validating it, which can cause runtime
errors if malformed. Add a check to ensure authHeader exists and starts with
"Bearer " before splitting and extracting the token. If the format is invalid,
handle the error appropriately, such as returning an unauthorized response or
throwing an error.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
17.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants