Backend agent table public permissions - #1864
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds internal Sitenova endpoints for retrieving decrypted runtime connections and validating public table reads. The change introduces typed contracts, use cases, provider registrations, end-user signing-key access, and Cedar-based column filtering. ChangesSitenova internal access
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant InternalClient
participant SitenovaInternalController
participant SitenovaGetConnectionUseCase
participant SitenovaEndUserAuthService
InternalClient->>SitenovaInternalController: GET connection by connectionId
SitenovaInternalController->>SitenovaGetConnectionUseCase: execute connectionId
SitenovaGetConnectionUseCase->>SitenovaEndUserAuthService: getEndUserSigningKey(connectionId)
SitenovaEndUserAuthService-->>SitenovaGetConnectionUseCase: signing key
SitenovaGetConnectionUseCase-->>SitenovaInternalController: runtime connection data
SitenovaInternalController-->>InternalClient: connection response
sequenceDiagram
participant InternalClient
participant SitenovaInternalController
participant SitenovaValidatePublicReadUseCase
participant CedarPermissionsService
InternalClient->>SitenovaInternalController: POST table and column validation
SitenovaInternalController->>SitenovaValidatePublicReadUseCase: execute validation request
SitenovaValidatePublicReadUseCase->>CedarPermissionsService: check public access and readable columns
CedarPermissionsService-->>SitenovaValidatePublicReadUseCase: authorization result
SitenovaValidatePublicReadUseCase-->>SitenovaInternalController: validation response
SitenovaInternalController-->>InternalClient: allowed and readable columns
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the backend SiteNova microservice’s internal (microservice-JWT) API to support satellite/universal-backend runtimes by (1) handing out decrypted connection credentials + end-user JWT signing key, and (2) validating anonymous/public table-read permissions (including readable-column projection) using the same Cedar policy logic as the core.
Changes:
- Added internal endpoints to fetch decrypted connection info (+ end-user JWT signing key) and to validate public-read access + readable columns.
- Introduced new SiteNova internal request/response DTOs/DS/ROs and new use cases, wired through DI tokens and the module.
- Exposed an auth-service method to retrieve the per-connection end-user JWT signing key for internal callers.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/microservices/sitenova-microservice/use-cases/sitenova-validate-public-read.use.case.ts | New use case bridging Cedar public-read evaluation and readable-columns projection for satellites. |
| backend/src/microservices/sitenova-microservice/use-cases/sitenova-use-cases.interface.ts | Added interfaces and typings for new internal use cases. |
| backend/src/microservices/sitenova-microservice/use-cases/sitenova-get-connection.use.case.ts | New use case to return decrypted connection credentials and end-user JWT signing key (rejects agent connections). |
| backend/src/microservices/sitenova-microservice/sitenova.module.ts | Registered new use cases in DI container for the SiteNova microservice. |
| backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts | Added internal routes for connection credential retrieval and public-read validation. |
| backend/src/microservices/sitenova-microservice/services/sitenova-enduser-auth.service.ts | Added a public method to expose the end-user signing key to internal callers. |
| backend/src/microservices/sitenova-microservice/dto/sitenova.dtos.ts | Added request DTO for public-read validation (tableName + optional columnNames). |
| backend/src/microservices/sitenova-microservice/data-structures/sitenova.ds.ts | Added DS classes for the new internal use cases. |
| backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts | New internal response objects for decrypted connection payload + public-read validation result. |
| backend/src/common/data-injection.tokens.ts | Added UseCaseType tokens for the new SiteNova internal use cases. |
Suppressed comments (1)
backend/src/microservices/sitenova-microservice/use-cases/sitenova-validate-public-read.use.case.ts:50
- When
columnNamesis provided as an empty array,readableColumnsstaysnull. This makes the response indistinguishable from the "columnNames was not submitted" case, and contradicts the response description (null only when not submitted). Returning an empty array when an empty array is submitted keeps the API semantics consistent.
let readableColumns: Array<string> | null = null;
if (columnNames && columnNames.length > 0) {
const readable = await this.cedarPermissions.getReadableColumnsForPublic(connectionId, tableName, columnNames);
readableColumns = columnNames.filter((columnName) => readable.has(columnName));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+26
| import { Injectable } from '@nestjs/common'; | ||
| import AbstractUseCase from '../../../common/abstract-use.case.js'; | ||
| import { IDatabaseContext } from '../../../common/database-context.interface.js'; | ||
| import { CedarAction, PUBLIC_USER_ID } from '../../../entities/cedar-authorization/cedar-action-map.js'; | ||
| import { CedarAuthorizationService } from '../../../entities/cedar-authorization/cedar-authorization.service.js'; | ||
| import { CedarPermissionsService } from '../../../entities/cedar-authorization/cedar-permissions.service.js'; | ||
| import { SitenovaValidatePublicReadDs } from '../data-structures/sitenova.ds.js'; | ||
| import { SitenovaPublicReadValidationRO } from '../data-structures/sitenova-internal-responses.ds.js'; | ||
| import { ISitenovaValidatePublicRead } from './sitenova-use-cases.interface.js'; | ||
|
|
||
| // Bridges SitenovaPublicReadGuard's Cedar evaluation (allowed?) and the public readable-columns | ||
| // projection to the universal-backend service, which runs the actual query itself but must apply | ||
| // the exact same public-read policy the core applies on its own /sitenova data routes. | ||
| @Injectable() | ||
| export class SitenovaValidatePublicReadUseCase | ||
| extends AbstractUseCase<SitenovaValidatePublicReadDs, SitenovaPublicReadValidationRO> | ||
| implements ISitenovaValidatePublicRead | ||
| { | ||
| protected _dbContext: IDatabaseContext | null = null; | ||
|
|
||
| constructor( | ||
| private readonly cedarAuthService: CedarAuthorizationService, | ||
| private readonly cedarPermissions: CedarPermissionsService, | ||
| ) { | ||
| super(); | ||
| } |
Comment on lines
+42
to
+78
| @ApiOperation({ | ||
| summary: 'Return decrypted connection credentials plus the end-user JWT signing key.', | ||
| description: | ||
| 'Serves the universal-backend service so it can open the customer database itself and ' + | ||
| 'sign/verify generated-site end-user tokens. Agent connections are rejected (no stored credentials).', | ||
| }) | ||
| @ApiResponse({ status: 200, type: SitenovaConnectionForSiteRuntimeRO }) | ||
| @Get('/connection/:connectionId') | ||
| public async getConnection( | ||
| @SlugUuid('connectionId') connectionId: string, | ||
| ): Promise<SitenovaConnectionForSiteRuntimeRO> { | ||
| return await this.getConnectionUseCase.execute({ connectionId }, InTransactionEnum.OFF); | ||
| } | ||
|
|
||
| @ApiOperation({ | ||
| summary: 'Evaluate the connection public policy for an anonymous table read.', | ||
| description: | ||
| 'Same Cedar evaluation SitenovaPublicReadGuard applies on the core /sitenova data routes, plus ' + | ||
| 'the publicly readable subset of the submitted columns — for satellites running the query themselves.', | ||
| }) | ||
| @ApiResponse({ status: 200, type: SitenovaPublicReadValidationRO }) | ||
| @ApiBody({ type: SitenovaValidatePublicReadDto }) | ||
| @HttpCode(HttpStatus.OK) | ||
| @Post('/validate-public-read/:connectionId') | ||
| public async validatePublicRead( | ||
| @SlugUuid('connectionId') connectionId: string, | ||
| @Body() body: SitenovaValidatePublicReadDto, | ||
| ): Promise<SitenovaPublicReadValidationRO> { | ||
| return await this.validatePublicReadUseCase.execute( | ||
| { | ||
| connectionId, | ||
| tableName: body.tableName, | ||
| columnNames: body.columnNames ?? null, | ||
| }, | ||
| InTransactionEnum.OFF, | ||
| ); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts (1)
59-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace string concatenation with template literals.
Both Swagger descriptions violate the same repository rule.
backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts#L59-L61: use one template literal for the signing-key description.backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts#L44-L46: use one template literal for the endpoint description.As per coding guidelines, “Use template literals instead of string concatenation.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts` around lines 59 - 61, Replace the concatenated Swagger description in backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts lines 59-61 with a single template literal. Apply the same conversion to the endpoint description in backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts lines 44-46; preserve the existing text exactly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts`:
- Around line 49-53: The getConnection endpoint currently trusts the
caller-controlled connectionId and returns decrypted credentials without
connection-level authorization. Update getConnection and its authorization flow
to require a valid connections[*] or service authorization claim, or an
equivalent guard proving access to that specific connection, before invoking
getConnectionUseCase.execute and returning secrets; reject unauthorized or
mismatched connection requests.
---
Nitpick comments:
In
`@backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts`:
- Around line 59-61: Replace the concatenated Swagger description in
backend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.ts
lines 59-61 with a single template literal. Apply the same conversion to the
endpoint description in
backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts
lines 44-46; preserve the existing text exactly.
🪄 Autofix
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 Plus
Run ID: f00fae52-d5dd-484b-9a03-9d9c065319e8
📒 Files selected for processing (10)
backend/src/common/data-injection.tokens.tsbackend/src/microservices/sitenova-microservice/data-structures/sitenova-internal-responses.ds.tsbackend/src/microservices/sitenova-microservice/data-structures/sitenova.ds.tsbackend/src/microservices/sitenova-microservice/dto/sitenova.dtos.tsbackend/src/microservices/sitenova-microservice/services/sitenova-enduser-auth.service.tsbackend/src/microservices/sitenova-microservice/sitenova-internal.controller.tsbackend/src/microservices/sitenova-microservice/sitenova.module.tsbackend/src/microservices/sitenova-microservice/use-cases/sitenova-get-connection.use.case.tsbackend/src/microservices/sitenova-microservice/use-cases/sitenova-use-cases.interface.tsbackend/src/microservices/sitenova-microservice/use-cases/sitenova-validate-public-read.use.case.ts
Comment on lines
+49
to
+53
| @Get('/connection/:connectionId') | ||
| public async getConnection( | ||
| @SlugUuid('connectionId') connectionId: string, | ||
| ): Promise<SitenovaConnectionForSiteRuntimeRO> { | ||
| return await this.getConnectionUseCase.execute({ connectionId }, InTransactionEnum.OFF); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
controller="$(fd -a 'sitenova-internal.controller.ts' | head -n1)"
sed -n '1,120p' "$controller"
rg -n -C5 \
'UseGuards|UseInterceptors|Jwt|JWT|microservice|SitenovaInternalController|connectionId' \
backend/srcRepository: rocket-admin/rocketadmin
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
controller="$(fd -a 'sitenova-internal.controller.ts' | head -n1)"
module="$(fd -a 'sitenova.module.ts' | head -n1)"
echo "== controller =="
sed -n '1,140p' "$controller"
echo
echo "== sitenova module =="
sed -n '1,180p' "$module"
echo
echo "== auth guard decorators/modules in sitenova internal area =="
rg -n --iglob '*.ts' \
'UseGuards|Guard|HttpUnauthorized|UnauthorizedException|Jwt|jwt|SitenovaInternal|`@Controller`' \
backend/src/microservices/sitenova-microservice backend/src/common backend/src/decorators backend/src/guards backend/src/filters 2>/dev/null \
| head -n 200
echo
echo "== focused UseGuards/Jwt/JWT in relevant files =="
while IFS= read -r f; do
echo "--- $f"
rg -n -C3 '(UseGuards|AuthGuard|jwt|jwt|JWT|microservice|sitenova-internal|ConnectionReadGuard|Sitenova' "$f" \
| head -n 120
done < <(fd -a --iglob '*.ts' backend/src | rg 'sitenova|auth-guard|connection|microservice|jwt|interceptor|guard' | head -n 80)Repository: rocket-admin/rocketadmin
Length of output: 18051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SaaSAuthMiddleware =="
fd -a 'saas-auth.middleware.ts' backend/src | sed -n '1,20p' | while read -r f; do
echo "--- $f"
sed -n '1,240p' "$f"
done
echo
echo "== SitenovaGetConnectionUseCase =="
usecase="$(fd -a 'sitenova-get-connection.use.case.ts' backend/src | head -n1)"
sed -n '1,160p' "$usecase"
echo
echo "== connection decrypt/signing key methods =="
rg -n -C4 'getDecrypted|decrypt.*password|endUserJwtKey|getEndUserSigningKey|secret-access-log|secret.*access|SitenovaGetConnection' backend/src/microservices/sitenova-microservice backend/src/common backend/src/authorization backend/src/entities -g '*.ts' | head -n 240
echo
echo "== SaaSAuthMiddleware focused symbols/usages =="
rg -n -C4 'SaaSAuthMiddleware|jwt|connection|companyId|tenant|internal|microservice|connectionId|Authorization|bearer|secret-access-log|secretAccess' backend/src -g '*.ts' | head -n 300Repository: rocket-admin/rocketadmin
Length of output: 50380
IDOR (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: External
Reachability path
● Entry
backend/src/microservices/sitenova-microservice/sitenova.module.ts:106
SitenovaInternalController
│
▼
● Sink
backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts
Add connection-level authorization to the SitNova connection-credentials endpoint.
SaaSAuthMiddleware only checks the microservice JWT and accepts request_id; it does not scope the request to a connection or tenant. getConnection() passes the caller-controlled connectionId directly to findAndDecryptConnection(), then returns decrypted credentials and the HS256 signing key. Require an explicit connections[*]/service authorization claim or a matching connection guard before returning connection secrets.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/microservices/sitenova-microservice/sitenova-internal.controller.ts`
around lines 49 - 53, The getConnection endpoint currently trusts the
caller-controlled connectionId and returns decrypted credentials without
connection-level authorization. Update getConnection and its authorization flow
to require a valid connections[*] or service authorization claim, or an
equivalent guard proving access to that specific connection, before invoking
getConnectionUseCase.execute and returning secrets; reject unauthorized or
mismatched connection requests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit