feat: add GetConnectionsInfoByIds use case and corresponding API endpoint#1703
Conversation
📝 WalkthroughWalkthroughAdds a new SaaS endpoint to fetch connection metadata by a list of connection IDs, including request/response DTOs, a use-case implementation querying the global DB, controller route, DI token, and module/provider registration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as SaasController
participant UseCase as GetConnectionsInfoByIdsUseCase
participant DB as GlobalDBContext
Client->>Controller: POST /saas/connections/info (GetConnectionsInfoByIdsDS)
Controller->>UseCase: execute(GetConnectionsInfoByIdsDS)
UseCase->>UseCase: validate connectionIds (non-empty)
UseCase->>DB: connectionRepository.findBy({ id: In(connectionIds) })
DB-->>UseCase: Array<ConnectionEntity>
UseCase->>UseCase: map entities -> FoundConnectionInfoRO[]
UseCase-->>Controller: FoundConnectionInfoRO[]
Controller-->>Client: 200 OK with FoundConnectionInfoRO[]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new SaaS microservice capability to fetch connection records by a list of connection IDs, wiring it through a new use case, DI token, and controller endpoint.
Changes:
- Introduced
GetConnectionsInfoByIdsUseCaseand request DS to fetch connections by ID list. - Added
/saas/connections/infoPOST endpoint and registered the use case in the SaaS module + DI tokens. - Extended SaaS use-case interfaces to include the new operation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/microservices/saas-microservice/use-cases/saas-use-cases.interface.ts | Adds interface contract for the new “get connections by ids” use case. |
| backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts | Implements DB lookup by connectionIds. |
| backend/src/microservices/saas-microservice/saas.module.ts | Registers new use case + adds route to SaaS auth middleware. |
| backend/src/microservices/saas-microservice/saas.controller.ts | Exposes new POST endpoint for retrieving connections info by IDs. |
| backend/src/microservices/saas-microservice/data-structures/get-connections-info-by-ids.ds.ts | Adds validated request payload for the new endpoint. |
| backend/src/common/data-injection.tokens.ts | Adds a new UseCaseType token for DI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @ApiOperation({ summary: 'Get connections info by ids' }) | ||
| @ApiBody({ type: GetConnectionsInfoByIdsDS }) | ||
| @ApiResponse({ | ||
| status: 200, | ||
| type: [ConnectionEntity], | ||
| }) | ||
| @Post('/connections/info') | ||
| async getConnectionsInfoByIds( | ||
| @Body() connectionsData: GetConnectionsInfoByIdsDS, | ||
| ): Promise<Array<ConnectionEntity>> { | ||
| return await this.getConnectionsInfoByIdsUseCase.execute(connectionsData); |
There was a problem hiding this comment.
The endpoint is documented and typed to return ConnectionEntity[], which includes sensitive fields (e.g., username, password, privateSSHKey, signing_key, cert) defined on ConnectionEntity. Returning the entity from a SaaS-facing API risks leaking credentials/encrypted secrets. Consider returning a dedicated DTO with only non-sensitive fields (e.g., id, title, type, is_frozen, timestamps) and update the Swagger ApiResponse type accordingly.
| const connections = await this._dbContext.connectionRepository.findBy({ | ||
| id: In(connectionIds), | ||
| }); | ||
|
|
||
| return connections; | ||
| } |
There was a problem hiding this comment.
This use case loads full ConnectionEntity records via connectionRepository.findBy({ id: In(connectionIds) }), which will include credential fields and other secrets (even if still encrypted) and then returns them directly. To avoid data exposure, fetch only the required columns (TypeORM select) and/or map results into a safe response DTO before returning.
| @Injectable() | ||
| export class GetConnectionsInfoByIdsUseCase | ||
| extends AbstractUseCase<GetConnectionsInfoByIdsDS, Array<ConnectionEntity>> | ||
| implements IGetConnectionsInfoByIds | ||
| { | ||
| constructor( | ||
| @Inject(BaseType.GLOBAL_DB_CONTEXT) | ||
| protected _dbContext: IGlobalDatabaseContext, | ||
| ) { |
There was a problem hiding this comment.
Most other SaaS microservice use cases are explicitly request-scoped (e.g., CreateConnectionForHostedDbUseCase, DeleteConnectionForHostedDbUseCase). This use case is left as default scope, but it depends on GlobalDatabaseContext which is request-scoped; making the scope explicit (@Injectable({ scope: Scope.REQUEST })) would better match the module’s established pattern and avoid surprising DI scope behavior.
| @ApiOperation({ summary: 'Get connections info by ids' }) | ||
| @ApiBody({ type: GetConnectionsInfoByIdsDS }) | ||
| @ApiResponse({ | ||
| status: 200, | ||
| type: [ConnectionEntity], | ||
| }) | ||
| @Post('/connections/info') | ||
| async getConnectionsInfoByIds( | ||
| @Body() connectionsData: GetConnectionsInfoByIdsDS, | ||
| ): Promise<Array<ConnectionEntity>> { | ||
| return await this.getConnectionsInfoByIdsUseCase.execute(connectionsData); | ||
| } |
There was a problem hiding this comment.
There’s existing e2e coverage for other /saas/* endpoints (e.g., backend/test/ava-tests/saas-tests/hosted-connection-e2e.test.ts), but this new /saas/connections/info behavior isn’t covered. Adding an e2e test for success + validation failure (empty connectionIds) would help prevent regressions, especially around response shape/sanitization.
| protected async implementation(inputData: GetConnectionsInfoByIdsDS): Promise<Array<ConnectionEntity>> { | ||
| const { connectionIds } = inputData; | ||
| if (!connectionIds || connectionIds.length === 0) { | ||
| throw new HttpException( | ||
| { | ||
| message: Messages.CONNECTION_ID_MISSING, | ||
| }, | ||
| HttpStatus.BAD_REQUEST, | ||
| ); | ||
| } | ||
|
|
||
| const connections = await this._dbContext.connectionRepository.findBy({ | ||
| id: In(connectionIds), | ||
| }); |
There was a problem hiding this comment.
Authorization-wise, this endpoint accepts arbitrary connectionIds and returns whatever exists in the global DB without scoping to a companyId (or other tenant identifier). With only the SaaS JWT protecting the route, a caller with that token could enumerate connection IDs across companies. Consider including companyId in the request and adding it to the query predicate (or otherwise validating that each returned connection belongs to the intended tenant).
| export interface IGetConnectionsInfoByIds { | ||
| execute(inputData: GetConnectionsInfoByIdsDS): Promise<Array<ConnectionEntity>>; | ||
| } |
There was a problem hiding this comment.
The use case interface exposes Promise<ConnectionEntity[]> as its return type. Since ConnectionEntity contains sensitive credential fields, it would be safer to define and return a dedicated response DTO (only the fields needed by SaaS) and update this interface to use that DTO type instead of the full entity.
| @ApiOperation({ summary: 'Get connections info by ids' }) | ||
| @ApiBody({ type: GetConnectionsInfoByIdsDS }) | ||
| @ApiResponse({ | ||
| status: 200, | ||
| type: [ConnectionEntity], | ||
| }) | ||
| @Post('/connections/info') | ||
| async getConnectionsInfoByIds( |
There was a problem hiding this comment.
@Post('/connections/info') will return HTTP 201 by default in NestJS unless @HttpCode(200) is set. The Swagger decorator declares status: 200, so docs and actual status code may diverge. Consider adding @HttpCode(HttpStatus.OK) (or changing the documented status to 201) to keep behavior consistent.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/microservices/saas-microservice/saas.controller.ts`:
- Around line 335-346: The controller method getConnectionsInfoByIds currently
returns ConnectionEntity which leaks sensitive fields; create a dedicated
response DTO (e.g., ConnectionInfoResponseDto) that exposes only safe fields
(id, title, status flags, createdAt/updatedAt, etc.), update the `@ApiResponse`
type to use that DTO array, and map the results of
getConnectionsInfoByIdsUseCase.execute(connectionsData) to an array of
ConnectionInfoResponseDto inside the getConnectionsInfoByIds method before
returning; ensure mapping explicitly picks allowed properties and does not
return credential or secret columns.
In
`@backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts`:
- Around line 34-36: The query in get-connections-info-by-ids.use.case.ts uses
this._dbContext.connectionRepository.findBy({ id: In(connectionIds) }) and
returns records by arbitrary IDs without tenant/owner scoping; update this to
enforce authorization by adding the caller's scope constraints (e.g.,
tenantId/companyId and/or ownerId) to the query or call a scoped repository
method (e.g., connectionRepository.findByScoped or findBy with { id:
In(connectionIds), tenantId: currentUser.tenantId, ownerId: currentUser.id })
and validate permissions before returning results so only connections within the
caller's authorized scope are fetched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9321bcd7-5f29-4084-ad2d-34fd121bdadf
📒 Files selected for processing (6)
backend/src/common/data-injection.tokens.tsbackend/src/microservices/saas-microservice/data-structures/get-connections-info-by-ids.ds.tsbackend/src/microservices/saas-microservice/saas.controller.tsbackend/src/microservices/saas-microservice/saas.module.tsbackend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.tsbackend/src/microservices/saas-microservice/use-cases/saas-use-cases.interface.ts
| const connections = await this._dbContext.connectionRepository.findBy({ | ||
| id: In(connectionIds), | ||
| }); |
There was a problem hiding this comment.
Add authorization scoping before fetching by arbitrary IDs.
The current query returns any matching connection.id records without checking caller scope (tenant/company/owner). With only service-token auth at the route level, this becomes an over-broad data access path. Please enforce scope constraints before findBy (or use a scoped repository method).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts`
around lines 34 - 36, The query in get-connections-info-by-ids.use.case.ts uses
this._dbContext.connectionRepository.findBy({ id: In(connectionIds) }) and
returns records by arbitrary IDs without tenant/owner scoping; update this to
enforce authorization by adding the caller's scope constraints (e.g.,
tenantId/companyId and/or ownerId) to the query or call a scoped repository
method (e.g., connectionRepository.findByScoped or findBy with { id:
In(connectionIds), tenantId: currentUser.tenantId, ownerId: currentUser.id })
and validate permissions before returning results so only connections within the
caller's authorized scope are fetched.
…onInfoRO and add new data structure
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts (1)
35-37:⚠️ Potential issue | 🔴 CriticalEnforce authorization scope in the
findByquery before returning connection metadata.Line 35-Line 37 fetches by arbitrary IDs only; combined with the unguarded controller route, this can expose cross-tenant/cross-owner connection data. Scope this query by authorized tenant/company/owner (or use a scoped repository method) before returning results.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts` around lines 35 - 37, The current call to this._dbContext.connectionRepository.findBy({ id: In(connectionIds) }) returns connections by arbitrary IDs and must be scoped to the caller; update the query in the use case (get-connections-info-by-ids use case) to include the caller's authorization identifiers (e.g., tenantId/companyId/ownerId) from the auth context (e.g., this._authContext or injected credentials) or call a scoped repository method (e.g., connectionRepository.findByScoped) so the where clause includes both id: In(connectionIds) and the tenant/owner/company filter before returning results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/microservices/saas-microservice/data-structures/found-connection-info.ro.ts`:
- Around line 11-57: The DTO FoundConnectionInfoRO currently marks several
properties as required but the mapper buildConnectionInfoRO passes through
nullable/optional values from ConnectionEntity; update the listed properties
(type, host, port, database, schema, sid, ssl, authSource) to be
optional/nullable in the DTO by changing their TypeScript types to include |
null (or make them optional with ?) and annotate each with `@ApiProperty`({
required: false, nullable: true }) so the Swagger schema and runtime typing
match the entity/mapper behavior; keep other properties unchanged.
---
Duplicate comments:
In
`@backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.ts`:
- Around line 35-37: The current call to
this._dbContext.connectionRepository.findBy({ id: In(connectionIds) }) returns
connections by arbitrary IDs and must be scoped to the caller; update the query
in the use case (get-connections-info-by-ids use case) to include the caller's
authorization identifiers (e.g., tenantId/companyId/ownerId) from the auth
context (e.g., this._authContext or injected credentials) or call a scoped
repository method (e.g., connectionRepository.findByScoped) so the where clause
includes both id: In(connectionIds) and the tenant/owner/company filter before
returning results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bdbc747d-d8e6-49e8-b40e-f7c70eb65bd1
📒 Files selected for processing (4)
backend/src/microservices/saas-microservice/data-structures/found-connection-info.ro.tsbackend/src/microservices/saas-microservice/saas.controller.tsbackend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.tsbackend/src/microservices/saas-microservice/use-cases/saas-use-cases.interface.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/microservices/saas-microservice/use-cases/saas-use-cases.interface.ts
- backend/src/microservices/saas-microservice/saas.controller.ts
| @ApiProperty({ enum: ConnectionTypesEnum }) | ||
| type: ConnectionTypesEnum; | ||
|
|
||
| @ApiProperty() | ||
| host: string; | ||
|
|
||
| @ApiProperty() | ||
| port: number; | ||
|
|
||
| @ApiProperty() | ||
| database: string; | ||
|
|
||
| @ApiProperty() | ||
| schema: string; | ||
|
|
||
| @ApiProperty() | ||
| sid: string; | ||
|
|
||
| @ApiProperty() | ||
| ssh: boolean; | ||
|
|
||
| @ApiProperty() | ||
| ssl: boolean; | ||
|
|
||
| @ApiProperty() | ||
| createdAt: Date; | ||
|
|
||
| @ApiProperty() | ||
| updatedAt: Date; | ||
|
|
||
| @ApiProperty() | ||
| isTestConnection: boolean; | ||
|
|
||
| @ApiProperty() | ||
| is_frozen: boolean; | ||
|
|
||
| @ApiProperty() | ||
| masterEncryption: boolean; | ||
|
|
||
| @ApiProperty() | ||
| azure_encryption: boolean; | ||
|
|
||
| @ApiProperty() | ||
| authSource: string; | ||
|
|
||
| @ApiProperty() | ||
| dataCenter: string | null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify nullable/optional source fields in entity vs non-null DTO contract
rg -n "type\\?:|host\\?:|port\\?:|database\\?:|schema\\?:|sid\\?:|ssl\\?:|authSource\\?:|dataCenter\\?:" backend/src/entities/connection/connection.entity.ts
rg -n "type: ConnectionTypesEnum|host: string;|port: number;|database: string;|schema: string;|sid: string;|ssl: boolean;|authSource: string;|dataCenter: string \\| null;" backend/src/microservices/saas-microservice/data-structures/found-connection-info.ro.tsRepository: rocket-admin/rocketadmin
Length of output: 525
🏁 Script executed:
# Find the mapper that creates FoundConnectionInfoRO
rg -n "FoundConnectionInfoRO|toFoundConnectionInfo" backend/src --type ts -A 3 | head -100Repository: rocket-admin/rocketadmin
Length of output: 5285
🏁 Script executed:
# Read the complete FoundConnectionInfoRO file to verify current state
cat -n backend/src/microservices/saas-microservice/data-structures/found-connection-info.ro.tsRepository: rocket-admin/rocketadmin
Length of output: 1362
🏁 Script executed:
# Check the connection entity file to verify field declarations
cat -n backend/src/entities/connection/connection.entity.ts | head -120Repository: rocket-admin/rocketadmin
Length of output: 4298
🏁 Script executed:
# Read the mapper function completely
sed -n '42,80p' backend/src/microservices/saas-microservice/use-cases/get-connections-info-by-ids.use.case.tsRepository: rocket-admin/rocketadmin
Length of output: 772
Fix DTO nullability to match nullable entity fields returned by mapper.
The FoundConnectionInfoRO declares several fields as required/non-null, but ConnectionEntity defines them as nullable/optional, and the buildConnectionInfoRO mapper passes these values through unchanged. This creates an inaccurate API contract where the Swagger schema doesn't reflect that these fields can be null or undefined.
Fields requiring updates (currently required, should be optional/nullable):
type,host,database,schema,sid,ssl,authSource— are explicitly nullable in the entity (| null)port— has@Column({ default: null }), allowing null values in the database
Proposed fix
-import { ApiProperty } from '@nestjs/swagger';
+import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js';
export class FoundConnectionInfoRO {
`@ApiProperty`()
id: string;
`@ApiProperty`()
title: string;
- `@ApiProperty`({ enum: ConnectionTypesEnum })
- type: ConnectionTypesEnum;
+ `@ApiPropertyOptional`({ enum: ConnectionTypesEnum, nullable: true })
+ type: ConnectionTypesEnum | null;
- `@ApiProperty`()
- host: string;
+ `@ApiPropertyOptional`({ nullable: true })
+ host: string | null;
- `@ApiProperty`()
- port: number;
+ `@ApiPropertyOptional`({ nullable: true })
+ port: number | null;
- `@ApiProperty`()
- database: string;
+ `@ApiPropertyOptional`({ nullable: true })
+ database: string | null;
- `@ApiProperty`()
- schema: string;
+ `@ApiPropertyOptional`({ nullable: true })
+ schema: string | null;
- `@ApiProperty`()
- sid: string;
+ `@ApiPropertyOptional`({ nullable: true })
+ sid: string | null;
`@ApiProperty`()
ssh: boolean;
- `@ApiProperty`()
- ssl: boolean;
+ `@ApiPropertyOptional`({ nullable: true })
+ ssl: boolean | null;
`@ApiProperty`()
createdAt: Date;
`@ApiProperty`()
updatedAt: Date;
`@ApiProperty`()
isTestConnection: boolean;
`@ApiProperty`()
is_frozen: boolean;
`@ApiProperty`()
masterEncryption: boolean;
`@ApiProperty`()
azure_encryption: boolean;
- `@ApiProperty`()
- authSource: string;
+ `@ApiPropertyOptional`({ nullable: true })
+ authSource: string | null;
- `@ApiProperty`()
+ `@ApiPropertyOptional`({ nullable: true })
dataCenter: string | null;
}📝 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.
| @ApiProperty({ enum: ConnectionTypesEnum }) | |
| type: ConnectionTypesEnum; | |
| @ApiProperty() | |
| host: string; | |
| @ApiProperty() | |
| port: number; | |
| @ApiProperty() | |
| database: string; | |
| @ApiProperty() | |
| schema: string; | |
| @ApiProperty() | |
| sid: string; | |
| @ApiProperty() | |
| ssh: boolean; | |
| @ApiProperty() | |
| ssl: boolean; | |
| @ApiProperty() | |
| createdAt: Date; | |
| @ApiProperty() | |
| updatedAt: Date; | |
| @ApiProperty() | |
| isTestConnection: boolean; | |
| @ApiProperty() | |
| is_frozen: boolean; | |
| @ApiProperty() | |
| masterEncryption: boolean; | |
| @ApiProperty() | |
| azure_encryption: boolean; | |
| @ApiProperty() | |
| authSource: string; | |
| @ApiProperty() | |
| dataCenter: string | null; | |
| `@ApiProperty`() | |
| id: string; | |
| `@ApiProperty`() | |
| title: string; | |
| `@ApiPropertyOptional`({ enum: ConnectionTypesEnum, nullable: true }) | |
| type: ConnectionTypesEnum | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| host: string | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| port: number | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| database: string | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| schema: string | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| sid: string | null; | |
| `@ApiProperty`() | |
| ssh: boolean; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| ssl: boolean | null; | |
| `@ApiProperty`() | |
| createdAt: Date; | |
| `@ApiProperty`() | |
| updatedAt: Date; | |
| `@ApiProperty`() | |
| isTestConnection: boolean; | |
| `@ApiProperty`() | |
| is_frozen: boolean; | |
| `@ApiProperty`() | |
| masterEncryption: boolean; | |
| `@ApiProperty`() | |
| azure_encryption: boolean; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| authSource: string | null; | |
| `@ApiPropertyOptional`({ nullable: true }) | |
| dataCenter: string | null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/microservices/saas-microservice/data-structures/found-connection-info.ro.ts`
around lines 11 - 57, The DTO FoundConnectionInfoRO currently marks several
properties as required but the mapper buildConnectionInfoRO passes through
nullable/optional values from ConnectionEntity; update the listed properties
(type, host, port, database, schema, sid, ssl, authSource) to be
optional/nullable in the DTO by changing their TypeScript types to include |
null (or make them optional with ?) and annotate each with `@ApiProperty`({
required: false, nullable: true }) so the Swagger schema and runtime typing
match the entity/mapper behavior; keep other properties unchanged.
Summary by CodeRabbit