Feat upgrade api keys #601
Conversation
|
|
Rachid Flih seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughWalkthroughThe changes primarily enhance the security and usability of API key management across the authentication system. Key updates include a new endpoint for generating API keys, improved hashing methods for key validation, and enhanced error messaging. The user interface has been refined to provide immediate feedback through modal dialogs upon API key creation, while sensitive information handling has been tightened. These adjustments ensure a more standardized approach to API key handling and facilitate easier debugging. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range comments (1)
packages/api/src/@core/auth/auth.service.ts (1)
Line range hint
327-366:
Enhance error handling and logging.Consider adding logging for successful API key generation and more descriptive error messages.
+ this.logger.log(`Generating API key for user ${userId} in project ${projectId}`); const foundProject = await this.prisma.projects.findUnique({ where: { id_project: projectId }, }); if (!foundProject) { this.logger.error(`Project not found: ${projectId}`); throw new ReferenceError('Project not found'); } const foundUser = await this.prisma.users.findUnique({ where: { id_user: userId }, }); if (!foundUser) { this.logger.error(`User not found: ${userId}`); throw new ReferenceError('User Not Found'); } const base_key = `sk_${process.env.ENV}_${uuidv4()}`; const hashed_key = crypto.createHash('sha256').update(base_key).digest('hex'); const new_api_key = await this.prisma.api_keys.create({ data: { id_api_key: uuidv4(), api_key_hash: hashed_key, name: keyName, id_project: projectId as string, id_user: userId as string, }, }); if (!new_api_key) { this.logger.error('Failed to create API key in the database'); throw new ReferenceError('API key undefined'); } this.logger.log(`API key generated successfully for user ${userId} in project ${projectId}`); return { api_key: base_key, ...new_api_key };
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (3)
docker-compose.dev.ymlis excluded by!**/*.ymldocker-compose.source.ymlis excluded by!**/*.ymldocker-compose.ymlis excluded by!**/*.yml
Files selected for processing (4)
- packages/api/src/@core/auth/auth.controller.ts (1 hunks)
- packages/api/src/@core/auth/auth.service.ts (4 hunks)
- packages/api/src/@core/auth/strategies/auth-header-api-key.strategy.ts (2 hunks)
- packages/api/src/@core/connections/@utils/index.ts (1 hunks)
Additional context used
Biome
packages/api/src/@core/auth/strategies/auth-header-api-key.strategy.ts
[error] 5-5: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
Additional comments not posted (3)
packages/api/src/@core/auth/strategies/auth-header-api-key.strategy.ts (2)
14-14: LGTM! Configuration change forHeaderAPIKeyStrategy.The update to use the
x-api-keyheader without a prefix is a good change for enhancing the specificity of API key handling.
22-24: LGTM! Improved security with API key hashing.The addition of the hashing mechanism using SHA-256 enhances security by ensuring that the API key is not exposed in its original form during processing.
packages/api/src/@core/auth/auth.controller.ts (1)
107-107: LGTM! Improved route clarity forgenerateApiKey.The change to use the
api_keysendpoint enhances the API's clarity by explicitly indicating that the method is responsible for generating API keys.
| async getProjectIdForApiKey(hashed_apiKey: string) { | ||
| try { | ||
| // Decode the JWT to verify if it's valid and get the payload | ||
| const decoded = this.jwtService.verify(apiKey, { | ||
| secret: process.env.JWT_SECRET, | ||
| }); | ||
|
|
||
| //const hashed_api_key = this.hashApiKey(apiKey); | ||
| const saved_api_key = await this.prisma.api_keys.findUnique({ | ||
| where: { | ||
| api_key_hash: apiKey, | ||
| api_key_hash: hashed_apiKey, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Add logging and improve error handling.
Consider adding logging for successful retrieval and handling cases where the API key is not found.
+ this.logger.log(`Retrieving project ID for API key`);
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: hashed_apiKey,
},
});
if (!saved_api_key) {
this.logger.error(`API key not found: ${hashed_apiKey}`);
throw new ReferenceError('API key not found');
}
this.logger.log(`Project ID retrieved successfully for API key`);
return saved_api_key.id_project;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getProjectIdForApiKey(hashed_apiKey: string) { | |
| try { | |
| // Decode the JWT to verify if it's valid and get the payload | |
| const decoded = this.jwtService.verify(apiKey, { | |
| secret: process.env.JWT_SECRET, | |
| }); | |
| //const hashed_api_key = this.hashApiKey(apiKey); | |
| const saved_api_key = await this.prisma.api_keys.findUnique({ | |
| where: { | |
| api_key_hash: apiKey, | |
| api_key_hash: hashed_apiKey, | |
| }, | |
| }); | |
| async getProjectIdForApiKey(hashed_apiKey: string) { | |
| this.logger.log(`Retrieving project ID for API key`); | |
| const saved_api_key = await this.prisma.api_keys.findUnique({ | |
| where: { | |
| api_key_hash: hashed_apiKey, | |
| }, | |
| }); | |
| if (!saved_api_key) { | |
| this.logger.error(`API key not found: ${hashed_apiKey}`); | |
| throw new ReferenceError('API key not found'); | |
| } | |
| this.logger.log(`Project ID retrieved successfully for API key`); | |
| return saved_api_key.id_project; |
| @@ -2,6 +2,7 @@ import { HeaderAPIKeyStrategy } from 'passport-headerapikey'; | |||
| import { PassportStrategy } from '@nestjs/passport'; | |||
| import { Injectable, UnauthorizedException } from '@nestjs/common'; | |||
| import { AuthService } from '../auth.service'; | |||
| import * as crypto from 'crypto'; | |||
There was a problem hiding this comment.
Use the node: protocol for Node.js built-in modules.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
- import * as crypto from 'crypto';
+ import * as crypto from 'node:crypto';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.
| import * as crypto from 'crypto'; | |
| import * as crypto from 'node:crypto'; |
Tools
Biome
[error] 5-5: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
|
|
||
| // TO DO : add Expiration in part 3 | ||
|
|
||
| // Decode the JWT to verify if it's valid and get the payload | ||
| const decoded = this.jwtService.verify(apiKey, { | ||
| secret: process.env.JWT_SECRET, | ||
| }); | ||
| // const decoded = this.jwtService.verify(apiKey, { | ||
| // secret: process.env.JWT_SECRET, | ||
| // }); | ||
|
|
||
|
|
||
| //const hashed_api_key = this.hashApiKey(apiKey); | ||
| // pseudo-code: | ||
| // 1 - SHA256 the API key from the header | ||
| const hashed_key = crypto.createHash('sha256').update(apiKey).digest('hex'); | ||
|
|
||
|
|
||
| // 2- check against DB | ||
| // if not found, return false | ||
| const saved_api_key = await this.prisma.api_keys.findUnique({ | ||
| where: { | ||
| api_key_hash: apiKey, | ||
| api_key_hash: hashed_key, | ||
| }, | ||
| }); | ||
|
|
||
| if (!saved_api_key) { | ||
| throw new ReferenceError('Api Key undefined'); | ||
| } | ||
| if (String(decoded.project_id) !== String(saved_api_key.id_project)) { | ||
| throw new ReferenceError( | ||
| 'Failed to validate API key: projectId mismatch.', | ||
| ); | ||
| } | ||
|
|
||
| // Validate that the JWT payload matches the provided userId and projectId | ||
| if (String(decoded.sub) !== String(saved_api_key.id_user)) { | ||
| throw new ReferenceError( | ||
| 'Failed to validate API key: userId mismatch.', | ||
| ); | ||
| throw new ReferenceError('API Key not found.'); | ||
| } | ||
| // if (String(decoded.project_id) !== String(saved_api_key.id_project)) { | ||
| // throw new ReferenceError( | ||
| // 'Failed to validate API key: projectId mismatch.', | ||
| // ); | ||
| // } | ||
|
|
||
| // // Validate that the JWT payload matches the provided userId and projectId | ||
| // if (String(decoded.sub) !== String(saved_api_key.id_user)) { | ||
| // throw new ReferenceError( | ||
| // 'Failed to validate API key: userId mismatch.', | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
Improve logging, error handling, and remove commented-out code.
Add logging for validation steps, improve error messages, and remove commented-out code for clarity.
+ this.logger.log(`Validating API key`);
const hashed_key = crypto.createHash('sha256').update(apiKey).digest('hex');
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: hashed_key,
},
});
if (!saved_api_key) {
this.logger.error(`API key not found: ${hashed_key}`);
throw new ReferenceError('API key not found');
}
this.logger.log(`API key validated successfully`);
return true;Committable suggestion was skipped due to low confidence.
| console.log('**********') | ||
| console.log(token); | ||
| console.log('**********') |
There was a problem hiding this comment.
Avoid logging sensitive information.
Logging sensitive information such as tokens can pose a security risk. Consider removing or obfuscating the token in the logs.
- console.log('**********')
- console.log(token);
- console.log('**********')
+ console.log('**********')
+ console.log('Token received');
+ console.log('**********')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.
| console.log('**********') | |
| console.log(token); | |
| console.log('**********') | |
| console.log('**********') | |
| console.log('Token received'); | |
| console.log('**********') |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range comments (1)
apps/webapp/src/components/ApiKeys/columns.tsx (1)
[!TIP]
Codebase VerificationPartial removal of
tokencolumn.The
tokencolumn has been partially removed fromapps/webapp/src/components/ApiKeys/columns.tsx. However, related functionality such as copying the token is still present in the code. Ensure that all references to thetokenare properly cleaned up to avoid potential issues.
apps/webapp/src/components/ApiKeys/columns.tsx: References totokenin thehandleCopyfunction and state management logic.Analysis chain
Line range hint
1-1:
Removal oftokencolumn.The column for displaying the
tokenhas been removed. Ensure that this change does not negatively impact the user experience and functionality.
The
tokencolumn appears to still be referenced in theapps/webapp/src/components/ApiKeys/columns.tsxfile. We need to verify if thetokencolumn is indeed removed and if the related functionality, such as copying the token, is impacted.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `token` column from the API keys table. # Test: Search for the usage of the `token` column in the codebase. Expect: No occurrences of `token` in the context of the API keys table. rg --type js --type ts 'token' -A 10 | rg 'columns'Length of output: 7297
Script:
#!/bin/bash # Check if the `token` column is defined in the `apps/webapp/src/components/ApiKeys/columns.tsx` file. rg 'token' apps/webapp/src/components/ApiKeys/columns.tsx -A 10 # Check if there are any other references to the `token` column in the `apps/webapp/src/components/ApiKeys/columns.tsx` file. rg 'token' apps/webapp/src/components/ApiKeys/columns.tsxLength of output: 885
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
packages/api/swagger/swagger-spec.yamlis excluded by!**/*.yaml
Files selected for processing (7)
- apps/webapp/src/app/(Dashboard)/api-keys/page.tsx (3 hunks)
- apps/webapp/src/components/ApiKeys/columns.tsx (1 hunks)
- apps/webapp/src/components/ApiKeys/schema.ts (1 hunks)
- apps/webapp/src/hooks/delete/useDeleteApiKey.tsx (1 hunks)
- packages/api/prisma/schema.prisma (4 hunks)
- packages/api/src/@core/auth/auth.service.ts (10 hunks)
- packages/api/src/main.ts (1 hunks)
Additional context used
Biome
packages/api/src/@core/auth/auth.service.ts
[error] 450-450: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
Additional comments not posted (19)
apps/webapp/src/components/ApiKeys/schema.ts (1)
4-7: Removal oftokenproperty inapiKeySchema.The
tokenproperty has been removed from the schema definition. Ensure that all parts of the codebase that relied on this property are updated accordingly.Verification successful
No issues found with the removal of the
tokenproperty fromapiKeySchema.The
tokenproperty was not found in the context ofapiKeySchemaacross the codebase. Therefore, its removal does not impact any dependent code.
apiKeySchemais defined inapps/webapp/src/components/ApiKeys/schema.tsand is not referenced elsewhere with thetokenproperty.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `token` property from the `apiKeySchema`. # Test: Search for the usage of `apiKeySchema` and `token` in the codebase. Expect: No occurrences of `token` in the context of `apiKeySchema`. rg --type js --type ts 'apiKeySchema' -A 10 | rg 'token'Length of output: 54
Script:
#!/bin/bash # Description: Verify the impact of removing the `token` property from the `apiKeySchema`. # Search for any usage of `token` in the codebase to ensure it is not used independently. rg --type js --type ts 'token' # Search for any usage of `apiKeySchema` in the codebase to ensure no dependent code is affected. rg --type js --type ts 'apiKeySchema'Length of output: 225882
apps/webapp/src/hooks/delete/useDeleteApiKey.tsx (1)
11-11: Change in endpoint URL.The endpoint URL has been changed from
/auth/api-keys/to/auth/api_keys/. Ensure that all parts of the codebase that interact with this endpoint are updated accordingly.Verification successful
Change in endpoint URL verified.
The endpoint URL has been changed from
/auth/api-keys/to/auth/api_keys/in the codebase. The change is correctly applied inapps/webapp/src/hooks/delete/useDeleteApiKey.tsx, and no other occurrences of the old or new endpoint URL were found.
apps/webapp/src/hooks/delete/useDeleteApiKey.tsx:const response = await fetch(`${config.API_URL}/auth/api_keys/${apiKeyData.id_api_key}`, {Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of changing the endpoint URL from `/auth/api-keys/` to `/auth/api_keys/`. # Test: Search for the usage of the old endpoint URL in the codebase. Expect: No occurrences of `/auth/api-keys/`. rg --type js --type ts '/auth/api-keys/'Length of output: 40
Script:
#!/bin/bash # Description: Verify the usage of the new endpoint URL `/auth/api_keys/` in the codebase. # Test: Search for the usage of the new endpoint URL in the codebase. Expect: Occurrences of `/auth/api_keys/`. rg --type js --type ts '/auth/api_keys/'Length of output: 188
Script:
#!/bin/bash # Description: Verify the consistent usage of the new endpoint URL `/auth/api_keys/` in the codebase. # Test: Search for any other occurrences of the new endpoint URL in the codebase. rg '/auth/api_keys/'Length of output: 168
packages/api/src/main.ts (2)
61-61: LGTM! Swagger documentation updated correctly.The Swagger documentation now correctly reflects the new
api_keysecurity scheme.
51-58: Verify the new API key configuration.The new API key configuration using
.addApiKeylooks correct. Ensure that all clients are updated to use the newx-api-keyheader for authentication.Verification successful
Verify the new API key configuration.
The new API key configuration using
.addApiKeylooks correct and is being used in the authentication strategy (auth-header-api-key.strategy.ts). Ensure that all clients are updated to use the newx-api-keyheader for authentication.
packages/api/src/@core/auth/strategies/auth-header-api-key.strategy.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all clients are updated to use the new `x-api-key` header for authentication. # Test: Search for the usage of the `x-api-key` header in the codebase. Expect: Occurrences of the new header. rg --type javascript --type typescript -A 5 $'x-api-key'Length of output: 90
Script:
#!/bin/bash # Description: Verify that all clients are updated to use the new `x-api-key` header for authentication. # Test: Search for the usage of the `x-api-key` header in the codebase. Expect: Occurrences of the new header. rg --type js --type ts -A 5 'x-api-key'Length of output: 926
apps/webapp/src/app/(Dashboard)/api-keys/page.tsx (4)
54-54: LGTM! State management for new API key added correctly.The new state variables
newApiKeyandisKeyModalOpenare correctly implemented to manage the display of the newly created API key and its expiration time.Also applies to: 58-58
65-74: LGTM! Lifecycle management fornewApiKeyadded correctly.The
useEffecthook correctly manages the lifecycle ofnewApiKey, ensuring that the key is only available for a limited time.
121-127: LGTM! API key storage and modal opening added correctly.The
onSubmitfunction now correctly stores the API key and its expiration time in state and opens the modal upon successful creation of the API key.
246-261: LGTM! Modal dialog for displaying new API key added correctly.The new modal dialog provides a user-friendly way to display the newly created API key and its expiration time.
packages/api/src/@core/auth/auth.service.ts (8)
Line range hint
46-60:
LGTM! Reset token verification and password update added correctly.The
resetPasswordmethod now correctly verifies the reset token and securely updates the user's password in the database.
68-73: LGTM! Reset token verification using bcrypt added correctly.The
verifyResetTokenmethod now correctly compares the request token with the database token using bcrypt.
Line range hint
79-103:
LGTM! Reset token generation and storage added correctly.The
requestPasswordResetmethod now correctly generates a reset token, hashes it using bcrypt, and updates the user's reset token in the database.
Line range hint
113-131:
LGTM! Logging for sending reset email added correctly.The
sendResetEmailmethod now correctly logs the sending of the reset email for better traceability.
162-170: LGTM! Exclusion ofapi_key_hashfrom returned API keys added correctly.The
getApiKeysmethod now correctly excludes theapi_key_hashfield from the returned API keys, improving security.
Line range hint
336-377:
LGTM! Secure API key generation and storage added correctly.The
generateApiKeyForUsermethod now correctly generates a base key using a UUID and environment variables, hashes it with SHA-256, and stores it in the database.
395-402: LGTM! Hashed API key usage added correctly.The
getProjectIdForApiKeymethod now correctly accepts a hashed API key instead of a plain text API key, improving security.
411-447: LGTM! Secure API key validation added correctly.The
validateApiKeymethod now correctly hashes the API key and checks it against the database, improving security.Also applies to: 453-453
packages/api/prisma/schema.prisma (3)
1736-1755: LGTM!The
ecom_addressesmodel is correctly defined with appropriate fields and relationships.
1757-1759: LGTM! But verify the completeness of the model.The
ecom_fulfilment_ordersmodel is minimal. Ensure that this meets the application requirements.
1761-1763: LGTM! But verify the completeness of the model.The
ecom_order_line_itemsmodel is minimal. Ensure that this meets the application requirements.
…-Upgrade-API-Keys-to-sk_env_UUID
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range comments (1)
packages/api/src/@core/auth/auth.service.ts (1)
Line range hint
53-72:
Remove commented-out code and redundant code duplication.The code segment includes commented-out code and redundant code duplication. Clean up the code for better readability and maintainability.
- // Verify the reset token - const isValidToken = await this.verifyResetToken( - checkResetRequestIsValid.reset_token, - reset_token, - ); if (!isValidToken) { throw new BadRequestException('Invalid reset token'); } // Hash the new password const hashedPassword = await bcrypt.hash(new_password, 10); // Update the user's password in the database - const updatedPassword = await this.prisma.users.update({ where: { email }, data: { password_hash: hashedPassword }, }); - console.log(updatedPassword); return { message: 'Password reset successfully' };
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (4)
docker-compose.dev.ymlis excluded by!**/*.ymldocker-compose.source.ymlis excluded by!**/*.ymldocker-compose.ymlis excluded by!**/*.ymlpackages/api/swagger/swagger-spec.yamlis excluded by!**/*.yaml
Files selected for processing (3)
- packages/api/prisma/schema.prisma (2 hunks)
- packages/api/src/@core/auth/auth.service.ts (12 hunks)
- packages/api/src/main.ts (1 hunks)
Additional context used
Biome
packages/api/src/@core/auth/auth.service.ts
[error] 20-20: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 142-142: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 142-142: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
Additional comments not posted (11)
packages/api/src/main.ts (2)
56-56: LGTM! Verify the integration of the new security schema.The code correctly sets the security schema to use the new
api_keysecurity scheme.Ensure that the new security schema is correctly integrated and consistent with the rest of the codebase.
Verification successful
Integration of the new security schema verified.
The
api_keysecurity scheme is defined and utilized in various parts of the codebase, indicating proper integration and usage.
packages/api/src/main.ts: Line 56 sets the security schema.- Multiple references to
api_keyin configuration, service, and component files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new security schema. # Test: Search for the usage of `document.security` in the codebase. rg --type python -A 5 $'document.security'Length of output: 72
Script:
#!/bin/bash # Description: Verify the integration of the new security schema. # Test: Search for the usage of `document.security` in TypeScript/JavaScript files. rg --type js --type ts -A 5 'document.security'Length of output: 406
Script:
#!/bin/bash # Description: Verify the definition and usage of the `api_key` security scheme in the Swagger/OpenAPI configuration. # Test: Search for the definition of the `api_key` security scheme in TypeScript/JavaScript files. rg --type js --type ts -A 5 'api_key'Length of output: 84640
46-53: LGTM! Verify the integration of API key security.The code correctly adds API key security configuration to the Swagger documentation.
Ensure that the new API key security configuration is correctly integrated and consistent with the rest of the codebase.
packages/api/src/@core/auth/auth.service.ts (5)
81-86: LGTM!The code correctly verifies the reset token using bcrypt.
185-190: LGTM!The code correctly retrieves API keys for a given project.
Line range hint
362-403:
LGTM!The code correctly generates and stores a hashed API key.
437-473: LGTM!The code correctly validates the API key by hashing it and checking against the database.
98-98: Remove commented-out code.The code segment includes commented-out code. Clean up the code for better readability and maintainability.
- // Send email with resetToken (implementation depends on your email service) - await this.sendResetEmail(email, resetToken); return { message: 'Password reset link sent' };Likely invalid or redundant comment.
packages/api/prisma/schema.prisma (4)
1649-1649: LGTM!The code correctly adds the new
remote_deletedfield to theecom_customersmodel.
1729-1748: LGTM!The code correctly renames the model and adds the new
id_ecom_orderfield to theecom_addressesmodel.
1750-1752: LGTM!The code correctly adds the new primary key field to the
ecom_fulfilment_ordersmodel.
1754-1755: LGTM!The code correctly adds the new primary key field to the
ecom_order_line_itemsmodel.
| host: process.env.SMTP_HOST, | ||
| port: Number(process.env.SMTP_PORT), | ||
| host: process.env.SMTP_HOST, | ||
| port: Number(process.env.SMTP_PORT), | ||
| //secure: false, | ||
| auth: { | ||
| user: process.env.SMTP_USER, | ||
| pass: process.env.SMTP_PASSWORD, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix syntax error and remove commented-out code.
The code segment has a syntax error and includes commented-out code. Clean up the code for better readability and maintainability.
- host: process.env.SMTP_HOST,
- port: Number(process.env.SMTP_PORT),
- //secure: false,
- auth: {
- user: process.env.SMTP_USER,
- pass: process.env.SMTP_PASSWORD,
- },
+ const transporter = nodemailer.createTransport({
+ host: process.env.SMTP_HOST,
+ port: Number(process.env.SMTP_PORT),
+ secure: false,
+ auth: {
+ user: process.env.SMTP_USER,
+ pass: process.env.SMTP_PASSWORD,
+ },
});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.
| host: process.env.SMTP_HOST, | |
| port: Number(process.env.SMTP_PORT), | |
| host: process.env.SMTP_HOST, | |
| port: Number(process.env.SMTP_PORT), | |
| //secure: false, | |
| auth: { | |
| user: process.env.SMTP_USER, | |
| pass: process.env.SMTP_PASSWORD, | |
| }, | |
| }, | |
| const transporter = nodemailer.createTransport({ | |
| host: process.env.SMTP_HOST, | |
| port: Number(process.env.SMTP_PORT), | |
| secure: false, | |
| auth: { | |
| user: process.env.SMTP_USER, | |
| pass: process.env.SMTP_PASSWORD, | |
| }, | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- packages/api/prisma/schema.prisma (3 hunks)
- packages/api/src/@core/auth/auth.service.ts (11 hunks)
Additional context used
Biome
packages/api/src/@core/auth/auth.service.ts
[error] 7-7: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
GitHub Check: Build and Test (18.x)
packages/api/src/@core/auth/auth.service.ts
[warning] 3-3:
'AuthError' is defined but never used
Additional comments not posted (6)
packages/api/src/@core/auth/auth.service.ts (2)
391-398: Add logging and improve error handling.Consider adding logging for successful retrieval and handling cases where the API key is not found.
+ this.logger.log(`Retrieving project ID for API key`); const saved_api_key = await this.prisma.api_keys.findUnique({ where: { api_key_hash: hashed_apiKey, }, }); if (!saved_api_key) { this.logger.error(`API key not found: ${hashed_apiKey}`); throw new ReferenceError('API key not found'); } this.logger.log(`Project ID retrieved successfully for API key`); return saved_api_key.id_project;
407-443: Improve logging, error handling, and remove commented-out code.Add logging for validation steps, improve error messages, and remove commented-out code for clarity.
+ this.logger.log(`Validating API key`); const hashed_key = crypto.createHash('sha256').update(apiKey).digest('hex'); const saved_api_key = await this.prisma.api_keys.findUnique({ where: { api_key_hash: hashed_key, }, }); if (!saved_api_key) { this.logger.error(`API key not found: ${hashed_key}`); throw new ReferenceError('API key not found'); } this.logger.log(`API key validated successfully`); return true;packages/api/prisma/schema.prisma (4)
1649-1649: LGTM!The addition of the
remote_deletedfield for soft deletion is a good practice for maintaining data integrity.
1731-1750: LGTM!The renaming to
ecom_addressesand the addition of theid_ecom_orderfield improve the schema's relational capabilities and naming conventions.
1752-1754: LGTM!The addition of the
ecom_fulfilment_ordersmodel with a primary key aligns with business requirements for order processing functionalities.
1756-1758: LGTM!The addition of the
ecom_order_line_itemsmodel with a primary key aligns with business requirements for order processing functionalities.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor