feat(#3734): RBAC-gate the Usage tab behind permission check - #3746
Conversation
Changed Packages
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3746 +/- ##
==========================================
+ Coverage 61.49% 61.51% +0.01%
==========================================
Files 2534 2534
Lines 101630 101649 +19
Branches 28460 28476 +16
==========================================
+ Hits 62496 62526 +30
+ Misses 38564 38553 -11
Partials 570 570
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
/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.
|
|
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! |
|
This pull request adds a new top-level directory under |
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
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
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.
yarn build:api-reports:only --ci was failing because report.api.md had drifted from the actual public API (translation-key and extension-output union ordering), the same stale-report class of CI failure fixed on PR #3747. Regenerate via yarn build:api-reports:only. Co-authored-by: Cursor <cursoragent@cursor.com>
37086f8 to
d920cc1
Compare
|
Rebased |
|
After the rebase onto #4185, this PR no longer defines the permission — it only gates UsageTab with the existing This is the RHDHPLAN-1509 / RHIDP-15440 Usage-tab slice of Issue 24 (#4062 / RHIDP-15273), not issue 4 (#4042). After merge, #4062 should treat UsageTab gating as done. |
Keep a bad spec.owner from crashing the denied fallback, drop the unused permission-common dependency, and tighten tests around the permission object and loading skeleton. Signed-off-by: Marek Libra <marek.libra@gmail.com>
|
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
clearing my requested change bit @mareklibra
found nothings else with my revisit of this, at this point I'm fine with you curating with fullsend review and merging when you think it is good to go
|
🤖 Finished Retro · ✅ Success · Started 1:58 PM UTC · Completed 2:12 PM UTC Commit: |
Retro: PR #3746 — RBAC-gate the Usage tab behind permission checkTimeline
AssessmentCode agent performed correctly given its inputs. The original issue explicitly instructed creating a Code quality was high. The agent produced 8 well-structured test cases with 100% coverage on new code. The permission-denied UX pattern (loading skeleton, denied fallback with optional contact-owner link, graceful handling of invalid owner refs) was praised by the reviewer. Two fix iterations were avoidable. The code agent omitted a changeset and didn't regenerate API reports. Both are CI requirements that the agent should have handled in its initial commit. Existing issues with new evidence
Proposals filed
|
2 participants
RBAC-gate the Usage tab using the permission already defined on main by #4185 (
aiCatalogAssetAccessUsageDocsPermission/ai-catalog.asset.access.usage-docs, resource-scoped). Users without that permission see a “contact owner” fallback instead of TechDocs or external links.This does not define a new permission, does not add
boost-common(already a frontend dependency), and is not aBasicPermissionnamedai-catalog.asset.read.usage-docs.Scope: RHDHPLAN-1509 / RHIDP-15440 — Usage-tab slice of Issue 24 (#4062 / RHIDP-15273). Independent of issue 4 (#4042) and of #4314.
After merge, #4062 should treat UsageTab gating as done and cover the remaining graduated-visibility frontend (other Tier 2 sections, list counts, admin-link visibility).
Changes
UsageTabwithusePermission+resourceRef: stringifyEntityRef(entity): loading skeleton, allowed = existing docs, denied = permission message and optional owner link fromspec.ownerparseEntityRef(bare name,group:,user:); invalidspec.owneromits the link instead of throwing@backstage/plugin-permission-reactto the boost frontend pluginusagePermissionDeniedandusageContactOwnerplugin.tsxUsageTab.test.tsxcovering loading, allowed (TechDocs / links / no docs), denied (with/without/invalid owner), and the exact permission + resourceRefWhen the Backstage permission framework is not configured (dev mode),
usePermissiondefaults toallowed: true.Closes #3734