From bf12d1b0c8f19105fa582c14e630376dcad80f24 Mon Sep 17 00:00:00 2001 From: Shaun Ford Date: Wed, 11 Jul 2018 15:47:54 -0700 Subject: [PATCH 1/5] Add feature flag for user group case sensitivity --- .../com/nike/cerberus/dao/PermissionsDao.java | 5 ++ .../nike/cerberus/dao/SafeDepositBoxDao.java | 4 ++ .../cerberus/mapper/PermissionsMapper.java | 3 + .../cerberus/mapper/SafeDepositBoxMapper.java | 2 + .../cerberus/service/PermissionsService.java | 67 ++++++++++++++++--- .../service/SafeDepositBoxService.java | 13 +++- src/main/resources/cms.conf | 4 +- .../cerberus/mapper/PermissionsMapper.xml | 23 +++++++ .../cerberus/mapper/SafeDepositBoxMapper.xml | 28 ++++++++ .../service/PermissionsServiceTest.java | 41 ++++++++++-- 10 files changed, 174 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/nike/cerberus/dao/PermissionsDao.java b/src/main/java/com/nike/cerberus/dao/PermissionsDao.java index 1a8b25bd7..f2f39174c 100644 --- a/src/main/java/com/nike/cerberus/dao/PermissionsDao.java +++ b/src/main/java/com/nike/cerberus/dao/PermissionsDao.java @@ -23,6 +23,7 @@ public class PermissionsDao { + private final PermissionsMapper permissionsMapper; @Inject @@ -37,4 +38,8 @@ public Boolean doesIamPrincipalHaveRoleForSdb(String sdbId, String iamPrincipalA public Boolean doesUserPrincipalHaveRoleForSdb(String sdbId, Set rolesThatAllowPermission, Set userGroupsThatPrincipalBelongsTo) { return permissionsMapper.doesUserPrincipalHaveGivenRoleForSdb(sdbId, rolesThatAllowPermission, userGroupsThatPrincipalBelongsTo); } + + public Boolean doesUserHavePermsForRoleAndSdbCaseInsensitive(String sdbId, Set rolesThatAllowPermission, Set userGroupsThatPrincipalBelongsTo) { + return permissionsMapper.doesUserHavePermsForRoleAndSdbCaseInsensitive(sdbId, rolesThatAllowPermission, userGroupsThatPrincipalBelongsTo); + } } diff --git a/src/main/java/com/nike/cerberus/dao/SafeDepositBoxDao.java b/src/main/java/com/nike/cerberus/dao/SafeDepositBoxDao.java index f00bc6131..191cae2d0 100644 --- a/src/main/java/com/nike/cerberus/dao/SafeDepositBoxDao.java +++ b/src/main/java/com/nike/cerberus/dao/SafeDepositBoxDao.java @@ -49,6 +49,10 @@ public List getUserAssociatedSafeDepositBoxes(final Set getUserAssociatedSafeDepositBoxesIgnoreCase(final Set userGroups) { + return safeDepositBoxMapper.getUserAssociatedSafeDepositBoxesIgnoreCase(userGroups); + } + public List getIamPrincipalAssociatedSafeDepositBoxes(final String iamPrincipalArn) { return safeDepositBoxMapper.getIamPrincipalAssociatedSafeDepositBoxes(iamPrincipalArn); } diff --git a/src/main/java/com/nike/cerberus/mapper/PermissionsMapper.java b/src/main/java/com/nike/cerberus/mapper/PermissionsMapper.java index ccd4ec4ec..3bed60b22 100644 --- a/src/main/java/com/nike/cerberus/mapper/PermissionsMapper.java +++ b/src/main/java/com/nike/cerberus/mapper/PermissionsMapper.java @@ -30,4 +30,7 @@ Boolean doesUserPrincipalHaveGivenRoleForSdb(@Param("sdbId") String sdbId, @Param("rolesThatAllowPermission") Set rolesThatAllowPermission, @Param("userGroupsThatPrincipalBelongsTo") Set userGroupsThatPrincipalBelongsTo); + Boolean doesUserHavePermsForRoleAndSdbCaseInsensitive(@Param("sdbId") String sdbId, + @Param("rolesThatAllowPermission") Set rolesThatAllowPermission, + @Param("userGroupsThatPrincipalBelongsTo") Set userGroupsThatPrincipalBelongsTo); } diff --git a/src/main/java/com/nike/cerberus/mapper/SafeDepositBoxMapper.java b/src/main/java/com/nike/cerberus/mapper/SafeDepositBoxMapper.java index d49b633a8..39f901748 100644 --- a/src/main/java/com/nike/cerberus/mapper/SafeDepositBoxMapper.java +++ b/src/main/java/com/nike/cerberus/mapper/SafeDepositBoxMapper.java @@ -34,6 +34,8 @@ public interface SafeDepositBoxMapper { List getUserAssociatedSafeDepositBoxes(@Param("userGroups") Set userGroups); + List getUserAssociatedSafeDepositBoxesIgnoreCase(@Param("userGroups") Set userGroups); + List getIamPrincipalAssociatedSafeDepositBoxes(@Param("iamPrincipalArn") final String iamPrincipalArn); SafeDepositBoxRecord getSafeDepositBox(@Param("id") String id); diff --git a/src/main/java/com/nike/cerberus/service/PermissionsService.java b/src/main/java/com/nike/cerberus/service/PermissionsService.java index 74f6328ca..27543814a 100644 --- a/src/main/java/com/nike/cerberus/service/PermissionsService.java +++ b/src/main/java/com/nike/cerberus/service/PermissionsService.java @@ -16,6 +16,7 @@ package com.nike.cerberus.service; +import com.google.inject.name.Named; import com.nike.backstopper.exception.ApiException; import com.nike.cerberus.SecureDataAction; import com.nike.cerberus.dao.PermissionsDao; @@ -30,31 +31,38 @@ import javax.inject.Inject; import javax.inject.Singleton; +import java.util.Collection; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import static com.nike.cerberus.record.RoleRecord.ROLE_OWNER; @Singleton public class PermissionsService { + public static final String USER_GROUPS_CASE_SENSITIVE = "cms.user.groups.caseSensitive"; + protected final Logger log = LoggerFactory.getLogger(getClass()); private final RoleService roleService; private final UserGroupPermissionService userGroupPermissionService; private final IamPrincipalPermissionService iamPrincipalPermissionService; private final PermissionsDao permissionsDao; + private final boolean userGroupsCaseSensitive; @Inject public PermissionsService(RoleService roleService, UserGroupPermissionService userGroupPermissionService, IamPrincipalPermissionService iamPrincipalPermissionService, - PermissionsDao permissionsDao) { + PermissionsDao permissionsDao, + @Named(USER_GROUPS_CASE_SENSITIVE) boolean userGroupsCaseSensitive) { this.roleService = roleService; this.userGroupPermissionService = userGroupPermissionService; this.iamPrincipalPermissionService = iamPrincipalPermissionService; this.permissionsDao = permissionsDao; + this.userGroupsCaseSensitive = userGroupsCaseSensitive; } /** @@ -78,9 +86,9 @@ public boolean doesPrincipalHaveOwnerPermissions(CerberusPrincipal principal, Sa } break; case USER: - if (principal.getUserGroups().contains(sdb.getOwner())) { - principalHasOwnerPermissions = true; - } + principalHasOwnerPermissions = userGroupsCaseSensitive ? + principal.getUserGroups().contains(sdb.getOwner()) : + containsIgnoreCase(principal.getUserGroups(), sdb.getOwner()); break; } return principalHasOwnerPermissions; @@ -124,10 +132,12 @@ public boolean doesPrincipalHaveReadPermission(CerberusPrincipal principal, Stri case USER: // if the the principal is a user principal ensure that one of the users groups is associated with the sdb Set userGroupPermissions = userGroupPermissionService.getUserGroupPermissions(sdbId); - principalHasPermissionAssociationWithSdb = userGroupPermissions - .stream() - .filter(perm -> principal.getUserGroups().contains(perm.getName())) // filter for permissions on the sdb that have groups that the authenticated user belongs too. - .count() > 0; // if there is more than one, then the SDB is has read, write or owner all of which allow read. + Set userGroups = userGroupPermissions.stream() + .map(UserGroupPermission::getName) + .collect(Collectors.toSet()); + principalHasPermissionAssociationWithSdb = userGroupsCaseSensitive ? + doesHaveIntersection(userGroups, principal.getUserGroups()) : + doesHaveIntersectionIgnoreCase(userGroups, principal.getUserGroups()); break; } return principalHasPermissionAssociationWithSdb; @@ -140,7 +150,9 @@ public boolean doesPrincipalHavePermission(CerberusPrincipal principal, String s hasPermission = permissionsDao.doesIamPrincipalHaveRoleForSdb(sdbId, principal.getName(), action.getAllowedRoles()); break; case USER: - hasPermission = permissionsDao.doesUserPrincipalHaveRoleForSdb(sdbId, action.getAllowedRoles(), principal.getUserGroups()); + hasPermission = userGroupsCaseSensitive ? + permissionsDao.doesUserPrincipalHaveRoleForSdb(sdbId, action.getAllowedRoles(), principal.getUserGroups()) : + permissionsDao.doesUserHavePermsForRoleAndSdbCaseInsensitive(sdbId, action.getAllowedRoles(), principal.getUserGroups()); break; default: log.error("Unknown Principal Type: {}, returning hasPermission: false", principal.getPrincipalType().getName()); @@ -156,4 +168,41 @@ public boolean doesPrincipalHavePermission(CerberusPrincipal principal, String s return hasPermission; } + + /** + * Does a case-insensitive check to see if the collection contains the given String + * @param items List of strings from which to search + * @param object String for which to search + * @return True if the object exists in the array, false if not + */ + private boolean containsIgnoreCase(Collection items, String object) { + return items.stream() + .anyMatch(item -> item.equalsIgnoreCase(object)); + } + + /** + * Checks if any string is contained in both the first collection and the second collection + * @return True if both collections contain any one string, false if not + */ + private boolean doesHaveIntersection(Collection co1, Collection co2) { + Set co1LowerCase = co1.stream() + .map(String::toLowerCase) + .collect(Collectors.toSet()); + + return co2.stream() + .anyMatch(str -> co1LowerCase.contains(str.toLowerCase())); + } + + /** + * Checks (ignoring case) if any string is contained in both the first collection and the second collection + * @return True if both collections contain any one string, false if not + */ + private boolean doesHaveIntersectionIgnoreCase(Collection co1, Collection co2) { + Set co1LowerCase = co1.stream() + .map(String::toLowerCase) + .collect(Collectors.toSet()); + + return co2.stream() + .anyMatch(str -> co1LowerCase.contains(str.toLowerCase())); + } } \ No newline at end of file diff --git a/src/main/java/com/nike/cerberus/service/SafeDepositBoxService.java b/src/main/java/com/nike/cerberus/service/SafeDepositBoxService.java index 76a9d19ea..49eda335e 100644 --- a/src/main/java/com/nike/cerberus/service/SafeDepositBoxService.java +++ b/src/main/java/com/nike/cerberus/service/SafeDepositBoxService.java @@ -45,6 +45,7 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Singleton; import java.time.OffsetDateTime; import java.util.LinkedList; @@ -53,6 +54,8 @@ import java.util.Set; import java.util.stream.Collectors; +import static com.nike.cerberus.service.PermissionsService.USER_GROUPS_CASE_SENSITIVE; + /** * Business logic for interacting with safe deposit boxes. */ @@ -87,6 +90,8 @@ public class SafeDepositBoxService { private final SecureDataVersionDao secureDataVersionDao; + private final Boolean userGroupsCaseSensitive; + @Inject public SafeDepositBoxService(SafeDepositBoxDao safeDepositBoxDao, UserGroupDao userGroupDao, @@ -100,7 +105,8 @@ public SafeDepositBoxService(SafeDepositBoxDao safeDepositBoxDao, DateTimeSupplier dateTimeSupplier, AwsIamRoleArnParser awsIamRoleArnParser, SecureDataService secureDataService, - SecureDataVersionDao secureDataVersionDao) { + SecureDataVersionDao secureDataVersionDao, + @Named(USER_GROUPS_CASE_SENSITIVE) Boolean userGroupsCaseSensitive){ this.safeDepositBoxDao = safeDepositBoxDao; this.userGroupDao = userGroupDao; @@ -115,6 +121,7 @@ public SafeDepositBoxService(SafeDepositBoxDao safeDepositBoxDao, this.awsIamRoleArnParser = awsIamRoleArnParser; this.secureDataService = secureDataService; this.secureDataVersionDao = secureDataVersionDao; + this.userGroupsCaseSensitive = userGroupsCaseSensitive; } /** @@ -132,7 +139,9 @@ public List getAssociatedSafeDepositBoxes(final CerberusP sdbRecords = safeDepositBoxDao.getIamPrincipalAssociatedSafeDepositBoxes(principal.getName()); break; case USER: - sdbRecords = safeDepositBoxDao.getUserAssociatedSafeDepositBoxes(principal.getUserGroups()); + sdbRecords = userGroupsCaseSensitive ? + safeDepositBoxDao.getUserAssociatedSafeDepositBoxes(principal.getUserGroups()) : + safeDepositBoxDao.getUserAssociatedSafeDepositBoxesIgnoreCase(principal.getUserGroups()); break; default: throw new ApiException(DefaultApiError.UNKNOWN_PRINCIPAL_TYPE); diff --git a/src/main/resources/cms.conf b/src/main/resources/cms.conf index 8c2bf8fcd..cc63b9486 100644 --- a/src/main/resources/cms.conf +++ b/src/main/resources/cms.conf @@ -140,7 +140,7 @@ cms.jobs.configuredJobs=[ # # Fully qualified classname, will be created with guice and injected into the event processor service # -# You can add your own here as long as its availible on the classpath +# You can add your own here as long as its available on the classpath # See https://github.com/Nike-Inc/cerberus-util-scripts/blob/master/bash_scripts/start-jar-script.sh#L72 # ex set JVM_BEHAVIOR_ARGS='-cp /opt/cerberus/*.jar' with your jar there. # You can disable event logging locally or for your specific env by removing the default LoggingEventProcessor @@ -164,3 +164,5 @@ cms.auth.token.hash.algorithm="PBKDF2WithHmacSHA512" cms.user.token.ttl=1h cms.user.token.maxRefreshCount=24 cms.iam.token.ttl=1h + +cms.user.groups.caseSensitive=true \ No newline at end of file diff --git a/src/main/resources/com/nike/cerberus/mapper/PermissionsMapper.xml b/src/main/resources/com/nike/cerberus/mapper/PermissionsMapper.xml index 65f92e223..76a09b211 100644 --- a/src/main/resources/com/nike/cerberus/mapper/PermissionsMapper.xml +++ b/src/main/resources/com/nike/cerberus/mapper/PermissionsMapper.xml @@ -63,4 +63,27 @@ ) as HAS_PERMS + + \ No newline at end of file diff --git a/src/main/resources/com/nike/cerberus/mapper/SafeDepositBoxMapper.xml b/src/main/resources/com/nike/cerberus/mapper/SafeDepositBoxMapper.xml index a3268d212..ad537de3f 100644 --- a/src/main/resources/com/nike/cerberus/mapper/SafeDepositBoxMapper.xml +++ b/src/main/resources/com/nike/cerberus/mapper/SafeDepositBoxMapper.xml @@ -102,6 +102,34 @@ SDB.ID ASC + +