Po- 5773 Tech Debt - Create reusable business-unit route permission guard#2709
Po- 5773 Tech Debt - Create reusable business-unit route permission guard#2709TimDanielsCQI wants to merge 30 commits into
Conversation
TimDanielsCQI
added
rel:opal-frontend-pr-2709
and removed
rel:opal-frontend-pr-5773
labels
|
@TimDanielsCQI could you also run |
Arnabsubedi233
left a comment
Arnabsubedi233
left a comment
There was a problem hiding this comment.
just some small changed
| import { IOpalFinesAccountMinorCreditorDetailsHeader } from '../../fines-acc-minor-creditor-details/interfaces/fines-acc-minor-creditor-details-header.interface'; | ||
| import { BusinessUnitIdResolver } from '@hmcts/opal-frontend-common/guards/business-unit-route-permissions'; | ||
|
|
||
| type FinesAccountRouteType = 'defendant' | 'minor-creditor'; |
There was a problem hiding this comment.
could we put this in its own file then import.
There was a problem hiding this comment.
Agreed as it is used in two places. Make a types folder within this resolver folder and and move it there then it can be used here and on the spec.ts
| return storedBusinessUnitId; | ||
| } | ||
|
|
||
| if (accountType === 'defendant') { |
There was a problem hiding this comment.
can we remove the hard coding of 'defendant' etc and then import that in as a constant in a file.
| import { IOpalFinesAccountMinorCreditorDetailsHeader } from '../../fines-acc-minor-creditor-details/interfaces/fines-acc-minor-creditor-details-header.interface'; | ||
| import { BusinessUnitIdResolver } from '@hmcts/opal-frontend-common/guards/business-unit-route-permissions'; | ||
|
|
||
| type FinesAccountRouteType = 'defendant' | 'minor-creditor'; |
There was a problem hiding this comment.
Agreed as it is used in two places. Make a types folder within this resolver folder and and move it there then it can be used here and on the spec.ts
iamfrankiemoran
left a comment
iamfrankiemoran
left a comment
There was a problem hiding this comment.
Sorry just noticed the naming convention as well
|
I think I have made all requested changes now. Build is failing on functional tests (on master as well) so I can't see sonar cube and coverage yet. |
4 participants
Jira link
See PO-5773
Change description
Current Position:
Some account maintenance routes rely on UI-level business-unit permission checks before navigating, but the route guards currently only check global permission IDs. This means the route layer cannot consistently enforce "user has permission X in the account's business unit" across direct navigation scenarios. We cannot reset FinesAccountStore on these journeys because amend/change/add pages depend on it.
Problem
routePermissionsGuard is generic and only checks whether the user has a permission anywhere. It does not know which business unit applies to the route. For account journeys, the relevant BU comes from account state or account heading data, not from static route configuration.
Common library dependency note
This PO-5773 change spans two repositories:
The frontend branch depends on new common-library behaviour introduced in the matching
opal-frontend-common-ui-libPO-5773 branch. Specifically, the common library adds/updates the BU-aware route permission guard so account-scoped denied routes are resolved relative to the current account route.During local testing, the frontend was run against a locally packed common-library tarball. That local tarball dependency has not been committed, because a
file:dependency would only work on a developer machine and would break CI/QA.At the moment, the committed
opal-frontendbranch still points at the currently published common library version:"@hmcts/opal-frontend-common": "^0.0.82"
Testing done
Unit tests
Browser testing
This the test plan I used to regression test all the routes that now use this generic guard:
PO-5773_ROUTE_BROWSER_TEST_PLAN.xls
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist