-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Tenant api key Entity and Service #8716
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces the implementation of a Tenant API Key feature across multiple packages. The changes include the addition of a new model, entity, service, repositories, and a database migration for managing tenant API keys. This implementation supports both TypeORM and MikroORM, allowing for flexibility across various database systems. The new functionality facilitates secure generation, storage, and management of API keys specific to individual tenants. Changes
Sequence DiagramsequenceDiagram
participant Client
participant TenantApiKeyService
participant TypeOrmRepo
participant MikroOrmRepo
participant Database
Client->>TenantApiKeyService: findApiKeyByOptions()
TenantApiKeyService->>TypeOrmRepo: Query API key
TenantApiKeyService->>MikroOrmRepo: Query API key
TypeOrmRepo->>Database: Fetch tenant API key
MikroOrmRepo->>Database: Fetch tenant API key
Database-->>TenantApiKeyService: Return API key or null
TenantApiKeyService-->>Client: Return API key details
Possibly Related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (2)
packages/core/src/lib/database/migrations/1736330073887-CreateTenantApiKeyTable.ts (1)
93-98
: Optimize index creation in SQLite migrationIn the
sqliteUpQueryRunner
method, the indexes ontenant_api_key
are created on lines 93-95 and then immediately dropped on lines 96-98 before recreating them later on lines 107-109. This redundant creation and dropping of indexes can be avoided to streamline the migration process.Apply this diff to remove the unnecessary index creation:
- await queryRunner.query(`CREATE INDEX "IDX_686a4464635f3dbb77411dd5e8" ON "tenant_api_key" ("isActive") `); - await queryRunner.query(`CREATE INDEX "IDX_21056fa2d8b060d3049f50ec0b" ON "tenant_api_key" ("isArchived") `); - await queryRunner.query(`CREATE INDEX "IDX_df44cc2b5ba1a4cc95850eea7c" ON "tenant_api_key" ("tenantId") `);packages/contracts/src/lib/tenant-api-key.model.ts (1)
10-10
: Consider simplifying theITenantApiKeyCreateInput
interfaceSince
ITenantApiKeyCreateInput
does not add any new properties toITenantApiKey
, you might consider usingITenantApiKey
directly without defining a separate interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/contracts/src/index.ts
(1 hunks)packages/contracts/src/lib/tenant-api-key.model.ts
(1 hunks)packages/core/src/lib/core/entities/index.ts
(2 hunks)packages/core/src/lib/core/entities/internal.ts
(1 hunks)packages/core/src/lib/database/migrations/1736330073887-CreateTenantApiKeyTable.ts
(1 hunks)packages/core/src/lib/tenant-api-key/repository/index.ts
(1 hunks)packages/core/src/lib/tenant-api-key/repository/mikro-orm-tanant-api-key.repository.ts
(1 hunks)packages/core/src/lib/tenant-api-key/repository/type-orm-tanant-api-key.repository.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Cspell
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
[warning] 8-8:
Unknown word (tanant)
packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
[warning] 5-5:
Unknown word (tanant)
[warning] 6-6:
Unknown word (tanant)
packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
[warning] 6-6:
Unknown word (tanant)
packages/core/src/lib/tenant-api-key/repository/index.ts
[warning] 1-1:
Unknown word (tanant)
[warning] 2-2:
Unknown word (tanant)
🪛 GitHub Actions: Check Spelling and Typos with cspell
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
[warning] 8-8: Spelling error: 'tanant' is misspelled (should be 'tenant')
packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
[warning] 5-6: Spelling error: 'tanant' is misspelled (should be 'tenant')
packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
[warning] 6-6: Spelling error: 'tanant' is misspelled (should be 'tenant')
packages/core/src/lib/tenant-api-key/repository/index.ts
[warning] 1-2: Spelling error: 'tanant' is misspelled (should be 'tenant')
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (8)
packages/core/src/lib/database/migrations/1736330073887-CreateTenantApiKeyTable.ts (1)
59-69
: PostgreSQL migration script is correctThe
postgresUpQueryRunner
method correctly creates thetenant_api_key
table with all necessary fields and constraints for PostgreSQL.packages/core/src/lib/tenant-api-key/repository/mikro-orm-tanant-api-key.repository.ts (1)
1-4
: Repository implementation is correctThe
MikroOrmTenantApiKeyRepository
class correctly extends theMikroOrmBaseEntityRepository
with theTenantApiKey
entity.packages/contracts/src/lib/tenant-api-key.model.ts (1)
3-8
: Interface definition is correctThe
ITenantApiKey
interface appropriately extendsIBasePerTenantEntityModel
and includes all required properties.packages/core/src/lib/tenant-api-key/repository/type-orm-tanant-api-key.repository.ts (1)
7-10
: LGTM! Repository implementation follows best practices.The implementation correctly extends TypeORM's Repository class and properly initializes it using dependency injection.
packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts (1)
22-39
: LGTM! Query implementation includes proper security filtersThe implementation correctly filters for active and non-archived records, including tenant status checks.
packages/core/src/lib/core/entities/index.ts (1)
143-143
: LGTM!The
TenantApiKey
entity is properly integrated into both the imports list andcoreEntities
array, maintaining alphabetical order.Also applies to: 301-301
packages/contracts/src/index.ts (1)
136-136
: LGTM!The export for
tenant-api-key.model
is properly added, maintaining alphabetical order and consistency with other model exports.packages/core/src/lib/core/entities/internal.ts (1)
147-147
: LGTM!The export for
tenant-api-key.entity
is properly added, maintaining alphabetical order and consistency with other entity exports.
packages/core/src/lib/tenant-api-key/repository/mikro-orm-tanant-api-key.repository.ts
Outdated
Show resolved
Hide resolved
packages/core/src/lib/tenant-api-key/repository/type-orm-tanant-api-key.repository.ts
Outdated
Show resolved
Hide resolved
@ApiProperty({ type: () => String }) | ||
@IsNotEmpty() | ||
@IsString() | ||
@MultiORMColumn() | ||
apiKey: string; | ||
|
||
@ApiProperty({ type: () => String }) | ||
@IsNotEmpty() | ||
@IsString() | ||
@MultiORMColumn() | ||
apiSecret: string; | ||
|
||
@ApiPropertyOptional({ type: () => String }) | ||
@IsNotEmpty() | ||
@IsString() | ||
@MultiORMColumn({ nullable: true }) | ||
openAiSecretKey?: string; | ||
|
||
@ApiPropertyOptional({ type: () => String }) | ||
@IsNotEmpty() | ||
@IsString() | ||
@MultiORMColumn({ nullable: true }) | ||
openAiOrganizationId?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider encrypting sensitive fields
The entity stores sensitive credentials in plain text. Consider:
- Encrypting these fields before storage
- Implementing a custom transformer for automatic encryption/decryption
- Adding
@Exclude()
decorator to prevent serialization of sensitive data
Would you like me to provide an example implementation of field encryption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @coderabbitai provide an example implementation of field encryption here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GloireMutaliko21 Here's an example implementation for encrypting sensitive fields:
// encryption.service.ts
import { Injectable } from '@nestjs/common';
import { createCipheriv, createDecipheriv, randomBytes } from 'crypto';
@Injectable()
export class EncryptionService {
private readonly algorithm = 'aes-256-gcm';
private readonly key = Buffer.from(process.env.ENCRYPTION_KEY, 'base64');
encrypt(text: string): string {
// Generate a random initialization vector
const iv = randomBytes(16);
const cipher = createCipheriv(this.algorithm, this.key, iv);
// Encrypt the data
let encrypted = cipher.update(text, 'utf8', 'hex');
encrypted += cipher.final('hex');
// Get the auth tag
const authTag = cipher.getAuthTag();
// Combine IV, encrypted data and auth tag
return `${iv.toString('hex')}:${authTag.toString('hex')}:${encrypted}`;
}
decrypt(encryptedData: string): string {
// Split the stored data
const [ivHex, authTagHex, encryptedText] = encryptedData.split(':');
// Convert hex to buffers
const iv = Buffer.from(ivHex, 'hex');
const authTag = Buffer.from(authTagHex, 'hex');
// Create decipher
const decipher = createDecipheriv(this.algorithm, this.key, iv);
decipher.setAuthTag(authTag);
// Decrypt the data
let decrypted = decipher.update(encryptedText, 'hex', 'utf8');
decrypted += decipher.final('utf8');
return decrypted;
}
}
// encrypted-column.transformer.ts
import { ValueTransformer } from 'typeorm';
import { EncryptionService } from './encryption.service';
export class EncryptedColumnTransformer implements ValueTransformer {
constructor(private readonly encryptionService: EncryptionService) {}
to(value: string): string {
return value ? this.encryptionService.encrypt(value) : null;
}
from(value: string): string {
return value ? this.encryptionService.decrypt(value) : null;
}
}
// tenant-api-key.entity.ts
import { Exclude } from 'class-transformer';
import { EncryptedColumnTransformer } from './encrypted-column.transformer';
import { EncryptionService } from './encryption.service';
const encryptionService = new EncryptionService();
const transformer = new EncryptedColumnTransformer(encryptionService);
@MultiORMEntity('tenant_api_key')
export class TenantApiKey extends TenantBaseEntity implements ITenantApiKey {
@Exclude()
@MultiORMColumn({ transformer })
apiKey: string;
@Exclude()
@MultiORMColumn({ transformer })
apiSecret: string;
@Exclude()
@MultiORMColumn({ nullable: true, transformer })
openAiSecretKey?: string;
@Exclude()
@MultiORMColumn({ nullable: true, transformer })
openAiOrganizationId?: string;
}
Key points about this implementation:
- Uses AES-256-GCM for strong encryption
- Stores IV and auth tag with encrypted data for secure decryption
- Automatically encrypts/decrypts via TypeORM transformer
- Excludes sensitive fields from serialization
- Uses environment variable for encryption key
Don't forget to:
- Add
ENCRYPTION_KEY
to your environment variables (32 bytes, base64 encoded) - Add
EncryptionService
to your module providers - Update your migration to ensure column types can handle encrypted data (recommend
TEXT
type)
✏️ Learnings added
Learnt from: GloireMutaliko21
PR: ever-co/ever-gauzy#8716
File: packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts:14-36
Timestamp: 2025-01-08T12:06:16.684Z
Learning: Sensitive fields in entities should be encrypted before storage using a custom TypeORM transformer with AES-256-GCM encryption, and should be excluded from serialization using the @Exclude() decorator.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/lib/tenant-api-key/repository/mikro-orm-tenant-api-key.repository.ts (1)
4-4
: Consider adding custom repository methodsWhile extending the base repository provides basic CRUD operations, consider adding custom methods for specific tenant API key operations like key validation, rotation, or expiry checks if these are part of your requirements.
Would you like me to suggest some common API key management methods that could be useful to implement?
packages/core/src/lib/tenant-api-key/repository/type-orm-tenant-api-key.repository.ts (1)
6-11
: Consider enhancing the repository implementation.While the basic implementation is correct and follows NestJS/TypeORM patterns, consider the following improvements:
- Add JSDoc documentation to describe the repository's purpose and usage
- Implement a common interface (e.g.,
ITenantApiKeyRepository
) to ensure consistent API across different ORM implementations- Consider adding custom methods for common tenant API key operations
Example enhancement:
+/** + * Repository for managing tenant API keys using TypeORM. + * Provides data access layer for TenantApiKey entity. + */ +export interface ITenantApiKeyRepository { + findByApiKey(apiKey: string): Promise<TenantApiKey>; + // Add other common operations +} + @Injectable() -export class TypeOrmTenantApiKeyRepository extends Repository<TenantApiKey> { +export class TypeOrmTenantApiKeyRepository extends Repository<TenantApiKey> implements ITenantApiKeyRepository { constructor(@InjectRepository(TenantApiKey) readonly repository: Repository<TenantApiKey>) { super(repository.target, repository.manager, repository.queryRunner); } + + async findByApiKey(apiKey: string): Promise<TenantApiKey> { + return this.findOne({ where: { apiKey } }); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/lib/tenant-api-key/repository/index.ts
(1 hunks)packages/core/src/lib/tenant-api-key/repository/mikro-orm-tenant-api-key.repository.ts
(1 hunks)packages/core/src/lib/tenant-api-key/repository/type-orm-tenant-api-key.repository.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/lib/tenant-api-key/repository/index.ts
- packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
- packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
- packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
🔇 Additional comments (2)
packages/core/src/lib/tenant-api-key/repository/mikro-orm-tenant-api-key.repository.ts (1)
1-4
: Clean and well-structured repository implementation!The implementation follows best practices by:
- Extending the base repository with proper typing
- Maintaining a clear separation of concerns
- Following the repository pattern
Let's verify the complete implementation context:
✅ Verification successful
Implementation follows established patterns correctly!
The repository setup is consistent with the codebase's patterns where:
- MikroORM repositories are automatically registered via MikroOrmModule.forFeature()
- Explicit provider registration is not needed for MikroORM repositories
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the complete tenant API key implementation # Check for the corresponding entity implementation echo "Checking TenantApiKey entity implementation..." ast-grep --pattern 'class TenantApiKey extends TenantBaseEntity implements ITenantApiKey { $$$ }' # Verify module configuration echo "Checking module configuration..." rg -A 10 "TenantApiKeyModule" # Verify repository registration echo "Checking repository registration in module..." rg -A 5 "MikroOrmTenantApiKeyRepository" # Check for corresponding service implementation echo "Checking for service implementation..." ast-grep --pattern 'class $service implements $interface { constructor( $$$ private readonly $repo: MikroOrmTenantApiKeyRepository ) { $$$ } }'Length of output: 5579
Script:
#!/bin/bash # Check module configuration details echo "Checking TenantApiKeyModule content..." cat packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts echo -e "\nChecking parent module imports..." rg -l "import.*TenantApiKeyModule" --type ts echo -e "\nChecking providers in tenant-api-key.module.ts..." rg "providers.*MikroOrmTenantApiKeyRepository" --type tsLength of output: 1028
Script:
#!/bin/bash # Check MikroORM repository registration pattern and parent module # Check if other MikroORM repositories follow the same pattern echo "Checking other MikroORM repository registrations..." rg "MikroOrm.*Repository" --type ts packages/core/src/lib/**/*.module.ts # Check core module structure echo -e "\nChecking core module structure..." fd -t f "core.module.ts" packages/core/src # If core module exists, check its content if [ -f "packages/core/src/core.module.ts" ]; then echo -e "\nChecking core module content..." cat packages/core/src/core.module.ts fiLength of output: 12840
packages/core/src/lib/tenant-api-key/repository/type-orm-tenant-api-key.repository.ts (1)
1-4
: LGTM! Clean import structure following NestJS conventions.The imports are well-organized, including all necessary dependencies for a TypeORM repository implementation in a NestJS application.
import { TenantApiKey } from '../tenant-api-key.entity'; | ||
|
||
@Injectable() | ||
export class TypeOrmTenantApiKeyRepository extends Repository<TenantApiKey> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Critical: API keys and secrets stored in plain text
Significant security vulnerabilities found in tenant API key implementation:
- API keys, secrets, and OpenAI credentials stored as plain text in database
- No encryption/hashing mechanism implemented for sensitive credentials
- Basic access controls insufficient for API key protection
Required changes:
- Implement encryption/hashing for all API keys and secrets
- Add audit logging for API key usage
- Consider implementing rate limiting
- Review and enhance access control mechanisms
🔗 Analysis chain
Verify security measures for API key handling.
Since this repository handles sensitive API keys, ensure:
- API keys are stored securely (hashed) in the database
- Proper access controls are implemented at the service layer
- Rate limiting and audit logging are considered
Let's verify the entity implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if TenantApiKey entity has proper column definitions
# and if API keys are stored securely
# Look for hash/encryption related imports and usage
rg -A 5 "import.*crypto|import.*bcrypt" "packages/core/src/lib/tenant-api-key"
# Check entity definition for proper column types
ast-grep --pattern 'class TenantApiKey {
$$$
@Column($$$)
apiKey: string;
$$$
}'
Length of output: 169
Script:
#!/bin/bash
# Find TenantApiKey entity file
fd -e ts "tenant-api-key.entity"
# Look for security-related code in core package
rg -t ts "class TenantApiKey" packages/core
# Check for any encryption/hashing utilities
fd -e ts "crypto|hash|encrypt" packages/core
# Look for service implementation
fd -e ts "tenant-api-key.service"
Length of output: 664
Script:
#!/bin/bash
# Check entity implementation
cat packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
# Check service implementation
cat packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts
# Look for base entity security measures
rg -t ts "class TenantBaseEntity" packages/core
Length of output: 3278
Script:
#!/bin/bash
# Check TenantBaseEntity implementation
cat packages/core/src/lib/core/entities/tenant-base.entity.ts
# Look for any encryption utilities in the broader codebase
rg -t ts "encrypt|hash|crypto" packages/core/src
Length of output: 70347
View your CI Pipeline Execution ↗ for commit 001d03f.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts (2)
14-14
: Consider replacingconsole.log
with a centralized logging mechanismUsing
console.log
for logging may not be consistent with the project's logging practices. Consider utilizing a centralized logging system or TypeORM's built-in logging features to ensure consistent log handling across the application.
90-110
: Simplify SQLite migration by including the foreign key constraint initiallyThe current SQLite migration creates the table, adds indexes, drops them, creates a temporary table with the foreign key constraint, and then replaces the original table. This process can be streamlined by adding the foreign key constraint directly in the initial
CREATE TABLE
statement. This eliminates the need for temporary tables and reduces the number of steps.Apply this diff to simplify the migration:
await queryRunner.query( `CREATE TABLE "tenant_api_key" ( "deletedAt" datetime, "id" varchar PRIMARY KEY NOT NULL, "createdAt" datetime NOT NULL DEFAULT (datetime('now')), "updatedAt" datetime NOT NULL DEFAULT (datetime('now')), "isActive" boolean DEFAULT (1), "isArchived" boolean DEFAULT (0), "archivedAt" datetime, "tenantId" varchar, "apiKey" varchar NOT NULL, "apiSecret" varchar NOT NULL, + CONSTRAINT "FK_df44cc2b5ba1a4cc95850eea7c2" FOREIGN KEY ("tenantId") REFERENCES "tenant" ("id") ON DELETE CASCADE ON UPDATE NO ACTION )` ); - await queryRunner.query(`CREATE INDEX "IDX_686a4464635f3dbb77411dd5e8" ON "tenant_api_key" ("isActive") `); - await queryRunner.query(`CREATE INDEX "IDX_21056fa2d8b060d3049f50ec0b" ON "tenant_api_key" ("isArchived") `); - await queryRunner.query(`CREATE INDEX "IDX_df44cc2b5ba1a4cc95850eea7c" ON "tenant_api_key" ("tenantId") `); - // Subsequent steps for dropping and recreating indexes and tables can be removed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/contracts/src/lib/tenant-api-key.model.ts
(1 hunks)packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/contracts/src/lib/tenant-api-key.model.ts
- packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
🔇 Additional comments (3)
packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts (3)
5-6
: Class and migration name consistencyThe class name
CreateTenantApiKeyTable1736357496620
and thename
property are consistent, ensuring that the migration is identifiable and traceable.
16-30
: Proper handling of different database types in theup
methodThe
up
method correctly handles different database types using a switch statement, ensuring that the appropriate migration runner is invoked based on the database in use.
37-52
: Proper handling of different database types in thedown
methodSimilarly, the
down
method appropriately handles the rollback for different databases, maintaining consistency with theup
method.
packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts (2)
20-26
:⚠️ Potential issueEnhance security measures for API Key
While the basic security measures are good, consider these essential improvements:
Implement encryption for storage (as per previous discussion):
- Use AES-256-GCM encryption
- Add a custom TypeORM transformer
- Store IV and auth tag with encrypted data
Add stronger validation rules:
@ApiProperty({ type: () => String, description: 'The API Key for authentication.' }) @IsNotEmpty() @IsString() +@Length(32, 64) +@Matches(/^[A-Za-z0-9_-]+$/) @IsSecret() @Exclude() -@MultiORMColumn() +@MultiORMColumn({ transformer: new EncryptedColumnTransformer() }) apiKey: string;
32-38
:⚠️ Potential issueApply consistent security measures to API Secret
Apply the same security improvements as suggested for apiKey:
@ApiProperty({ type: () => String, description: 'The API Secret for secure authentication.' }) @IsNotEmpty() @IsString() +@Length(64, 128) +@Matches(/^[A-Za-z0-9_-]+$/) @IsSecret() @Exclude() -@MultiORMColumn() +@MultiORMColumn({ transformer: new EncryptedColumnTransformer() }) apiSecret: string;
🧹 Nitpick comments (1)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts (1)
1-39
: Consider implementing key rotation mechanismTo enhance security further, consider implementing a key rotation mechanism that allows:
- Generation of new API keys while maintaining old ones during transition
- Gradual deprecation of old keys
- Tracking of key usage and age
This would require additional fields like:
expiresAt
rotatedAt
status
(active/deprecated/expired)Would you like me to provide an example implementation of the key rotation mechanism?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts
(1 hunks)packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts
🧰 Additional context used
📓 Learnings (1)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts (1)
Learnt from: GloireMutaliko21
PR: ever-co/ever-gauzy#8716
File: packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts:14-36
Timestamp: 2025-01-08T12:06:16.832Z
Learning: Sensitive fields in entities should be encrypted before storage using a custom TypeORM transformer with AES-256-GCM encryption, and should be excluded from serialization using the @Exclude() decorator.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (2)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts (2)
10-10
: Fix typo in import pathThe import path contains a spelling error:
-import { MikroOrmTenantApiKeyRepository } from './repository/mikro-orm-tanant-api-key.repository'; +import { MikroOrmTenantApiKeyRepository } from './repository/mikro-orm-tenant-api-key.repository';
12-14
: LGTM! Well-structured entity definitionThe entity is properly configured with MultiORM support and correct inheritance hierarchy.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
New Features
Technical Improvements
Database Changes
tenant_api_key
table with support for PostgreSQL, SQLite, and MySQL.