Support additional auth types

kork-security/src/main/java/com/netflix/spinnaker/security/AllowedAccountAuthority.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2023 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.security;
+
+import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
+import java.util.Locale;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import org.springframework.security.core.GrantedAuthority;
+
+/**
+ * Represents an authorization to use an account. Allowed account authorities are based on an older
+ * security mechanism used by Spinnaker before other account types besides cloud providers and
+ * permissions were added.
+ */
+@Getter
+@EqualsAndHashCode(of = "account")
+@NonnullByDefault
+public class AllowedAccountAuthority implements GrantedAuthority {
+  private final String account;
+  private final String authority;
+
+  public AllowedAccountAuthority(String account) {
+    this.account = account.toLowerCase(Locale.ROOT);
+    authority = AllowedAccountsAuthorities.PREFIX + this.account;
+  }
+}

kork-security/src/main/java/com/netflix/spinnaker/security/AuthenticatedRequest.java

@@ -20,6 +20,7 @@
 
 import com.google.common.base.Preconditions;
 import com.netflix.spinnaker.kork.common.Header;
+import java.security.Principal;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -33,6 +34,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
+import org.springframework.security.core.AuthenticatedPrincipal;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.userdetails.User;
@@ -85,9 +87,16 @@ default Optional<String> getSpinnakerUser() {
      * @return the user id of the provided principal
      */
     default Optional<String> getSpinnakerUser(Object principal) {
-      return (principal instanceof UserDetails)
-          ? Optional.ofNullable(((UserDetails) principal).getUsername())
-          : get(Header.USER);
+      if (principal instanceof UserDetails) {
+        return Optional.ofNullable(((UserDetails) principal).getUsername());
+      }
+      if (principal instanceof AuthenticatedPrincipal) {
+        return Optional.ofNullable(((AuthenticatedPrincipal) principal).getName());
+      }
+      if (principal instanceof Principal) {
+        return Optional.ofNullable(((Principal) principal).getName());
+      }
+      return Optional.ofNullable(principal).map(Object::toString).or(() -> get(Header.USER));
     }
   }
 

kork-security/src/main/java/com/netflix/spinnaker/security/User.java

@@ -18,14 +18,14 @@
 
 import static java.util.Collections.unmodifiableCollection;
 import static java.util.Collections.unmodifiableList;
-import static java.util.stream.Collectors.toList;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.springframework.security.core.GrantedAuthority;
-import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.util.StringUtils;
 
@@ -57,10 +57,11 @@ public class User implements UserDetails {
 
   @Override
   public List<? extends GrantedAuthority> getAuthorities() {
-    return roles.stream()
-        .filter(StringUtils::hasText)
-        .map(SimpleGrantedAuthority::new)
-        .collect(toList());
+    Stream<AllowedAccountAuthority> accountAuthorities =
+        allowedAccounts.stream().filter(StringUtils::hasText).map(AllowedAccountAuthority::new);
+    Stream<GrantedAuthority> roleAuthorities =
+        roles.stream().filter(StringUtils::hasText).map(SpinnakerAuthorities::forRoleName);
+    return Stream.concat(accountAuthorities, roleAuthorities).collect(Collectors.toList());
   }
 
   /** Not used */

kork-security/src/test/groovy/com/netflix/spinnaker/security/AuthenticatedRequestSpec.groovy

@@ -18,6 +18,9 @@ package com.netflix.spinnaker.security
 
 import com.netflix.spinnaker.kork.common.Header
 import org.slf4j.MDC
+import org.springframework.security.authentication.TestingAuthenticationToken
+import org.springframework.security.core.AuthenticatedPrincipal
+import org.springframework.security.core.context.SecurityContextHolder
 import spock.lang.Specification
 
 class AuthenticatedRequestSpec extends Specification {
@@ -136,4 +139,18 @@ class AuthenticatedRequestSpec extends Specification {
     then:
     closure.run()
   }
+
+  void "should support AuthenticatedPrincipal"() {
+    when:
+    def user = new TestingAuthenticationToken(new TestPrincipal(name: 'foo'), '',
+      [SpinnakerAuthorities.forRoleName('alpha')])
+    SecurityContextHolder.context.authentication = user
+
+    then:
+    AuthenticatedRequest.spinnakerUser.get() == 'foo'
+  }
+
+  static class TestPrincipal implements AuthenticatedPrincipal {
+    String name
+  }
 }

kork-security/src/test/groovy/com/netflix/spinnaker/security/UserSpec.groovy

@@ -69,7 +69,16 @@ class UserSpec extends Specification {
   def "should filter out empty roles"() {
     expect:
       new User(roles: [""]).getAuthorities().isEmpty()
-      new User(roles: ["", "bar"]).getAuthorities()*.getAuthority() == ["bar"]
-      new User(roles: ["foo", "", "bar"]).getAuthorities()*.getAuthority() == ["foo", "bar"]
+      new User(roles: ["", "bar"]).getAuthorities()*.getAuthority() == ["ROLE_bar"]
+      new User(roles: ["foo", "", "bar"]).getAuthorities()*.getAuthority() == ["ROLE_foo", "ROLE_bar"]
+  }
+
+  def "should translate allowed accounts into granted authorities"() {
+    setup:
+      def user = new User(allowedAccounts: ['a', 'b', 'c'])
+
+    expect:
+      user.getAuthorities()*.getAuthority() ==
+        ['ALLOWED_ACCOUNT_a', 'ALLOWED_ACCOUNT_b', 'ALLOWED_ACCOUNT_c']
   }
 }

kork-web/src/main/groovy/com/netflix/spinnaker/filters/AuthenticatedRequestFilter.groovy

@@ -17,24 +17,24 @@
 package com.netflix.spinnaker.filters
 
 import com.netflix.spinnaker.kork.common.Header
-import com.netflix.spinnaker.security.AllowedAccountsAuthorities
+import com.netflix.spinnaker.security.AllowedAccountAuthority
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import groovy.util.logging.Slf4j
+import org.springframework.security.core.context.SecurityContext
 import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.core.context.SecurityContextImpl
-import org.springframework.security.core.userdetails.UserDetails
+import org.springframework.security.web.context.HttpRequestResponseHolder
+import org.springframework.security.web.context.HttpSessionSecurityContextRepository
+import org.springframework.security.web.context.SecurityContextRepository
+import org.springframework.web.filter.OncePerRequestFilter
 
-import javax.servlet.Filter
 import javax.servlet.FilterChain
-import javax.servlet.FilterConfig
 import javax.servlet.ServletException
-import javax.servlet.ServletRequest
-import javax.servlet.ServletResponse
 import javax.servlet.http.HttpServletRequest
+import javax.servlet.http.HttpServletResponse
 import java.security.cert.X509Certificate
 
 @Slf4j
-class AuthenticatedRequestFilter implements Filter {
+class AuthenticatedRequestFilter extends OncePerRequestFilter {
   private static final String X509_CERTIFICATE = "javax.servlet.request.X509Certificate"
 
   /*
@@ -54,63 +54,77 @@ class AuthenticatedRequestFilter implements Filter {
   private final boolean extractSpinnakerUserOriginHeader
   private final boolean forceNewSpinnakerRequestId
   private final boolean clearAuthenticatedRequestPostFilter
+  private final SecurityContextRepository securityContextRepository
 
   public AuthenticatedRequestFilter(boolean extractSpinnakerHeaders = false,
                                     boolean extractSpinnakerUserOriginHeader = false,
                                     boolean forceNewSpinnakerRequestId = false,
-                                    boolean clearAuthenticatedRequestPostFilter = true) {
+                                    boolean clearAuthenticatedRequestPostFilter = true,
+                                    SecurityContextRepository securityContextRepository = null) {
     this.extractSpinnakerHeaders = extractSpinnakerHeaders
     this.extractSpinnakerUserOriginHeader = extractSpinnakerUserOriginHeader
     this.forceNewSpinnakerRequestId = forceNewSpinnakerRequestId
     this.clearAuthenticatedRequestPostFilter = clearAuthenticatedRequestPostFilter
+    this.securityContextRepository = securityContextRepository ?: new HttpSessionSecurityContextRepository()
   }
 
   @Override
-  void init(FilterConfig filterConfig) throws ServletException {}
-
-  @Override
-  void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-    def spinnakerUser = null
-    def spinnakerAccounts = null
-    HashMap<String, String> otherSpinnakerHeaders = new HashMap<>()
+  protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException {
+    String spinnakerUser = null
+    String spinnakerAccounts = null
+    Map<String, String> otherSpinnakerHeaders = [:]
 
     try {
-      def session = ((HttpServletRequest) request).getSession(false)
-      def securityContext = (SecurityContextImpl) session?.getAttribute("SPRING_SECURITY_CONTEXT")
-      if (!securityContext) {
-        securityContext = SecurityContextHolder.getContext()
+      SecurityContext securityContext
+      // first check if there is a session with a SecurityContext (but don't create the session yet)
+      if (securityContextRepository.containsContext(request)) {
+        def holder = new HttpRequestResponseHolder(request, response)
+        securityContext = securityContextRepository.loadContext(holder)
+      } else {
+        // otherwise, try checking SecurityContextHolder as this may be a fresh session
+        securityContext = SecurityContextHolder.context
       }
 
-      def principal = securityContext?.authentication?.principal
-      if (principal && principal instanceof UserDetails) {
-        spinnakerUser = principal.username
-        spinnakerAccounts = AllowedAccountsAuthorities.getAllowedAccounts(principal).join(",")
+      // next, if an authenticated user is present, get their username and allowed account authorities
+      // (when using OAuth2, the principal may be an OAuth2User or OidcUser rather than a UserDetails instance)
+      def auth = securityContext.authentication
+      if (auth && auth.authenticated) {
+        spinnakerUser = auth.name
+        spinnakerAccounts = auth.authorities
+          .grep(AllowedAccountAuthority)
+          .collect { (it as AllowedAccountAuthority).account }
+          .join(',')
       }
     } catch (Exception e) {
       log.error("Unable to extract spinnaker user and account information", e)
     }
 
     if (extractSpinnakerHeaders) {
-      def httpServletRequest = (HttpServletRequest) request
-      spinnakerUser = spinnakerUser ?: httpServletRequest.getHeader(Header.USER.getHeader())
-      spinnakerAccounts = spinnakerAccounts ?: httpServletRequest.getHeader(Header.ACCOUNTS.getHeader())
-
-      Enumeration<String> headers = httpServletRequest.getHeaderNames()
+      // for backend services using the x-spinnaker-user header for authentication
+      spinnakerUser = spinnakerUser ?: request.getHeader(Header.USER.header)
+      spinnakerAccounts = spinnakerAccounts ?: request.getHeader(Header.ACCOUNTS.header)
 
-      for (header in headers) {
+      for (header in request.headerNames) {
         String headerUpper = header.toUpperCase()
 
         if (headerUpper.startsWith(Header.XSpinnakerPrefix)) {
-          otherSpinnakerHeaders.put(headerUpper, httpServletRequest.getHeader(header))
+          otherSpinnakerHeaders[headerUpper] = request.getHeader(header)
         }
       }
     }
 
+    // normalize anonymous principal name in case of misconfiguration
+    // [anonymous] => anonymous (occasional use in Kayenta, potentially used in Orca; shows up in test data)
+    // anonymousUser => anonymous (default principal in Spring Security's AnonymousAuthenticationFilter)
+    // __unrestricted_user__ => anonymous (principal used in Fiat for anonymous)
+    if (spinnakerUser ==~ /\[?anonymous]?User|__unrestricted_user__/) {
+      spinnakerUser = 'anonymous'
+    }
+
     if (extractSpinnakerUserOriginHeader) {
-      otherSpinnakerHeaders.put(
-        Header.USER_ORIGIN.getHeader(),
-        "deck".equalsIgnoreCase(((HttpServletRequest) request).getHeader("X-RateLimit-App")) ? "deck" : "api"
-      )
+      def app = request.getHeader('X-RateLimit-App')
+      def origin = 'deck'.equalsIgnoreCase(app) ? 'deck' : 'api'
+      otherSpinnakerHeaders[Header.USER_ORIGIN.header] = origin
     }
 
     if (forceNewSpinnakerRequestId) {
@@ -151,7 +165,4 @@ class AuthenticatedRequestFilter implements Filter {
       }
     }
   }
-
-  @Override
-  void destroy() {}
 }