Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update how we save and log org claims on whoami calls #8254

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Update how we save and log org claims on whoami calls #8254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,12 +576,21 @@ private ApiUser getCurrentApiUserNoCache() {
return nonOktaUser.orElseGet(() -> getCurrentApiUserFromIdentity(userIdentity));
}

/*
`getCurrentUserInfoForWhoAmI()` can be removed and replaced with `getCurrentUserInfo()` as part of #7602 or whenever we stop migrating users over from Okta
*/
public UserInfo getCurrentUserInfoForWhoAmI() {
ApiUser currentUser = getCurrentApiUser();

Optional<OrganizationRoles> currentOrgRoles = _orgService.getCurrentOrganizationRoles();
boolean isAdmin = _authService.isSiteAdmin();
if (!_featureFlagsConfig.isOktaMigrationEnabled() && !isAdmin) {
setRolesAndFacilities(currentOrgRoles, currentUser);
if (!isAdmin) {
if (currentOrgRoles.isPresent()) {
PartialOktaUser oktaUser = _oktaRepo.findUser(currentUser.getLoginEmail());
return consolidateUser(currentUser, oktaUser);
Copy link
Collaborator Author

@emyl3 emyl3 Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Making use of the consolidateUser method that also handles the comparison check, logging (regardless of the status of the feature flag) and setting the user's role and facilities (if the feature flag is off)

} else {
log.info("No org roles for User ID: {}", currentUser.getInternalId());
}
}
return new UserInfo(currentUser, currentOrgRoles, isAdmin);
}
Expand Down
10 changes: 2 additions & 8 deletions backend/src/main/java/gov/cdc/usds/simplereport/service/LoggedInAuthorizationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,9 @@ public List<OrganizationRoleClaims> findAllOrganizationRoles() {
List<OrganizationRoleClaims> oktaOrgRoleClaims =
_extractor.convert(currentAuth.getAuthorities());

if (!isSiteAdmin()) {
if (!isSiteAdmin() && _featureFlagsConfig.isOktaMigrationEnabled()) {
String username = currentAuth.getName();
List<OrganizationRoleClaims> dbOrgRoleClaims =
Copy link
Collaborator Author

@emyl3 emyl3 Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Before I was doing the comparison check way lower in the stack for whoami calls.

Moved the check to the level of the getCurrentUserInfoForWhoAmI method to avoid instances where users who were not migrated were being checked (thus, logging an error).

_dbOrgRoleClaimsService.getOrganizationRoleClaims(username);
_dbOrgRoleClaimsService.checkOrgRoleClaimsEquality(
oktaOrgRoleClaims, dbOrgRoleClaims, username);
if (_featureFlagsConfig.isOktaMigrationEnabled()) {
return dbOrgRoleClaims;
}
return _dbOrgRoleClaimsService.getOrganizationRoleClaims(username);
}
return oktaOrgRoleClaims;
}
Expand Down
Loading