feat(permission): enables customization of permission sources and agg…

…regation (#481)

Adds a `ResourcePermissionSource` - a place to get permissions from, and
`ResourcePermissionProvider` - the API for Permissions consumers

The default `ResourcePermissionProvider` is a single source 
ResourcePermissionProvider that reads Permissions from the resource objects,
however an AggregatingResourcePermissionProvider is available that will 
merge all Permissions from all configured ResourcePermissionSources.

This can be enabled by setting `auth.permissions.provider.<resource type>: aggregate`

There is a `DefaultResourcePermissionConfiguration` that maintains the
status quo by providing default ResourcePermissionSources that read
Permissions directly off of the objects, but allows opting out of some
or all of the default opinions if someone wants to customize, extend,
or override any of the default behaviour.

Additional improvements:

* refactor(fiat-roles):
    - make DefaultPermissionsResolver immutable
    - don't return null values from Permissions#get
    - clean up loadAll

* refactor(permissions): Permissions should always have a non null value for all Authorization types

Cleans up Permission construction via Permissions.Builder to ensure we populate an
immutable collection (empty if unspecified) for all Authorization types when building
Permission objects.

Redirects empty build() results to a shared Permissions.EMPTY.

Fixes up a couple of accidentally non-final constant values.

* refactor(permissions): Add Notnull annotations on resource permission provider/source

Also moves Authorization.ALL to EnumSet

fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/Authorization.java

@@ -16,16 +16,15 @@
 
 package com.netflix.spinnaker.fiat.model;
 
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.EnumSet;
 import java.util.Set;
 
 public enum Authorization {
   READ,
   WRITE,
   EXECUTE;
 
-  public static Set<Authorization> ALL =
-      Collections.unmodifiableSet(new HashSet<>(Arrays.asList(values())));
+  public static final Set<Authorization> ALL =
+      Collections.unmodifiableSet(EnumSet.allOf(Authorization.class));
 }

fiat-core/src/main/java/com/netflix/spinnaker/fiat/model/resources/Permissions.java

@@ -27,19 +27,19 @@
 
 /**
  * Representation of authorization configuration for a resource. This object is immutable, which
- * makes it challenging when working with Jackson's {{ObjectMapper}} and Spring's
- * {{\@ConfigurationProperties}}. The {@link Builder} is a helper class for the latter use case.
+ * makes it challenging when working with Jackson's {@code ObjectMapper} and Spring's
+ * {@code @ConfigurationProperties}. The {@link Builder} is a helper class for the latter use case.
  */
 @ToString
 @EqualsAndHashCode
 public class Permissions {
 
-  public static Permissions EMPTY = new Permissions.Builder().build();
+  public static final Permissions EMPTY = Builder.fromMap(Collections.emptyMap());
 
   private final Map<Authorization, List<String>> permissions;
 
   private Permissions(Map<Authorization, List<String>> p) {
-    this.permissions = p;
+    this.permissions = Collections.unmodifiableMap(p);
   }
 
   /**
@@ -69,10 +69,6 @@ public boolean isAuthorized(Set<Role> userRoles) {
     return !getAuthorizations(userRoles).isEmpty();
   }
 
-  public boolean isEmpty() {
-    return permissions.isEmpty();
-  }
-
   public Set<Authorization> getAuthorizations(Set<Role> userRoles) {
     val r = userRoles.stream().map(Role::getName).collect(Collectors.toList());
     return getAuthorizations(r);
@@ -90,7 +86,7 @@ public Set<Authorization> getAuthorizations(List<String> userRoles) {
   }
 
   public List<String> get(Authorization a) {
-    return permissions.get(a);
+    return permissions.getOrDefault(a, Collections.emptyList());
   }
 
   /**
@@ -105,6 +101,25 @@ public List<String> get(Authorization a) {
    */
   public static class Builder extends LinkedHashMap<Authorization, List<String>> {
 
+    private static Permissions fromMap(Map<Authorization, List<String>> authConfig) {
+      final Map<Authorization, List<String>> perms = new EnumMap<>(Authorization.class);
+      for (Authorization auth : Authorization.values()) {
+        perms.put(
+            auth,
+            Optional.ofNullable(authConfig.get(auth))
+                .map(
+                    groups ->
+                        groups.stream()
+                            .map(String::trim)
+                            .filter(s -> !s.isEmpty())
+                            .map(String::toLowerCase)
+                            .collect(Collectors.toList()))
+                .map(Collections::unmodifiableList)
+                .orElse(Collections.emptyList()));
+      }
+      return new Permissions(perms);
+    }
+
     @JsonCreator
     public static Builder factory(Map<Authorization, List<String>> data) {
       return new Builder().set(data);
@@ -127,19 +142,11 @@ public Builder add(Authorization a, List<String> groups) {
     }
 
     public Permissions build() {
-      final Map<Authorization, List<String>> perms = new HashMap<>();
-      this.forEach(
-          (auth, groups) -> {
-            List<String> lowerGroups =
-                Collections.unmodifiableList(
-                    groups.stream()
-                        .map(String::trim)
-                        .filter(s -> !s.isEmpty())
-                        .map(String::toLowerCase)
-                        .collect(Collectors.toList()));
-            perms.put(auth, lowerGroups);
-          });
-      return new Permissions(Collections.unmodifiableMap(perms));
+      final Permissions result = fromMap(this);
+      if (!result.isRestricted()) {
+        return Permissions.EMPTY;
+      }
+      return result;
     }
   }
 }

fiat-core/src/test/groovy/com/netflix/spinnaker/fiat/model/resources/PermissionsSpec.groovy

@@ -42,10 +42,18 @@ class PermissionsSpec extends Specification {
       .enable(SerializationFeature.INDENT_OUTPUT)
       .enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS)
 
-  String permissionJson = '''{
-  "READ" : [ "foo" ],
-  "WRITE" : [ "bar" ]
-}'''
+  String permissionJson = '''\
+    {
+      "READ" : [ "foo" ],
+      "WRITE" : [ "bar" ]
+    }'''.stripIndent()
+
+  String permissionSerialized = '''\
+    {
+      "READ" : [ "foo" ],
+      "WRITE" : [ "bar" ],
+      "EXECUTE" : [ ]
+    }'''.stripIndent()
 
   def "should deserialize"() {
     when:
@@ -54,6 +62,7 @@ class PermissionsSpec extends Specification {
     then:
     p.get(R) == ["foo"]
     p.get(W) == ["bar"]
+    p.get(E) == []
 
     when:
     Permissions.Builder b = mapper.readValue(permissionJson, Permissions.Builder)
@@ -62,6 +71,7 @@ class PermissionsSpec extends Specification {
     then:
     p.get(R) == ["foo"]
     p.get(W) == ["bar"]
+    p.get(E) == []
   }
 
   def "should serialize"() {
@@ -70,7 +80,7 @@ class PermissionsSpec extends Specification {
     b.set([(R): ["foo"], (W): ["bar"]])
 
     then:
-    permissionJson ==  mapper.writeValueAsString(b.build())
+    permissionSerialized ==  mapper.writeValueAsString(b.build())
   }
 
   def "can deserialize to builder from serialized Permissions"() {
@@ -85,7 +95,6 @@ class PermissionsSpec extends Specification {
 
     then:
     p1 == p2
-    b1 == b2
   }
 
   def "should trim and lower"() {

fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java

@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.fiat.permissions;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
 import com.netflix.spinnaker.fiat.config.FiatAdminConfig;
 import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig;
 import com.netflix.spinnaker.fiat.model.UserPermission;
@@ -36,31 +37,35 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import lombok.NoArgsConstructor;
 import lombok.NonNull;
-import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
 
 @Component
-@NoArgsConstructor
 @Slf4j
 public class DefaultPermissionsResolver implements PermissionsResolver {
 
-  @Autowired @Setter private UserRolesProvider userRolesProvider;
-
-  @Autowired @Setter private ResourceProvider<ServiceAccount> serviceAccountProvider;
-
-  @Autowired @Setter private List<ResourceProvider<? extends Resource>> resourceProviders;
-
-  @Autowired @Setter private FiatAdminConfig fiatAdminConfig;
+  private final UserRolesProvider userRolesProvider;
+  private final ResourceProvider<ServiceAccount> serviceAccountProvider;
+  private final ImmutableList<ResourceProvider<? extends Resource>> resourceProviders;
+  private final FiatAdminConfig fiatAdminConfig;
+  private final ObjectMapper mapper;
 
   @Autowired
-  @Qualifier("objectMapper")
-  @Setter
-  private ObjectMapper mapper;
+  public DefaultPermissionsResolver(
+      UserRolesProvider userRolesProvider,
+      ResourceProvider<ServiceAccount> serviceAccountProvider,
+      List<ResourceProvider<? extends Resource>> resourceProviders,
+      FiatAdminConfig fiatAdminConfig,
+      @Qualifier("objectMapper") ObjectMapper mapper) {
+    this.userRolesProvider = userRolesProvider;
+    this.serviceAccountProvider = serviceAccountProvider;
+    this.resourceProviders = ImmutableList.copyOf(resourceProviders);
+    this.fiatAdminConfig = fiatAdminConfig;
+    this.mapper = mapper;
+  }
 
   @Override
   public UserPermission resolveUnrestrictedUser() {
@@ -123,13 +128,12 @@ private UserPermission getUserPermission(String userId, Set<Role> roles) {
   }
 
   @Override
-  @SuppressWarnings("unchecked")
   public Map<String, UserPermission> resolve(@NonNull Collection<ExternalUser> users) {
     Map<String, Collection<Role>> allServiceAccountRoles = getServiceAccountRoles();
 
     Collection<ExternalUser> serviceAccounts =
         users.stream()
-            .filter(user -> allServiceAccountRoles.keySet().contains(user.getId()))
+            .filter(user -> allServiceAccountRoles.containsKey(user.getId()))
             .collect(Collectors.toList());
 
     // Service accounts should already have external roles set. Remove them from the list so they
@@ -160,11 +164,10 @@ private Map<String, Collection<Role>> getAndMergeUserRoles(
     Map<String, Collection<Role>> userToRoles = userRolesProvider.multiLoadRoles(users);
 
     users.forEach(
-        user -> {
-          userToRoles
-              .computeIfAbsent(user.getId(), ignored -> new ArrayList<>())
-              .addAll(user.getExternalRoles());
-        });
+        user ->
+            userToRoles
+                .computeIfAbsent(user.getId(), ignored -> new ArrayList<>())
+                .addAll(user.getExternalRoles()));
 
     if (log.isDebugEnabled()) {
       try {

fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/AccessControlledResourcePermissionSource.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2019 Google, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.fiat.providers;
+
+import com.netflix.spinnaker.fiat.model.resources.Permissions;
+import com.netflix.spinnaker.fiat.model.resources.Resource;
+import java.util.Optional;
+import javax.annotation.Nonnull;
+
+public final class AccessControlledResourcePermissionSource<T extends Resource.AccessControlled>
+    implements ResourcePermissionSource<T> {
+
+  @Override
+  @Nonnull
+  public Permissions getPermissions(@Nonnull T resource) {
+    return Optional.ofNullable(resource)
+        .map(Resource.AccessControlled::getPermissions)
+        .filter(Permissions::isRestricted)
+        .orElse(Permissions.EMPTY);
+  }
+}

fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/AggregatingResourcePermissionProvider.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2019 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.fiat.providers;
+
+import com.netflix.spinnaker.fiat.model.Authorization;
+import com.netflix.spinnaker.fiat.model.resources.Permissions;
+import com.netflix.spinnaker.fiat.model.resources.Resource;
+import java.util.List;
+import javax.annotation.Nonnull;
+
+/**
+ * AggregatingResourcePermissionProvider additively combines permissions from all
+ * ResourcePermissionSources to build a resulting Permission object.
+ *
+ * @param <T> the type of Resource for this AggregatingResourcePermissionProvider
+ */
+public class AggregatingResourcePermissionProvider<T extends Resource>
+    implements ResourcePermissionProvider<T> {
+
+  private final List<ResourcePermissionSource<T>> resourcePermissionSources;
+
+  public AggregatingResourcePermissionProvider(
+      List<ResourcePermissionSource<T>> resourcePermissionSources) {
+    this.resourcePermissionSources = resourcePermissionSources;
+  }
+
+  @Override
+  @Nonnull
+  public Permissions getPermissions(@Nonnull T resource) {
+    Permissions.Builder builder = new Permissions.Builder();
+    for (ResourcePermissionSource<T> source : resourcePermissionSources) {
+      Permissions permissions = source.getPermissions(resource);
+      if (permissions.isRestricted()) {
+        for (Authorization auth : Authorization.values()) {
+          builder.add(auth, permissions.get(auth));
+        }
+      }
+    }
+
+    return builder.build();
+  }
+}

fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseProvider.java → fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseResourceProvider.java

@@ -32,7 +32,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 
 @Slf4j
-public abstract class BaseProvider<R extends Resource> implements ResourceProvider<R> {
+public abstract class BaseResourceProvider<R extends Resource> implements ResourceProvider<R> {
 
   private static final Integer CACHE_KEY = 0;