Skip to content

Commit

Permalink
Remove strict authorization flow configuration.
Browse files Browse the repository at this point in the history 
Now all users must have at least one associated privilege and access rule. Previously, none strict connections did not require an access rule.
  • Loading branch information
@Gcolon021
Gcolon021 committed Dec 17, 2024
1 parent d7934dd commit 317ba1f
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 50 deletions.
55 changes: 11 additions & 44 deletions ...a/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,13 @@ public class AuthorizationService {
protected SessionService sessionService;
private final RoleService roleService;

/**
* Applications that have strict access control. If the application is strict a user must have both privileges and access rules.
* If the application is not strict, the user only needs privileges. Access rules are optional.
*/
private final Set<String> strictConnections = new HashSet<>();

@Autowired
public AuthorizationService(AccessRuleService accessRuleService,
SessionService sessionService,
RoleService roleService,
@Value("${strict.authorization.applications.connections}") String strictConnections) {
RoleService roleService) {
this.accessRuleService = accessRuleService;
this.sessionService = sessionService;
this.roleService = roleService;
if (strictConnections != null && !strictConnections.isEmpty()) {
this.strictConnections.addAll(Arrays.asList(strictConnections.split(",")));
}
}

/**
Expand Down Expand Up @@ -143,33 +133,10 @@ public boolean isAuthorized(Application application, Object requestBody, User us
return false;
}

Set<AccessRule> accessRules;
String label = "";
if (user.getConnection() != null) {
// Open Access doesn't currently use a connection
label = user.getConnection().getLabel();
}

if (this.strictConnections.contains(label)) {
accessRules = this.accessRuleService.getAccessRulesForUserAndApp(user, application);
if (accessRules.isEmpty()) {
logger.info("ACCESS_LOG ___ {},{},{} ___ has been denied access to execute query ___ {} ___ in application ___ {} ___ NO ACCESS RULES EVALUATED", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName);
return false;
}
} else {
Set<Privilege> privileges = user.getPrivilegesByApplication(application);
// List all privileges of the user
logger.info("ACCESS_LOG ___ {},{},{} ___ has the following privileges: {}", user.getUuid().toString(), user.getEmail(), user.getName(), privileges.stream().map(Privilege::getName).collect(Collectors.joining(", ")));
if (privileges == null || privileges.isEmpty()) {
logger.info("ACCESS_LOG ___ {},{},{} ___ has been denied access to execute query ___ {} ___ in application ___ {} __ USER HAS NO PRIVILEGES ASSOCIATED TO THE APPLICATION, BUT APPLICATION HAS PRIVILEGES", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName);
return false;
}

accessRules = this.accessRuleService.cachedPreProcessAccessRules(user, privileges);
if (accessRules.isEmpty()) {
logger.info("ACCESS_LOG ___ {},{},{} ___ has been granted access to execute query ___ {} ___ in application ___ {} ___ NO ACCESS RULES EVALUATED", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName);
return true;
}
Set<AccessRule> accessRules = this.accessRuleService.getAccessRulesForUserAndApp(user, application);
if (accessRules.isEmpty()) {
logger.info("ACCESS_LOG ___ {},{},{} ___ has been denied access to execute query ___ {} ___ in application ___ {} ___ NO ACCESS RULES EVALUATED", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName);
return false;
}

logger.info("ACCESS_LOG ___ {},{},{} ___ has the following access rules: {}", user.getUuid().toString(), user.getEmail(), user.getName(), accessRules.stream().map(AccessRule::toString).collect(Collectors.joining(", ")));
Expand All @@ -180,9 +147,9 @@ public boolean isAuthorized(Application application, Object requestBody, User us
Set<AccessRule> failedRules = evaluationResult.failedRules();

logger.info("ACCESS_LOG ___ {},{},{} ___ has been {} access to execute query ___ {} ___ in application ___ {} ___ {}", user.getUuid().toString(), user.getEmail(), user.getName(), (result ? "granted" : "denied"), formattedQuery, applicationName, (result ? "passed by " + passRuleName : "failed by rules: ["
+ failedRules.stream()
.map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName()))
.collect(Collectors.joining(", ")) + "]"));
+ failedRules.stream()
.map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName()))
.collect(Collectors.joining(", ")) + "]"));

return result;
}
Expand Down Expand Up @@ -240,9 +207,9 @@ public boolean openAccessRequestIsValid(Map<String, Object> inputMap) {
Set<AccessRule> failedRules = evaluationResult.failedRules();

logger.info("ACCESS_LOG ___ AN OPEN ACCESS USER ___ has been {} access to execute query ___ {} ___ in application ___ OPEN ACCESS ___ {}", (result ? "granted" : "denied"), requestBody, (result ? "passed by " + passRuleName : "failed by rules: ["
+ failedRules.stream()
.map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName()))
.collect(Collectors.joining(", ")) + "]"));
+ failedRules.stream()
.map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName()))
.collect(Collectors.joining(", ")) + "]"));

return result;
}
Expand Down
4 changes: 0 additions & 4 deletions pic-sure-auth-services/src/main/resources/application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ fence.parent.consent.group.concept.path=\\_consents\\
fence.topmed.consent.group.concept.path=\\_topmed_consents\\
fence.variant.annotation.columns=Variant_consequence_calculated,Variant_class,Gene_with_variant,Variant_severity,Variant_frequency_in_gnomAD,Variant_frequency_as_text

# We have two different authorization flows. One is strict and the other is not strict.
# Strict requires both access rules and privilege rules to be present for the user.
strict.authorization.applications.connections=${STRICT_AUTHORIZATION_APPLICATIONS:OKTA,FENCE,OPEN,RAS}

# Application Token configurations.
# This configuration is used to generate a token for the application to access the PIC-SURE API.
application.client.secret=${APPLICATION_CLIENT_SECRET}
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AuthorizationServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void setUp() {
SecurityContextHolder.setContext(securityContext);

accessRuleService = new AccessRuleService(accessRuleRepository, "false", "false", "false", "false","false", "false");
authorizationService = new AuthorizationService(accessRuleService, sessionService, roleService,"fence,okta,open");
authorizationService = new AuthorizationService(accessRuleService, sessionService, roleService);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...u/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ public void setUp() {

when(sessionService.isSessionExpired(any(String.class))).thenReturn(false);
accessRuleService = new AccessRuleService(accessRuleRepository, "false", "false", "false", "false","false", "false");
authorizationService = new AuthorizationService(accessRuleService, sessionService, roleService, "fence,okta");
authorizationService = new AuthorizationService(accessRuleService, sessionService, roleService);
}

@Test
Expand Down

0 comments on commit 317ba1f

Please sign in to comment.