Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Commit

Permalink
Merge pull request #170 from Nike-Inc/feature/flag_user_group_case_se…
Browse files Browse the repository at this point in the history
…nsitivity

Add feature flag for user group case sensitivity
  • Loading branch information
sdford authored Jul 12, 2018
2 parents 4600d8f + 3c0414a commit d6d9419
Show file tree
Hide file tree
Showing 12 changed files with 275 additions and 17 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ cms.event.processors.com.nike.cerberus.event.processor.LoggingEventProcessor | N
cms.event.processors.com.nike.cerberus.event.processor.AuditLogProcessor | No | defaults to false, Boolean of whether or not to enable audit logging, see #events below
cms.audit.bucket="bucket-name" | No | [See Logging Event Processor below](https://github.com/Nike-Inc/cerberus-management-service/tree/feature/audit_logging#audit-log-processor)
cms.audit.bucket_region="bucket-region" | No | [See Logging Event Processor below](https://github.com/Nike-Inc/cerberus-management-service/tree/feature/audit_logging#audit-log-processor)
cms.user.groups.caseSensitive | YES | Property to enable/disable case-sensitive user AD group permission checks

KMS Policies are bound to IAM Principal IDs rather than ARNs themselves. Because of this, we validate the policy at authentication time
to ensure that if an IAM role has been deleted and re-created, that we grant access to the new principal ID.
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
# limitations under the License.
#

version=3.18.2
version=3.19.0
groupId=com.nike.cerberus
artifactId=cms
5 changes: 5 additions & 0 deletions src/main/java/com/nike/cerberus/dao/PermissionsDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

public class PermissionsDao {


private final PermissionsMapper permissionsMapper;

@Inject
Expand All @@ -37,4 +38,8 @@ public Boolean doesIamPrincipalHaveRoleForSdb(String sdbId, String iamPrincipalA
public Boolean doesUserPrincipalHaveRoleForSdb(String sdbId, Set<String> rolesThatAllowPermission, Set<String> userGroupsThatPrincipalBelongsTo) {
return permissionsMapper.doesUserPrincipalHaveGivenRoleForSdb(sdbId, rolesThatAllowPermission, userGroupsThatPrincipalBelongsTo);
}

public Boolean doesUserHavePermsForRoleAndSdbCaseInsensitive(String sdbId, Set<String> rolesThatAllowPermission, Set<String> userGroupsThatPrincipalBelongsTo) {
return permissionsMapper.doesUserHavePermsForRoleAndSdbCaseInsensitive(sdbId, rolesThatAllowPermission, userGroupsThatPrincipalBelongsTo);
}
}
4 changes: 4 additions & 0 deletions src/main/java/com/nike/cerberus/dao/SafeDepositBoxDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ public List<SafeDepositBoxRecord> getUserAssociatedSafeDepositBoxes(final Set<St
return safeDepositBoxMapper.getUserAssociatedSafeDepositBoxes(userGroups);
}

public List<SafeDepositBoxRecord> getUserAssociatedSafeDepositBoxesIgnoreCase(final Set<String> userGroups) {
return safeDepositBoxMapper.getUserAssociatedSafeDepositBoxesIgnoreCase(userGroups);
}

public List<SafeDepositBoxRecord> getIamPrincipalAssociatedSafeDepositBoxes(final String iamPrincipalArn) {
return safeDepositBoxMapper.getIamPrincipalAssociatedSafeDepositBoxes(iamPrincipalArn);
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/nike/cerberus/mapper/PermissionsMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,7 @@ Boolean doesUserPrincipalHaveGivenRoleForSdb(@Param("sdbId") String sdbId,
@Param("rolesThatAllowPermission") Set<String> rolesThatAllowPermission,
@Param("userGroupsThatPrincipalBelongsTo") Set<String> userGroupsThatPrincipalBelongsTo);

Boolean doesUserHavePermsForRoleAndSdbCaseInsensitive(@Param("sdbId") String sdbId,
@Param("rolesThatAllowPermission") Set<String> rolesThatAllowPermission,
@Param("userGroupsThatPrincipalBelongsTo") Set<String> userGroupsThatPrincipalBelongsTo);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public interface SafeDepositBoxMapper {

List<SafeDepositBoxRecord> getUserAssociatedSafeDepositBoxes(@Param("userGroups") Set<String> userGroups);

List<SafeDepositBoxRecord> getUserAssociatedSafeDepositBoxesIgnoreCase(@Param("userGroups") Set<String> userGroups);

List<SafeDepositBoxRecord> getIamPrincipalAssociatedSafeDepositBoxes(@Param("iamPrincipalArn") final String iamPrincipalArn);

SafeDepositBoxRecord getSafeDepositBox(@Param("id") String id);
Expand Down
64 changes: 55 additions & 9 deletions src/main/java/com/nike/cerberus/service/PermissionsService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -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<UserGroupPermission> 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<String> userGroups = userGroupPermissions.stream()
.map(UserGroupPermission::getName)
.collect(Collectors.toSet());
principalHasPermissionAssociationWithSdb = userGroupsCaseSensitive ?
doesHaveIntersection(userGroups, principal.getUserGroups()) :
doesHaveIntersectionIgnoreCase(userGroups, principal.getUserGroups());
break;
}
return principalHasPermissionAssociationWithSdb;
Expand All @@ -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());
Expand All @@ -156,4 +168,38 @@ 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<String> 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<String> co1, Collection<String> co2) {
return co2.stream()
.anyMatch(co1::contains);
}

/**
* 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<String> co1, Collection<String> co2) {
Set<String> co1LowerCase = co1.stream()
.map(String::toLowerCase)
.collect(Collectors.toSet());

return co2.stream()
.map(String::toLowerCase)
.anyMatch(co1LowerCase::contains);
}
}
13 changes: 11 additions & 2 deletions src/main/java/com/nike/cerberus/service/SafeDepositBoxService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -87,6 +90,8 @@ public class SafeDepositBoxService {

private final SecureDataVersionDao secureDataVersionDao;

private final Boolean userGroupsCaseSensitive;

@Inject
public SafeDepositBoxService(SafeDepositBoxDao safeDepositBoxDao,
UserGroupDao userGroupDao,
Expand All @@ -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;
Expand All @@ -115,6 +121,7 @@ public SafeDepositBoxService(SafeDepositBoxDao safeDepositBoxDao,
this.awsIamRoleArnParser = awsIamRoleArnParser;
this.secureDataService = secureDataService;
this.secureDataVersionDao = secureDataVersionDao;
this.userGroupsCaseSensitive = userGroupsCaseSensitive;
}

/**
Expand All @@ -132,7 +139,9 @@ public List<SafeDepositBoxSummary> 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);
Expand Down
9 changes: 8 additions & 1 deletion src/main/resources/cms.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -164,3 +164,10 @@ cms.auth.token.hash.algorithm="PBKDF2WithHmacSHA512"
cms.user.token.ttl=1h
cms.user.token.maxRefreshCount=24
cms.iam.token.ttl=1h

# Set to true to make user group permissions case-sensitive, false for case-insensitive
#
# For example:
# When true, if an SDB grants access to AD group 'Lst-foo', then users in group 'Lst-Foo' will not have access
# When false, if an SDB grants access to AD group 'Lst-foo', then users in group 'Lst-Foo' will have access
cms.user.groups.caseSensitive=true
23 changes: 23 additions & 0 deletions src/main/resources/com/nike/cerberus/mapper/PermissionsMapper.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,27 @@
) as HAS_PERMS
</select>

<select id="doesUserHavePermsForRoleAndSdbCaseInsensitive" resultType="Boolean">
SELECT NOT 0 >= (
SELECT
COUNT(*)
FROM SAFE_DEPOSIT_BOX
LEFT JOIN USER_GROUP_PERMISSIONS ON SAFE_DEPOSIT_BOX.ID = USER_GROUP_PERMISSIONS.SDBOX_ID
LEFT JOIN USER_GROUP ON USER_GROUP_PERMISSIONS.USER_GROUP_ID = USER_GROUP.ID
LEFT JOIN ROLE ON USER_GROUP_PERMISSIONS.ROLE_ID = ROLE.ID
WHERE
SAFE_DEPOSIT_BOX.ID = #{sdbId}
AND
UPPER(USER_GROUP.NAME) IN
<foreach item="userGroup" collection="userGroupsThatPrincipalBelongsTo" separator="," open="(" close=")">
UPPER(#{userGroup})
</foreach>
AND
ROLE.NAME IN
<foreach item="role" collection="rolesThatAllowPermission" separator="," open="(" close=")">
#{role}
</foreach>
) as HAS_PERMS
</select>

</mapper>
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,34 @@
SDB.ID ASC
</select>

<select id="getUserAssociatedSafeDepositBoxesIgnoreCase" resultType="SafeDepositBoxRecord">
SELECT
DISTINCT SDB.ID,
SDB.CATEGORY_ID,
SDB.NAME,
SDB.DESCRIPTION,
SDB.PATH,
SDB.CREATED_BY,
SDB.LAST_UPDATED_BY,
SDB.CREATED_TS,
SDB.LAST_UPDATED_TS
FROM
SAFE_DEPOSIT_BOX SDB
INNER JOIN
USER_GROUP_PERMISSIONS UGP ON SDB.ID = UGP.SDBOX_ID
INNER JOIN
USER_GROUP UG ON UGP.USER_GROUP_ID = UG.ID
WHERE
UPPER(UG.NAME) IN
<foreach item="item" index="index" collection="userGroups"
open="(" separator="," close=")">
UPPER(#{item})
</foreach>
ORDER BY
SDB.CATEGORY_ID ASC,
SDB.ID ASC
</select>

<select id="getIamPrincipalAssociatedSafeDepositBoxes" resultType="SafeDepositBoxRecord">
SELECT
DISTINCT SDB.ID,
Expand Down
Loading

0 comments on commit d6d9419

Please sign in to comment.