FLPATH-3243 | [UI] Plugin skeleton - App shell and navigation#2425
FLPATH-3243 | [UI] Plugin skeleton - App shell and navigation#2425asmasarw wants to merge 6 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
|
Review Summary by QodoImplement DCM backend API with RBAC and sidebar navigation
WalkthroughsDescription• Implement DCM backend API with token and access endpoints • Add RBAC permission integration and authorization checks • Create collapsible Administration sidebar menu with DCM and RBAC items • Configure permission system with RBAC backend plugin integration • Add SSO token management with caching and configuration support Diagramflowchart LR
A["Backend Plugin"] -->|"registers"| B["RBAC Backend"]
A -->|"registers"| C["DCM Backend"]
C -->|"provides"| D["/token endpoint"]
C -->|"provides"| E["/access endpoint"]
D -->|"fetches from"| F["Red Hat SSO"]
E -->|"checks"| G["RBAC Permissions"]
H["Frontend Root"] -->|"renders"| I["Administration Menu"]
I -->|"contains"| J["DCM Submenu"]
I -->|"contains"| K["RBAC Submenu"]
File Changes1. workspaces/dcm/packages/backend/src/index.ts
|
Code Review by Qodo
1. Guest RBAC superuser
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
| permission: | ||
| enabled: true | ||
| rbac: | ||
| policyFileReload: true | ||
| pluginsWithPermission: | ||
| - catalog | ||
| - scaffolder | ||
| - permission | ||
| - dcm | ||
| admin: | ||
| users: | ||
| - name: group:default/admins | ||
| - name: user:default/guest | ||
| superUsers: | ||
| - name: group:default/admins | ||
| - name: user:default/guest |
There was a problem hiding this comment.
1. Guest rbac superuser 🐞 Bug ⛨ Security
Guest auth is enabled and auto-sign-in is configured, but the guest user is also configured as an RBAC admin and superUser, effectively bypassing authorization for anyone who can access the app.
Agent Prompt
### Issue description
`user:default/guest` is configured as an RBAC admin and superUser while guest auth + auto sign-in is enabled, making the instance effectively admin-by-default.
### Issue Context
This is a configuration chain issue: guest provider + auto sign-in + guest listed in RBAC admin/superUsers.
### Fix Focus Areas
- workspaces/dcm/app-config.yaml[42-61]
- workspaces/dcm/packages/app/src/App.tsx[79-81]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const getToken = | ||
| (options: RouterOptions): RequestHandler => | ||
| async (_, response) => { | ||
| const { logger, config } = options; | ||
|
|
||
| assert(typeof config !== 'undefined', 'Config is undefined'); | ||
|
|
||
| logger.info('Requesting new access token'); | ||
|
|
||
| const ssoBaseUrl = | ||
| config.getOptionalString('dcm.ssoBaseUrl') ?? DEFAULT_SSO_BASE_URL; | ||
| const clientId = config.getString('dcm.clientId'); | ||
| const clientSecret = config.getString('dcm.clientSecret'); | ||
|
|
||
| const tokenUrl = `${ssoBaseUrl}/auth/realms/redhat-external/protocol/openid-connect/token`; | ||
| const body = new URLSearchParams({ | ||
| client_id: clientId, | ||
| client_secret: clientSecret, | ||
| scope: 'api.console', | ||
| grant_type: 'client_credentials', | ||
| }); | ||
|
|
||
| const rhSsoResponse = await fetch(tokenUrl, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, | ||
| body: body.toString(), | ||
| }); | ||
|
|
||
| if (rhSsoResponse.ok) { | ||
| const json = (await rhSsoResponse.json()) as { | ||
| access_token: string; | ||
| expires_in: number; | ||
| }; | ||
| const bodyRes: GetTokenResponse = { | ||
| accessToken: json.access_token, | ||
| expiresAt: Date.now() + json.expires_in * 1000, | ||
| }; | ||
| response.json(bodyRes); |
There was a problem hiding this comment.
2. Un-gated token endpoint 🐞 Bug ⛨ Security
The DCM backend exposes GET /token that returns an RH SSO client_credentials access token to any user-cookie caller, without performing any permission check; combined with guest auto-sign-in this can hand out privileged external tokens to arbitrary visitors.
Agent Prompt
### Issue description
`GET /api/dcm/token` returns an RH SSO `client_credentials` access token to any authenticated user-cookie caller, without any RBAC/permission gate.
### Issue Context
- `/access` performs permission evaluation but `/token` does not.
- Guest auth is configured with auto sign-in, increasing exposure.
### Fix Focus Areas
- workspaces/dcm/plugins/dcm-backend/src/plugin.ts[48-59]
- workspaces/dcm/plugins/dcm-backend/src/routes/token.ts[23-64]
- workspaces/dcm/plugins/dcm-backend/src/routes/access.ts[22-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export async function createRouter( | ||
| options: RouterOptions, | ||
| ): Promise<express.Router> { | ||
| const router = Router(); | ||
| router.use(express.json()); | ||
|
|
||
| // TEMPLATE NOTE: | ||
| // Zod is a powerful library for data validation and recommended in particular | ||
| // for user-defined schemas. In this case we use it for input validation too. | ||
| // | ||
| // If you want to define a schema for your API we recommend using Backstage's | ||
| // OpenAPI tooling: https://backstage.io/docs/next/openapi/01-getting-started | ||
| const todoSchema = z.object({ | ||
| title: z.string(), | ||
| entityRef: z.string().optional(), | ||
| }); | ||
|
|
||
| router.post('/todos', async (req, res) => { | ||
| const parsed = todoSchema.safeParse(req.body); | ||
| if (!parsed.success) { | ||
| throw new InputError(parsed.error.toString()); | ||
| } | ||
|
|
||
| const result = await todoList.createTodo(parsed.data, { | ||
| credentials: await httpAuth.credentials(req, { allow: ['user'] }), | ||
| }); | ||
| router.use(express.json()); | ||
|
|
||
| res.status(201).json(result); | ||
| const permissionsIntegrationRouter = createPermissionIntegrationRouter({ | ||
| permissions: dcmPluginPermissions, | ||
| }); | ||
| router.use(permissionsIntegrationRouter); | ||
|
|
||
| router.get('/todos', async (_req, res) => { | ||
| res.json(await todoList.listTodos()); | ||
| router.get('/health', (_req, res) => { | ||
| res.json({ status: 'ok' }); | ||
| }); | ||
|
|
||
| router.get('/todos/:id', async (req, res) => { | ||
| res.json(await todoList.getTodo({ id: req.params.id })); | ||
| }); | ||
| router.get('/token', getToken(options)); | ||
| router.get('/access', getAccess(options)); | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
There was a problem hiding this comment.
Code Review — FLPATH-3243 | Plugin skeleton - App shell and navigation
Overall the structure is reasonable for a skeleton. The sidebar navigation, RBAC integration, and backend route layout are sound. However, there are several issues ranging from a security gap to dead code and missing tests.
Security Issues (must fix)
P0 — /token endpoint has no authorization gate
The /access endpoint correctly evaluates permissions via the RBAC system, but /token does not. It hands out an RH SSO client_credentials access token to any caller with a user-cookie — no RBAC check is performed. The backend acts as an open proxy for minting SSO tokens.
The caller doesn't read clientId/clientSecret directly (those stay server-side), but the backend uses them on the caller's behalf and returns the minted token. Combined with guest auth + auto sign-in, every visitor automatically gets a privileged api.console SSO token.
sequenceDiagram
actor Visitor as Anonymous Visitor
participant App as Backstage Frontend
participant BE as DCM Backend<br/>/api/dcm
participant SSO as Red Hat SSO
Note over Visitor,App: Guest auth + auto sign-in is enabled
Visitor->>App: Opens browser
App->>App: Auto sign-in as guest
App-->>Visitor: Session cookie set
Note over Visitor,SSO: ⚠ No permission check anywhere in this flow
Visitor->>BE: GET /api/dcm/token (user-cookie)
Note right of BE: Auth policy: allow user-cookie ✓<br/>RBAC check: NONE ✗
BE->>BE: Read dcm.clientId +<br/>dcm.clientSecret from config
BE->>SSO: POST /auth/realms/.../token<br/>grant_type=client_credentials<br/>scope=api.console
SSO-->>BE: { access_token, expires_in }
BE-->>Visitor: { accessToken: "eyJ...", expiresAt: ... }
Note over Visitor: Visitor now holds a privileged<br/>api.console SSO token
rect rgba(100, 200, 100, 0.1)
Note over Visitor,SSO: Compare: /access correctly checks RBAC
Visitor->>BE: GET /api/dcm/access (user-cookie)
BE->>BE: httpAuth.credentials(req)
BE->>BE: permissions.authorize([dcmPluginReadPermission])
BE-->>Visitor: { decision: ALLOW | DENY }
end
Fix: Add the same authorize() check from /access to the /token handler, so that only users with dcmPluginReadPermission can obtain a token.
Guest user as RBAC superUser
app-config.yaml grants user:default/guest both admin and superUser roles while guest auth + auto sign-in is enabled. This bypasses all RBAC. If this is intentional for dev, add a clear comment (e.g. # DEV ONLY) or move it to app-config.local.yaml.
Bugs / Correctness
Token caching utility exists but is never used
util/tokenUtil.ts implements getTokenFromApi with proper cache-backed flow (60s expiry buffer, Backstage CacheService), yet routes/token.ts calls RH SSO directly on every request — duplicating the fetch logic without caching. Under load this will hammer the SSO endpoint.
Fix: Wire the /token route to use getTokenFromApi from tokenUtil.ts.
config is optional in RouterOptions but asserted at runtime
RouterOptions.config is typed as config?: RootConfigService (optional), but both /token and tokenUtil.ts immediately assert(typeof config !== 'undefined'). If config is required for the plugin to function (it is), make it required in the interface and let TypeScript catch it at compile time.
License header typo
Multiple files have "CONDITIONS OF THE License" instead of "CONDITIONS OF THE LICENSE" (line 12 in routes/token.ts, routes/access.ts, plugin.ts, util/tokenUtil.ts, permissions.ts).
Dead Code / Cleanup
services/TodoListService.ts— the entire template scaffold (157 lines) is still present and not imported anywhere. Remove it.zoddependency — no longer used after the/todosendpoints were removed. Remove frompackage.json.- Unused deps from template —
@backstage/catalog-client,@backstage/errors,@backstage/plugin-catalog-node,@backstage/typesare only referenced by the deadTodoListService.ts. Remove them.
Test Coverage
- Both test files (
router.test.ts,plugin.test.ts) only test/health. There are zero tests for/tokenand/access. - The router test doesn't pass
config, so any test hitting/tokenwould crash on theassert. - Suggested minimum: test
/accessreturns ALLOW/DENY based on mocked permission decisions; test/tokenwith a mockedfetch.
Frontend Notes
- Active route matching is fragile —
location.pathname === '/dcm'won't highlight the sidebar for sub-routes like/dcm/clusters/123. UsestartsWithinstead. - Repetitive theme casting —
(theme.palette as { type?: string; mode?: string }).type === 'dark'appears 4+ times. Extract a helper. - Numerous
!importantoverrides — suggests the styles are fighting upstream sidebar styles. Consider more specific selectors or theme mechanisms.
Summary of Action Items
| Priority | Item |
|---|---|
| P0 | Add permission check to /token endpoint |
| P1 | Wire /token to use getTokenFromApi (cache-backed) |
| P1 | Make config required in RouterOptions |
| P1 | Delete TodoListService.ts and remove unused deps (zod, catalog-client, errors, catalog-node, types) |
| P2 | Add tests for /token and /access |
| P2 | Fix license header typo (License → LICENSE) |
| P2 | Use startsWith for sidebar active state |
| P3 | Add comment about guest superUser being dev-only |
| P3 | Extract dark-mode detection helper in Root.tsx |
| P3 | Add missing changesets |
2 participants
FLPATH-3243 | [UI] Plugin skeleton - App shell and navigation