From 50773e43370dfc46408ddddef6436be6c3011c8b Mon Sep 17 00:00:00 2001 From: Justin Field Date: Mon, 21 Oct 2019 11:56:52 -0700 Subject: [PATCH] refactor: Removed all legacy policy generation logic that hasn't been necessary since Highlander (implenting /v1/secret internally). Policies are no longer used by this service, so we will just return a empty list. (#214) --- .../service/AuthenticationService.java | 113 ++---------------- .../service/AuthenticationServiceTest.java | 51 +------- 2 files changed, 8 insertions(+), 156 deletions(-) diff --git a/src/main/java/com/nike/cerberus/service/AuthenticationService.java b/src/main/java/com/nike/cerberus/service/AuthenticationService.java index 18573ed30..9e1c4a877 100644 --- a/src/main/java/com/nike/cerberus/service/AuthenticationService.java +++ b/src/main/java/com/nike/cerberus/service/AuthenticationService.java @@ -28,11 +28,9 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.benmanes.caffeine.cache.Cache; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; -import com.google.common.collect.Sets; import com.google.inject.Inject; import com.google.inject.Singleton; import com.google.inject.name.Named; @@ -56,7 +54,6 @@ import com.nike.cerberus.error.KeyInvalidForAuthException; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; import com.nike.cerberus.record.AwsIamRoleRecord; -import com.nike.cerberus.record.SafeDepositBoxRoleRecord; import com.nike.cerberus.security.CerberusPrincipal; import com.nike.cerberus.util.AwsIamRoleArnParser; import com.nike.cerberus.util.DateTimeSupplier; @@ -73,12 +70,7 @@ import java.nio.charset.Charset; import java.time.Duration; import java.time.OffsetDateTime; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import static com.nike.cerberus.security.CerberusPrincipal.METADATA_KEY_GROUPS; import static com.nike.cerberus.security.CerberusPrincipal.METADATA_KEY_IS_ADMIN; @@ -104,7 +96,6 @@ public class AuthenticationService { public static final String LOOKUP_SELF_POLICY = "lookup-self"; public static final int KMS_SIZE_LIMIT = 4096; - private final SafeDepositBoxDao safeDepositBoxDao; private final AwsIamRoleDao awsIamRoleDao; private final AuthConnector authServiceConnector; private final KmsService kmsService; @@ -113,7 +104,6 @@ public class AuthenticationService { private final String adminGroup; private final DateTimeSupplier dateTimeSupplier; private final AwsIamRoleArnParser awsIamRoleArnParser; - private final Slugger slugger; private final AuthTokenService authTokenService; private final String userTokenTTL; private final String iamTokenTTL; @@ -129,8 +119,7 @@ public class AuthenticationService { private Set adminRoleArnSet; @Inject - public AuthenticationService(SafeDepositBoxDao safeDepositBoxDao, - AwsIamRoleDao awsIamRoleDao, + public AuthenticationService(AwsIamRoleDao awsIamRoleDao, AuthConnector authConnector, KmsService kmsService, KmsClientFactory kmsClientFactory, @@ -139,14 +128,12 @@ public AuthenticationService(SafeDepositBoxDao safeDepositBoxDao, @Named(MAX_TOKEN_REFRESH_COUNT) int maxTokenRefreshCount, DateTimeSupplier dateTimeSupplier, AwsIamRoleArnParser awsIamRoleArnParser, - Slugger slugger, AuthTokenService authTokenService, @Named(USER_TOKEN_TTL) String userTokenTTL, @Named(IAM_TOKEN_TTL) String iamTokenTTL, AwsIamRoleService awsIamRoleService, @Named(CACHE_ENABLED) boolean cacheEnabled, @Named(CACHE) Cache cache) { - this.safeDepositBoxDao = safeDepositBoxDao; this.awsIamRoleDao = awsIamRoleDao; this.authServiceConnector = authConnector; this.kmsService = kmsService; @@ -156,7 +143,6 @@ public AuthenticationService(SafeDepositBoxDao safeDepositBoxDao, this.dateTimeSupplier = dateTimeSupplier; this.awsIamRoleArnParser = awsIamRoleArnParser; this.maxTokenRefreshCount = maxTokenRefreshCount; - this.slugger = slugger; this.authTokenService = authTokenService; this.userTokenTTL = userTokenTTL; this.iamTokenTTL = iamTokenTTL; @@ -256,13 +242,8 @@ public IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials) { public AuthTokenResponse stsAuthenticate(final String iamPrincipalArn) { final Map authPrincipalMetadata = generateCommonIamPrincipalAuthMetadata(iamPrincipalArn); authPrincipalMetadata.put(CerberusPrincipal.METADATA_KEY_AWS_IAM_PRINCIPAL_ARN, iamPrincipalArn); - - final AwsIamRoleRecord iamRoleRecord; - iamRoleRecord = getIamPrincipalRecord(iamPrincipalArn); - - final Set policies = buildCompleteSetOfPolicies(iamPrincipalArn); - AuthTokenResponse authResponse = createToken(iamRoleRecord.getAwsIamRoleArn(), PrincipalType.IAM, policies, authPrincipalMetadata, iamTokenTTL); - return authResponse; + final AwsIamRoleRecord iamRoleRecord = getIamPrincipalRecord(iamPrincipalArn); // throws error if iam principal not associated with SDB + return createToken(iamRoleRecord.getAwsIamRoleArn(), PrincipalType.IAM, authPrincipalMetadata, iamTokenTTL); } private IamRoleAuthResponse cachingKmsAuthenticate(IamPrincipalCredentials credentials, Map authPrincipalMetadata) { @@ -292,8 +273,7 @@ private IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials, Ma throw e; } - final Set policies = buildCompleteSetOfPolicies(credentials.getIamPrincipalArn()); - AuthTokenResponse authResponse = createToken(iamRoleRecord.getAwsIamRoleArn(), PrincipalType.IAM, policies, authPrincipalMetadata, iamTokenTTL); + AuthTokenResponse authResponse = createToken(iamRoleRecord.getAwsIamRoleArn(), PrincipalType.IAM, authPrincipalMetadata, iamTokenTTL); byte[] authResponseJson; try { @@ -320,7 +300,6 @@ private IamRoleAuthResponse authenticate(IamPrincipalCredentials credentials, Ma private AuthTokenResponse createToken(String principal, PrincipalType principalType, - Set policies, Map metadata, String vaultStyleTTL) { @@ -343,7 +322,7 @@ private AuthTokenResponse createToken(String principal, return new AuthTokenResponse() .setClientToken(tokenResult.getToken()) - .setPolicies(policies) + .setPolicies(Collections.emptySet()) .setMetadata(metadata) .setLeaseDuration(Duration.between(tokenResult.getCreated(), tokenResult.getExpires()).getSeconds()) .setRenewable(PrincipalType.USER.equals(principalType)); @@ -466,71 +445,7 @@ private AuthTokenResponse generateToken(final String username, final Set meta.put(CerberusPrincipal.METADATA_KEY_TOKEN_REFRESH_COUNT, String.valueOf(refreshCount)); meta.put(CerberusPrincipal.METADATA_KEY_MAX_TOKEN_REFRESH_COUNT, String.valueOf(maxTokenRefreshCount)); - final Set policies = buildPolicySet(userGroups); - - return createToken(username, PrincipalType.USER, policies, meta, userTokenTTL); - } - - /** - * Builds the policy set to be associated with the to-be generated auth token. The lookup-self policy is - * included by default. All other associated policies are based on the groups the user is a member of. - * - * @param groups Groups the user is a member of - * @return Set of policies to be associated - */ - private Set buildPolicySet(final Set groups) { - final Set policies = Sets.newHashSet(LOOKUP_SELF_POLICY); - final List sdbRoles = safeDepositBoxDao.getUserAssociatedSafeDepositBoxRoles(groups); - - sdbRoles.forEach(i -> { - policies.add(buildPolicyName(i.getSafeDepositBoxName(), i.getRoleName())); - }); - - return policies; - } - - /** - * Builds the policy set with permissions given to the specific IAM principal - * (e.g. arn:aws:iam::1111111111:instance-profile/example), as well as the base role that is assumed by that - * principal (i.e. arn:aws:iam::1111111111:role/example) - * @param iamPrincipalArn - The given IAM principal ARN during authentication - * @return - List of all policies the given ARN has access to - */ - protected Set buildCompleteSetOfPolicies(final String iamPrincipalArn) { - - final Set allPolicies = buildPolicySet(iamPrincipalArn); - - return allPolicies; - } - - /** - * Builds the policy set to be associated with the to-be generated auth token. The lookup-self policy is - * included by default. All other associated policies are based on what permissions are granted to the IAM role. - * - * @param iamRoleArn IAM role ARN - * @return Set of policies to be associated - */ - private Set buildPolicySet(final String iamRoleArn) { - final String accountRootArn = awsIamRoleArnParser.convertPrincipalArnToRootArn(iamRoleArn); - final Set policies = Sets.newHashSet(LOOKUP_SELF_POLICY); - final List sdbRolesForIamPrincipal; - // This may cause issues for user/instance-profile if someone's relying on the old code that converts everything - // that's not a role ARN. - if (awsIamRoleArnParser.isAssumedRoleArn(iamRoleArn)) { - logger.debug("Detected assumed-role ARN, attempting to collect policies for the principal's base role..."); - String baseIamRoleArn = awsIamRoleArnParser.convertPrincipalArnToRoleArn(iamRoleArn); - sdbRolesForIamPrincipal = - safeDepositBoxDao.getIamAssumedRoleAssociatedSafeDepositBoxRoles(iamRoleArn, baseIamRoleArn, accountRootArn); - } else { - sdbRolesForIamPrincipal = - safeDepositBoxDao.getIamRoleAssociatedSafeDepositBoxRoles(iamRoleArn, accountRootArn); - } - - sdbRolesForIamPrincipal.forEach(i -> { - policies.add(buildPolicyName(i.getSafeDepositBoxName(), i.getRoleName())); - }); - - return policies; + return createToken(username, PrincipalType.USER, meta, userTokenTTL); } protected AwsIamRoleRecord getIamPrincipalRecord(String iamPrincipalArn) { @@ -719,18 +634,4 @@ protected Optional findIamRoleAssociatedWithSdb(String iamPrin return iamRole; } - - /** - * Outputs the expected policy name format that was used in Vault, when Cerberus used Vault to store AC information. - * - * @param sdbName Safe deposit box name. - * @param roleName Role for safe deposit box. - * @return Formatted policy name. - */ - public String buildPolicyName(final String sdbName, final String roleName) { - Preconditions.checkArgument(StringUtils.isNotBlank(sdbName), "sdbName cannot be blank!"); - Preconditions.checkArgument(StringUtils.isNotBlank(roleName), "roleName cannot be blank!"); - - return slugger.toSlug(sdbName) + '-' + StringUtils.lowerCase(roleName); - } } diff --git a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java index ba93a158d..ddedc174e 100644 --- a/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java +++ b/src/test/java/com/nike/cerberus/service/AuthenticationServiceTest.java @@ -34,15 +34,12 @@ import com.nike.cerberus.error.DefaultApiError; import com.nike.cerberus.record.AwsIamRoleKmsKeyRecord; import com.nike.cerberus.record.AwsIamRoleRecord; -import com.nike.cerberus.record.SafeDepositBoxRoleRecord; import com.nike.cerberus.security.CerberusPrincipal; import com.nike.cerberus.server.config.CmsConfig; import com.nike.cerberus.util.AwsIamRoleArnParser; import com.nike.cerberus.util.DateTimeSupplier; import com.nike.cerberus.util.Slugger; import org.apache.commons.lang3.RandomStringUtils; -import org.assertj.core.util.Lists; -import org.assertj.core.util.Sets; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -50,14 +47,12 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.HashMap; -import java.util.List; import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; -import static com.nike.cerberus.service.AuthenticationService.LOOKUP_SELF_POLICY; import static com.nike.cerberus.util.AwsIamRoleArnParser.AWS_IAM_ROLE_ARN_TEMPLATE; import static org.junit.Assert.assertEquals; @@ -116,7 +111,6 @@ public void setup() { initMocks(this); objectMapper = CmsConfig.configureObjectMapper(); authenticationService = new AuthenticationService( - safeDepositBoxDao, awsIamRoleDao, authConnector, kmsService, @@ -126,7 +120,6 @@ public void setup() { MAX_LIMIT, dateTimeSupplier, awsIamRoleArnParser, - slugger, authTokenService, "1h", "1h", @@ -255,48 +248,6 @@ public void test_that_getKeyId_only_validates_kms_policy_one_time_within_interva verify(kmsService, times(1)).validateKeyAndPolicy(awsIamRoleKmsKeyRecord, principalArn); } - @Test - public void test_that_buildCompleteSetOfPolicies_returns_all_policies() { - - String accountId = "0000000000"; - String roleName = "role/path"; - String read = "read"; - String write = "write"; - String owner = "owner"; - String principalArn = String.format("arn:aws:iam::%s:assumed-role/%s/token-session", accountId, roleName); - - String roleArn = String.format(AWS_IAM_ROLE_ARN_TEMPLATE, accountId, roleName); - String rootArn = String.format("arn:aws:iam::%s:root", accountId); - when(awsIamRoleArnParser.isAssumedRoleArn(principalArn)).thenReturn(true); - when(awsIamRoleArnParser.convertPrincipalArnToRoleArn(principalArn)).thenReturn(roleArn); - when(awsIamRoleArnParser.convertPrincipalArnToRootArn(roleArn)).thenReturn(rootArn); - when(awsIamRoleArnParser.convertPrincipalArnToRootArn(principalArn)).thenReturn(rootArn); - - String sdbName1 = "principal arn sdb 1"; - String sdbName2 = "principal arn sdb 2"; - SafeDepositBoxRoleRecord principalArnRecord1 = new SafeDepositBoxRoleRecord().setRoleName(read).setSafeDepositBoxName(sdbName1); - SafeDepositBoxRoleRecord principalArnRecord2 = new SafeDepositBoxRoleRecord().setRoleName(write).setSafeDepositBoxName(sdbName2); - - String roleArnSdb = "role arn sdb"; - SafeDepositBoxRoleRecord roleArnRecord = new SafeDepositBoxRoleRecord().setRoleName(owner).setSafeDepositBoxName(roleArnSdb); - - List principalArnRecords = Lists.newArrayList(principalArnRecord1, principalArnRecord2, roleArnRecord); - when(safeDepositBoxDao.getIamAssumedRoleAssociatedSafeDepositBoxRoles(principalArn, roleArn, rootArn)).thenReturn(principalArnRecords); - - List expectedPolicies = Lists.newArrayList( - "principal-arn-sdb-1-read", - "principal-arn-sdb-2-write", - "role-arn-sdb-owner", - LOOKUP_SELF_POLICY - ); - - Set expected = Sets.newHashSet(expectedPolicies); - Set result = authenticationService.buildCompleteSetOfPolicies(principalArn); - - assertEquals(expected.size(), result.size()); - assertTrue(result.containsAll(expected)); - } - @Test public void test_that_findIamRoleAssociatedWithSdb_returns_first_matching_iam_role_record_if_found() { @@ -489,4 +440,4 @@ public void tests_that_refreshUserToken_throws_access_denied_token_when_count_is assertTrue(e instanceof ApiException); assertTrue(((ApiException) e).getApiErrors().contains(DefaultApiError.MAXIMUM_TOKEN_REFRESH_COUNT_REACHED)); } -} \ No newline at end of file +}