feat! : Added alternative object storage #1484
Conversation
Signed-off-by: Ankita Patidar <[email protected]>
WalkthroughReplaces the deprecated AWS-specific service and library with a new provider-agnostic storage abstraction. Adds MinIO and S3 providers behind a StorageService, updates DI across apps to use it, removes the old aws library, extends common constants, fixes a cloud-wallet URL, updates role guard logic, and revises environment/config mappings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Svc as StorageService
participant Prov as Storage Provider
participant Back as Storage Backend
Note over Svc: On init: selects provider by FILE_STORAGE_PROVIDER<br/>(minio → MinioProvider, else S3Provider)
Caller->>Svc: uploadFileToBucket(buffer, ext, filename, bucket, encoding, path?)
Svc->>Prov: uploadFile(bucket, key, buffer, {encoding,mimeType})
Prov->>Back: PUT object (key, data, headers)
Back-->>Prov: 200 / ETag
Prov-->>Svc: object URL
Svc-->>Caller: object URL
Caller->>Svc: getFile(key)
Svc->>Prov: getFile(bucket, key)
Prov->>Back: GET object
Back-->>Prov: data Buffer/stream
Prov-->>Svc: Buffer
Svc-->>Caller: Buffer
Caller->>Svc: storeObject(persistent, key, body)
Svc->>Prov: storeObject(bucket, persistent, key, body)
Prov->>Back: PUT JSON object
Back-->>Prov: 200
Prov-->>Svc: object URL
Svc-->>Caller: object URL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/cloud-wallet/src/cloud-wallet.service.ts (1)
158-158: Fix the syntax error.There's an extra closing brace
}at the end of the template literal, resulting in/${proofRecordId}}. This will cause a syntax error.Apply this diff to fix the syntax error:
- const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}}`; + const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}`;apps/organization/src/organization.service.ts (1)
495-512: Add validation for environment variable and improve error handling.The method has a few potential issues:
- Line 503:
process.env.ORG_LOGO_BUCKETis not validated. If undefined, it could cause runtime errors downstream in the storage service.- Line 497:
orgLogo.split(',')[1]assumes the data URL contains a comma. WhileisValidBase64should prevent this, adding explicit validation here improves robustness.Apply this diff to add validation:
async uploadFile(orgLogo: string): Promise<string> { try { + if (!process.env.ORG_LOGO_BUCKET) { + throw new InternalServerErrorException('ORG_LOGO_BUCKET environment variable is not configured'); + } + const updatedOrglogo = orgLogo.split(',')[1]; + if (!updatedOrglogo) { + throw new BadRequestException('Invalid base64 data URL format'); + } + const imgData = Buffer.from(updatedOrglogo, CommonConstants.ENCODING); const logoUrl = await this.storageService.uploadFileToBucket( imgData, this.IMG_EXT, this.ORG_LOGO_PREFIX, process.env.ORG_LOGO_BUCKET, CommonConstants.ENCODING, this.ORG_LOGO_FOLDER ); return logoUrl; } catch (error) { this.logger.error(`In getting imageUrl : ${JSON.stringify(error)}`); throw new RpcException(error.response ? error.response : error); } }
🧹 Nitpick comments (9)
apps/api-gateway/src/authz/guards/user-role.guard.ts (1)
6-20: Consider adding type annotations for the user object.The
request.userobject lacks type definitions, reducing type safety. Consider defining an interface for the expected user structure withrealm_access.rolesto catch potential issues at compile time.Example interface:
interface RequestUser { realm_access?: { roles?: string[]; }; }Then annotate the guard to expect this type on the request object.
apps/cloud-wallet/src/cloud-wallet.service.ts (1)
52-53: Address the TODO comment.The TODO indicates that
cacheServiceis a duplicate, unused variable that should be removed.Do you want me to help verify if this variable is used elsewhere in the codebase, or open a new issue to track this cleanup task?
apps/organization/src/organization.service.ts (1)
73-75: Consider making IMG_EXT dynamic instead of hardcoding 'png'.The
IMG_EXTconstant is hardcoded as'png', but the incoming data URL might specify a different image format (e.g.,data:image/jpeg;base64,...). This mismatch could cause issues if the storage provider or consumers expect the correct file extension.Consider extracting the image type from the data URL:
private getImageExtension(dataUrl: string): string { const match = dataUrl.match(/data:image\/([a-zA-Z]*);base64,/); return match?.[1] || 'png'; }Then use it in
uploadFile:async uploadFile(orgLogo: string): Promise<string> { try { const updatedOrglogo = orgLogo.split(',')[1]; + const extension = this.getImageExtension(orgLogo); const imgData = Buffer.from(updatedOrglogo, CommonConstants.ENCODING); const logoUrl = await this.storageService.uploadFileToBucket( imgData, - this.IMG_EXT, + extension, this.ORG_LOGO_PREFIX, process.env.ORG_LOGO_BUCKET, CommonConstants.ENCODING, this.ORG_LOGO_FOLDER ); return logoUrl; } catch (error) { this.logger.error(`In getting imageUrl : ${JSON.stringify(error)}`); throw new RpcException(error.response ? error.response : error); } }apps/utility/src/utilities.service.ts (1)
51-61: Consider preserving error details for debugging.The error handling at line 59 throws a generic error message without preserving the original error details, which could make debugging more difficult.
Apply this diff to preserve error context:
} catch (error) { this.logger.error(error); - throw new Error('An error occurred while uploading data Error::::::'); + throw new RpcException(error.response ? error.response : error); }This aligns with the error handling pattern used elsewhere in the codebase (e.g., issuance.controller.ts lines 376-378).
libs/storage/package.json (1)
19-19: Consider migrating to AWS SDK v3.The package retains aws-sdk v2 (2.1510.0), which entered maintenance mode in September 2024 and reached end-of-support in September 2025. AWS SDK v2 now receives only critical security fixes, with no new features.
Based on learnings: AWS recommends migrating to AWS SDK v3 (@aws-sdk/*) for:
- Modular imports that reduce bundle size
- Active feature updates and long-term support
- Improved middleware/plugin model
Since this PR introduces a storage abstraction layer with S3Provider, consider scheduling a follow-up migration to AWS SDK v3 for the S3 provider implementation to ensure long-term maintainability.
libs/storage/src/storage.service.spec.ts (1)
1-18: Consider expanding test coverage for storage abstraction.The test file correctly updates all references from AwsService to StorageService. However, the test suite only verifies that the service is defined. Consider adding tests for:
- Provider selection based on FILE_STORAGE_PROVIDER environment variable
- StorageService methods (uploadFileToBucket, uploadCsvFile, getFile, deleteFile, storeObject)
- Error handling for storage operations
- Behavior differences between MinIO and S3 providers
Would you like me to generate a more comprehensive test suite for the StorageService?
libs/storage/src/provider/s3.provider.ts (1)
27-28: S3 URL builder ignores dual-stack / regional patternsHard-coding
https://${bucket}.s3.${region}.amazonaws.com/${key}fails for buckets in regions withs3.${region}.amazonaws.comdeprecation (e.g., us-east-1 dualstack) or when the bucket name contains dots (SSL invalid). Preferthis.s3.getSignedUrl('getObject', ...)or usehttps://${process.env.AWS_S3_CDN_HOST}/${key}fed via config to avoid breakage.libs/storage/src/storage.service.ts (2)
46-47: Missing encoding when delegating to providerWhen uploading CSVs we never set
encoding, so providers sendundefinedto S3/MinIO headers. If the data is base64 or UTF-16 the object metadata is incorrect. Thread through an explicit encoding parameter.
54-58: Bucket name pulled fromprocess.envat call-siteEvery call reads
process.env.FILE_BUCKET. If the env var is missing, we silently passundefinedto the provider and get opaque S3 errors. Validate configuration up front (e.g., throw during service construction) so failures are explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
.env.sample(2 hunks)apps/api-gateway/src/authz/guards/user-role.guard.ts(1 hunks)apps/api-gateway/src/issuance/issuance.controller.ts(4 hunks)apps/api-gateway/src/issuance/issuance.module.ts(2 hunks)apps/api-gateway/src/organization/organization.module.ts(2 hunks)apps/api-gateway/src/organization/organization.service.ts(2 hunks)apps/api-gateway/src/user/user.controller.ts(2 hunks)apps/api-gateway/src/user/user.module.ts(2 hunks)apps/api-gateway/src/webhook/webhook.module.ts(2 hunks)apps/cloud-wallet/src/cloud-wallet.service.ts(1 hunks)apps/issuance/src/issuance.module.ts(2 hunks)apps/issuance/src/issuance.service.ts(3 hunks)apps/organization/src/organization.module.ts(2 hunks)apps/organization/src/organization.service.ts(5 hunks)apps/user/src/fido/fido.module.ts(3 hunks)apps/user/src/user.module.ts(2 hunks)apps/user/src/user.service.ts(0 hunks)apps/utility/src/utilities.module.ts(2 hunks)apps/utility/src/utilities.service.ts(1 hunks)libs/aws/src/aws.module.ts(0 hunks)libs/aws/src/aws.service.ts(0 hunks)libs/aws/src/index.ts(0 hunks)libs/aws/tsconfig.build.json(0 hunks)libs/common/src/common.constant.ts(1 hunks)libs/storage/package.json(2 hunks)libs/storage/src/index.ts(1 hunks)libs/storage/src/provider/minio.provider.ts(1 hunks)libs/storage/src/provider/s3.provider.ts(1 hunks)libs/storage/src/storage.interface.ts(1 hunks)libs/storage/src/storage.module.ts(1 hunks)libs/storage/src/storage.service.spec.ts(1 hunks)libs/storage/src/storage.service.ts(1 hunks)libs/storage/tsconfig.build.json(1 hunks)package.json(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (5)
- libs/aws/src/index.ts
- apps/user/src/user.service.ts
- libs/aws/tsconfig.build.json
- libs/aws/src/aws.module.ts
- libs/aws/src/aws.service.ts
🧰 Additional context used
🧬 Code graph analysis (6)
apps/organization/src/organization.module.ts (1)
libs/logger/src/logger.interface.ts (1)
Logger(6-20)
libs/storage/src/storage.module.ts (9)
apps/api-gateway/src/issuance/issuance.module.ts (1)
Module(12-26)apps/api-gateway/src/organization/organization.module.ts (1)
Module(13-29)apps/api-gateway/src/user/user.module.ts (1)
Module(13-28)apps/api-gateway/src/webhook/webhook.module.ts (1)
Module(12-26)apps/issuance/src/issuance.module.ts (1)
Module(24-69)apps/organization/src/organization.module.ts (1)
Module(27-63)apps/user/src/fido/fido.module.ts (1)
Module(25-61)apps/user/src/user.module.ts (1)
Module(29-67)apps/utility/src/utilities.module.ts (1)
Module(17-36)
libs/storage/src/storage.service.ts (6)
apps/issuance/src/issuance.service.ts (1)
Injectable(96-2121)apps/organization/src/organization.service.ts (1)
Injectable(71-2104)apps/utility/src/utilities.service.ts (1)
Injectable(7-62)libs/storage/src/storage.interface.ts (1)
IStorageProvider(14-22)libs/storage/src/provider/minio.provider.ts (1)
MinioProvider(5-57)libs/storage/src/provider/s3.provider.ts (1)
S3Provider(5-55)
libs/storage/src/provider/minio.provider.ts (1)
libs/storage/src/storage.interface.ts (2)
IStorageProvider(14-22)FileUploadOptions(9-12)
libs/storage/src/provider/s3.provider.ts (1)
libs/storage/src/storage.interface.ts (2)
IStorageProvider(14-22)FileUploadOptions(9-12)
apps/utility/src/utilities.service.ts (2)
libs/storage/src/storage.service.ts (1)
Injectable(7-77)apps/utility/src/utilities.repository.ts (2)
Injectable(6-51)getShorteningUrl(35-50)
🪛 dotenv-linter (3.3.0)
.env.sample
[warning] 196-196: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 197-197: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 198-198: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 198-198: [UnorderedKey] The AWS_ACCESS_KEY key should go before the AWS_ACCOUNT_ID key
(UnorderedKey)
[warning] 198-198: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 199-199: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 199-199: [UnorderedKey] The AWS_SECRET_KEY key should go before the S3_BUCKET_ARN key
(UnorderedKey)
[warning] 199-199: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 200-200: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 200-200: [UnorderedKey] The AWS_REGION key should go before the AWS_SECRET_KEY key
(UnorderedKey)
[warning] 200-200: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 203-203: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 203-203: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 204-204: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 204-204: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 205-205: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 205-205: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 206-206: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 206-206: [UnorderedKey] The MINIO_ACCESS_KEY key should go before the MINIO_ENDPOINT key
(UnorderedKey)
[warning] 206-206: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 207-207: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 207-207: [UnorderedKey] The MINIO_SECRET_KEY key should go before the MINIO_USE_SSL key
(UnorderedKey)
[warning] 207-207: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 209-209: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 209-209: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 210-210: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 210-210: [UnorderedKey] The FILE_BUCKET key should go before the STORE_OBJECT_BUCKET key
(UnorderedKey)
[warning] 210-210: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 211-211: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 211-211: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 211-211: [UnorderedKey] The ORG_LOGO_BUCKET key should go before the STORE_OBJECT_BUCKET key
(UnorderedKey)
[warning] 211-211: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (25)
libs/storage/tsconfig.build.json (1)
1-9: LGTM! Standard TypeScript build configuration.The build configuration correctly extends the root tsconfig, enables declaration file generation for the library's public API, and follows standard monorepo conventions for output paths and exclusions.
package.json (1)
202-203: LGTM! Jest path mapping correctly configured.The Jest moduleNameMapper correctly adds the @credebl/storage alias, enabling test resolution for the new storage library. The pattern is consistent with other library aliases in the configuration.
apps/user/src/fido/fido.module.ts (2)
22-22: LGTM! Correct migration to StorageService.The import has been correctly updated from the deprecated AWS-specific service to the new provider-agnostic StorageService, aligning with the PR's objective to support multiple storage providers.
41-41: LGTM! Provider correctly updated.The module providers array has been correctly updated to use StorageService instead of AwsService, completing the migration for this module.
tsconfig.json (1)
107-112: LGTM! Path aliases correctly configured.The TypeScript path mappings for the new storage library are correctly configured and follow the same pattern as other library aliases, enabling proper module resolution across the codebase.
apps/api-gateway/src/organization/organization.service.ts (2)
28-28: LGTM! Improved consistency with centralized constants.The import of CommonConstants enables the use of standardized constant values throughout the service, improving maintainability.
240-240: LGTM! Using centralized encoding constant.The change from a string literal to
CommonConstants.ENCODINGimproves code consistency and maintainability by using a centralized constant value.libs/storage/src/storage.module.ts (1)
1-8: LGTM! Clean module implementation.The StorageModule follows NestJS best practices by correctly providing and exporting StorageService, enabling dependency injection across the application. The module structure is clean and appropriate for a service module.
apps/issuance/src/issuance.module.ts (2)
15-15: LGTM! Import correctly updated.The import has been correctly changed from the deprecated AWS-specific service to the new StorageService from the storage library.
60-60: LGTM! Provider correctly updated.The module providers array has been correctly updated to include StorageService, completing the migration from AWS-specific storage to the new provider-agnostic implementation.
libs/common/src/common.constant.ts (1)
386-387: LGTM! Good practice to centralize the encoding constant.Adding
ENCODING = 'base64'toCommonConstantseliminates magic strings and ensures consistent encoding references across the codebase.libs/storage/src/index.ts (1)
1-2: LGTM! Standard barrel export pattern.The exports follow NestJS conventions for library modules and provide a clean public API for the storage abstraction.
apps/utility/src/utilities.module.ts (1)
34-34: LGTM! Dependency injection updated correctly.The replacement of
AwsServicewithStorageServicein the providers aligns with the storage abstraction migration across the codebase.apps/api-gateway/src/issuance/issuance.module.ts (1)
24-24: LGTM! Dependency injection updated correctly.The module providers correctly include
StorageService, maintaining consistency with the storage abstraction migration.apps/api-gateway/src/user/user.controller.ts (1)
64-64: LGTM! Constructor dependency updated correctly.The injection of
StorageServicereplaces the previous AWS-specific service, aligning with the provider-agnostic storage abstraction.apps/user/src/user.module.ts (1)
49-49: LGTM! Module providers updated correctly.The inclusion of
StorageServicein the providers maintains consistency with the storage abstraction migration across modules.apps/organization/src/organization.module.ts (1)
59-59: LGTM! Dependency injection updated correctly.The
StorageServiceis properly included in the providers, aligning with the storage abstraction migration.apps/api-gateway/src/organization/organization.module.ts (1)
10-10: LGTM! Storage service properly integrated.The module correctly imports and registers StorageService, replacing the deprecated AwsService. This aligns with the PR's objective to introduce a provider-agnostic storage abstraction.
Also applies to: 27-27
apps/api-gateway/src/user/user.module.ts (1)
9-9: LGTM! Storage service properly integrated.The module correctly imports and registers StorageService, consistent with the storage abstraction refactor.
Also applies to: 26-26
apps/api-gateway/src/issuance/issuance.controller.ts (2)
72-72: LGTM! Storage service properly integrated.The controller correctly switches from AwsService to StorageService for dependency injection, aligning with the new storage abstraction.
Also applies to: 92-92
375-378: StorageService.uploadCsvFile signature is consistent. It accepts the passedfile.buffer(asunknown), converts it internally to aBuffer, and correctly invokes both S3 and MinIO providers.apps/utility/src/utilities.service.ts (1)
4-4: LGTM! Storage service properly integrated.The service correctly switches from AwsService to StorageService for dependency injection, aligning with the new storage abstraction.
Also applies to: 9-13
apps/api-gateway/src/webhook/webhook.module.ts (1)
8-8: LGTM! Storage service properly integrated.The module correctly imports and registers StorageService, consistent with the storage abstraction refactor.
Also applies to: 24-26
libs/storage/package.json (1)
20-20: MinIO dependency version looks good.The minio package version (^8.0.6) is appropriate. Based on learnings, v8.x is the current major release with TypeScript support and compatibility fixes.
libs/storage/src/storage.interface.ts (1)
9-22: LGTM! Well-designed storage abstraction interfaces.The interface definitions are clean and well-structured:
FileUploadOptionsprovides clear type safety for upload configurationsIStorageProviderdefines a consistent contract for storage providers- Method signatures include bucket parameters for multi-bucket support
- Use of
unknowntype forbodyinstoreObjectis appropriately permissiveThis abstraction enables seamless switching between MinIO and S3 providers.
| # FILE_STORAGE_PROVIDER can be 'aws' or 'minio' | ||
| FILE_STORAGE_PROVIDER=minio | ||
|
|
||
| # AWS Configurations | ||
| AWS_ACCOUNT_ID=xxxxx // Please provide your AWS account Id | ||
| S3_BUCKET_ARN=arn:aws:s3:::xxxxx // Please provide your AWS bucket arn | ||
| AWS_ACCESS_KEY= // Please provide your access key | ||
| AWS_SECRET_KEY= // Please provide your secret key | ||
| AWS_REGION= // Please provide your aws region | ||
|
|
||
| # MinIO Config : Use the same buckets as below three mentioned with AWS S3 config | ||
| MINIO_ENDPOINT= // Please provide your minio endpoint | ||
| MINIO_PORT= // Please provide your minio port | ||
| MINIO_USE_SSL= // Please provide your minio ssl true/false | ||
| MINIO_ACCESS_KEY= // Please provide your minio access key | ||
| MINIO_SECRET_KEY= // Please provide your minio secret key | ||
|
|
||
| STORE_OBJECT_BUCKET= // Please provide your aws bucket | ||
| FILE_BUCKET= // Please provide your aws bucket for file sharing | ||
| ORG_LOGO_BUCKET= // Please provide your aws bucket for org logo |
There was a problem hiding this comment.
Address dotenv formatting issues.
The new storage provider configuration is functionally correct, but static analysis has identified multiple formatting issues that should be addressed:
- Missing quotes: Values should be quoted (e.g.,
FILE_STORAGE_PROVIDER="minio") - Space characters: Remove spaces around equal signs (e.g.,
AWS_ACCESS_KEY=notAWS_ACCESS_KEY=) - Missing ending blank line: Add a blank line at the end of the file
Apply these formatting fixes:
-# FILE_STORAGE_PROVIDER can be 'aws' or 'minio'
-FILE_STORAGE_PROVIDER=minio
+# FILE_STORAGE_PROVIDER can be 'aws' or 'minio'
+FILE_STORAGE_PROVIDER="minio"
# AWS Configurations
-AWS_ACCOUNT_ID=xxxxx // Please provide your AWS account Id
-S3_BUCKET_ARN=arn:aws:s3:::xxxxx // Please provide your AWS bucket arn
-AWS_ACCESS_KEY= // Please provide your access key
-AWS_SECRET_KEY= // Please provide your secret key
-AWS_REGION= // Please provide your aws region
+AWS_ACCOUNT_ID="xxxxx" // Please provide your AWS account Id
+S3_BUCKET_ARN="arn:aws:s3:::xxxxx" // Please provide your AWS bucket arn
+AWS_ACCESS_KEY="" // Please provide your access key
+AWS_SECRET_KEY="" // Please provide your secret key
+AWS_REGION="" // Please provide your aws region
# MinIO Config : Use the same buckets as below three mentioned with AWS S3 config
-MINIO_ENDPOINT= // Please provide your minio endpoint
-MINIO_PORT= // Please provide your minio port
-MINIO_USE_SSL= // Please provide your minio ssl true/false
-MINIO_ACCESS_KEY= // Please provide your minio access key
-MINIO_SECRET_KEY= // Please provide your minio secret key
+MINIO_ENDPOINT="" // Please provide your minio endpoint
+MINIO_PORT="" // Please provide your minio port
+MINIO_USE_SSL="" // Please provide your minio ssl true/false
+MINIO_ACCESS_KEY="" // Please provide your minio access key
+MINIO_SECRET_KEY="" // Please provide your minio secret key
-STORE_OBJECT_BUCKET= // Please provide your aws bucket
-FILE_BUCKET= // Please provide your aws bucket for file sharing
-ORG_LOGO_BUCKET= // Please provide your aws bucket for org logo
+STORE_OBJECT_BUCKET="" // Please provide your aws bucket
+FILE_BUCKET="" // Please provide your aws bucket for file sharing
+ORG_LOGO_BUCKET="" // Please provide your aws bucket for org logo
+📝 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.
| # FILE_STORAGE_PROVIDER can be 'aws' or 'minio' | |
| FILE_STORAGE_PROVIDER=minio | |
| # AWS Configurations | |
| AWS_ACCOUNT_ID=xxxxx // Please provide your AWS account Id | |
| S3_BUCKET_ARN=arn:aws:s3:::xxxxx // Please provide your AWS bucket arn | |
| AWS_ACCESS_KEY= // Please provide your access key | |
| AWS_SECRET_KEY= // Please provide your secret key | |
| AWS_REGION= // Please provide your aws region | |
| # MinIO Config : Use the same buckets as below three mentioned with AWS S3 config | |
| MINIO_ENDPOINT= // Please provide your minio endpoint | |
| MINIO_PORT= // Please provide your minio port | |
| MINIO_USE_SSL= // Please provide your minio ssl true/false | |
| MINIO_ACCESS_KEY= // Please provide your minio access key | |
| MINIO_SECRET_KEY= // Please provide your minio secret key | |
| STORE_OBJECT_BUCKET= // Please provide your aws bucket | |
| FILE_BUCKET= // Please provide your aws bucket for file sharing | |
| ORG_LOGO_BUCKET= // Please provide your aws bucket for org logo | |
| # FILE_STORAGE_PROVIDER can be 'aws' or 'minio' | |
| FILE_STORAGE_PROVIDER="minio" | |
| # AWS Configurations | |
| AWS_ACCOUNT_ID="xxxxx" // Please provide your AWS account Id | |
| S3_BUCKET_ARN="arn:aws:s3:::xxxxx" // Please provide your AWS bucket arn | |
| AWS_ACCESS_KEY="" // Please provide your access key | |
| AWS_SECRET_KEY="" // Please provide your secret key | |
| AWS_REGION="" // Please provide your aws region | |
| # MinIO Config : Use the same buckets as below three mentioned with AWS S3 config | |
| MINIO_ENDPOINT="" // Please provide your minio endpoint | |
| MINIO_PORT="" // Please provide your minio port | |
| MINIO_USE_SSL="" // Please provide your minio ssl true/false | |
| MINIO_ACCESS_KEY="" // Please provide your minio access key | |
| MINIO_SECRET_KEY="" // Please provide your minio secret key | |
| STORE_OBJECT_BUCKET="" // Please provide your aws bucket | |
| FILE_BUCKET="" // Please provide your aws bucket for file sharing | |
| ORG_LOGO_BUCKET="" // Please provide your aws bucket for org logo | |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 196-196: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 197-197: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 198-198: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 198-198: [UnorderedKey] The AWS_ACCESS_KEY key should go before the AWS_ACCOUNT_ID key
(UnorderedKey)
[warning] 198-198: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 199-199: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 199-199: [UnorderedKey] The AWS_SECRET_KEY key should go before the S3_BUCKET_ARN key
(UnorderedKey)
[warning] 199-199: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 200-200: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 200-200: [UnorderedKey] The AWS_REGION key should go before the AWS_SECRET_KEY key
(UnorderedKey)
[warning] 200-200: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 203-203: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 203-203: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 204-204: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 204-204: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 205-205: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 205-205: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 206-206: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 206-206: [UnorderedKey] The MINIO_ACCESS_KEY key should go before the MINIO_ENDPOINT key
(UnorderedKey)
[warning] 206-206: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 207-207: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 207-207: [UnorderedKey] The MINIO_SECRET_KEY key should go before the MINIO_USE_SSL key
(UnorderedKey)
[warning] 207-207: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 209-209: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 209-209: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 210-210: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 210-210: [UnorderedKey] The FILE_BUCKET key should go before the STORE_OBJECT_BUCKET key
(UnorderedKey)
[warning] 210-210: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 211-211: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 211-211: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 211-211: [UnorderedKey] The ORG_LOGO_BUCKET key should go before the STORE_OBJECT_BUCKET key
(UnorderedKey)
[warning] 211-211: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
In .env.sample around lines 192-211, fix dotenv formatting by (1) quoting all
assigned values (e.g., FILE_STORAGE_PROVIDER="minio", AWS_ACCOUNT_ID="xxxxx",
etc.), (2) removing any spaces around the '=' so assignments are like
KEY="value" with no surrounding spaces, (3) move inline comments off assignment
lines to their own comment lines above or below if needed (don’t leave comments
after values), and (4) ensure the file ends with a single trailing newline
(blank line) at EOF.
| if (!user?.realm_access.roles) { | ||
| throw new ForbiddenException('This role is not a holder.'); | ||
| } | ||
| if (!user?.userRole.includes('holder')) { | ||
|
|
||
| if (!user?.realm_access.roles.includes('holder')) { | ||
| throw new ForbiddenException('This role is not a holder.'); | ||
| } |
There was a problem hiding this comment.
Critical: Incomplete optional chaining will throw TypeError.
The optional chaining operator ?. only applies to the immediate property access. In user?.realm_access.roles, if user exists but realm_access is undefined, accessing .roles on undefined will throw a TypeError. The same issue exists on line 15 with .includes('holder').
Apply this diff to add proper optional chaining:
- if (!user?.realm_access.roles) {
- throw new ForbiddenException('This role is not a holder.');
+ if (!user?.realm_access?.roles) {
+ throw new ForbiddenException('User roles not found.');
}
- if (!user?.realm_access.roles.includes('holder')) {
+ if (!user.realm_access.roles.includes('holder')) {
throw new ForbiddenException('This role is not a holder.');
}Note: Line 15 no longer needs optional chaining after line 11's validation confirms the path exists.
📝 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.
| if (!user?.realm_access.roles) { | |
| throw new ForbiddenException('This role is not a holder.'); | |
| } | |
| if (!user?.userRole.includes('holder')) { | |
| if (!user?.realm_access.roles.includes('holder')) { | |
| throw new ForbiddenException('This role is not a holder.'); | |
| } | |
| if (!user?.realm_access?.roles) { | |
| throw new ForbiddenException('User roles not found.'); | |
| } | |
| if (!user.realm_access.roles.includes('holder')) { | |
| throw new ForbiddenException('This role is not a holder.'); | |
| } |
🤖 Prompt for AI Agents
In apps/api-gateway/src/authz/guards/user-role.guard.ts around lines 11 to 17,
the optional chaining is incomplete and may throw if realm_access is undefined;
change the existence check to guard the whole path (e.g., verify
user?.realm_access?.roles) and after that extract roles into a local const and
use roles.includes('holder') (no optional chaining) to perform the membership
check, throwing the same ForbiddenException when the path or the role is
missing.
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${ | ||
| threadId ? `?threadId=${threadId}` : '' | ||
| }`; |
There was a problem hiding this comment.
Fix the URL construction to remove the trailing slash before the query parameter.
The URL construction includes a hardcoded / before the query parameter, resulting in malformed URLs:
- With threadId:
.../proof-request/?threadId=xyz(extra/before?) - Without threadId:
.../proof-request/(trailing/)
Standard REST URL format should not have a trailing slash before query parameters: .../proof-request?threadId=xyz
Apply this diff to fix the URL construction:
- const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${
- threadId ? `?threadId=${threadId}` : ''
- }`;
+ const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}${
+ threadId ? `?threadId=${threadId}` : ''
+ }`;📝 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.
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${ | |
| threadId ? `?threadId=${threadId}` : '' | |
| }`; | |
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}${ | |
| threadId ? `?threadId=${threadId}` : '' | |
| }`; |
🤖 Prompt for AI Agents
In apps/cloud-wallet/src/cloud-wallet.service.ts around lines 179 to 181, the
URL is built with a hardcoded slash before the query string causing either an
extra slash before `?` or a trailing slash; remove the trailing `/` from the
base path and concatenate the query parameter only when present (e.g. build URL
as agentEndpoint + CLOUD_WALLET_GET_PROOF_REQUEST followed by `?threadId=...`
only if threadId exists), ensuring no extra `/` remains before the `?` and no
trailing slash is left when threadId is absent.
| private readonly storageService: StorageService, | ||
| @InjectQueue('bulk-issuance') private readonly bulkIssuanceQueue: Queue, | ||
| // TODO: Remove duplicate, unused variable | ||
| @Inject(CACHE_MANAGER) private readonly cacheService: Cache, | ||
| @Inject(ContextStorageServiceKey) | ||
| private readonly contextStorageService: ContextStorageService, | ||
| private readonly natsClient: NATSClient | ||
| ) {} |
There was a problem hiding this comment.
Constructor injection still references removed provider
ContextStorageServiceKey and natsClient remain injected even though the corresponding provider was removed with the AWS service refactor. Keeping this stale dependency will break DI at runtime because Nest can no longer resolve ContextStorageServiceKey. Please drop the injection (and related property) or ensure the token is still registered.
🤖 Prompt for AI Agents
In apps/issuance/src/issuance.service.ts around lines 109 to 114, the
constructor still injects @Inject(ContextStorageServiceKey) private readonly
natsClient: NATSClient even though that provider was removed; remove the
injection parameter and the natsClient property from the constructor (and delete
or update any usages of this.natsClient in the class), or if the dependency must
remain, re-register the ContextStorageServiceKey provider and ensure its token
matches the refactored AWS service; pick one approach and apply it consistently
so Nest DI can resolve the constructor.
| const getFileDetails = await this.storageService.getFile(importFileDetails.fileKey); | ||
| const csvData: string = getFileDetails.toString(); | ||
|
|
There was a problem hiding this comment.
StorageService.getFile() returns buffers
The new storage service resolves files to Buffer instances. Calling toString() without passing the expected encoding will use UTF‑8 implicitly, which corrupts binary CSVs uploaded with base64 encoding. Capture and honor the encoding (e.g., pass CommonConstants.ENCODING) when converting, or make StorageService.getFile return the decoded string directly.
🤖 Prompt for AI Agents
In apps/issuance/src/issuance.service.ts around lines 1290-1292, the code calls
toString() on the Buffer returned by StorageService.getFile which implicitly
uses UTF-8 and can corrupt CSVs uploaded as base64; update the call to decode
the buffer with the expected encoding (e.g., csvData =
getFileDetails.toString(CommonConstants.ENCODING)) or change
StorageService.getFile to return the decoded string directly so callers receive
the correctly decoded CSV content.
| await this.minioClient.putObject(bucket, objKey, buffer, null, { | ||
| ContentType: 'application/json', | ||
| ContentEncoding: CommonConstants.ENCODING | ||
| }); | ||
| const protocol = 'true' === process.env.MINIO_USE_SSL ? 'https' : 'http'; | ||
| return `${protocol}://${process.env.MINIO_ENDPOINT}:${process.env.MINIO_PORT}/${bucket}/${objKey}`; |
There was a problem hiding this comment.
Content headers case mismatch
MinIO expects lowercase header keys ('Content-Type', 'Content-Encoding') like in uploadFile. Here mixed-case keys (ContentType) are used, so metadata isn’t set. Align header casing to ensure persisted objects advertise the right MIME type.
🤖 Prompt for AI Agents
In libs/storage/src/provider/minio.provider.ts around lines 50 to 55, the
putObject call uses mixed-case metadata keys (ContentType, ContentEncoding)
which MinIO ignores; change them to lowercase header names exactly as used
elsewhere ('Content-Type' and 'Content-Encoding') so the MIME type and encoding
are persisted (keep the same values, e.g. 'application/json' and
CommonConstants.ENCODING), then return the same URL logic unchanged.
| // export interface IStorageProvider { | ||
| // uploadFile(buffer: Buffer, key: string, mimeType: string): Promise<string>; | ||
| // getFile(key: string): Promise<Buffer>; | ||
| // deleteFile(key: string): Promise<void>; | ||
| // // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| // storeObject(key: string, body: any): Promise<string>; | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
The file contains commented-out old interface definitions that should be removed to maintain clean code. Commented code creates confusion about which interface is currently in use and clutters the file.
Apply this diff to remove the commented code:
-// export interface IStorageProvider {
-// uploadFile(buffer: Buffer, key: string, mimeType: string): Promise<string>;
-// getFile(key: string): Promise<Buffer>;
-// deleteFile(key: string): Promise<void>;
-// // eslint-disable-next-line @typescript-eslint/no-explicit-any
-// storeObject(key: string, body: any): Promise<string>;
-// }
-
export interface FileUploadOptions {📝 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.
| // export interface IStorageProvider { | |
| // uploadFile(buffer: Buffer, key: string, mimeType: string): Promise<string>; | |
| // getFile(key: string): Promise<Buffer>; | |
| // deleteFile(key: string): Promise<void>; | |
| // // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| // storeObject(key: string, body: any): Promise<string>; | |
| // } | |
| export interface FileUploadOptions { |
🤖 Prompt for AI Agents
In libs/storage/src/storage.interface.ts around lines 1 to 7, remove the
commented-out legacy IStorageProvider interface block (all commented lines
shown) so the file contains only active code; delete the entire commented
section to eliminate dead/commented code and save the file.
| break; | ||
| case 's3': | ||
| default: | ||
| this.provider = new S3Provider(); | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Instantiate providers lazily via DI
Direct new MinioProvider() / new S3Provider() bypasses Nest's dependency injection, making it impossible to mock providers in tests or supply configuration through Nest’s ConfigService. Register providers as injectable classes and request them through DI instead of manual construction.
🤖 Prompt for AI Agents
In libs/storage/src/storage.service.ts around lines 16 to 20, the service is
instantiating providers directly (new S3Provider()/new MinioProvider()), which
bypasses Nest DI; instead register MinioProvider and S3Provider as injectable
providers in the storage module and inject them into StorageService via the
constructor (or inject a provider factory/token that maps the configured
provider to the correct implementation), then select the correct injected
instance at runtime rather than calling new; ensure providers can receive
ConfigService and be mocked in tests by relying on DI.
| return await this.provider.uploadFile(bucketName, key, fileBuffer, { | ||
| encoding, | ||
| mimeType: 'image/png' | ||
| }); |
There was a problem hiding this comment.
Hard-coded MIME type breaks non-PNG uploads
mimeType: 'image/png' is fixed regardless of the actual file extension. Uploading JPEG/SVG logos will get the wrong Content-Type, leading to broken rendering on clients. Derive the MIME type from ext (e.g., via mime.lookup) before delegating to the storage provider.
🤖 Prompt for AI Agents
In libs/storage/src/storage.service.ts around lines 35 to 38, the code
hard-codes mimeType: 'image/png' which breaks non-PNG uploads; instead derive
the MIME type from the file extension (use a library like
mime.lookup/mime.getType on the ext variable), provide a sensible fallback
(e.g., 'application/octet-stream' or 'image/png' if you prefer), and pass that
computed mimeType into this.provider.uploadFile so the provider receives the
correct Content-Type for the uploaded file.
| async uploadCsvFile(key: string, body: unknown): Promise<void> { | ||
| try { | ||
| await this.provider.uploadFile(process.env.FILE_BUCKET as string, key, Buffer.from(String(body)), { | ||
| mimeType: 'text/csv' | ||
| }); |
There was a problem hiding this comment.
CSV body serialization loses encoding
Buffer.from(String(body)) converts objects to [object Object] and discards original CSV contents. Ensure the caller provides a Buffer or stringify appropriately (e.g., when body is already CSV text). Otherwise uploaded files become unreadable.
🤖 Prompt for AI Agents
In libs/storage/src/storage.service.ts around lines 44 to 48, the current
uploadCsvFile uses Buffer.from(String(body)) which converts objects to “[object
Object]” and loses CSV contents; change the implementation to accept and
preserve CSV encoding by: if body is already a Buffer use it as-is; else if body
is a string call Buffer.from(body, 'utf8'); otherwise throw a clear error (or
require the caller to provide a Buffer/string) so objects are not coerced to
“[object Object]”; keep the mimeType as 'text/csv' when calling
provider.uploadFile.
1 participant
This is linked to the issue 1162 to support addition object storage in CREDEBL.
Other improvements -
Summary by CodeRabbit
New Features
Bug Fixes
Documentation