Skip to content
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

Merged
merged 8 commits into from
Jan 9, 2025

Conversation

GloireMutaliko21
Copy link
Contributor

@GloireMutaliko21 GloireMutaliko21 commented Jan 8, 2025

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

    • Added support for Tenant API Keys, enabling secure API access management for tenants.
    • Introduced new data model and service for managing tenant-specific API credentials.
  • Technical Improvements

    • Implemented cross-database migration support for Tenant API Key table.
    • Added ORM repositories for TypeORM and MikroORM to handle API key operations.
  • Database Changes

    • Created new tenant_api_key table with support for PostgreSQL, SQLite, and MySQL.

Copy link
Contributor

coderabbitai bot commented Jan 8, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This 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

File Change Summary
packages/contracts/src/index.ts Added export for tenant-api-key.model
packages/contracts/src/lib/tenant-api-key.model.ts Introduced ITenantApiKey and ITenantApiKeyCreateInput interfaces
packages/core/src/lib/core/entities/index.ts Added TenantApiKey to entity imports and exports
packages/core/src/lib/core/entities/internal.ts Exported tenant-api-key.entity
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts Created TenantApiKey entity with properties name, apiKey, and apiSecret
packages/core/src/lib/tenant-api-key/tenant-api-key.module.ts Implemented TenantApiKeyModule with ORM configurations
packages/core/src/lib/tenant-api-key/tenant-api-key.service.ts Added TenantApiKeyService with findApiKeyByOptions method
packages/core/src/lib/tenant-api-key/repository/mikro-orm-tenant-api-key.repository.ts Created MikroOrmTenantApiKeyRepository class
packages/core/src/lib/tenant-api-key/repository/type-orm-tenant-api-key.repository.ts Created TypeOrmTenantApiKeyRepository class
packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts Added migration for creating tenant_api_key table across database types
packages/core/src/lib/core/entities/base.entity.ts Updated timestamp management in base.entity.ts

Sequence Diagram

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

Possibly Related PRs

  • [Feature] Mention model and entity #8553: This PR adds a new export for the mention.model in the same file as the main PR, indicating a similar type of change involving the public API surface.
  • [Feat] Dashboard Widget Entity #8616: This PR introduces a new export for the dashboard-widget.model in the same file as the main PR, reflecting a related change to the public API surface.
  • [Feat] Tag Type #8692: This PR modifies the tag.model.ts file, which is related to the changes in the main PR as it also involves updates to models and exports in the contracts package.

Poem

🐰 Hop, hop, API keys so bright!
A tenant's secret, held tight
Repositories dance, migrations sing
Security's rabbit takes wing!
New models bloom in our code's light 🔑


📜 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 a27b3cb and 001d03f.

📒 Files selected for processing (4)
  • packages/contracts/src/lib/tenant-api-key.model.ts (1 hunks)
  • packages/core/src/lib/core/entities/base.entity.ts (2 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)

Finishing Touches

  • 📝 Generate Docstrings

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?

❤️ 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.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate unit testing code.
    • @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.

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. (Beta)
  • @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
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🧹 Nitpick comments (2)
packages/core/src/lib/database/migrations/1736330073887-CreateTenantApiKeyTable.ts (1)

93-98: Optimize index creation in SQLite migration

In the sqliteUpQueryRunner method, the indexes on tenant_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 the ITenantApiKeyCreateInput interface

Since ITenantApiKeyCreateInput does not add any new properties to ITenantApiKey, you might consider using ITenantApiKey directly without defining a separate interface.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between befc8cb and 82a47f6.

📒 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 correct

The postgresUpQueryRunner method correctly creates the tenant_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 correct

The MikroOrmTenantApiKeyRepository class correctly extends the MikroOrmBaseEntityRepository with the TenantApiKey entity.

packages/contracts/src/lib/tenant-api-key.model.ts (1)

3-8: Interface definition is correct

The ITenantApiKey interface appropriately extends IBasePerTenantEntityModel 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 filters

The 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 and coreEntities 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.

Comment on lines 14 to 36
@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;
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider encrypting sensitive fields

The entity stores sensitive credentials in plain text. Consider:

  1. Encrypting these fields before storage
  2. Implementing a custom transformer for automatic encryption/decryption
  3. Adding @Exclude() decorator to prevent serialization of sensitive data

Would you like me to provide an example implementation of field encryption?

Copy link
Contributor Author

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

Copy link
Contributor

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:

  1. Uses AES-256-GCM for strong encryption
  2. Stores IV and auth tag with encrypted data for secure decryption
  3. Automatically encrypts/decrypts via TypeORM transformer
  4. Excludes sensitive fields from serialization
  5. Uses environment variable for encryption key

Don't forget to:

  1. Add ENCRYPTION_KEY to your environment variables (32 bytes, base64 encoded)
  2. Add EncryptionService to your module providers
  3. 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.

packages/core/src/lib/tenant-api-key/repository/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@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

🧹 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 methods

While 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:

  1. Add JSDoc documentation to describe the repository's purpose and usage
  2. Implement a common interface (e.g., ITenantApiKeyRepository) to ensure consistent API across different ORM implementations
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 82a47f6 and d1415c3.

📒 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 ts

Length 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
fi

Length 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> {
Copy link
Contributor

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:

  1. API keys are stored securely (hashed) in the database
  2. Proper access controls are implemented at the service layer
  3. 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

Copy link

nx-cloud bot commented Jan 8, 2025

View your CI Pipeline Execution ↗ for commit 001d03f.

Command Status Duration Result
nx build desktop --base-href ./ ✅ Succeeded 1m 52s View ↗
nx build desktop-api --output-path=dist/apps/de... ✅ Succeeded 28s View ↗
nx run api:desktop-api ✅ Succeeded 1m 18s View ↗
nx build gauzy -c=production --prod --verbose ✅ Succeeded 3m 55s View ↗
nx run gauzy:desktop-ui --base-href ./ ✅ Succeeded 3m 34s View ↗
nx build api -c=production --prod ✅ Succeeded 1m 10s View ↗
nx build desktop-ui-lib --configuration=production ✅ Succeeded 29s View ↗
nx build desktop-ui-lib --configuration=develop... ✅ Succeeded 30s View ↗
Additional runs (53) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-01-09 09:52:43 UTC

Copy link
Contributor

@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

🧹 Nitpick comments (2)
packages/core/src/lib/database/migrations/1736357496620-CreateTenantApiKeyTable.ts (2)

14-14: Consider replacing console.log with a centralized logging mechanism

Using 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 initially

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between d1415c3 and 94ada7a.

📒 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 consistency

The class name CreateTenantApiKeyTable1736357496620 and the name property are consistent, ensuring that the migration is identifiable and traceable.


16-30: Proper handling of different database types in the up method

The 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 the down method

Similarly, the down method appropriately handles the rollback for different databases, maintaining consistency with the up method.

Copy link
Contributor

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

♻️ Duplicate comments (2)
packages/core/src/lib/tenant-api-key/tenant-api-key.entity.ts (2)

20-26: ⚠️ Potential issue

Enhance security measures for API Key

While the basic security measures are good, consider these essential improvements:

  1. 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
  2. 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 issue

Apply 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 mechanism

To enhance security further, consider implementing a key rotation mechanism that allows:

  1. Generation of new API keys while maintaining old ones during transition
  2. Gradual deprecation of old keys
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 94ada7a and a27b3cb.

📒 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 path

The 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 definition

The entity is properly configured with MultiORM support and correct inheritance hierarchy.

@rahul-rocket rahul-rocket merged commit 8f19a55 into develop Jan 9, 2025
12 of 16 checks passed
@rahul-rocket rahul-rocket deleted the feat/tenant-api-key-entity-and-api branch January 9, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants