Fetch OrganizationRoleClaims from DB when feature flag enabled #8030

emyl3 · 2024-08-13T19:47:08Z

BACKEND PULL REQUEST

Related Issue

Continuation of Initiate rolling migration, phase 2 #7598
See "Additional Information" section for work that remains

Changes Proposed

When the feature flag is enabled, fetch role and facility information for users from DB
Creation of DbOrgRoleClaimsService
- handles transforming a ApiUsers role and facility information to OrganizationRoleClaims and returning those claims
- method introduced to check for equality between the OrganizationRoleClaims from Okta and the DB and log if there is an error
- Refactoring of consolidateUser method in ApiUserService

Additional Information

Remaining work (will be introduced in separate PRs)
1. behind the feature flag, leverage our DB instead of okta's group api endpoint to fetch group information for orgs
2. utilize the method for checking equality of roles from Okta and DB and create an alert when this they are different

Testing

Envs

oktaMigrationEnabled FALSE on dev2
- dev2 metabase link to see roles and facilities at a glance
- roles and facilities should be added whenever we read role/facility info from Okta
oktaMigrationEnabled TRUE on dev3
- dev3 metabase link to see roles and facilities at a glance
- roles and facilities should be not be updated when we read role/facility info from Okta
- when we update role and facilities, we should still be updating Okta and DB
- list of users in the org will be incorrect and still fetched from Okta until bullet point number 1 from "Remaining work" is completed

Some scenarios to try in both environments:

Site Admin role
- ghost into different orgs (no role/facility saved in DB)
- can manage users
  - send account setup email
  - edit name
  - edit email
  - reset password
  - reset MFA
  - delete user
  - update organization/facility/role access
- add organization admin
Org Admin role
- can manage users
  - send account setup email
  - edit name
  - edit email
  - reset password
  - reset MFA
  - delete user
  - update facility/role access
- add user

⚠️ On dev3, when fetching a user that is not in the DB (e.g. [email protected]), it will show the following due to a MisconfiguredUserException being thrown:

I think this makes sense if we are only enabling this flag after all users have been migrated over. Once the other work goes in, this user wouldn't be appear in the "Manage users" page so this error wouldn't be visible to end-users.

emyl3 · 2024-08-15T18:15:23Z

backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java

-    OrganizationRoleClaims claims = optClaims.orElseThrow(UnidentifiedUserException::new);
-
-    // use the target user's org so response is built correctly even if site admin is the requester
-    Organization org = _orgService.getOrganization(claims.getOrganizationExternalId());


⚠️ Let me know if I am missing something but I found the logic from what was previously lines 732-743 unnecessary because we can get OrganizationRoles from the OrganizationRoleClaims using this OrganizationService method.

emyl3 · 2024-08-15T18:16:30Z

backend/src/main/java/gov/cdc/usds/simplereport/service/DbOrgRoleClaimsService.java

+   * @param dbClaims - List<OrganizationRoleClaims> from DB
+   * @return boolean
+   */
+  public boolean checkOrgRoleClaimsEquality(


⚠️ Currently only using this in the tests but will use it later to introduce logging and alerting based on differing org roles from Okta vs DB

...ain/java/gov/cdc/usds/simplereport/config/authorization/DemoAuthenticationConfiguration.java

backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java

backend/src/main/java/gov/cdc/usds/simplereport/service/DbOrgRoleClaimsService.java

fzhao99

great to see this started elisa! left some initial questions for you

mpbrown

Tested in dev2 and dev3 and things look good overall! Great job with all this migration work!

sonarqubecloud · 2024-08-21T15:37:19Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

emyl3 · 2024-08-21T18:31:19Z

@mpbrown @fzhao99 ready for re-review! (redeployed changes to dev2 and dev3)

fzhao99

thanks so much for your hard work on this elisa!

backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java

mehansen

thank you for all your hard work on this! it's so cool to see this coming together 🤩

do you think we need to use the db roles for this permission check too?
https://github.com/CDCgov/prime-simplereport/blob/refs/heads/elisa/7598-read-roles-from-db/backend/src/main/java/gov/cdc/usds/simplereport/config/authorization/UserAuthorizationVerifier.java#L100

mehansen · 2024-08-23T17:35:01Z

backend/src/main/java/gov/cdc/usds/simplereport/service/DbOrgRoleClaimsService.java

this is just a thought, not feedback - I haven't looked at all the usages of OrganizationRoleClaims but do you think we'll be able to clean that up as a concept entirely once the migration is complete? it seems like we mostly use it to hold the values of org, facilities, roles that we've pulled from the Okta groups before we can convert them into their objects.

Yes, I think you are right! Once fully migrated, we will be able to clean it up!! 😮‍💨 🧹 🤩

mehansen · 2024-08-23T19:23:08Z

forgive me if these are included in the remaining work you called out, but I noticed a couple unexpected places where we're using Okta groups as source of truth:

getting count of users in facility (I think for support admin delete facility tool)
activating org admins and getting their emails
check to see if we're reprovisioning a user in a different org
getting the org to use when updating user privileges

emyl3 · 2024-08-23T19:30:47Z

forgive me if these are included in the remaining work you called out, but I noticed a couple unexpected places where we're using Okta groups as source of truth:

getting count of users in facility (I think for support admin delete facility tool)

activating org admins and getting their emails

check to see if we're reprovisioning a user in a different org

getting the org to use when updating user privileges

I didn't explicitly call these out but I was going to search the entire repo for where we were calling the GroupApi and bundle all that work together. This is a very helpful starting point to make sure I have all the different usages covered 🙌 Thank you, Merethe! 🗒️

emyl3 · 2024-08-23T20:27:37Z

thank you for all your hard work on this! it's so cool to see this coming together 🤩

do you think we need to use the db roles for this permission check too? https://github.com/CDCgov/prime-simplereport/blob/refs/heads/elisa/7598-read-roles-from-db/backend/src/main/java/gov/cdc/usds/simplereport/config/authorization/UserAuthorizationVerifier.java#L100

ohhhh nice catch! I will update this 😅 Thank you!!! 🙌 🐛 🔍

mehansen · 2024-08-23T21:23:04Z

I didn't explicitly call these out but I was going to search the entire repo for where we were calling the GroupApi and bundle all that work together. This is a very helpful starting point to make sure I have all the different usages covered 🙌 Thank you, Merethe! 🗒️

oh nice that should definitely do it. everything looks good! the permission check update could totally go in a follow-up PR too if you want to just merge this

emyl3 · 2024-08-27T14:46:37Z

I didn't explicitly call these out but I was going to search the entire repo for where we were calling the GroupApi and bundle all that work together. This is a very helpful starting point to make sure I have all the different usages covered 🙌 Thank you, Merethe! 🗒️

oh nice that should definitely do it. everything looks good! the permission check update could totally go in a follow-up PR too if you want to just merge this

I'll fix the permission check in a separate PR 🙏 Thank you for your flexibility.

emyl3 · 2024-08-27T15:54:09Z

Going to merge this in after demo 💪

emyl3 force-pushed the elisa/7598-read-roles-from-db branch 8 times, most recently from 3938acf to 7bea9fb Compare August 15, 2024 16:59

Fetch OrganizationRoleClaims from DB when feature flag enabled

d5ba960

emyl3 force-pushed the elisa/7598-read-roles-from-db branch from 7bea9fb to d5ba960 Compare August 15, 2024 17:59

emyl3 temporarily deployed to dev2 August 15, 2024 18:11 — with GitHub Actions Inactive

emyl3 commented Aug 15, 2024

View reviewed changes

emyl3 temporarily deployed to dev2 August 15, 2024 18:18 — with GitHub Actions Inactive

emyl3 temporarily deployed to dev2 August 15, 2024 18:22 — with GitHub Actions Inactive

emyl3 requested review from mehansen, DanielSass, mpbrown, fzhao99 and bobbywells52 August 16, 2024 16:36

emyl3 marked this pull request as ready for review August 16, 2024 16:36