feat(authorization): add workspace target scopes and resource preview#5
Merged
Conversation
- README: reflect current repo structure with backend, Biome, openspec - SECURITY.md: professional security policy with disclosure process, CVSS-based response timeline, scope definitions, and attribution
- Frontend (npm): group Astro/Tailwind, auto-merge patches - Backend (Gradle): group Spring/Kotlin/Testing, auto-merge patches - GitHub Actions: weekly schedule with auto-merge - Bazel: manual review for high-risk updates - Security updates enabled with fix prefix
…arget scope resolution
Contributor
|
Hi @yacosta738! 👋 Before we can merge this pull request, the CLA must be signed. Please leave this exact comment on this PR to complete signing: Your signature is recorded and linked to your GitHub account. You only need to do this once. This check is required for all external contributors. Org members bypass this step automatically. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a target-aware workspace scope model, persists per-workspace allowed target IDs, refactors authorization to apply scope-reduction after resolving base permission, exposes GET /api/authorization/resources/{resourceId}/preview, wires an R2DBC scope resolver, updates specs/docs, and adds unit and H2/Postgres integration tests validating allow and deny scenarios. ChangesDocumentation, Specifications, and Configuration
Target-Aware Authorization Scope Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying profiletailors with
|
| Latest commit: |
6be1db2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5c2229da.profiletailors-com.pages.dev |
| Branch Preview URL: | https://feature-backend-scopes-execu.profiletailors-com.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointPostgresIntegrationTest.kt (1)
52-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Detekt
LargeClassfailure on this integration test class.CI currently fails on
WorkspaceAccessSummaryEndpointPostgresIntegrationTest(LargeClass). Add a class-level suppression (matching the H2 counterpart) or split the suite before merge.Minimal unblocker
`@Testcontainers`(disabledWithoutDocker = true) `@TestInstance`(TestInstance.Lifecycle.PER_CLASS) +@Suppress("LargeClass") class WorkspaceAccessSummaryEndpointPostgresIntegrationTest(🤖 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 `@server/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointPostgresIntegrationTest.kt` at line 52, The class WorkspaceAccessSummaryEndpointPostgresIntegrationTest is tripping Detekt's LargeClass rule; to fix, add a class-level suppression annotation matching the H2 counterpart (e.g., annotate WorkspaceAccessSummaryEndpointPostgresIntegrationTest with `@Suppress`("LargeClass")) or alternatively refactor/split the test methods into smaller test classes (preserving test setup/fixtures) so the class no longer violates LargeClass; update imports if needed and ensure the suppression is applied at the class declaration for WorkspaceAccessSummaryEndpointPostgresIntegrationTest.
🤖 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 @.github/dependabot.yml:
- Around line 47-48: Remove the unsupported keys "automerge" and
"automerge-type" from .github/dependabot.yml (they appear at the locations
listed) because Dependabot's config schema ignores them; delete those key/value
lines and instead add a GitHub Actions workflow (triggering on pull_request from
Dependabot) that verifies CI and uses the GitHub CLI to run "gh pr merge --auto"
to perform auto-merge.
- Around line 157-162: Remove the invalid top-level "security-updates" block and
move any needed configuration into the valid "updates" structure: delete the
top-level security-updates key and its children (enabled, commit-message) and
instead add equivalent configuration under the "updates" -> "groups" section
using "applies-to: security-updates" and group-level "commit-message" settings;
reference the "security-updates" identifier to remove it and the "updates",
"groups", "applies-to", and "commit-message" keys to place the configuration
correctly.
In `@openspec/specs/authorization/spec.md`:
- Around line 241-249: The spec is self-contradictory: it adds a "Target-Aware
Resource Preview Proving Capability" (resource-preview-by-resourceId, base
permission 'workspace:resource:read') that evaluates a targetResourceId while
elsewhere forbidding new protected endpoints for
/api/authorization/workspace-access/current; reconcile by either (A) explicitly
permitting exactly one new protected endpoint for the new capability (describe
its path and request shape and state that it is the only new protected endpoint
allowed), or (B) remove/modify the new capability text so authorization is
evaluated via the existing /api/authorization/workspace-access/current flow
(remove resource-preview-by-resourceId wording and targetResourceId
requirement). Update the spec sections that reference
/api/authorization/workspace-access/current to reflect the chosen approach so
the document consistently allows the target-aware check and remains testable.
In `@openspec/specs/platform/spec.md`:
- Around line 106-110: The spec is ambiguous about where the new target-aware
proving capability lives: reconcile the conflict between requiring a
target-aware proving capability that uses explicit targetResourceId in WORKSPACE
context (backend-scopes-execution / targetResourceId / WORKSPACE / GLOBAL / USER
/ SYSTEM) and the later claim that the proving slice must remain only
/api/authorization/workspace-access/current; update the spec so it either (A)
explicitly states the existing endpoint
/api/authorization/workspace-access/current is extended to accept an explicit
targetResourceId parameter and perform target-aware evaluation in WORKSPACE
context, or (B) introduces a clearly named new endpoint (e.g.,
/api/authorization/workspace-access/target-aware) for the target-aware proving
capability; ensure the chosen approach is reflected wherever the proving slice
is described and that references to WORKSPACE, GLOBAL, USER, SYSTEM contexts and
the targetResourceId input are consistent.
In `@SECURITY.md`:
- Line 78: The file ends without a final newline; append a single trailing
newline character to the end of SECURITY.md so the last line "For general
security inquiries: **security@profiletailors.com**" is terminated by a newline
(satisfy MD047).
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/authorization/domain/AuthorizationModels.kt`:
- Around line 21-26: Replace the fully qualified type in the AuthorizationScope
data class with a simple import: add an import for ResourceContextType
(com.profiletailors.smp.platform.domain.ResourceContextType) and change the
parameter type in AuthorizationScope (val resourceContextType) from the FQN to
ResourceContextType to match the existing ResourceContext import and improve
consistency.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/authorization/infrastructure/R2dbcWorkspaceTargetScopeResolver.kt`:
- Around line 60-63: The JSON deserialization in decodeAllowedTargetIds can
throw JsonProcessingException with no context; update it to catch
JsonProcessingException (or JsonParseException) and rethrow a new exception that
includes workspace and principal identifiers so debugging is easier—e.g., change
decodeAllowedTargetIds to accept workspaceId and principalId (or pass them from
the caller in resolveTargetScope/resolve methods), wrap
objectMapper.readValue(...) in a try/catch, and on failure throw an
IllegalStateException (or custom exception) with a message containing
workspaceId, principalId and the rawJson (or a truncated preview) and include
the original exception as the cause to preserve the stacktrace.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/GetResourcePreviewHandlerTest.kt`:
- Around line 23-33: Tests repeat literal IDs and paths (e.g., "principal-1",
"workspace-1", "resource-1", "subject-123"); extract these into private const
val top-level constants and replace repeated occurrences in the test file
(GetResourcePreviewHandlerTest) to satisfy the style gate. Add constants with
clear names like PRINCIPAL_ID, PRINCIPAL_SUBJECT, WORKSPACE_ID, RESOURCE_ID (and
any repeated path strings used across lines 23–152) and update usages in
PrincipalContext, ResourceContext, test setup (CapturingAuditHook/handler
instantiation) and all assertions to reference those constants.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/WorkspaceAuthorizationServiceTest.kt`:
- Around line 38-43: Introduce a test constant for the repeated resource id and
replace all literal occurrences ("resource-1") with that constant; e.g., add a
private val RESOURCE_ID = "resource-1" (or similarly named) at the top of the
test class and update ResourceContext construction in targetAwareResourceContext
and any assertions in WorkspaceAuthorizationServiceTest to use RESOURCE_ID
instead of the literal, ensuring you update all other occurrences mentioned
(lines around 390 and 472) so the tests no longer duplicate the string literal.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointIntegrationTest.kt`:
- Around line 81-203: Extract repeated literals in
ResourcePreviewEndpointIntegrationTest into private constants (e.g.,
RESOURCE_PREVIEW_PATH = "/api/authorization/resources/%s/preview",
AUTH_HEADER_VALUE = "Bearer valid-token", WORKSPACE_HEADER = "X-Workspace-Id",
WORKSPACE_ID = "workspace-1", PERMISSION_RESOURCE_READ =
"workspace:resource:read", REQUEST_NAME_GET_PREVIEW =
"com.profiletailors.smp.authorization.application.GetResourcePreviewQuery",
PRINCIPAL_ID = "principal-1", ROLE_MEMBER = "member") and replace all inline
usages across the test methods (including calls to webTestClient.get().uri(...),
header(...), expectBody/jsonPath checks, and assertAuthorizationFacts
AuthorizationDecisionAuditFact constructions) with these constants so the path,
headers, token, permission, requestName, principalId, workspaceId and roleKeys
are referenced centrally; keep format specifier for resource id in
RESOURCE_PREVIEW_PATH and use String.format or Kotlin string templates when
building the uri, and update the other occurrences in the file range noted (and
lines 225-281) accordingly.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointPostgresIntegrationTest.kt`:
- Around line 81-203: The tests duplicate literals (request path
"/api/authorization/resources/resource-1/preview", auth header "Bearer
valid-token", header name "X-Workspace-Id", workspace/principal IDs
"workspace-1"/"principal-1", and the audit requestName
"com.profiletailors.smp.authorization.application.GetResourcePreviewQuery")
across multiple test functions (e.g., allows resource preview..., denies
resource preview..., denies resource preview when base permission is missing...,
scope resolver remains narrow...), causing static-analysis failures; fix by
introducing private constants at the top of the test class (e.g., PREVIEW_URI,
AUTH_TOKEN, WORKSPACE_ID, PRINCIPAL_ID, AUDIT_REQUEST_NAME) and replace all
repeated literal occurrences in the test methods and in the assertions
(including the duplicated block around lines 225-281) to reference these
constants so the suite matches the H2 refactor pattern.
---
Outside diff comments:
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointPostgresIntegrationTest.kt`:
- Line 52: The class WorkspaceAccessSummaryEndpointPostgresIntegrationTest is
tripping Detekt's LargeClass rule; to fix, add a class-level suppression
annotation matching the H2 counterpart (e.g., annotate
WorkspaceAccessSummaryEndpointPostgresIntegrationTest with
`@Suppress`("LargeClass")) or alternatively refactor/split the test methods into
smaller test classes (preserving test setup/fixtures) so the class no longer
violates LargeClass; update imports if needed and ensure the suppression is
applied at the class declaration for
WorkspaceAccessSummaryEndpointPostgresIntegrationTest.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4ee64495-753a-4b3e-a484-1b93e437b45a
📒 Files selected for processing (24)
.github/dependabot.ymlREADME.mdSECURITY.mdopenspec/specs/authorization/spec.mdopenspec/specs/governance/spec.mdopenspec/specs/platform/spec.mdserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/application/GetCurrentWorkspaceAccessSummaryQuery.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/application/GetResourcePreviewQuery.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/application/WorkspaceAuthorizationService.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/domain/AuthorizationModels.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/infrastructure/AuthorizationBootstrapConfiguration.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/infrastructure/R2dbcWorkspaceTargetScopeResolver.ktserver/smp/src/main/kotlin/com/profiletailors/smp/authorization/infrastructure/http/ResourcePreviewController.ktserver/smp/src/main/kotlin/com/profiletailors/smp/platform/application/PlatformContracts.ktserver/smp/src/main/resources/db/changelog/authorization/007-create-workspace-target-scopes.yamlserver/smp/src/main/resources/db/changelog/db.changelog-master.yamlserver/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/GetCurrentWorkspaceAccessSummaryHandlerTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/GetResourcePreviewHandlerTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/WorkspaceAuthorizationServiceTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/infrastructure/db/LiquibaseBaselineChangelogTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointPostgresIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointPostgresIntegrationTest.kt
… Bazel dependency review
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/specs/platform/spec.md (1)
217-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify which proving slice line 221 refers to.
Line 221 states "The proving slice MUST remain limited to
/api/authorization/workspace-access/current" but the spec now defines two distinct proving slices:
- The API-key replacement proving slice at
/api/authorization/workspace-access/current(lines 217-223)- The new target-aware scope execution proving slice at
/api/authorization/resources/{resourceId}/preview(lines 106-112)The use of "THE proving slice" (singular) in line 221 creates ambiguity about which slice is being constrained.
📝 Proposed clarification
-The proving slice MUST remain limited to `/api/authorization/workspace-access/current`. +The API-key replacement proving slice MUST remain limited to `/api/authorization/workspace-access/current`.Or alternatively, restructure to make the scope clearer:
For the supported API-key replacement capability, the platform MUST apply one explicit runtime rule: after the replacement operation completes, the successor API key MUST be accepted and the predecessor API key MUST be denied. The completed replacement rule MUST NOT allow any overlap window where both predecessor and successor are accepted on `/api/authorization/workspace-access/current`. -The proving slice MUST remain limited to `/api/authorization/workspace-access/current`. -The proving slice MUST NOT broaden into new endpoints, service-account rotation, dual-active rollover windows, inventory or detail APIs, or generalized credential-family management. -The proving slice MUST NOT broaden into broad issuance/admin platform behavior beyond what is minimally necessary to execute one API-key replacement path. +The API-key replacement proving capability MUST remain limited to `/api/authorization/workspace-access/current`. +The API-key replacement proving capability MUST NOT broaden into new endpoints, service-account rotation, dual-active rollover windows, inventory or detail APIs, or generalized credential-family management. +The API-key replacement proving capability MUST NOT broaden into broad issuance/admin platform behavior beyond what is minimally necessary to execute one API-key replacement path.🤖 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 `@openspec/specs/platform/spec.md` around lines 217 - 223, Change the ambiguous phrase "THE proving slice" to explicitly name the API-key replacement proving slice and its endpoint: replace "THE proving slice MUST remain limited to `/api/authorization/workspace-access/current`" with wording such as "The API-key replacement proving slice for `/api/authorization/workspace-access/current` MUST remain limited to that endpoint" so it cannot be confused with the target-aware scope execution proving slice at `/api/authorization/resources/{resourceId}/preview`; ensure any subsequent references to "proving slice" likewise specify "API-key replacement proving slice" when referring to this behavior.
🤖 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 @.github/dependabot.yml:
- Around line 9-13: Each Dependabot update block is missing the required
schedule.interval field; for every schedule block (the YAML key "schedule") add
interval: "weekly" (or your chosen frequency) under schedule so the snippet
becomes schedule: day: "monday" time: "09:00" timezone: "UTC" interval: "weekly"
— apply this change to all four update blocks referenced (also where similar
schedule blocks appear around the other sections).
In `@openspec/changes/archive/2026-05-18-backend-scopes-execution/design.md`:
- Line 141: Update the path string in the change table row so it points to the
archived location: replace the current value
`openspec/changes/backend-scopes-execution/design.md` with the archived path
that includes the `archive/2026-05-18-backend-scopes-execution/` segment (so the
table entry references `archive/2026-05-18-backend-scopes-execution/design.md`),
ensuring the table row in this file (`|
`openspec/changes/backend-scopes-execution/design.md` | Create | ... |`) matches
the actual archived file location for correct traceability.
In `@openspec/specs/platform/spec.md`:
- Around line 106-121: Add a short clarifying sentence immediately after the new
endpoint declaration so readers understand the two proving slices are distinct:
explicitly state that GET /api/authorization/resources/{resourceId}/preview is a
separate, target-aware proving capability evaluated in WORKSPACE context using
the explicit targetResourceId and does not overlap with or extend the proving
slice represented by /api/authorization/workspace-access/current (and therefore
does not affect the API-key replacement proving slice discussed later).
---
Outside diff comments:
In `@openspec/specs/platform/spec.md`:
- Around line 217-223: Change the ambiguous phrase "THE proving slice" to
explicitly name the API-key replacement proving slice and its endpoint: replace
"THE proving slice MUST remain limited to
`/api/authorization/workspace-access/current`" with wording such as "The API-key
replacement proving slice for `/api/authorization/workspace-access/current` MUST
remain limited to that endpoint" so it cannot be confused with the target-aware
scope execution proving slice at
`/api/authorization/resources/{resourceId}/preview`; ensure any subsequent
references to "proving slice" likewise specify "API-key replacement proving
slice" when referring to this behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 23a1f70d-4899-4560-a73e-67d3db6740fe
📒 Files selected for processing (14)
.github/dependabot.ymlopenspec/changes/archive/2026-05-18-backend-scopes-execution/design.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/exploration.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/proposal.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/specs/authorization/spec.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/specs/governance/spec.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/specs/platform/spec.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/state.yamlopenspec/changes/archive/2026-05-18-backend-scopes-execution/tasks.mdopenspec/changes/archive/2026-05-18-backend-scopes-execution/verify-report.mdopenspec/specs/authorization/spec.mdopenspec/specs/platform/spec.mdserver/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointPostgresIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/WorkspaceAccessSummaryEndpointPostgresIntegrationTest.kt
… for major dependencies
…builder pattern and improve readability
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@openspec/changes/archive/2026-05-18-backend-scopes-execution/design.md`:
- Line 373: Add blank lines immediately before and after each subsection heading
that uses four hashes (e.g., "#### Allow: base permission + matching scope" and
the other similar "#### ..." headings flagged in the comment) so there is an
empty line separating headings from surrounding paragraphs (fixes MD022); then
ensure the file ends with exactly one trailing newline character (no extra blank
lines or missing EOF newline) to satisfy MD047.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/integration/support/ResourcePreviewEndpointTestBase.kt`:
- Around line 332-334: The test JWT in ResourcePreviewEndpointTestBase uses
hard-coded .issuedAt(Instant.parse(...)) and .expiresAt(Instant.parse(...)) that
are already expired; update the JWT construction to use dynamic, non-expired
timestamps (e.g., set .issuedAt(Instant.now()) and
.expiresAt(Instant.now().plus(Duration.ofMinutes(...))) or derive from a test
Clock) so the token remains valid during test execution and avoids brittle auth
failures.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 676c822e-5193-42b1-b0c6-0262487cc737
📒 Files selected for processing (8)
.github/dependabot.ymlopenspec/changes/archive/2026-05-18-backend-scopes-execution/design.mdopenspec/specs/platform/spec.mdserver/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/GetResourcePreviewHandlerTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/authorization/application/WorkspaceAuthorizationServiceTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/ResourcePreviewEndpointPostgresIntegrationTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/integration/support/ResourcePreviewEndpointTestBase.kt
…x formatting in test base
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@openspec/changes/archive/2026-05-18-backend-scopes-execution/design.md`:
- Around line 324-327: The document leaves undefined the behavior when there is
"no persisted scope row" for a capability; pick and state one deterministic rule
(either "absence = unrestricted within proving capability" or "absence =
deny/require scope presence for scoped principals") and update all places that
reference this behavior (notably the prose around "no persisted scope row" and
the "proving matrix"/"proving capability" sections) to reflect that single rule;
ensure the proving matrix entries and any examples mention the chosen semantics
explicitly and update wording for "scoped principals" and the proving matrix
rows so every decision path deterministically references the chosen rule.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/integration/support/ResourcePreviewEndpointTestBase.kt`:
- Around line 86-212: Add a test that verifies when the base permission exists
but there is no workspace_target_scopes row the preview is allowed: create a new
test e.g. `allows resource preview when base permission exists and no scope row`
that calls seedMemberWithPreviewPermission() but does NOT call
seedTargetScope(...), then issue the same GET to RESOURCE_PREVIEW_PATH with
BEARER_TOKEN and WORKSPACE_HEADER/WORKSPACE_ID and assert status isOk and
previewAllowed true; finally assert the authorization audit fact (using
GET_RESOURCE_PREVIEW_QUERY, PERMISSION_RESOURCE_READ, PRINCIPAL_ID,
WORKSPACE_ID) records decision ALLOW with reasonCode
AuthorizationReasonCode.ROLE_PERMISSION and roleKeys listOf("member").
🪄 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: ASSERTIVE
Plan: Pro
Run ID: cb72d03b-0151-476b-800c-b0d5e15ad769
📒 Files selected for processing (2)
openspec/changes/archive/2026-05-18-backend-scopes-execution/design.mdserver/smp/src/test/kotlin/com/profiletailors/smp/integration/support/ResourcePreviewEndpointTestBase.kt
…ase permission and no scope rows
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
…up and enhancing cleanup logic
|
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
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
2 participants
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
Add workspace target scopes execution and resource preview functionality to the authorization system.
Changes
Commits
specs: update authorization, governance, and platform specsfeat(authorization): add workspace target scopes and resource preview endpointsrefactor(authorization): enhance WorkspaceAuthorizationService with target scope resolutiontest(authorization): add integration tests for resource preview endpointstest(authorization): update existing authorization tests for new scope functionalityFiles Changed