feat(homepage): add permission-aware default cards backend#2855
feat(homepage): add permission-aware default cards backend#2855karthikjeeyar merged 19 commits intoredhat-developer:mainfrom
Conversation
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
christoph-jerolimov
force-pushed
the
homepage/rbac
branch
from
2f57d38 to
6f0ef5a
Compare
christoph-jerolimov
force-pushed
the
homepage/rbac
branch
from
0629c32 to
4d4dca1
Compare
karthikjeeyar
left a comment
karthikjeeyar
left a comment
There was a problem hiding this comment.
This is a strong step forward 🎉 This feature turns "everyone sees the same static tiles" to "you see what you are meant to see", I have some small comments, but nothing takes away the overall approach.
| const requests = names.map(name => ({ | ||
| permission: createPermission({ name, attributes: {} }), |
There was a problem hiding this comment.
In homepage-common, homepageDefaultWidgetsReadPermission is defined as a resource permission. This means it expects a resourceRef (like a widget ID) to evaluate conditional policies correctly.
Currently, the homepage backend still checks this via createPermission with empty attributes, which is just a basic global name-level check. This won't trigger any logic that relies on specific card IDs.
Should we be adding a second permissions.authorize call and pass the resourceRef for each card to support these conditional policies? Or is if.permissions meant to stay high-level? I
It’s worth clarifying the intended model in the documentation
christoph-jerolimov
force-pushed
the
homepage/rbac
branch
from
8ade6b2 to
d20e76e
Compare
christoph-jerolimov
requested review from
a team and
Eswaraiahsapram
as code owners
Code Review by Qodo
1.
|
Review Summary by QodoAdd permission-aware default cards backend with RBAC integration
WalkthroughsDescription• Add permission-aware default cards backend with RBAC support • Implement visibility filtering based on users, groups, and permissions • Create shared types package for frontend-backend communication • Register homepage resource type with HAS_CARD_ID permission rule • Add frontend API client and integrate with both legacy and NFS systems Diagramflowchart LR
Config["app-config.yaml<br/>defaultWidgets tree"]
Backend["Backend Plugin<br/>DefaultWidgetsService"]
Visibility["Visibility Evaluation<br/>users/groups/permissions"]
API["HTTP Endpoint<br/>/default-widgets"]
Frontend["Frontend API Client<br/>DefaultWidgetsApiClient"]
UI["UI Components<br/>Debug pages"]
Config -->|loadDefaultWidgets| Backend
Backend -->|buildUserContext| Visibility
Visibility -->|filterToVisibleLeaves| API
API -->|getDefaultWidgets| Frontend
Frontend -->|display| UI
RBAC["RBAC Plugin<br/>HAS_CARD_ID rule"]
RBAC -.->|permission decisions| Visibility
File Changes1. workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts
|
rhdh-qodo-merge
Bot
added
documentation
christoph-jerolimov
force-pushed
the
homepage/rbac
branch
from
6a9cd2b to
bf4c1e9
Compare
karthikjeeyar
left a comment
karthikjeeyar
left a comment
There was a problem hiding this comment.
Changes looks good to me.
- Conditional RBAC policy isn't handled. can we have followup ticket for this?
- Good to have a small blurb in plugin documentation to capture the new implementation
homepage.defaultWidgets - some test cases are failing.
| xs: { w: 12, h: 9 } | ||
| xxs: { w: 12, h: 14 } | ||
| - id: onboarding | ||
| ref: Onboarding |
There was a problem hiding this comment.
This should match with the mountpoint ID 'rhdh-onboarding-section`
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
…end client Add a default-cards feature that reads a recursive card tree from app-config.yaml, evaluates per-node visibility rules (users, groups, permissions — OR semantics), and returns the filtered leaf cards to the frontend. Adds a homepage-common package for shared types, a DefaultCardsApiClient in the frontend plugin and registers it in both the legacy and NFS extension systems. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Add i18n support fields (titleKey, descriptionKey) to CardNode and VisibleCard, and rename the visibility field to if to align with Backstage API conventions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Add a dedicated homepage-default-card resource type with a HAS_CARD_ID rule so that RBAC policies can apply conditions to individual cards. Register via addResourceType instead of addPermissions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
…y card config - Rename defaultCards → defaultWidgets across config, types, API, backend service, routes, permissions, and tests - Remove label, title, titleKey, description, descriptionKey, priority from card/widget node config; replace with generic props attribute - Add debug pages for viewing available and default widgets - Switch from js-yaml to yaml package - Export defaultWidgetsApiRef from the dynamic-home-page plugin Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
…tWidget Add `ref` as optional string on DefaultWidgetNode and required string on VisibleDefaultWidget. Rename CardVisibility to DefaultWidgetVisibility, CardNode to DefaultWidgetNode, and VisibleCard to VisibleDefaultWidget to align naming with the defaultWidgets domain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
…yout Drop the `customizable` config option and its plumbing through the backend service, types, and tests. Also add `ref` field and rename `layouts` to `layout` in the app-config for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Add API-driven grid components that fetch default widget configuration from the backend when configured in app-config, falling back to mount-point-only behavior when no default widgets are configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Rename homepageDefaultCardPermissionResourceRef to homepageDefaultWidgetPermissionResourceRef and hasCardId rule to hasWidgetId for consistency with the widget terminology. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
christoph-jerolimov
force-pushed
the
homepage/rbac
branch
from
bf4c1e9 to
3f8304a
Compare
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
karthikjeeyar
force-pushed
the
homepage/rbac
branch
from
619970f to
2977476
Compare
karthikjeeyar
force-pushed
the
homepage/rbac
branch
from
2977476 to
25ef841
Compare
|
karthikjeeyar
left a comment
karthikjeeyar
left a comment
There was a problem hiding this comment.
We can address these in followup issues:
- Conditional RBAC policy isn't handled. can we have followup ticket for this?
- Good to have a small blurb in plugin documentation to capture the new implementation homepage.defaultWidgets
/lgtm
2 participants
Hey, I just made a Pull Request!
WIP
✔️ Checklist