feat(api): Add support for more Spring Security APIs (#966)

* feat(api): Add support for more Spring Security APIs

This implements a few relevant Spring Security features and updates Fiat to use them in addition to the newly added AccessControlled interface in Kork.

- Introduces Spinnaker-specific GrantedAuthority values for admins, account managers, and anonymous users.
- Adds conversion from a user's set of roles to a set of granted authorities based on Spring Security conventions (`ROLE_foo` style authorities).
- Adds an implementation for FiatPermissionEvaluator::hasPermission for checking permissions on AccessControlled objects.
- Updates to Permissions/UserPermission to support GrantedAuthority conversions and checks.
- Adds spring-security-core as an API dependency to fiat-core as we now reference GrantedAuthority, Authentication, and some other core security interfaces.
- Updates Authorization parsing to support case-insensitive versions as Spring Security defines some common ones in lowercase like read/write/etc.

* feat(api): Add feature flag for granted authorities

* fix(test): null check

* docs(api): Add javadoc to AuthenticationConverter classes

* update(api): Use granted authorities feature flag

fiat-api/fiat-api.gradle

@@ -39,3 +39,7 @@ dependencies {
 
   testImplementation "org.slf4j:slf4j-api"
 }
+
+test {
+  useJUnitPlatform()
+}

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/AuthenticatedRequestAuthenticationConverter.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2022 Apple, 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.shared;
+
+import com.netflix.spinnaker.security.AuthenticatedRequest;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.authority.AuthorityUtils;
+import org.springframework.security.web.authentication.AuthenticationConverter;
+import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
+
+/**
+ * Provides an Authentication for an HTTP request using the current {@link
+ * AuthenticatedRequest#getSpinnakerUser()}.
+ *
+ * @see AuthenticatedRequest
+ */
+public class AuthenticatedRequestAuthenticationConverter implements AuthenticationConverter {
+  @Override
+  public Authentication convert(HttpServletRequest request) {
+    return AuthenticatedRequest.getSpinnakerUser()
+        .map(
+            user ->
+                (Authentication) new PreAuthenticatedAuthenticationToken(user, "N/A", List.of()))
+        .orElseGet(
+            () ->
+                new AnonymousAuthenticationToken(
+                    "anonymous",
+                    "anonymous",
+                    AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")));
+  }
+}

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java

@@ -32,6 +32,7 @@
 import okhttp3.OkHttpClient;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
@@ -43,6 +44,7 @@
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
 import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
+import org.springframework.security.web.authentication.AuthenticationConverter;
 import retrofit.Endpoints;
 import retrofit.RestAdapter;
 import retrofit.converter.JacksonConverter;
@@ -85,9 +87,33 @@ public FiatService fiatService(
         .create(FiatService.class);
   }
 
+  /**
+   * When enabled, this authenticates the {@code X-SPINNAKER-USER} HTTP header using permissions
+   * obtained from {@link FiatPermissionEvaluator#getPermission(String)}. This feature is part of a
+   * larger effort to adopt standard Spring Security APIs rather than using Fiat directly where
+   * possible.
+   */
+  @ConditionalOnProperty("services.fiat.granted-authorities.enabled")
   @Bean
-  FiatWebSecurityConfigurerAdapter fiatSecurityConfig(FiatStatus fiatStatus) {
-    return new FiatWebSecurityConfigurerAdapter(fiatStatus);
+  AuthenticationConverter fiatAuthenticationFilter(FiatPermissionEvaluator permissionEvaluator) {
+    return new FiatAuthenticationConverter(permissionEvaluator);
+  }
+
+  /**
+   * Provides the previous behavior of using PreAuthenticatedAuthenticationToken with no granted
+   * authorities to indicate an authenticated user or an AnonymousAuthenticationToken with an
+   * "ANONYMOUS" role for anonymous authenticated users.
+   */
+  @ConditionalOnMissingBean
+  @Bean
+  AuthenticationConverter defaultAuthenticationConverter() {
+    return new AuthenticatedRequestAuthenticationConverter();
+  }
+
+  @Bean
+  FiatWebSecurityConfigurerAdapter fiatSecurityConfig(
+      FiatStatus fiatStatus, AuthenticationConverter authenticationConverter) {
+    return new FiatWebSecurityConfigurerAdapter(fiatStatus, authenticationConverter);
   }
 
   @Bean
@@ -97,12 +123,15 @@ FiatAccessDeniedExceptionHandler fiatAccessDeniedExceptionHandler(
     return new FiatAccessDeniedExceptionHandler(exceptionMessageDecorator);
   }
 
-  private class FiatWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter {
+  private static class FiatWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter {
     private final FiatStatus fiatStatus;
+    private final AuthenticationConverter authenticationConverter;
 
-    private FiatWebSecurityConfigurerAdapter(FiatStatus fiatStatus) {
+    private FiatWebSecurityConfigurerAdapter(
+        FiatStatus fiatStatus, AuthenticationConverter authenticationConverter) {
       super(true);
       this.fiatStatus = fiatStatus;
+      this.authenticationConverter = authenticationConverter;
     }
 
     @Override
@@ -114,7 +143,8 @@ protected void configure(HttpSecurity http) throws Exception {
           .anonymous()
           .and()
           .addFilterBefore(
-              new FiatAuthenticationFilter(fiatStatus), AnonymousAuthenticationFilter.class);
+              new FiatAuthenticationFilter(fiatStatus, authenticationConverter),
+              AnonymousAuthenticationFilter.class);
     }
   }
 }

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConverter.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2022 Apple, 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.shared;
+
+import com.netflix.spinnaker.fiat.model.SpinnakerAuthorities;
+import com.netflix.spinnaker.fiat.model.UserPermission;
+import com.netflix.spinnaker.kork.common.Header;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import lombok.RequiredArgsConstructor;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.authentication.AuthenticationConverter;
+import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
+
+/**
+ * Converts an {@code X-SPINNAKER-USER} HTTP header into an Authentication object containing a list
+ * of roles and other {@linkplain SpinnakerAuthorities granted authorities} in its granted
+ * authorities.
+ */
+@RequiredArgsConstructor
+public class FiatAuthenticationConverter implements AuthenticationConverter {
+  private final FiatPermissionEvaluator permissionEvaluator;
+
+  @Override
+  public Authentication convert(HttpServletRequest request) {
+    String user = request.getHeader(Header.USER.getHeader());
+    if (user != null) {
+      UserPermission.View permission = permissionEvaluator.getPermission(user);
+      if (permission != null) {
+        return new PreAuthenticatedAuthenticationToken(
+            user, "N/A", permission.toGrantedAuthorities());
+      }
+    }
+    return new AnonymousAuthenticationToken(
+        "anonymous", "anonymous", List.of(SpinnakerAuthorities.ANONYMOUS_AUTHORITY));
+  }
+}

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationFilter.java

@@ -16,58 +16,40 @@
 
 package com.netflix.spinnaker.fiat.shared;
 
-import com.netflix.spinnaker.security.AuthenticatedRequest;
 import java.io.IOException;
-import java.util.ArrayList;
-import javax.servlet.*;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpFilter;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
-import lombok.val;
-import org.springframework.security.authentication.AnonymousAuthenticationToken;
 import org.springframework.security.core.Authentication;
-import org.springframework.security.core.authority.AuthorityUtils;
+import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
+import org.springframework.security.web.authentication.AuthenticationConverter;
 
 @Slf4j
-public class FiatAuthenticationFilter implements Filter {
+public class FiatAuthenticationFilter extends HttpFilter {
 
   private final FiatStatus fiatStatus;
+  private final AuthenticationConverter authenticationConverter;
 
-  public FiatAuthenticationFilter(FiatStatus fiatStatus) {
+  public FiatAuthenticationFilter(
+      FiatStatus fiatStatus, AuthenticationConverter authenticationConverter) {
     this.fiatStatus = fiatStatus;
+    this.authenticationConverter = authenticationConverter;
   }
 
   @Override
-  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+  protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
       throws IOException, ServletException {
-    if (!fiatStatus.isEnabled()) {
-      chain.doFilter(request, response);
-      return;
+    if (fiatStatus.isEnabled()) {
+      SecurityContext ctx = SecurityContextHolder.createEmptyContext();
+      Authentication auth = authenticationConverter.convert(req);
+      ctx.setAuthentication(auth);
+      SecurityContextHolder.setContext(ctx);
+      log.debug("Set SecurityContext to user: {}", auth.getPrincipal().toString());
     }
-
-    Authentication auth =
-        AuthenticatedRequest.getSpinnakerUser()
-            .map(
-                username ->
-                    (Authentication)
-                        new PreAuthenticatedAuthenticationToken(username, null, new ArrayList<>()))
-            .orElseGet(
-                () ->
-                    new AnonymousAuthenticationToken(
-                        "anonymous",
-                        "anonymous",
-                        AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")));
-
-    val ctx = SecurityContextHolder.createEmptyContext();
-    ctx.setAuthentication(auth);
-    SecurityContextHolder.setContext(ctx);
-    log.debug("Set SecurityContext to user: {}", auth.getPrincipal().toString());
-    chain.doFilter(request, response);
+    chain.doFilter(req, res);
   }
-
-  @Override
-  public void init(FilterConfig filterConfig) throws ServletException {}
-
-  @Override
-  public void destroy() {}
 }

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatClientConfigurationProperties.java

@@ -43,6 +43,9 @@ public class FiatClientConfigurationProperties {
 
   @NestedConfigurationProperty private RetryConfiguration retry = new RetryConfiguration();
 
+  @NestedConfigurationProperty
+  private GrantedAuthorities grantedAuthorities = new GrantedAuthorities();
+
   public RetryConfiguration getRetry() {
     retry.setDynamicConfigService(dynamicConfigService);
     return retry;
@@ -93,4 +96,9 @@ public double getRetryMultiplier() {
           Double.class, "fiat.retry.retryMultiplier", retryMultiplier);
     }
   }
+
+  @Data
+  public static class GrantedAuthorities {
+    private boolean enabled;
+  }
 }

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java

@@ -21,12 +21,14 @@
 import com.netflix.spectator.api.Id;
 import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.fiat.model.Authorization;
+import com.netflix.spinnaker.fiat.model.SpinnakerAuthorities;
 import com.netflix.spinnaker.fiat.model.UserPermission;
 import com.netflix.spinnaker.fiat.model.resources.Account;
 import com.netflix.spinnaker.fiat.model.resources.Authorizable;
 import com.netflix.spinnaker.fiat.model.resources.ResourceType;
 import com.netflix.spinnaker.kork.exceptions.IntegrationException;
 import com.netflix.spinnaker.kork.telemetry.caffeine.CaffeineStatsCounter;
+import com.netflix.spinnaker.security.AccessControlled;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import java.io.Serializable;
 import java.util.Arrays;
@@ -148,6 +150,25 @@ private static RetryHandler buildRetryHandler(
   @Override
   public boolean hasPermission(
       Authentication authentication, Object resource, Object authorization) {
+    if (!fiatStatus.isGrantedAuthoritiesEnabled()) {
+      return false;
+    }
+    if (!fiatStatus.isEnabled()) {
+      return true;
+    }
+    if (authentication == null || resource == null) {
+      log.warn(
+          "Permission denied because at least one of the required arguments was null. authentication={}, resource={}",
+          authentication,
+          resource);
+      return false;
+    }
+    if (authentication.getAuthorities().contains(SpinnakerAuthorities.ADMIN_AUTHORITY)) {
+      return true;
+    }
+    if (resource instanceof AccessControlled) {
+      return ((AccessControlled) resource).isAuthorized(authentication, authorization);
+    }
     return false;
   }
 
@@ -223,7 +244,7 @@ public boolean hasPermission(
 
     // Service accounts don't have read/write authorizations.
     if (!r.equals(ResourceType.SERVICE_ACCOUNT)) {
-      a = Authorization.valueOf(authorization.toString());
+      a = Authorization.parse(authorization);
     }
 
     if (a == Authorization.CREATE) {

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatStatus.java

@@ -35,6 +35,7 @@ public class FiatStatus {
 
   private final AtomicBoolean enabled;
   private final AtomicBoolean legacyFallbackEnabled;
+  private final AtomicBoolean grantedAuthoritiesEnabled;
 
   @Autowired
   public FiatStatus(
@@ -47,13 +48,18 @@ public FiatStatus(
     this.enabled = new AtomicBoolean(fiatClientConfigurationProperties.isEnabled());
     this.legacyFallbackEnabled =
         new AtomicBoolean(fiatClientConfigurationProperties.isLegacyFallback());
+    this.grantedAuthoritiesEnabled =
+        new AtomicBoolean(fiatClientConfigurationProperties.getGrantedAuthorities().isEnabled());
 
     PolledMeter.using(registry)
         .withName("fiat.enabled")
         .monitorValue(enabled, value -> enabled.get() ? 1 : 0);
     PolledMeter.using(registry)
         .withName("fiat.legacyFallback.enabled")
         .monitorValue(legacyFallbackEnabled, value -> legacyFallbackEnabled.get() ? 1 : 0);
+    PolledMeter.using(registry)
+        .withName("fiat.granted-authorities.enabled")
+        .monitorValue(grantedAuthoritiesEnabled, value -> grantedAuthoritiesEnabled.get() ? 1 : 0);
   }
 
   public boolean isEnabled() {
@@ -64,6 +70,10 @@ public boolean isLegacyFallbackEnabled() {
     return legacyFallbackEnabled.get();
   }
 
+  public boolean isGrantedAuthoritiesEnabled() {
+    return grantedAuthoritiesEnabled.get();
+  }
+
   @Scheduled(fixedDelay = 30000L)
   void refreshStatus() {
     try {
@@ -76,6 +86,10 @@ void refreshStatus() {
       legacyFallbackEnabled.set(
           dynamicConfigService.isEnabled(
               "fiat.legacyFallback", fiatClientConfigurationProperties.isLegacyFallback()));
+      grantedAuthoritiesEnabled.set(
+          dynamicConfigService.isEnabled(
+              "fiat.granted-authorities",
+              fiatClientConfigurationProperties.getGrantedAuthorities().isEnabled()));
     } catch (Exception e) {
       log.warn("Unable to refresh fiat status, reason: {}", e.getMessage());
     }