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

* fix issue where removing an assumed-role's SDB permissions will mak… #182

Merged
merged 4 commits into from
Jan 9, 2019

Conversation

mayitbeegh
Copy link
Contributor

…e the assumed-role lose access to the SDB even if the base IAM role has permissions

  • fix issue where an assumed-role doesn't inherit base IAM role's admin privileges

…e the assumed-role lose access to the SDB even if the base IAM role has permissions

* fix issue where an assumed-role doesn't inherit base IAM role's admin privileges
@coveralls
Copy link

coveralls commented Jan 4, 2019

Coverage Status

Coverage increased (+0.3%) to 55.312% when pulling 0347086 on fix/sts-lost-sdb-access into baccaf3 on master.

@@ -46,6 +46,12 @@ public SafeDepositBoxDao(final SafeDepositBoxMapper safeDepositBoxMapper) {
return safeDepositBoxMapper.getIamRoleAssociatedSafeDepositBoxRoles(awsIamRoleArn, iamRootArn);
}

public List<SafeDepositBoxRoleRecord> getIamAssumedRoleAssociatedSafeDepositBoxRoles(final String iamAssumedRoleArn,
final String awsIamRoleArn,
final String iamRootArn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace looks off

@@ -59,6 +65,12 @@ public SafeDepositBoxDao(final SafeDepositBoxMapper safeDepositBoxMapper) {
return safeDepositBoxMapper.getIamPrincipalAssociatedSafeDepositBoxes(iamPrincipalArn, iamRootArn);
}

public List<SafeDepositBoxRecord> getAssumedRoleAssociatedSafeDepositBoxes(final String iamAssumedRoleArn,
final String iamRoleArn,
final String iamRootArn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace looks off

@@ -27,6 +27,12 @@ Boolean doesIamPrincipalHaveGivenRoleForSdb(@Param("sdbId") String sdbId,
@Param("iamRootArn") String iamRootArn,
@Param("rolesThatAllowPermission") Set<String> rolesThatAllowPermission);

Boolean doesAssumedRoleHaveGivenRoleForSdb(@Param("sdbId") String sdbId,
@Param("assumedRoleArn") String assumedRoleArn,
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace looks off

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

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

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

List<SafeDepositBoxRecord> getIamAssumedRoleAssociatedSafeDepositBoxes(@Param("iamAssumedRoleArn") final String iamAssumedRoleArn,
@Param("iamRoleArn") final String iamRoleArn,
@Param("iamRootArn") final String iamRootArn);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace looks off

@@ -42,6 +42,30 @@
) as HAS_PERM;
</select>

<select id="doesAssumedRoleHaveGivenRoleForSdb" resultType="Boolean">
SELECT NOT 0 >= (
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace looks off

@@ -113,6 +113,18 @@ public boolean isRoleArn(final String arn) {
return iamRoleArnMatcher.find();
}

/**
* Returns true if the ARN is in format 'arn:aws:iam::000000000:role/example' and false if not
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this example arn is for a role arn and not an assume role arn

Copy link
Contributor

@fieldju fieldju left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants