fix(homepage): fix conditional permission checks and use props from default config widgets also when customization is enabled by christoph-jerolimov · Pull Request #3231 · redhat-developer/rhdh-plugins

christoph-jerolimov · 2026-05-22T12:52:36Z

Hey, I just made a Pull Request!

todo: add some test cases and some documentation here

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or Updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

rhdh-qodo-merge · 2026-05-22T12:52:52Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. ~~Tests broken by refactor~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

Backend tests still construct UserContext.permissionDecisions and call `isVisible(visibility,
ctx), but the PR changes the shape to policyDecisions and changes isVisible` to accept a
DefaultWidgetNode, so tests will fail to compile/run in CI. Plugin tests also mock
permissions.authorize but the code now calls authorizeConditional, causing runtime failures in
the test backend.

Code

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[R25-56]

Relevance

⭐⭐⭐ High
Repo fixes CI-breakage quickly; PR #3137 shows tests updated with behavior changes.
PR-#3137

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The production code now requires defaultWidget + ctx.policyDecisions, but tests still use
permissionDecisions and mock only authorize, so the test suite will fail after this change.
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[25-56]
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts[26-30]
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts[24-54]
workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts[71-86]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Backend tests are written against the old visibility API (`isVisible(visibility, ctx)`) and old `UserContext.permissionDecisions`, but the implementation now requires `isVisible(defaultWidget, ctx)` and `UserContext.policyDecisions` populated from `permissions.authorizeConditional()`.

### Issue Context
This will break TypeScript compilation and/or runtime in tests (permission mocks don’t implement `authorizeConditional`).

### Fix Focus Areas
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[25-56]
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts[26-30]
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts[24-90]
- workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts[71-133]

### What to change
- Update `evaluateVisibility.test.ts`:
 - Build `UserContext` with `policyDecisions: Map<string, PolicyDecision>`.
 - Update `isVisible(...)` call sites to pass a `DefaultWidgetNode` (e.g., `{ if: { ... }, id: 'x', ref: 'x' }`) rather than a raw visibility block.
- Update `plugin.test.ts` permission mocks to provide `authorizeConditional` (and return `PolicyDecision` objects with `result: 'ALLOW'|'DENY'|'CONDITIONAL'`).
- If any tests only need ALLOW/DENY behavior, return `{ result: 'ALLOW' }` / `{ result: 'DENY' }` for each request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Conditional hides permission-groups 🐞 Bug ≡ Correctness

Description

If a group node (children-only, no id) is gated by permissions and receives a CONDITIONAL
decision with HAS_WIDGET_ID, matches() will evaluate the condition against the group node, which
can never satisfy HAS_WIDGET_ID due to missing id, hiding the entire subtree. This makes
conditional permissions unsafe to use on grouping nodes.

Code

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[R42-51]

Relevance

⭐⭐ Medium
No historical evidence on conditional checks for group nodes; unclear intended semantics.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Group nodes are enforced to have no id/ref, but the conditional rule HAS_WIDGET_ID requires
defaultWidget.id, so conditional matches will always fail on groups, pruning the subtree.
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[38-55]
workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts[28-43]
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/loadDefaultWidgets.ts[55-66]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Conditional permission evaluation is applied to the current node, but group nodes are guaranteed to have no `id`. This makes `HAS_WIDGET_ID` (and any id-based rule) impossible to satisfy for groups, potentially hiding whole groups even when some leaves would match.

### Issue Context
The default widgets schema enforces: groups have `children` and must not have `id/ref`.

### Fix Focus Areas
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts[38-55]
- workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts[28-43]
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/loadDefaultWidgets.ts[55-66]

### What to change
Options:
- If a node is a group (`children?.length > 0`), and a decision is CONDITIONAL, consider the group visible if **any descendant leaf** matches the conditions (walk children and apply `matches` to leaves).
- Alternatively, enforce at config validation time that `if.permissions` is not allowed on group nodes when conditional rules are in play (fail fast with a clear error message).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Rule semantics inconsistent 🐞 Bug ≡ Correctness

Description

The HAS_WIDGET_ID rule now returns false when widgetIds is missing/empty, but toQuery still
returns { values: widgetIds } (i.e., undefined), which can create inconsistent behavior between
query-based and in-memory evaluation. A misconfigured policy that omits widgetIds can unexpectedly
become deny-all, and the apply/toQuery mismatch is a correctness risk.

Code

workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts[R40-49]

Relevance

⭐⭐ Medium
No prior precedent found for apply/toQuery mismatch in permission rules.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
apply now returns false for missing/empty widgetIds while toQuery still forwards undefined,
creating divergent interpretations of the same params.
workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts[28-50]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`apply()` treats missing/empty `widgetIds` as **no match** (false), while `toQuery()` forwards `widgetIds` (possibly undefined), which often represents **no filtering** in query semantics. These should be consistent.

### Issue Context
The previous implementation treated missing/empty `widgetIds` as match-all (true). The new code changes that but does not update `toQuery`/schema accordingly.

### Fix Focus Areas
- workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts[28-50]

### What to change
Choose one consistent policy:
- **Fail-open**: if `widgetIds` missing/empty, return `true` in `apply()` and keep `toQuery.values` undefined.
- **Fail-closed**: if `widgetIds` missing/empty, keep `apply()` as false, but then update one of:
 - `paramsSchema` to require non-empty `widgetIds`, or
 - `toQuery()` to return a query that matches nothing when `widgetIds` is missing/empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Permission semantics changed 🐞 Bug ≡ Correctness

Description

buildUserContext now treats every string in if.permissions as a homepage default-widget
*resource* permission (action: 'read', resourceType: 'homepage-default-widget') and queries it
via authorizeConditional, which effectively stops supporting arbitrary permission names as
described in the backend README. Any if.permissions values that are not registered as homepage
resource permissions are likely to fail closed (DENY), changing widget visibility behavior.

Code

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/buildUserContext.ts[R62-75]

Relevance

⭐ Low
PR #2855 review rejected changing permission authorization; PR #2944 documents current fail-closed
semantics.
PR-#2855
PR-#2944

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code forces resourceType/action for all permission strings, while the README states
arbitrary permission names are allowed; additionally the plugin registers only the homepage
default-widgets read permission as a resource permission.
workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/buildUserContext.ts[58-79]
workspaces/homepage/plugins/homepage-backend/README.md[36-46]
workspaces/homepage/plugins/homepage-backend/src/plugin.ts[47-54]
workspaces/homepage/plugins/homepage-common/src/permissions.ts[24-44]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The backend README documents that `if.permissions` can reference “any listed permission name”, but the new implementation always creates a homepage resource permission (`action: read`, `resourceType: homepage-default-widget`) and calls `authorizeConditional()`. This is a behavior change that can break existing configs that referenced non-homepage or non-resource permissions.

### Issue Context
The homepage backend only registers `homepage.default-widgets.read` as a resource permission for `homepage-default-widget`, so other names will not be registered under this resource type.

### Fix Focus Areas
- workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/buildUserContext.ts[58-79]
- workspaces/homepage/plugins/homepage-backend/src/plugin.ts[47-54]
- workspaces/homepage/plugins/homepage-backend/README.md[36-46]
- workspaces/homepage/plugins/homepage-common/src/permissions.ts[24-44]

### What to change
Pick one of these approaches:
1) **Constrain and validate**: explicitly document + validate that `if.permissions` must contain only `homepage.default-widgets.read` (or other registered homepage permissions). Fail fast during config load if unknown permission names are used.
2) **Support both**:
  - For known homepage resource permissions, use `authorizeConditional()`.
  - For other permission names, fall back to `permissions.authorize()` with `createPermission({ name, attributes: {} })` (old behavior).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

rhdh-gh-app · 2026-05-22T12:53:02Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
app-legacy	workspaces/homepage/packages/app-legacy	none	`v0.0.0`
@red-hat-developer-hub/backstage-plugin-dynamic-home-page	workspaces/homepage/plugins/dynamic-home-page	patch	`v1.13.3`
@red-hat-developer-hub/backstage-plugin-homepage-backend	workspaces/homepage/plugins/homepage-backend	patch	`v0.1.2`

rhdh-qodo-merge · 2026-05-22T13:02:28Z

Review Summary by Qodo

Implement conditional permission checks for homepage default widgets

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Migrate from simple ALLOW/DENY decisions to conditional policy decisions
• Update permission checks to support conditional criteria evaluation
• Refactor permission resource type from VisibleDefaultWidget to DefaultWidgetNode
• Add conditional policies configuration and permission rules matching logic
• Update app configurations for RBAC and conditional policies support

Diagram

flowchart LR
  A["buildUserContext"] -->|"authorizeConditional"| B["PolicyDecision"]
  B -->|"ALLOW/CONDITIONAL"| C["evaluateVisibility"]
  C -->|"matches conditions"| D["Permission Rules"]
  D -->|"HAS_WIDGET_ID"| E["Filtered Widgets"]

File Changes

1. workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/buildUserContext.ts ✨ Enhancement +23/-14

Switch to conditional permission authorization

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/buildUserContext.ts

2. workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts ✨ Enhancement +54/-15

Add conditional criteria matching logic

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts

3. workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts ✨ Enhancement +3/-3

Replace PermissionDecision with PolicyDecision type

workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts

View more (8)

4. workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts ✨ Enhancement +2/-2

Update resource type to DefaultWidgetNode

workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts

5. workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts ✨ Enhancement +13/-25

Simplify permission rules and remove type assertions

workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts

6. workspaces/homepage/app-config-admin.yaml ⚙️ Configuration changes +2/-1

Clear superUsers configuration

workspaces/homepage/app-config-admin.yaml

7. workspaces/homepage/app-config-developer.yaml ⚙️ Configuration changes +2/-1

Clear superUsers configuration

workspaces/homepage/app-config-developer.yaml

8. workspaces/homepage/app-config.yaml ⚙️ Configuration changes +12/-6

Add conditional policies and update RBAC config

workspaces/homepage/app-config.yaml

9. workspaces/homepage/conditional-policies.yaml ⚙️ Configuration changes +13/-0

Add conditional policy for widget access

workspaces/homepage/conditional-policies.yaml

10. workspaces/homepage/packages/app-legacy/src/App.tsx ✨ Enhancement +6/-3

Make headline parameters optional and add test widget

workspaces/homepage/packages/app-legacy/src/App.tsx

11. workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx 🐞 Bug fix +3/-3

Refactor empty state rendering logic

workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx

…sion checks Migrate tests from simple permissionDecisions (string values) to policyDecisions (PolicyDecision objects) and fix isVisible tests to pass DefaultWidgetNode with `if` field. Add tests for CONDITIONAL decisions covering allOf, anyOf, not, and unknown rule cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

codecov · 2026-05-22T20:34:09Z

Codecov Report

❌ Patch coverage is 55.12821% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.83%. Comparing base (fb32b28) to head (aa8477a).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3231   +/-   ##
=======================================
  Coverage   53.82%   53.83%           
=======================================
  Files        2362     2363    +1     
  Lines       84815    84854   +39     
  Branches    23508    23520   +12     
=======================================
+ Hits        45650    45678   +28     
- Misses      37714    37725   +11     
  Partials     1451     1451

Flag	Coverage Δ		*Carryforward flag
adoption-insights	`83.58% <ø> (ø)`		Carriedforward from 5a5d2d9
ai-integrations	`70.03% <ø> (ø)`		Carriedforward from 5a5d2d9
app-defaults	`69.60% <ø> (ø)`		Carriedforward from 5a5d2d9
augment	`47.62% <ø> (ø)`		Carriedforward from 5a5d2d9
bulk-import	`72.86% <ø> (ø)`		Carriedforward from 5a5d2d9
cost-management	`16.49% <ø> (ø)`		Carriedforward from 5a5d2d9
dcm	`32.85% <ø> (ø)`		Carriedforward from 5a5d2d9
extensions	`61.79% <ø> (ø)`		Carriedforward from 5a5d2d9
global-floating-action-button	`74.30% <ø> (ø)`		Carriedforward from 5a5d2d9
global-header	`61.68% <ø> (ø)`		Carriedforward from 5a5d2d9
homepage	`51.60% <55.12%> (+0.60%)`	⬆️
konflux	`91.01% <ø> (ø)`		Carriedforward from 5a5d2d9
lightspeed	`68.33% <ø> (ø)`		Carriedforward from 5a5d2d9
mcp-integrations	`81.59% <ø> (ø)`		Carriedforward from 5a5d2d9
orchestrator	`36.36% <ø> (ø)`		Carriedforward from 5a5d2d9
quickstart	`62.88% <ø> (ø)`		Carriedforward from 5a5d2d9
sandbox	`79.49% <ø> (ø)`		Carriedforward from 5a5d2d9
scorecard	`83.84% <ø> (ø)`		Carriedforward from 5a5d2d9
theme	`64.54% <ø> (ø)`		Carriedforward from 5a5d2d9
translations	`8.49% <ø> (ø)`		Carriedforward from 5a5d2d9
x2a	`78.59% <ø> (ø)`		Carriedforward from 5a5d2d9

*This pull request uses carry forward flags. Click here to find out more.

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb32b28...aa8477a. Read the comment docs.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

sonarqubecloud · 2026-05-23T20:08:19Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fix(homepage): fix conditional permission checks

2c829a5

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

christoph-jerolimov requested a review from karthikjeeyar May 22, 2026 12:52

christoph-jerolimov self-assigned this May 22, 2026

christoph-jerolimov requested review from a team and Eswaraiahsapram as code owners May 22, 2026 12:52

github-actions Bot added the workspace/homepage label May 22, 2026

rhdh-qodo-merge Bot added enhancement New feature or request Bug fix labels May 22, 2026

christoph-jerolimov force-pushed the homepage/add-conditional-permission-checks branch from 291231f to 23c33ca Compare May 22, 2026 20:29

fix(homepage): use default widget props when customization is enabled

c173028

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

christoph-jerolimov force-pushed the homepage/add-conditional-permission-checks branch from 23c33ca to c173028 Compare May 22, 2026 22:07

fix(homepage): add changesets

5a5d2d9

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

christoph-jerolimov changed the title ~~fix(homepage): fix conditional permission checks~~ fix(homepage): fix conditional permission checks and use props from default config widgets also when customization is enabled May 22, 2026

fix(homepage): fix e2e

aa8477a

Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>

christoph-jerolimov force-pushed the homepage/add-conditional-permission-checks branch from 4a97c97 to aa8477a Compare May 23, 2026 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(homepage): fix conditional permission checks and use props from default config widgets also when customization is enabled#3231

fix(homepage): fix conditional permission checks and use props from default config widgets also when customization is enabled#3231
christoph-jerolimov wants to merge 5 commits into
redhat-developer:mainfrom
christoph-jerolimov:homepage/add-conditional-permission-checks

christoph-jerolimov commented May 22, 2026 •

edited

Loading

Uh oh!

rhdh-qodo-merge Bot commented May 22, 2026 •

edited

Loading

Uh oh!

rhdh-gh-app Bot commented May 22, 2026 •

edited

Loading

Uh oh!

rhdh-qodo-merge Bot commented May 22, 2026

Uh oh!

codecov Bot commented May 22, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented May 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

christoph-jerolimov commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hey, I just made a Pull Request!

✔️ Checklist

Uh oh!

rhdh-qodo-merge Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

rhdh-gh-app Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changed Packages

Uh oh!

rhdh-qodo-merge Bot commented May 22, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

codecov Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud Bot commented May 23, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

christoph-jerolimov commented May 22, 2026 •

edited

Loading

rhdh-qodo-merge Bot commented May 22, 2026 •

edited

Loading

rhdh-gh-app Bot commented May 22, 2026 •

edited

Loading

codecov Bot commented May 22, 2026 •

edited

Loading