Skip to content

Commit

Permalink
feat(core): Add account manager roles config (#928)
Browse files Browse the repository at this point in the history
This adds a Fiat configuration option for the Account Management API in Clouddriver for listing which roles are allowed to manage accounts in the API.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Cameron Motevasselani <cmotevasselani@gmail.com>
  • Loading branch information
3 people authored Apr 1, 2022
1 parent 4047a11 commit dd191f0
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class UserPermission {
private Set<BuildService> buildServices = new LinkedHashSet<>();
private Set<Resource> extensionResources = new LinkedHashSet<>();
private boolean admin = false;
private boolean accountManager;

/**
* Custom setter to normalize the input. lombok.accessors.chain is enabled, so setters must return
Expand Down Expand Up @@ -118,6 +119,7 @@ public static class View extends Viewable.BaseView {
Set<BuildService.View> buildServices;
HashMap<ResourceType, Set<Authorizable>> extensionResources;
boolean admin;
boolean accountManager;
boolean legacyFallback = false;
boolean allowAccessToUnknownApplications = false;

Expand Down Expand Up @@ -149,6 +151,7 @@ public View(UserPermission permission) {
}

this.admin = permission.isAdmin();
this.accountManager = permission.isAccountManager();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.netflix.spinnaker.fiat.config;

import java.util.List;
import lombok.Data;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.context.annotation.Configuration;

/** Configures Fiat aspects of the account management API. */
@Configuration
@ConfigurationProperties("fiat.account.manager")
@Data
public class AccountManagerConfig {

/** List of roles allowed to programmatically manage accounts. */
private List<String> roles = List.of();
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.netflix.spinnaker.fiat.config.AccountManagerConfig;
import com.netflix.spinnaker.fiat.config.FiatAdminConfig;
import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig;
import com.netflix.spinnaker.fiat.model.UserPermission;
Expand All @@ -29,6 +30,7 @@
import com.netflix.spinnaker.fiat.roles.UserRolesProvider;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -51,6 +53,7 @@ public class DefaultPermissionsResolver implements PermissionsResolver {
private final ResourceProvider<ServiceAccount> serviceAccountProvider;
private final ImmutableList<ResourceProvider<? extends Resource>> resourceProviders;
private final FiatAdminConfig fiatAdminConfig;
private final AccountManagerConfig accountManagerConfig;
private final ObjectMapper mapper;

@Autowired
Expand All @@ -59,11 +62,13 @@ public DefaultPermissionsResolver(
ResourceProvider<ServiceAccount> serviceAccountProvider,
List<ResourceProvider<? extends Resource>> resourceProviders,
FiatAdminConfig fiatAdminConfig,
AccountManagerConfig accountManagerConfig,
@Qualifier("objectMapper") ObjectMapper mapper) {
this.userRolesProvider = userRolesProvider;
this.serviceAccountProvider = serviceAccountProvider;
this.resourceProviders = ImmutableList.copyOf(resourceProviders);
this.fiatAdminConfig = fiatAdminConfig;
this.accountManagerConfig = accountManagerConfig;
this.mapper = mapper;
}

Expand Down Expand Up @@ -103,15 +108,26 @@ public void clearCache() {
}
}

private boolean resolveAdminRole(Set<Role> roles) {
List<String> adminRoles = fiatAdminConfig.getAdmin().getRoles();
return roles.stream().map(Role::getName).anyMatch(adminRoles::contains);
private boolean hasAdminRole(Set<Role> roles) {
Set<String> adminRoles = Set.copyOf(fiatAdminConfig.getAdmin().getRoles());
Set<String> userRoles = roles.stream().map(Role::getName).collect(Collectors.toSet());
return !Collections.disjoint(adminRoles, userRoles);
}

private boolean hasAccountManagerRole(Set<Role> roles) {
Set<String> accountManagerRoles = Set.copyOf(accountManagerConfig.getRoles());
Set<String> userRoles = roles.stream().map(Role::getName).collect(Collectors.toSet());
return !Collections.disjoint(accountManagerRoles, userRoles);
}

@SuppressWarnings("unchecked")
private UserPermission getUserPermission(String userId, Set<Role> roles) {
UserPermission permission =
new UserPermission().setId(userId).setRoles(roles).setAdmin(resolveAdminRole(roles));
new UserPermission()
.setId(userId)
.setRoles(roles)
.setAdmin(hasAdminRole(roles))
.setAccountManager(hasAccountManagerRole(roles));

for (ResourceProvider provider : resourceProviders) {
try {
Expand Down Expand Up @@ -198,8 +214,8 @@ private Map<String, UserPermission> resolveResources(
return new UserPermission()
.setId(userId)
.setRoles(userRoles)
.setAdmin(resolveAdminRole(userRoles))
.addResources(getResources(userId, userRoles, resolveAdminRole(userRoles)));
.setAdmin(hasAdminRole(userRoles))
.addResources(getResources(userId, userRoles, hasAdminRole(userRoles)));
})
.collect(Collectors.toMap(UserPermission::getId, Function.identity()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.fiat.permissions

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.fiat.config.AccountManagerConfig
import com.netflix.spinnaker.fiat.config.FiatAdminConfig
import com.netflix.spinnaker.fiat.config.FiatRoleConfig
import com.netflix.spinnaker.fiat.model.Authorization
Expand Down Expand Up @@ -105,7 +106,7 @@ class DefaultPermissionsResolverSpec extends Specification {
def "should resolve the anonymous user permission, when enabled"() {
setup:
@Subject DefaultPermissionsResolver resolver = new DefaultPermissionsResolver(
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new ObjectMapper())
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new AccountManagerConfig(), new ObjectMapper())

when:
def result = resolver.resolveUnrestrictedUser()
Expand All @@ -124,7 +125,7 @@ class DefaultPermissionsResolverSpec extends Specification {
def testUserId = "testUserId"
UserRolesProvider userRolesProvider = Mock(UserRolesProvider)
@Subject DefaultPermissionsResolver resolver = new DefaultPermissionsResolver(
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new ObjectMapper())
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new AccountManagerConfig(), new ObjectMapper())

def role1 = new Role("group1")
def role2 = new Role("gRoUP2") // to test case insensitivity.
Expand Down Expand Up @@ -182,7 +183,7 @@ class DefaultPermissionsResolverSpec extends Specification {
UserRolesProvider userRolesProvider = Mock(UserRolesProvider)

@Subject DefaultPermissionsResolver resolver = new DefaultPermissionsResolver(
userRolesProvider, serviceAccountProvider, resourceProviders, fiatAdminConfig, new ObjectMapper())
userRolesProvider, serviceAccountProvider, resourceProviders, fiatAdminConfig, new AccountManagerConfig(), new ObjectMapper())

def role1 = new Role("delivery-team")
def testUser = new ExternalUser().setId(testUserId).setExternalRoles([role1])
Expand All @@ -204,7 +205,7 @@ class DefaultPermissionsResolverSpec extends Specification {
setup:
UserRolesProvider userRolesProvider = Mock(UserRolesProvider)
@Subject DefaultPermissionsResolver resolver = new DefaultPermissionsResolver(
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new ObjectMapper())
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new AccountManagerConfig(), new ObjectMapper())

def role1 = new Role("group1")
def role2 = new Role("group2")
Expand Down Expand Up @@ -258,7 +259,7 @@ class DefaultPermissionsResolverSpec extends Specification {
setup:
UserRolesProvider userRolesProvider = Mock(UserRolesProvider)
@Subject DefaultPermissionsResolver resolver = new DefaultPermissionsResolver(
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new ObjectMapper())
userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), new AccountManagerConfig(), new ObjectMapper())

def role1 = new Role(group1SvcAcct.memberOf[0])
def svc1 = new ExternalUser().setId(group1SvcAcct.name).setExternalRoles([role1])
Expand Down Expand Up @@ -293,4 +294,24 @@ class DefaultPermissionsResolverSpec extends Specification {
result.remove("user1") == expectedUser1
result.isEmpty()
}

def "should resolve account manager permissions"() {
setup:
def accountManagerConfig = new AccountManagerConfig()
accountManagerConfig.roles = ['sre']

def testUsername = 'ron'
def userRolesProvider = Mock(UserRolesProvider)
@Subject def resolver = new DefaultPermissionsResolver(userRolesProvider, serviceAccountProvider, resourceProviders, new FiatAdminConfig(), accountManagerConfig, new ObjectMapper())
def role = new Role('sre')
def user = new ExternalUser().setId(testUsername).setExternalRoles([role])

when:
def result = resolver.resolveAndMerge(user)

then:
1 * userRolesProvider.loadRoles({ u -> u.id == testUsername }) >> [role]
def expected = new UserPermission().setId(testUsername).setRoles(Set.of(role)).setAccountManager(true)
result == expected
}
}

0 comments on commit dd191f0

Please sign in to comment.