diff --git a/workspaces/homepage/.changeset/nasty-goats-live.md b/workspaces/homepage/.changeset/nasty-goats-live.md new file mode 100644 index 0000000000..8c57178720 --- /dev/null +++ b/workspaces/homepage/.changeset/nasty-goats-live.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-dynamic-home-page': patch +--- + +Fix issue that defaul widget props are ignored when customization is enabled. diff --git a/workspaces/homepage/.changeset/thirty-spies-tell.md b/workspaces/homepage/.changeset/thirty-spies-tell.md new file mode 100644 index 0000000000..3e84eb71be --- /dev/null +++ b/workspaces/homepage/.changeset/thirty-spies-tell.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-homepage-backend': patch +--- + +Fix conditional permission checks. diff --git a/workspaces/homepage/app-config-admin.yaml b/workspaces/homepage/app-config-admin.yaml index 89c8494505..c1d2158326 100644 --- a/workspaces/homepage/app-config-admin.yaml +++ b/workspaces/homepage/app-config-admin.yaml @@ -22,4 +22,5 @@ permission: users: - name: user:default/admin-user superUsers: - - name: user:default/admin-user + [] + # - name: user:default/admin-user diff --git a/workspaces/homepage/app-config-developer.yaml b/workspaces/homepage/app-config-developer.yaml index a53ff86861..13348a2417 100644 --- a/workspaces/homepage/app-config-developer.yaml +++ b/workspaces/homepage/app-config-developer.yaml @@ -22,4 +22,5 @@ permission: users: - name: user:default/developer-user superUsers: - - name: user:default/developer-user + [] + # - name: user:default/developer-user diff --git a/workspaces/homepage/app-config.yaml b/workspaces/homepage/app-config.yaml index 75ebdae559..72ecc17f7f 100644 --- a/workspaces/homepage/app-config.yaml +++ b/workspaces/homepage/app-config.yaml @@ -87,6 +87,85 @@ homepage: # For 1.x this references a mount point (`config.id`). # For 2.x this references a homepage widget extension (name). defaultWidgets: + - id: test-no-if + ref: headline + props: + title: 'This widget has no "if" condition and should always render' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-user + ref: headline + if: + users: [user:development/guest] + props: + title: 'This widget has an "if" condition that allows only the guest user to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-another-user + ref: headline + if: + users: [user:development/another-user] + props: + title: 'This widget has an "if" condition that allows only another user to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-group + ref: headline + if: + groups: [group:development/guests] + props: + title: 'This widget has an "if" condition that allows only the guests group to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-another-group + ref: headline + if: + groups: [group:development/another-group] + props: + title: 'This widget has an "if" condition that allows only another group to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-user-can-read-catalog-entities + ref: headline + if: + permissions: [catalog.entity.read] + props: + title: 'This widget has an "if" condition that allows only users with catalog read permissions to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-user-can-create-catalog-entities + ref: headline + if: + permissions: [catalog.entity.create] + props: + title: 'This widget has an "if" condition that allows only users with catalog create permissions to see it' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-user-can-read-this-widget + ref: headline + if: + permissions: [homepage.default-widgets.read, catalog.entity.read] + props: + title: 'This widget has an "if" condition that allows only users with access to read default widgets to see it (id: test-if-user-can-read-this-widget)' + layout: + xl: { w: 12, h: 1 } + + - id: test-if-user-can-read-another-widget + ref: headline + if: + permissions: [homepage.default-widgets.read] + props: + title: 'This widget has an "if" condition that allows only users with access to read default widgets to see it (id: test-if-user-can-read-another-widget)' + layout: + xl: { w: 12, h: 1 } + - id: onboarding ref: 'rhdh-onboarding-section' layout: @@ -230,9 +309,7 @@ auth: # see https://backstage.io/docs/auth/ to learn about auth providers providers: # See https://backstage.io/docs/auth/guest/provider - guest: - userEntityRef: user:default/guest-user - ownershipEntityRefs: [] + guest: {} scaffolder: # see https://backstage.io/docs/features/software-templates/configuration for software template options @@ -279,6 +356,15 @@ permission: - homepage admin: users: - - name: user:default/guest-user + - name: user:development/guest + + # if you make the guest user super admin here you might see "unexpected" default widgets + # on the homepage, as some widgets are only visible to users with access to read default widgets, + # which the guest user has then by default. + # You can either remove the guest user from the admin users or configure the superUsers instead. superUsers: - - name: user:default/guest-user + [] + # - name: user:development/guest + policyFileReload: true + policies-csv-file: ../../rbac-policy.csv + conditionalPoliciesFile: ../../conditional-policies.yaml diff --git a/workspaces/homepage/conditional-policies.yaml b/workspaces/homepage/conditional-policies.yaml new file mode 100644 index 0000000000..afd61c30df --- /dev/null +++ b/workspaces/homepage/conditional-policies.yaml @@ -0,0 +1,13 @@ +--- +result: CONDITIONAL +roleEntityRef: role:default/test-if-user-can-read-this-widget +pluginId: homepage +resourceType: homepage-default-widget +permissionMapping: + - read +conditions: + rule: HAS_WIDGET_ID + resourceType: homepage-default-widget + params: + widgetIds: + - test-if-user-can-read-this-widget diff --git a/workspaces/homepage/e2e-tests/homepageCustomizable.test.ts b/workspaces/homepage/e2e-tests/homepageCustomizable.test.ts index fe3b1a09aa..cb51c16158 100644 --- a/workspaces/homepage/e2e-tests/homepageCustomizable.test.ts +++ b/workspaces/homepage/e2e-tests/homepageCustomizable.test.ts @@ -86,7 +86,7 @@ test.describe.serial('Dynamic Home Page Customization', () => { sharedPage.getByText(/Good (morning|afternoon|evening)/), ).toBeVisible(); - await homePageCustomization.addWidget('Access'); + await homePageCustomization.addWidget('Quick Access Card'); await expect(sharedPage.getByText('Quick Access')).toBeVisible(); }); diff --git a/workspaces/homepage/e2e-tests/pages/homePageCustomization.ts b/workspaces/homepage/e2e-tests/pages/homePageCustomization.ts index d0daf2030c..606298849e 100644 --- a/workspaces/homepage/e2e-tests/pages/homePageCustomization.ts +++ b/workspaces/homepage/e2e-tests/pages/homePageCustomization.ts @@ -30,7 +30,8 @@ export class HomePageCustomization { // Locators private readonly editButton = () => this.page.getByText('Edit'); - private readonly saveButton = () => this.page.getByText('Save'); + private readonly saveButton = () => + this.page.getByText('Save', { exact: true }); private readonly clearAllButton = () => this.page.getByText('Clear all'); private readonly restoreDefaultsButton = () => this.page.getByText('Restore defaults'); diff --git a/workspaces/homepage/packages/app-legacy/src/App.tsx b/workspaces/homepage/packages/app-legacy/src/App.tsx index cb8e540d4a..f2411c41ce 100644 --- a/workspaces/homepage/packages/app-legacy/src/App.tsx +++ b/workspaces/homepage/packages/app-legacy/src/App.tsx @@ -44,6 +44,7 @@ import { searchPage } from './components/search/SearchPage'; import { Root } from './components/Root'; import { DebugHomepageAvailableWidgets } from './components/homepage/DebugHomepageAvailableWidgets'; import { DebugHomepageDefaultWidgets } from './components/homepage/DebugHomepageDefaultWidgets'; +import { DebugHomepageSavedWidgets } from './components/homepage/DebugHomepageSavedWidgets'; import { AlertDisplay, @@ -170,9 +171,9 @@ const createHeadline = ({ priority, }: { id: string; - title: string; - align: string; - priority: number; + title?: string; + align?: string; + priority?: number; }): HomePageCardMountPoint => ({ Component: Headline, config: { @@ -243,6 +244,9 @@ const cardMountPoints: HomePageCardMountPoint[] = [ }, }, // Add extra mount points to verify same components can mount multiple times + createHeadline({ + id: 'headline', + }), createHeadline({ id: 'headline-left', title: 'Left title', @@ -454,6 +458,10 @@ const routes = ( path="/debug-default-widgets" element={} /> + } + /> ); diff --git a/workspaces/homepage/packages/app-legacy/src/components/Root/Root.tsx b/workspaces/homepage/packages/app-legacy/src/components/Root/Root.tsx index a405e05c48..3bbd590714 100644 --- a/workspaces/homepage/packages/app-legacy/src/components/Root/Root.tsx +++ b/workspaces/homepage/packages/app-legacy/src/components/Root/Root.tsx @@ -98,6 +98,11 @@ export const Root = ({ children }: PropsWithChildren<{}>) => ( to="debug-default-widgets" text="Default Widgets" /> + diff --git a/workspaces/homepage/packages/app-legacy/src/components/homepage/DebugHomepageSavedWidgets.tsx b/workspaces/homepage/packages/app-legacy/src/components/homepage/DebugHomepageSavedWidgets.tsx new file mode 100644 index 0000000000..711b0d8672 --- /dev/null +++ b/workspaces/homepage/packages/app-legacy/src/components/homepage/DebugHomepageSavedWidgets.tsx @@ -0,0 +1,42 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { CodeSnippet, Content, Header, Page } from '@backstage/core-components'; + +import { stringify } from 'yaml'; + +export const DebugHomepageSavedWidgets = () => { + const savedWidgetString = localStorage.getItem('/home.customHomepage/home'); + const savedWidgets = savedWidgetString + ? JSON.parse(JSON.parse(savedWidgetString)) + : []; + + return ( + +
+ + + + + ); +}; diff --git a/workspaces/homepage/plugins/dynamic-home-page/src/components/DefaultWidgetsCustomizableGrid.tsx b/workspaces/homepage/plugins/dynamic-home-page/src/components/DefaultWidgetsCustomizableGrid.tsx index da074705cc..137ce79634 100644 --- a/workspaces/homepage/plugins/dynamic-home-page/src/components/DefaultWidgetsCustomizableGrid.tsx +++ b/workspaces/homepage/plugins/dynamic-home-page/src/components/DefaultWidgetsCustomizableGrid.tsx @@ -37,6 +37,7 @@ import { dynamicHomePagePlugin } from '../plugin'; import { useTranslation } from '../hooks/useTranslation'; import { useContainerQuery } from '../hooks/useContainerQuery'; import { getCardTitle, getCardDescription } from '../utils/customizable-cards'; +import { getTranslatedTextWithFallback } from '../translations/utils'; export interface DefaultWidgetsCustomizableGridProps { defaultWidgets: VisibleDefaultWidget[]; @@ -62,28 +63,6 @@ export const DefaultWidgetsCustomizableGrid = ({ return map; }, [mountPoints]); - const widgetsToRender = useMemo(() => { - // If config exists, use it exclusively; otherwise fall back to mount points - if (defaultWidgets.length > 0) { - return defaultWidgets.map((widget, index) => ({ - id: widget.id || `config-widget-${index}`, - ref: widget.ref, - layout: widget.layout, - source: 'config' as const, - })); - } - - // Fallback: render all mount points - return mountPoints - .filter(mp => mp.config?.id) - .map(mp => ({ - id: mp.config!.id, - ref: mp.config!.id, - layout: undefined, - source: 'mountpoint' as const, - })); - }, [defaultWidgets, mountPoints]); - const { children, config } = useMemo(() => { const childDictionary: Record< string, @@ -91,23 +70,12 @@ export const DefaultWidgetsCustomizableGrid = ({ > = {}; const defaultConfig: LayoutConfiguration[] = []; - widgetsToRender.forEach(widget => { - const widgetId = widget.id; - const widgetRef = widget.ref ?? widgetId; - if (!widgetId || !widgetRef) { + mountPoints.forEach(mountPoint => { + const mountPointId = mountPoint.config?.id; + if (!mountPointId) { // eslint-disable-next-line no-console console.warn( - `Widget missing id or ref (id: ${String(widget.id)}, ref: ${String(widget.ref)}).`, - ); - return; - } - - const mountPoint = mountPointsById.get(widgetRef); - - if (!mountPoint || !mountPoint.config?.id) { - // eslint-disable-next-line no-console - console.warn( - `No mount point found for widget ref ${widgetRef}. Available mount points: ${[...mountPointsById.keys()].join(', ')}`, + `Skipping homepage mount point without config.id: ${JSON.stringify(mountPoint)}`, ); return; } @@ -131,28 +99,101 @@ export const DefaultWidgetsCustomizableGrid = ({ }; const cardExtension = createCardExtension({ - name: widgetId, + name: mountPointId, title, description, - layout: mountPoint.config.cardLayout, - settings: mountPoint.config.settings, + layout: mountPoint.config!.cardLayout, + settings: mountPoint.config!.settings, components: () => Promise.resolve(componentParts), }); const Card = dynamicHomePagePlugin.provide(cardExtension); - childDictionary[widgetId] = { - child: , + // Make mount points available as 'addable' cards. + childDictionary[mountPointId] = { + child: , title, }; + }); - const widgetLayout = widget.layout as + defaultWidgets.forEach(defaultWidget => { + const mountPoint = mountPointsById.get(defaultWidget.ref); + if (!mountPoint) { + // eslint-disable-next-line no-console + console.warn( + `Homepage default widget has invalid ref (id: ${defaultWidget.id}, ref: ${defaultWidget.ref}). No matching mount point found. Available mount points ids: ${Array.from(mountPointsById.keys()).join(', ')}. This widget will not be displayed!`, + ); + return; + } + + // Make default widgets available as 'addable' cards because they can have custom props! + let cardId = defaultWidget.ref; + if (defaultWidget.props) { + cardId = defaultWidget.id; + + const title = getTranslatedTextWithFallback( + t, + defaultWidget.props.titleKey as string | undefined, + defaultWidget.props.title as string | undefined, + ); + let description = + getTranslatedTextWithFallback( + t, + defaultWidget.props.descriptionKey as string | undefined, + defaultWidget.props.description as string | undefined, + ) ?? + (defaultWidget.props.title as string | undefined) ?? + (defaultWidget.props.debugContent as string | undefined); + if (title === description) { + description = undefined; + } + + const automaticallyWrapInInfoCard = false; + + const componentParts: ComponentParts = { + Content: props => ( + + ), + Actions: mountPoint.Actions as () => JSX.Element, + Settings: mountPoint.Settings as () => JSX.Element, + ContextProvider: automaticallyWrapInInfoCard + ? undefined + : props => ( + + ), + }; + + const cardExtension = createCardExtension({ + name: cardId, + title, + description, + layout: mountPoint.config!.cardLayout, + settings: mountPoint.config!.settings, + components: () => Promise.resolve(componentParts), + }); + + const Card = dynamicHomePagePlugin.provide(cardExtension); + + childDictionary[cardId] = { + child: , + title, + }; + } + + const widgetLayout = defaultWidget.layout as | Record | undefined; const layout = widgetLayout?.xl ?? {}; - defaultConfig.push({ - component: widgetId, + component: cardId, x: layout.x ?? 0, y: layout.y ?? 0, width: layout.w ?? 12, @@ -171,7 +212,7 @@ export const DefaultWidgetsCustomizableGrid = ({ .map(e => e.child), config: defaultConfig, }; - }, [widgetsToRender, mountPointsById, t]); + }, [defaultWidgets, mountPoints, mountPointsById, t]); return ( <> diff --git a/workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx b/workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx index 16542cf6e4..eac2acb94e 100644 --- a/workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx +++ b/workspaces/homepage/plugins/dynamic-home-page/src/components/HomePage.tsx @@ -46,9 +46,7 @@ export const HomePage = ({ let content: React.ReactNode; if (loading) { content = ; - } else if (defaultWidgets && defaultWidgets.length === 0 && !customizable) { - content = ; - } else if (defaultWidgets && defaultWidgets.length > 0) { + } else if (defaultWidgets) { if (customizable) { content = ( ); + } else if (defaultWidgets.length === 0) { + content = ; } else { content = ( ; @@ -39,10 +40,10 @@ export async function buildUserContext(opts: { opts; const userEntityRef = credentials.principal.userEntityRef; - const userEntity = await catalog.getEntityByRef(userEntityRef, { credentials, }); + if (!userEntity) { logger.warn( `User entity '${userEntityRef}' not found in catalog; group-based visibility will fail closed`, @@ -54,20 +55,28 @@ export async function buildUserContext(opts: { .map(relation => relation.targetRef), ); - const permissionDecisions = new Map(); + const policyDecisions = new Map(); if (referencedPermissions.size > 0) { const names = [...referencedPermissions]; - const requests = names.map(name => ({ - permission: createPermission({ name, attributes: {} }), - })); - const decisions = await permissions.authorize(requests, { credentials }); - decisions.forEach((decision, index) => { - permissionDecisions.set( - names[index], - decision.result === AuthorizeResult.ALLOW ? 'ALLOW' : 'DENY', - ); + + const conditionalPermissionRequests = names.map( + name => ({ + permission: createPermission({ + name, + attributes: { action: 'read' }, + resourceType: 'homepage-default-widget', + }), + }), + ); + + const conditionalDecisions = await permissions.authorizeConditional( + conditionalPermissionRequests, + { credentials }, + ); + conditionalDecisions.forEach((decision, index) => { + policyDecisions.set(names[index], decision); }); } - return { userEntityRef, groupEntityRefs, permissionDecisions }; + return { userEntityRef, groupEntityRefs, policyDecisions }; } diff --git a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts index a490e83160..2d8090105f 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.test.ts @@ -14,6 +14,10 @@ * limitations under the License. */ +import { + AuthorizeResult, + PolicyDecision, +} from '@backstage/plugin-permission-common'; import { filterToVisibleLeafIds, filterToVisibleLeaves, @@ -25,7 +29,7 @@ function makeCtx(partial?: Partial): UserContext { return { userEntityRef: 'user:default/alice', groupEntityRefs: new Set(), - permissionDecisions: new Map(), + policyDecisions: new Map(), ...partial, }; } @@ -33,68 +37,82 @@ function makeCtx(partial?: Partial): UserContext { describe('isVisible', () => { const ctx = makeCtx({ groupEntityRefs: new Set(['group:default/developers']), - permissionDecisions: new Map([ - ['perm.allowed', 'ALLOW'], - ['perm.denied', 'DENY'], + policyDecisions: new Map([ + ['perm.allowed', { result: AuthorizeResult.ALLOW }], + ['perm.denied', { result: AuthorizeResult.DENY }], ]), }); it('visible when no visibility block is provided', () => { - expect(isVisible(undefined, ctx)).toBe(true); + expect(isVisible({}, ctx)).toBe(true); }); it('visible when visibility block is empty', () => { - expect(isVisible({}, ctx)).toBe(true); + expect(isVisible({ if: {} }, ctx)).toBe(true); }); it('visible when visibility has only empty arrays', () => { - expect(isVisible({ users: [], groups: [], permissions: [] }, ctx)).toBe( - true, - ); + expect( + isVisible({ if: { users: [], groups: [], permissions: [] } }, ctx), + ).toBe(true); }); it('visible when user ref matches', () => { - expect(isVisible({ users: ['user:default/alice'] }, ctx)).toBe(true); + expect(isVisible({ if: { users: ['user:default/alice'] } }, ctx)).toBe( + true, + ); }); it('hidden when only user ref is set and does not match', () => { - expect(isVisible({ users: ['user:default/bob'] }, ctx)).toBe(false); + expect(isVisible({ if: { users: ['user:default/bob'] } }, ctx)).toBe(false); }); it('visible when any group ref in the list matches', () => { expect( isVisible( - { groups: ['group:default/platform', 'group:default/developers'] }, + { + if: { + groups: ['group:default/platform', 'group:default/developers'], + }, + }, ctx, ), ).toBe(true); }); it('hidden when no group ref in the list matches', () => { - expect(isVisible({ groups: ['group:default/platform'] }, ctx)).toBe(false); + expect(isVisible({ if: { groups: ['group:default/platform'] } }, ctx)).toBe( + false, + ); }); it('visible when any referenced permission is ALLOW', () => { expect( - isVisible({ permissions: ['perm.denied', 'perm.allowed'] }, ctx), + isVisible({ if: { permissions: ['perm.denied', 'perm.allowed'] } }, ctx), ).toBe(true); }); it('hidden when all referenced permissions are DENY', () => { - expect(isVisible({ permissions: ['perm.denied'] }, ctx)).toBe(false); + expect(isVisible({ if: { permissions: ['perm.denied'] } }, ctx)).toBe( + false, + ); }); it('hidden when a permission is missing from decisions (fails closed)', () => { - expect(isVisible({ permissions: ['perm.unknown'] }, ctx)).toBe(false); + expect(isVisible({ if: { permissions: ['perm.unknown'] } }, ctx)).toBe( + false, + ); }); it('visible via OR across fields (permission allows even when user and group miss)', () => { expect( isVisible( { - users: ['user:default/bob'], - groups: ['group:default/platform'], - permissions: ['perm.allowed'], + if: { + users: ['user:default/bob'], + groups: ['group:default/platform'], + permissions: ['perm.allowed'], + }, }, ctx, ), @@ -105,20 +123,196 @@ describe('isVisible', () => { expect( isVisible( { - users: ['user:default/bob'], - groups: ['group:default/platform'], - permissions: ['perm.denied'], + if: { + users: ['user:default/bob'], + groups: ['group:default/platform'], + permissions: ['perm.denied'], + }, }, ctx, ), ).toBe(false); }); + + describe('conditional permissions', () => { + const conditionFor = (widgetIds: string[]) => ({ + rule: 'HAS_WIDGET_ID', + resourceType: 'homepage-default-widget', + params: { widgetIds }, + }); + + const conditionalDecision = ( + conditions: PolicyDecision extends infer T + ? T extends { conditions: infer C } + ? C + : never + : never, + ): PolicyDecision => ({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'homepage', + resourceType: 'homepage-default-widget', + conditions, + }); + + it('visible when CONDITIONAL decision matches widget id', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + ['perm.cond', conditionalDecision(conditionFor(['my-widget']))], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(true); + }); + + it('hidden when CONDITIONAL decision does not match widget id', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + ['perm.cond', conditionalDecision(conditionFor(['other-widget']))], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(false); + }); + + it('visible when CONDITIONAL uses allOf and all conditions match', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + conditionalDecision({ + allOf: [conditionFor(['my-widget', 'other-widget'])], + }), + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(true); + }); + + it('hidden when CONDITIONAL uses allOf and one condition fails', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + conditionalDecision({ + allOf: [ + conditionFor(['my-widget']), + conditionFor(['other-widget']), + ], + }), + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(false); + }); + + it('visible when CONDITIONAL uses anyOf and one condition matches', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + conditionalDecision({ + anyOf: [ + conditionFor(['other-widget']), + conditionFor(['my-widget']), + ], + }), + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(true); + }); + + it('hidden when CONDITIONAL uses not and the inner condition matches', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + conditionalDecision({ not: conditionFor(['my-widget']) }), + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(false); + }); + + it('visible when CONDITIONAL uses not and the inner condition does not match', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + conditionalDecision({ not: conditionFor(['other-widget']) }), + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(true); + }); + + it('hidden when CONDITIONAL references an unknown rule', () => { + const ctxCond = makeCtx({ + policyDecisions: new Map([ + [ + 'perm.cond', + { + result: AuthorizeResult.CONDITIONAL, + pluginId: 'homepage', + resourceType: 'homepage-default-widget', + conditions: { + rule: 'UNKNOWN_RULE', + resourceType: 'homepage-default-widget', + params: {}, + }, + }, + ], + ]), + }); + expect( + isVisible( + { id: 'my-widget', if: { permissions: ['perm.cond'] } }, + ctxCond, + ), + ).toBe(false); + }); + }); }); describe('filterToVisibleLeafIds', () => { const ctx = makeCtx({ groupEntityRefs: new Set(['group:default/developers']), - permissionDecisions: new Map([['perm.admin', 'DENY']]), + policyDecisions: new Map([ + ['perm.admin', { result: AuthorizeResult.DENY }], + ]), }); it('returns an empty list for an empty tree', () => { @@ -226,7 +420,9 @@ describe('filterToVisibleLeafIds', () => { describe('filterToVisibleLeaves', () => { const ctx = makeCtx({ groupEntityRefs: new Set(['group:default/developers']), - permissionDecisions: new Map([['perm.admin', 'DENY']]), + policyDecisions: new Map([ + ['perm.admin', { result: AuthorizeResult.DENY }], + ]), }); it('returns full card objects with metadata', () => { diff --git a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts index 8cdf8ad39b..518218306f 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/evaluateVisibility.ts @@ -15,34 +15,44 @@ */ import { - DefaultWidgetNode, - DefaultWidgetVisibility, - UserContext, - VisibleDefaultWidget, -} from './types'; + PermissionCondition, + PermissionCriteria, + PermissionRuleParams, +} from '@backstage/plugin-permission-common'; +import { DefaultWidgetNode, UserContext, VisibleDefaultWidget } from './types'; +import { rules } from '../permissions/rules'; export function isVisible( - visibility: DefaultWidgetVisibility | undefined, + defaultWidget: DefaultWidgetNode, ctx: UserContext, ): boolean { + const visibility = defaultWidget?.if; if (!visibility) return true; - const hasAny = + const hasAnyCondition = (visibility.users?.length ?? 0) > 0 || (visibility.groups?.length ?? 0) > 0 || (visibility.permissions?.length ?? 0) > 0; - if (!hasAny) return true; + if (!hasAnyCondition) return true; const matchUser = visibility.users?.some(ref => ref === ctx.userEntityRef) ?? false; const matchGroup = visibility.groups?.some(ref => ctx.groupEntityRefs.has(ref)) ?? false; - const matchPermission = - visibility.permissions?.some( - p => ctx.permissionDecisions.get(p) === 'ALLOW', - ) ?? false; + const matchPolicy = + visibility.permissions?.some(p => { + const decision = ctx.policyDecisions.get(p); + if (!decision) return false; + return ( + decision.result === 'ALLOW' || + (decision.result === 'CONDITIONAL' && + matches(defaultWidget, decision.conditions)) + ); + }) ?? false; - return matchUser || matchGroup || matchPermission; + const matchAny = matchUser || matchGroup || matchPolicy; + + return matchAny; } export function filterToVisibleLeafIds( @@ -51,7 +61,7 @@ export function filterToVisibleLeafIds( ): string[] { const out: string[] = []; const walk = (node: DefaultWidgetNode) => { - if (!isVisible(node.if, ctx)) return; + if (!isVisible(node, ctx)) return; if (node.id !== undefined) out.push(node.id); node.children?.forEach(walk); }; @@ -65,7 +75,7 @@ export function filterToVisibleLeaves( ): VisibleDefaultWidget[] { const out: VisibleDefaultWidget[] = []; const walk = (node: DefaultWidgetNode) => { - if (!isVisible(node.if, ctx)) return; + if (!isVisible(node, ctx)) return; if (node.id !== undefined) { const card: VisibleDefaultWidget = { id: node.id, @@ -80,3 +90,32 @@ export function filterToVisibleLeaves( nodes.forEach(walk); return out; } + +const matches = ( + defaultWidget: DefaultWidgetNode, + filters?: PermissionCriteria< + PermissionCondition + >, +): boolean => { + if (!filters) { + return true; + } + + if ('allOf' in filters) { + return filters.allOf.every(filter => matches(defaultWidget, filter)); + } + + if ('anyOf' in filters) { + return filters.anyOf.some(filter => matches(defaultWidget, filter)); + } + + if ('not' in filters) { + return !matches(defaultWidget, filters.not); + } + + return ( + Object.values(rules) + .find(r => r.name === filters.rule) + ?.apply(defaultWidget, filters.params ?? {}) ?? false + ); +}; diff --git a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts index 20335ede2d..2af149b720 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/defaultWidgets/types.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { PolicyDecision } from '@backstage/plugin-permission-common'; + export type { DefaultWidgetNode, DefaultWidgetVisibility, @@ -21,10 +23,8 @@ export type { VisibleDefaultWidget, } from '@red-hat-developer-hub/backstage-plugin-homepage-common'; -export type PermissionDecision = 'ALLOW' | 'DENY'; - export interface UserContext { userEntityRef: string; groupEntityRefs: Set; - permissionDecisions: Map; + policyDecisions: Map; } diff --git a/workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts b/workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts index 8f59b63e9a..dcaf635c34 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/permissions/resource.ts @@ -16,14 +16,14 @@ import { createPermissionResourceRef } from '@backstage/plugin-permission-node'; import { - VisibleDefaultWidget, + DefaultWidgetNode, RESOURCE_TYPE_HOMEPAGE_DEFAULT_WIDGET, } from '@red-hat-developer-hub/backstage-plugin-homepage-common'; import { HomepageDefaultWidgetFilter } from './rules'; export const homepageDefaultWidgetPermissionResourceRef = createPermissionResourceRef< - VisibleDefaultWidget, + DefaultWidgetNode, HomepageDefaultWidgetFilter >().with({ pluginId: 'homepage', diff --git a/workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts b/workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts index 2a3bb1debf..131bec0cc5 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/permissions/rules.ts @@ -14,15 +14,10 @@ * limitations under the License. */ -import { - createPermissionRule, - type PermissionRule, -} from '@backstage/plugin-permission-node'; +import { createPermissionRule } from '@backstage/plugin-permission-node'; import { z } from 'zod/v3'; -import { - VisibleDefaultWidget, - RESOURCE_TYPE_HOMEPAGE_DEFAULT_WIDGET, -} from '@red-hat-developer-hub/backstage-plugin-homepage-common'; +import { DefaultWidgetNode } from '@red-hat-developer-hub/backstage-plugin-homepage-common'; + import { homepageDefaultWidgetPermissionResourceRef } from './resource'; export type HomepageDefaultWidgetFilter = { @@ -30,14 +25,11 @@ export type HomepageDefaultWidgetFilter = { values: Array | undefined; }; -type HasWidgetIdParams = { widgetIds?: string[] | undefined }; - const hasWidgetId = createPermissionRule({ name: 'HAS_WIDGET_ID', description: 'Should allow users to access homepage widgets with specified widget IDs', resourceRef: homepageDefaultWidgetPermissionResourceRef, - paramsSchema: z.object({ widgetIds: z .string() @@ -45,20 +37,16 @@ const hasWidgetId = createPermissionRule({ .optional() .describe('List of widget IDs to match on'), }), - apply: (widget: VisibleDefaultWidget, { widgetIds }: HasWidgetIdParams) => { - return widgetIds && widgetIds.length > 0 - ? widgetIds.includes(widget.id) - : true; + apply: (defaultWidget: DefaultWidgetNode, { widgetIds }) => { + if (!widgetIds || widgetIds.length === 0 || !defaultWidget.id) return false; + return widgetIds.includes(defaultWidget.id); }, - toQuery: ({ widgetIds }: HasWidgetIdParams) => ({ - key: 'widgetId', - values: widgetIds, - }), -} as any) as unknown as PermissionRule< - VisibleDefaultWidget, - HomepageDefaultWidgetFilter, - typeof RESOURCE_TYPE_HOMEPAGE_DEFAULT_WIDGET, - HasWidgetIdParams ->; + toQuery: ({ widgetIds }) => { + return { + key: 'widgetId', + values: widgetIds, + }; + }, +}); export const rules = { hasWidgetId }; diff --git a/workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts b/workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts index 92998d85b3..6e183549b8 100644 --- a/workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts +++ b/workspaces/homepage/plugins/homepage-backend/src/plugin.test.ts @@ -70,7 +70,7 @@ describe('homepagePlugin', () => { ], }, { - if: { permissions: ['homepage.platform.read'] }, + if: { permissions: ['homepage.default-widgets.read'] }, children: [{ id: 'platform-inner', ref: 'rhdh.platform' }], }, ], @@ -81,7 +81,7 @@ describe('homepagePlugin', () => { entities: [userEntityWithGroups(['group:default/developers'])], }), mockServices.permissions.mock({ - authorize: async requests => + authorizeConditional: async requests => requests.map(() => ({ result: AuthorizeResult.DENY })), }).factory, ], @@ -110,7 +110,7 @@ describe('homepagePlugin', () => { { id: 'public', ref: 'rhdh.public' }, { if: { - permissions: ['homepage.platform.read'], + permissions: ['homepage.default-widgets.read'], }, children: [ { @@ -126,7 +126,7 @@ describe('homepagePlugin', () => { }), catalogServiceMock.factory({ entities: [userEntityWithGroups([])] }), mockServices.permissions.mock({ - authorize: async requests => + authorizeConditional: async requests => requests.map(() => ({ result: AuthorizeResult.ALLOW })), }).factory, ], diff --git a/workspaces/homepage/rbac-policy.csv b/workspaces/homepage/rbac-policy.csv new file mode 100644 index 0000000000..13ecd609dc --- /dev/null +++ b/workspaces/homepage/rbac-policy.csv @@ -0,0 +1 @@ +g, user:development/guest, role:default/test-if-user-can-read-this-widget