feat(#3734): RBAC-gate the Usage tab behind permission check - #3746
feat(#3734): RBAC-gate the Usage tab behind permission check#3746fullsend-ai-coder[bot] wants to merge 4 commits into
Conversation
Add RBAC permission gating to the UsageTab entity content extension in the boost plugin. Users without the ai-catalog.asset.read.usage-docs permission now see a "contact owner" fallback instead of documentation content, satisfying RHDHPLAN-1509 requirements. Changes: - Define boostAiCatalogUsageDocsPermission (ai-catalog.asset.read.usage-docs) in boost-common/src/permissions.ts and add it to boostFunctionalPermissions - Add @backstage/plugin-permission-common, @backstage/plugin-permission-react, and boost-common as dependencies of the boost frontend plugin - Gate UsageTab rendering with usePermission hook: loading shows skeleton, denied shows permission message with optional "Contact owner" link derived from spec.owner, allowed renders existing docs content - Add usagePermissionDenied and usageContactOwner i18n keys - Remove the TODO(RHDHPLAN-1508) comment from plugin.tsx - Add UsageTab.test.tsx with 6 test cases covering loading, allowed (TechDocs, external links, no docs), and denied (with/without owner) When the Backstage permission framework is not configured (dev mode), usePermission defaults to allowed: true, preserving current behavior. Closes #3734
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
|
|
/fs-fix add changeset and fix the CI |
|
🤖 Finished Fix · ✅ Success · Started 9:12 AM UTC · Completed 9:24 AM UTC |
Add changeset for boost-common (minor) and boost (patch) covering the RBAC permission gating of the Usage tab. Regenerate API reports to include the new boostAiCatalogUsageDocsPermission export and updated permission tuple types. Addresses review feedback on #3746
🔧 Fix agent — iteration 1 (human-triggered)Added changeset and regenerated API reports to fix the CI 'check api reports' failure. The changeset covers both affected packages: boost-common (minor, new permission export) and boost (patch, UI changes). API reports now reflect the new boostAiCatalogUsageDocsPermission and updated permission aggregation tuples. Fixed (2):
Tests: passed Updated by fullsend fix agent |
|
/fs-fix fix CI |
|
🤖 Finished Fix · ✅ Success · Started 10:12 AM UTC · Completed 10:27 AM UTC |
The report.api.md had stale ordering for union members and translation keys. Regenerated with `yarn build:api-reports:only` so the CI `--ci` validation passes. Addresses review feedback on #3746
🔧 Fix agent — iteration 2 (human-triggered)Fixed CI failure by regenerating the boost plugin's report.api.md to match api-extractor output. The file had ordering discrepancies in union members and translation keys. Fixed (1):
Tests: passed Updated by fullsend fix agent |
|
Similar CI issues are in #3735, fixing them there first |
|
@rohitkrai03 FYI |
|
@mareklibra - I also want to cross reference this change with the openspec's I'm curating for RHDHPLAN-1508 before we merge this |
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
So this generally aligns/overlaps/compliments the RHDHPLAN-1508 graduated-visibility capability, but has one conflict needs attention.
Overlaps/Aligns/Compliments:
The PR touches these areas we spec'd in https://github.com/gabemontero/rhdh-plugins/tree/rhdhplan-1508-openspecs :
- Task 1.3 — Define ai-catalog.asset.read.usage-docs: PR defines this (tweak needed in conflict section
- Task 3.1 — RequirePermission gating on Tier 2 sections: PR Gates UsageTab via usePermission , no changes needed
- Task 3.2 — Restricted-access placeholder: PR employs "Contact owner" fallback with owner link, no changes needed
Conflict:
BasicPermission vs ResourcePermission
The PR defines the permission as a BasicPermission (no resourceType):
export const boostAiCatalogUsageDocsPermission = createPermission({
name: 'ai-catalog.asset.read.usage-docs',
attributes: { action: 'read' },
});
Our design (Decision 1) explicitly specifies it as a resource-based permission with resourceType: 'ai-catalog-asset':
export const boostAiCatalogUsageDocsPermission = createPermission({
name: 'ai-catalog.asset.read.usage-docs',
attributes: { action: 'read' },
resourceType: AI_CATALOG_ASSET_RESOURCE_TYPE,
});
This matters because:
- A BasicPermission only returns ALLOW/DENY — it can never return CONDITIONAL
- Without CONDITIONAL, deployers cannot scope usage-docs visibility by category, connector, or tenant (the whole point of our isAiAssetCategory, isFromConnector, isInTenant rules)
- If this merges as BasicPermission, we'd have to immediately change it to resource-based in RHDHPLAN-1508, which is a breaking change for any RBAC policies configured in the interim
Complementary (good work we'd keep)
- The frontend UX pattern — loading skeleton, denied fallback with contact-owner link derived from spec.owner — is solid and aligns with our restricted-access placeholder requirement
- 6 test cases with proper usePermission mocking — reusable pattern for other Tier 2 sections
- i18n keys (usagePermissionDenied, usageContactOwner) — exactly what we'd need
- Removing the TODO(RHDHPLAN-1508) comment — housekeeping we'd want
Not covered by PR (our spec still needed)
- ai-catalog.asset.read (Tier 1 discovery) — not in PR
- ai-catalog.admin — not in PR
- Backend field-level filtering — not in PR
- Backend authorizeConditional() + toQuery() — not in PR
- Conditional permission rules — not in PR
- Everything under epics RHIDP-15274, 15277, 15304 — not in PR
Jira note
The issue references RHIDP-15440 and RHDHPLAN-1509, not RHDHPLAN-1508. But the TODO it removes says RHDHPLAN-1508. The ai-catalog.asset.read.usage-docs permission is clearly ours (RHIDP-15272 under RHDHPLAN-1508). Either RHDHPLAN-1509 has overlapping scope with our plan, or this is simply the frontend vs. backend parts of an end to end flow. WDYT @rohitkrai03 ?
Recommendation
The PR's frontend work is good and complementary — but the permission definition needs to be ResourcePermission<'ai-catalog-asset'> before merging. I'd suggest posting a review comment flagging the BasicPermission→ResourcePermission issue and pointing to our design decision, so it can be fixed in this PR rather than creating immediate tech debt.
| * | ||
| * @public | ||
| */ | ||
| export const boostAiCatalogUsageDocsPermission = createPermission({ |
There was a problem hiding this comment.
This needs to be a ResourcePermission with resourceType: 'ai-catalog-asset', not a BasicPermission.
I'm backed up on reviews, so it is not merged yet (see https://github.com/gabemontero/rhdh-plugins/tree/rhdhplan-1508-openspecs), but the RHDHPLAN-1508 design (openspec/changes/ai-catalog-asset-governance/design.md, Decision 1) explicitly makes both ai-catalog.asset.read and ai-catalog.asset.read.usage-docs resource-based so they support CONDITIONAL evaluation. Without a resourceType, this permission can only return ALLOW/DENY — deployers won't be able to scope usage-docs visibility by category, connector, or tenant via RBAC conditional policies (isAiAssetCategory, isFromConnector, isInTenant).
We just landed the same pattern for boost.agent.list in b52a789b0a — upgrading it from basic to resource-based for 3-tier evaluation support. This permission should follow the same approach:
▎ export const boostAiCatalogUsageDocsPermission = createPermission({
▎ name: 'ai-catalog.asset.read.usage-docs',
▎ attributes: { action: 'read' },
▎ resourceType: AI_CATALOG_ASSET_RESOURCE_TYPE,
▎ });
▎
The frontend usePermission call in UsageTab.tsx works identically with either type — this is purely a backend/policy concern — so the fix is just adding the resourceType here and defining the AI_CATALOG_ASSET_RESOURCE_TYPE constant.
Everything else in this PR (the UsageTab UX, tests, i18n, contact-owner fallback) looks great and aligns with our graduated-visibility spec.
There was a problem hiding this comment.
now have a PR up for the RHDHPLAN-1508 openspecs: #3753
Address PR review feedback: make boostAiCatalogUsageDocsPermission a ResourcePermission (was BasicPermission), pass resourceRef to usePermission, fix stale permission-count tests, and parse owner refs instead of assuming a bare group name.
|
|
Fixed the BasicPermission vs ResourcePermission issue. The CI is failing, a fix is part of #3735 ( |
|
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
2 participants
Add RBAC permission gating to the UsageTab entity content extension in the boost plugin. Users without the ai-catalog.asset.read.usage-docs permission now see a "contact owner" fallback instead of documentation content, satisfying RHDHPLAN-1509 requirements.
Changes:
in boost-common/src/permissions.ts and add it to boostFunctionalPermissions
and boost-common as dependencies of the boost frontend plugin
denied shows permission message with optional "Contact owner" link
derived from spec.owner, allowed renders existing docs content
(TechDocs, external links, no docs), and denied (with/without owner)
When the Backstage permission framework is not configured (dev mode), usePermission defaults to allowed: true, preserving current behavior.
Closes #3734
Post-script verification
agent/3734-rbac-gate-usage-tab)f57257ed4644d864e5ab5c0f1056782a8f5f64e6..HEAD)