feat!: country configurable user scopes & roles #7301

rikukissa · 2024-07-02T06:36:46Z

Ticket: #6455
Farajaland: opencrvs/opencrvs-farajaland#1042

The design for change is quite disruptive and non-backwards compatible. I started exploring this from a more incremental direction but eventually decided it's better to do a larger permission overhaul now so we can then just focus on specifying more scopes as per country needs.

Changes

systemRole is now deprecated. Only the simple role of the user is being used, and matched against the default role configuration, or country config roles. The auth-microservice now provides the scopes.
The same workqueue query is now used for every user. This will get reworked with configurable workqueues.
Frontend now uses scopes for access management using usePermissions hook.
The frontend gateway types generation now also includes Scope and scopes.

To-do

Define a list of scopes in the commons package
- Re-sync the scope list from Notion
- Create the default system roles and scopes for Farajaland (and core defaults) based on this Notion document
Remove the concept of systemRole completely. Identify where these roles are still referred at and create as narrowed down scopes as possible using @jpye-finch 's permissions table as a guide

Frontend

Work through the @TODO comments and see JPF's answers in this PR.
What to do for persistence middleware. Design and talk with JPF: how does this persistenceMiddleware work now that a FIELD_AGENT role is not there anymore? Consider that configurable workqueues are also coming for 1.7
Based off Navigation.tsx, create a useNavigation -hook. Have the useNavigation hook return a list of the navigation items, that the Navigation.tsx then renders. Also, have the useNavigation return a homePage which is the first item of the array. Then use this in Home.tsx and HistoryNavigator.tsx to remove the useHomePage -hook.
You need to define which scopes are needed for routes using <ProtectedRoute roles={[. This should become scopes=[. Design: Could we define the routes so, that useNavigation could also use the scopes configuration to figure out what navigation items the users have?
Remove the remaining old user scopes in frontend. (references to 'sysadmin' and 'natlsysadmin')
Country config adds demo scope for all system roles when it's not a production (implemented here)
Rework tests that use systemRole. The sanity of some can be considered, if the test is not meaningful anymore; feel free to delete it or write a new one that is more connected to the current application logic.
Explore: In FHIR practitioner extensions, "http://opencrvs.org/specs/types" are these labels stringified JSON array used for something? If not, then remove with migration.
We should force clients with an older version of OpenCRVS previously install to refetch their user details. Ensure clients and server are always on the same version #6695 is related

Backend

packages/gateway/src/features/registration/utils.ts:57 the English role label was stored in the certificate collector details, not the role id. How does changing this to userDetails.role affect things? Is a migration needed?
Write a migration to remove user scopes from database. I've started writing the migration in this PR so use that as the base.
Remove the old user scopes for example 'certify' and 'declare': use the new user scopes in backend.
- Design: Consider how searchEvents would now work, when different user roles fetch all of the workqueues. Return an empty array of records if you don't have access? Scopes: Add search scopes to control what records a user can search for #6909

Locations

Design: Could we only return the locations from Location API the user has access to (separate PR/ticket!)

Tests & maintenance

Using the Farajaland PR draft a country config PR to make required changes in country config generic for all countries.
Go through the existing users in default-employees.csv. Go through that the navigation items look the same, comparing to QA or farajaland-dev.
Document GraphQL API changes to changelog
Test system clients as in Webhook integration clients still work ok

Unrelated technical requirements identified

Automatically regenerate client GraphQL types on code or gateway schema changes

packages/client/src/utils/userUtils.ts

naftis · 2024-07-03T08:05:07Z

packages/user-mgnt/src/server.ts

+      response: {
+        failAction: async (req, _2, err: Boom) => {
+          if (process.env.NODE_ENV === 'production') {
+            // In prod, log a limited error message and throw the default Bad Request error.
+            logger.error(`Response validationError: ${err.message}`)
+            throw badRequest(`Invalid response payload returned from handler`)
+          } else {
+            // During development, log and respond with the full error.
+            logger.error(
+              `${req.path} response has a validation error: ${err.message}`
+            )
+            throw err
+          }
+        }
+      },


#7295
Also good to amend for workflow

packages/auth/package.json

packages/auth/src/features/authenticate/handler.test.ts

rikukissa · 2024-07-05T08:05:40Z

packages/auth/src/features/authenticate/handler.test.ts

          status: 'active',
-          scope: ['admin'],
+          role: 'NATIONAL_SYSTEM_ADMIN',


user-mgnt doesn't return scopes anymore as it doesn't know what scopes are. Scopes are now controlled by CC's /roles endpoint in combination with the auth service which creates JWT tokens.

packages/auth/src/features/authenticate/handler.ts

.github/workflows/lint-and-test.yml

rikukissa · 2024-07-05T14:01:23Z

packages/auth/src/features/authenticateSuperUser/handler.ts

@@ -36,9 +38,16 @@ export default async function authenticateSuperUserHandler(
    throw unauthorized()
  }

+  if (result.status === 'deactivated') {


This is just to improve error messaging when you try to seed again after seeding once

packages/auth/src/features/authenticate/handler.ts

packages/auth/src/features/scopes/service.ts

packages/auth/tsconfig.json

packages/client/package.json

rikukissa · 2024-07-05T14:09:21Z

packages/client/src/forms/index.ts

@@ -1257,3 +1257,37 @@ export interface ICertificate {
  payments?: Payment[]
  data?: string
 }
+
+export function modifyFormField(


Improvement: Related to a small refactor I made here while debugging something

packages/user-mgnt/src/config/routes.ts

rikukissa · 2024-07-05T18:26:34Z

packages/user-mgnt/src/features/createUser/handler.ts

-      (process.env.NODE_ENV === 'development' || QA_ENV) &&
-      !userScopes.includes('demo')
-    ) {
-      userScopes.push('demo')


Good thing to notice is that the addition of the demo scope (used to control 2FA of users, email/sms notifications) is now made the responsibility of CC.

rikukissa · 2024-07-05T18:28:24Z

packages/user-mgnt/src/features/createUser/service.ts

@@ -111,14 +100,6 @@ export const createFhirPractitionerRole = async (
              code: user.systemRole
            }
          ]
-        },
-        {
-          coding: [


I wasn't able to immediately find if these are used or if they are used at all. I decided to remove these as end user friendly text labels shouldn't be stored in the database but we should use role ids and message objects instead

rikukissa · 2024-07-05T18:37:57Z

packages/auth/package.json

@@ -7,7 +7,7 @@
  "scripts": {


Auth service

The auth service's new responsibility is to, as part of authentication, connect scopes coming from CC to user's role field. This is done every time a user needs a JWT token

rikukissa · 2024-07-05T18:40:06Z

packages/client/package.json

@@ -12,7 +12,7 @@
    "build": "VITE_APP_VERSION=$VERSION NODE_OPTIONS=--max_old_space_size=8000 vite build",


Client

The most significant change is that the data model for user.role looks now a little different. Instead of a labels array with a single entity label for each supported language, the client now only receives a singular label, the is a message descriptor object. This object needs to be passed through react-intl to get the end-user friendly string.

rikukissa · 2024-07-05T18:41:29Z

packages/client/src/App.test.tsx

@@ -127,39 +127,3 @@ describe('when user has a valid token in local storage', () => {
    expect(loadLocations).toHaveBeenCalled()
  })
 })
-
-describe('it handles react errors', () => {


Improvement: I changed the error boundaries so that when running unit tests, errors won't get swallowed but instead are console.logged and thrown as you would expect in a test run

rikukissa · 2024-07-05T18:42:09Z

packages/client/src/components/StyledErrorBoundary.tsx

@@ -47,6 +47,10 @@ const StyledErrorBoundaryComponent = ({ intl, children }: IFullProps) => {
  const [authError, setAuthError] = useState(false)

  const onError = (error: Error) => {
+    if (import.meta.env.MODE === 'test') {


Improvement: part of https://github.com/opencrvs/opencrvs-core/pull/7301/files#r1667055909

rikukissa · 2024-07-05T18:43:23Z

packages/client/src/forms/user/query/queries.ts

@@ -11,35 +11,25 @@
 import { gql } from '@apollo/client'
 import { client } from '@client/utils/apolloClient'

-export const getSystemRolesQuery = gql`
-  query getSystemRoles($value: ComparisonInput) {


Not sure why we had these filtering and search parameters here in the first place

packages/client/src/i18n/reducer.ts

rikukissa · 2024-07-05T18:46:58Z

packages/client/src/setupTests.ts

+vi.mock('@client/utils/authApi', () => ({
+  invalidateToken: () => Promise.resolve()
+}))
+vi.mock('@client/pdfRenderer', () => ({
+  printPDF: () => Promise.resolve()
+}))


@Zangetsu101 as per my testing these weren't working previously but now are 😮

rikukissa · 2024-07-05T18:50:38Z

packages/client/src/user/userReducer.ts

@@ -65,21 +65,15 @@ const initialState: IUserFormState = {
  userDetailsStored: false,
  submitting: false,
  loadingRoles: false,
+  userRoles: [],


This could also be a React hook.. We just need a good template example of how to write offline-capable data fetch hooks 🤔

Why do we need to know the user roles? Can't we get the scopes from the JWT token?

Writing offline-capable fetch hooks should be possible via gateway using Apollo & fetchPolicy 'cache-first' or 'cache-and-network'

rikukissa · 2024-07-05T19:02:51Z

packages/client/vite.config.ts

@@ -90,7 +91,9 @@ export default defineConfig(({ mode }) => {
    },
    resolve: {
      alias: {
-        crypto: 'crypto-js'
+        crypto: 'crypto-js',
+        '@opencrvs/commons/build/dist/authentication':


Sorry, couldn't get both the TS compiler and vitest to agree with each other without this

rikukissa · 2024-07-05T19:03:45Z

packages/commons/src/http.ts

+  }
+}
+
+export async function fetchJSON<ResponseType = any>(


Our new shared HTTP client

ocrvs-7436: misc bugs

ocrvs-7419: misc bugs

ocrvs-7421: misc bugs

feat: separate query for sent for review tab

* fix: use correct scopes for request, approve, reject and make correction in root-resolver * fix: show `sentForApproval` tab for `RECORD_REGISTRATION_REQUEST_CORRECTION` scope * fix: unit test: update scope for req correction

* fix: allow fetchRegistration for actionable scopes * fix: only self assigned & unassign scope can unassign * fix: require assignment for print & issue actions * test: assignment test for certify & issue * fix: add missing actionable scopes

* fix: add missing scopes for outbox * fix: remove declare scopes from update action * fix: allow update scope to change role * test: update navigation tab scopes

This reverts commit 935d5de.

* fix: amend active tab id of advance search * fix: hide advance searching based on scopes * fix: remove marriage scope from advance search component permission scope * chore: refactor advance search section to receive param * chore: refactor advance search section creator with user office id * chore: add filter location function * fix: add comment for additional props * fix: refactor variable name * fix: amend accordion state function with office id * fix: update advance search test with user details

* fix: update unassign conditional query * fix: update wq record assignment value while assign and unassign based on hearth bundle * Revert "fix: update wq record assignment value while assign and unassign based on hearth bundle" This reverts commit b6a4aa7. * fix: add assignment property in declaration

* fix: update new sys admin scope in events middleware * fix: update scopes in events package

rikukissa added this to the v1.7.0 milestone Jul 2, 2024

rikukissa changed the title ~~Country configurable user scopes & roles~~ [OCRVS-6455] Country configurable user scopes & roles Jul 2, 2024

rikukissa mentioned this pull request Jul 2, 2024

Custom Users & Scopes (v1) #6455

Open

3 tasks

rikukissa added the 🚧 work in progress label Jul 2, 2024

naftis reviewed Jul 3, 2024

View reviewed changes

packages/client/src/utils/userUtils.ts Outdated Show resolved Hide resolved

naftis reviewed Jul 3, 2024

View reviewed changes