Skip to content

Commit

Permalink
refactor(fallback): Add FallbackPermissionsResolver and default imple…
Browse files Browse the repository at this point in the history
…mentation (#521)
  • Loading branch information
jonsie committed Dec 6, 2019
1 parent d85328b commit 17ed589
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 24 deletions.
@@ -0,0 +1,38 @@
package com.netflix.spinnaker.fiat.permissions;

import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;

import com.netflix.spinnaker.fiat.model.Authorization;
import com.netflix.spinnaker.fiat.model.resources.Permissions;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import javax.annotation.Nonnull;

public class DefaultFallbackPermissionsResolver implements FallbackPermissionsResolver {

private final Authorization fallbackFrom;
private final Authorization fallbackTo;

public DefaultFallbackPermissionsResolver(Authorization fallbackFrom, Authorization fallbackTo) {
this.fallbackFrom = fallbackFrom;
this.fallbackTo = fallbackTo;
}

@Override
public boolean shouldResolve(@Nonnull Permissions permissions) {
return permissions.isRestricted() && unpackPermissions(permissions).get(fallbackFrom).isEmpty();
}

@Override
public Permissions resolve(@Nonnull Permissions permissions) {
Map<Authorization, List<String>> authorizations = unpackPermissions(permissions);
authorizations.put(fallbackFrom, authorizations.get(fallbackTo));
return Permissions.Builder.factory(authorizations).build();
}

private Map<Authorization, List<String>> unpackPermissions(Permissions permissions) {
return Arrays.stream(Authorization.values()).collect(toMap(identity(), permissions::get));
}
}
@@ -0,0 +1,28 @@
package com.netflix.spinnaker.fiat.permissions;

import com.netflix.spinnaker.fiat.model.resources.Permissions;
import javax.annotation.Nonnull;

/**
* Resolve permissions. This is useful if you do not have a way to configure a particular permission
* and so want to apply one permission group type to another.
*/
public interface FallbackPermissionsResolver {

/**
* Determine if resolving fallback permissions is necessary - typically checking if permissions
* are restricted.
*
* @param permissions
* @return boolean
*/
boolean shouldResolve(@Nonnull Permissions permissions);

/**
* Resolve fallback permissions.
*
* @param permissions
* @return The resolved Permissions
*/
Permissions resolve(@Nonnull Permissions permissions);
}
Expand Up @@ -24,7 +24,7 @@

/**
* AggregatingResourcePermissionProvider additively combines permissions from all
* ResourcePermissionSources to build a resulting Permission object.
* ResourcePermissionSources to build a resulting Permissions object.
*
* @param <T> the type of Resource for this AggregatingResourcePermissionProvider
*/
Expand Down
Expand Up @@ -30,12 +30,6 @@
public final class ApplicationResourcePermissionSource
implements ResourcePermissionSource<Application> {

private final Authorization executeFallback;

public ApplicationResourcePermissionSource(Authorization executeFallback) {
this.executeFallback = executeFallback;
}

@Override
@Nonnull
public Permissions getPermissions(@Nonnull Application resource) {
Expand All @@ -47,12 +41,6 @@ public Permissions getPermissions(@Nonnull Application resource) {
Map<Authorization, List<String>> authorizations =
Arrays.stream(Authorization.values()).collect(toMap(identity(), storedPermissions::get));

// If the execute permission wasn't set, copy the permissions from whatever is specified in the
// config's executeFallback flag
if (authorizations.get(Authorization.EXECUTE).isEmpty()) {
authorizations.put(Authorization.EXECUTE, authorizations.get(executeFallback));
}

// CREATE permissions are not allowed on the resource level.
authorizations.remove(Authorization.CREATE);

Expand Down
Expand Up @@ -22,7 +22,9 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.netflix.spinnaker.fiat.model.resources.Application;
import com.netflix.spinnaker.fiat.model.resources.Permissions;
import com.netflix.spinnaker.fiat.model.resources.Role;
import com.netflix.spinnaker.fiat.permissions.FallbackPermissionsResolver;
import com.netflix.spinnaker.fiat.providers.internal.ClouddriverService;
import com.netflix.spinnaker.fiat.providers.internal.Front50Service;
import java.util.Collections;
Expand All @@ -38,17 +40,20 @@ public class DefaultApplicationResourceProvider extends BaseResourceProvider<App
private final Front50Service front50Service;
private final ClouddriverService clouddriverService;
private final ResourcePermissionProvider<Application> permissionProvider;
private final FallbackPermissionsResolver executeFallbackPermissionsResolver;

private final boolean allowAccessToUnknownApplications;

public DefaultApplicationResourceProvider(
Front50Service front50Service,
ClouddriverService clouddriverService,
ResourcePermissionProvider<Application> permissionProvider,
FallbackPermissionsResolver executeFallbackPermissionsResolver,
boolean allowAccessToUnknownApplications) {
this.front50Service = front50Service;
this.clouddriverService = clouddriverService;
this.permissionProvider = permissionProvider;
this.executeFallbackPermissionsResolver = executeFallbackPermissionsResolver;
this.allowAccessToUnknownApplications = allowAccessToUnknownApplications;
}

Expand Down Expand Up @@ -79,8 +84,15 @@ protected Set<Application> loadAll() throws ProviderException {
.collect(toImmutableList());

applications.forEach(
application ->
application.setPermissions(permissionProvider.getPermissions(application)));
application -> {
Permissions permissions = permissionProvider.getPermissions(application);

// Check to see if we need to fallback permissions to the configured fallback
application.setPermissions(
executeFallbackPermissionsResolver.shouldResolve(permissions)
? executeFallbackPermissionsResolver.resolve(permissions)
: permissions);
});

if (allowAccessToUnknownApplications) {
// no need to include applications w/o explicit permissions if we're allowing access to
Expand Down
@@ -0,0 +1,36 @@
package com.netflix.spinnaker.fiat.permissions

import com.netflix.spinnaker.fiat.model.Authorization
import com.netflix.spinnaker.fiat.model.resources.Permissions
import spock.lang.Specification
import spock.lang.Unroll

class DefaultFallbackPermissionsResolverSpec extends Specification {
private static final Authorization R = Authorization.READ
private static final Authorization W = Authorization.WRITE
private static final Authorization E = Authorization.EXECUTE
private static final Authorization C = Authorization.CREATE

def makePerms(Map<Authorization, List<String>> auths) {
return Permissions.Builder.factory(auths).build()
}

@Unroll
def "should add fallback permissions based on fallbackTo value" () {
setup:
FallbackPermissionsResolver fallbackResolver = new DefaultFallbackPermissionsResolver(fallbackFrom, fallbackTo)

when:
def result = fallbackResolver.resolve(makePerms(givenPermissions))

then:
makePerms(expectedPermissions) == result

where:
fallbackFrom || fallbackTo || givenPermissions || expectedPermissions
E || R || [:] || [:]
E || R || [(R): ['r']] || [(R): ['r'], (E): ['r']]
E || W || [(R): ['r'], (W): ['w']] || [(R): ['r'], (W): ['w'], (E): ['w']]
C || W || [(R): ['r'], (W): ['w']] || [(R): ['r'], (W): ['w'], (C): ['w']]
}
}
Expand Up @@ -20,6 +20,8 @@ import com.netflix.spinnaker.fiat.model.Authorization
import com.netflix.spinnaker.fiat.model.resources.Application
import com.netflix.spinnaker.fiat.model.resources.Permissions
import com.netflix.spinnaker.fiat.model.resources.Role
import com.netflix.spinnaker.fiat.permissions.DefaultFallbackPermissionsResolver
import com.netflix.spinnaker.fiat.permissions.FallbackPermissionsResolver
import com.netflix.spinnaker.fiat.providers.internal.ClouddriverService
import com.netflix.spinnaker.fiat.providers.internal.Front50Service
import org.apache.commons.collections4.CollectionUtils
Expand All @@ -34,7 +36,8 @@ class DefaultApplicationProviderSpec extends Specification {

ClouddriverService clouddriverService = Mock(ClouddriverService)
Front50Service front50Service = Mock(Front50Service)
ResourcePermissionProvider<Application> defaultProvider = new AggregatingResourcePermissionProvider<>([new ApplicationResourcePermissionSource(Authorization.READ)])
ResourcePermissionProvider<Application> defaultProvider = new AggregatingResourcePermissionProvider<>([new ApplicationResourcePermissionSource()])
FallbackPermissionsResolver fallbackPermissionsResolver = new DefaultFallbackPermissionsResolver(Authorization.EXECUTE, Authorization.READ)

@Subject DefaultApplicationResourceProvider provider

Expand All @@ -59,7 +62,7 @@ class DefaultApplicationProviderSpec extends Specification {
]
}

provider = new DefaultApplicationResourceProvider(front50Service, clouddriverService, defaultProvider, allowAccessToUnknownApplications)
provider = new DefaultApplicationResourceProvider(front50Service, clouddriverService, defaultProvider, fallbackPermissionsResolver, allowAccessToUnknownApplications)

when:
def restrictedResult = provider.getAllRestricted([new Role(role)] as Set<Role>, false)
Expand Down Expand Up @@ -92,7 +95,7 @@ class DefaultApplicationProviderSpec extends Specification {
when:
app.setPermissions(makePerms(givenPermissions))
provider = new DefaultApplicationResourceProvider(
front50Service, clouddriverService, defaultProvider, allowAccessToUnknownApplications)
front50Service, clouddriverService, defaultProvider, fallbackPermissionsResolver, allowAccessToUnknownApplications)
def resultApps = provider.getAll()

then:
Expand All @@ -114,11 +117,11 @@ class DefaultApplicationProviderSpec extends Specification {
def "should add fallback execute permissions based on executeFallback value" () {
setup:
def app = new Application().setName("app")
def provider = new AggregatingResourcePermissionProvider([new ApplicationResourcePermissionSource(fallback)])
FallbackPermissionsResolver fallbackResolver = new DefaultFallbackPermissionsResolver(Authorization.EXECUTE, fallback)

when:
app.setPermissions(makePerms(givenPermissions))
provider = new DefaultApplicationResourceProvider(front50Service, clouddriverService, provider, false)
provider = new DefaultApplicationResourceProvider(front50Service, clouddriverService, defaultProvider, fallbackResolver, false)
def resultApps = provider.getAll()

then:
Expand Down
Expand Up @@ -53,10 +53,8 @@ public ResourcePermissionProvider<Account> defaultAccountPermissionProvider(
value = "auth.permissions.source.application.resource.enabled",
matchIfMissing = true)
@Order
ResourcePermissionSource<Application> applicationResourcePermissionSource(
FiatServerConfigurationProperties fiatServerConfigurationProperties) {
return new ApplicationResourcePermissionSource(
fiatServerConfigurationProperties.getExecuteFallback());
ResourcePermissionSource<Application> applicationResourcePermissionSource() {
return new ApplicationResourcePermissionSource();
}

@Bean
Expand Down
Expand Up @@ -2,9 +2,12 @@

import com.google.common.collect.ImmutableList;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.fiat.model.Authorization;
import com.netflix.spinnaker.fiat.model.resources.Application;
import com.netflix.spinnaker.fiat.model.resources.Role;
import com.netflix.spinnaker.fiat.permissions.DefaultFallbackPermissionsResolver;
import com.netflix.spinnaker.fiat.permissions.ExternalUser;
import com.netflix.spinnaker.fiat.permissions.FallbackPermissionsResolver;
import com.netflix.spinnaker.fiat.providers.DefaultApplicationResourceProvider;
import com.netflix.spinnaker.fiat.providers.ResourcePermissionProvider;
import com.netflix.spinnaker.fiat.providers.internal.ClouddriverService;
Expand Down Expand Up @@ -74,14 +77,23 @@ DefaultApplicationResourceProvider applicationProvider(
Front50Service front50Service,
ClouddriverService clouddriverService,
ResourcePermissionProvider<Application> permissionProvider,
FallbackPermissionsResolver executeFallbackPermissionsResolver,
FiatServerConfigurationProperties properties) {
return new DefaultApplicationResourceProvider(
front50Service,
clouddriverService,
permissionProvider,
executeFallbackPermissionsResolver,
properties.isAllowAccessToUnknownApplications());
}

@Bean
DefaultFallbackPermissionsResolver executeFallbackPermissionsResolver(
FiatServerConfigurationProperties properties) {
return new DefaultFallbackPermissionsResolver(
Authorization.EXECUTE, properties.getExecuteFallback());
}

/**
* This AuthenticatedRequestFilter pulls the email and accounts out of the Spring security context
* in order to enabling forwarding them to downstream components.
Expand Down

0 comments on commit 17ed589

Please sign in to comment.