fix(requestlogging): quiet down anonymous request logging

don't spew out logs around missing auth headers during log as that's expected
spinnaker · Jul 16, 2019 · 5d328b5 · 5d328b5
1 parent 454cb9a
commit 5d328b5
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 57 deletions.
diff --git a/gate-core/src/main/groovy/com/netflix/spinnaker/gate/services/PermissionService.groovy b/gate-core/src/main/groovy/com/netflix/spinnaker/gate/services/PermissionService.groovy
@@ -23,6 +23,7 @@ import com.netflix.spinnaker.fiat.shared.FiatService
 import com.netflix.spinnaker.fiat.shared.FiatStatus
 import com.netflix.spinnaker.gate.security.SpinnakerUser
 import com.netflix.spinnaker.gate.services.commands.HystrixFactory
+import com.netflix.spinnaker.security.AuthenticatedRequest
 import com.netflix.spinnaker.security.User
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
@@ -54,8 +55,10 @@ class PermissionService {
     if (fiatStatus.isEnabled()) {
       HystrixFactory.newVoidCommand(HYSTRIX_GROUP, "login") {
         try {
-          fiatService.loginUser(userId, "")
-          permissionEvaluator.invalidatePermission(userId)
+          AuthenticatedRequest.allowAnonymous({
+            fiatService.loginUser(userId, "")
+            permissionEvaluator.invalidatePermission(userId)
+          })
         } catch (RetrofitError e) {
           throw classifyError(e)
         }
@@ -67,8 +70,10 @@ class PermissionService {
     if (fiatStatus.isEnabled()) {
       HystrixFactory.newVoidCommand(HYSTRIX_GROUP, "loginWithRoles") {
         try {
-          fiatService.loginWithRoles(userId, roles)
-          permissionEvaluator.invalidatePermission(userId)
+          AuthenticatedRequest.allowAnonymous({
+            fiatService.loginWithRoles(userId, roles)
+            permissionEvaluator.invalidatePermission(userId)
+          })
         } catch (RetrofitError e) {
           throw classifyError(e)
         }

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/WebhookService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/WebhookService.groovy
@@ -19,6 +19,7 @@ package com.netflix.spinnaker.gate.services
 import com.netflix.spinnaker.gate.security.RequestContext
 import com.netflix.spinnaker.gate.services.internal.EchoService
 import com.netflix.spinnaker.gate.services.internal.OrcaServiceSelector
+import com.netflix.spinnaker.security.AuthenticatedRequest
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
@@ -34,20 +35,28 @@ class WebhookService {
   Map webhooks(String type, String source, Map event) {
     if (event == null) {
       // Need this since Retrofit.Body does not work with null as Body
-      event = new HashMap();
+      event = new HashMap()
     }
-    echoService.webhooks(type, source, event)
+
+    return AuthenticatedRequest.allowAnonymous( {
+      echoService.webhooks(type, source, event)
+    })
   }
 
   Map webhooks(String type, String source, Map event, String gitHubSignature, String bitBucketEventType) {
     if (event == null) {
       // Need this since Retrofit.Body does not work with null as Body
-      event = new HashMap();
+      event = new HashMap()
     }
-    echoService.webhooks(type, source, event, gitHubSignature, bitBucketEventType)
+
+    return AuthenticatedRequest.allowAnonymous({
+      echoService.webhooks(type, source, event, gitHubSignature, bitBucketEventType)
+    })
   }
 
   List preconfiguredWebhooks() {
-    return orcaServiceSelector.withContext(RequestContext.get()).preconfiguredWebhooks()
+    return AuthenticatedRequest.allowAnonymous({
+      orcaServiceSelector.withContext(RequestContext.get()).preconfiguredWebhooks()
+    })
   }
 }
diff --git a/...oovy/com/netflix/spinnaker/gate/security/x509/X509AuthenticationUserDetailsService.groovy b/...oovy/com/netflix/spinnaker/gate/security/x509/X509AuthenticationUserDetailsService.groovy
@@ -138,71 +138,75 @@ class X509AuthenticationUserDetailsService implements AuthenticationUserDetailsS
   @PackageScope
   Collection<String> handleLogin(String email, X509Certificate x509) {
 
-    final Instant now = clock.instant()
-    final boolean loginDebounceEnabled = dynamicConfigService.isEnabled('x509.loginDebounce', false)
-    final boolean shouldLogin
-    if (loginDebounceEnabled) {
-      final Duration debounceWindow = Duration.ofSeconds(dynamicConfigService.getConfig(Long, 'x509.loginDebounce.debounceWindowSeconds', TimeUnit.MINUTES.toSeconds(5)))
-      final Optional<Instant> lastDebounced = Optional.ofNullable(loginDebounce.getIfPresent(email))
-      UserPermission.View fiatPermission = AuthenticatedRequest.allowAnonymous { fiatPermissionEvaluator.getPermission(email) }
-      shouldLogin = fiatPermission == null ||
-        lastDebounced.map({ now.isAfter(it.plus(debounceWindow)) }).orElse(true)
-    } else {
-      shouldLogin = true
-    }
+    return AuthenticatedRequest.allowAnonymous({ ->
+      final Instant now = clock.instant()
+      final boolean loginDebounceEnabled = dynamicConfigService.isEnabled('x509.loginDebounce', false)
+      boolean shouldLogin
+
+      if (loginDebounceEnabled) {
+        final Duration debounceWindow = Duration.ofSeconds(dynamicConfigService.getConfig(Long, 'x509.loginDebounce.debounceWindowSeconds', TimeUnit.MINUTES.toSeconds(5)))
+        final Optional<Instant> lastDebounced = Optional.ofNullable(loginDebounce.getIfPresent(email))
+        UserPermission.View fiatPermission = AuthenticatedRequest.allowAnonymous { fiatPermissionEvaluator.getPermission(email) }
+        shouldLogin = fiatPermission == null ||
+          lastDebounced.map({ now.isAfter(it.plus(debounceWindow)) }).orElse(true)
+      } else {
+        shouldLogin = true
+      }
 
-    def roles = [email]
+      def roles = [email]
 
-    if (rolesExtractor) {
-      def extractedRoles = rolesExtractor.fromCertificate(x509)
-      log.debug("Extracted roles from certificate for user {}: {}", email, extractedRoles)
-      roles += extractedRoles
-    }
+      if (rolesExtractor) {
+        def extractedRoles = rolesExtractor.fromCertificate(x509)
+        log.debug("Extracted roles from certificate for user {}: {}", email, extractedRoles)
+        roles += extractedRoles
+      }
 
-    if (shouldLogin) {
-      def id = registry
+      if (shouldLogin) {
+        def id = registry
           .createId("fiat.login")
           .withTag("type", "x509")
 
-      try {
-        if (rolesExtractor) {
-          permissionService.loginWithRoles(email, roles)
-          log.debug("Successful X509 authentication (user: {}, roleCount: {}, roles: {})", email, roles.size(), roles)
-        } else {
-          permissionService.login(email)
-          log.debug("Successful X509 authentication (user: {})", email)
-        }
-
-        id = id.withTag("success", true).withTag("fallback", "none")
-      } catch (Exception e) {
-        log.debug(
+        try {
+          if (rolesExtractor) {
+            permissionService.loginWithRoles(email, roles)
+            log.debug("Successful X509 authentication (user: {}, roleCount: {}, roles: {})", email, roles.size(), roles)
+          } else {
+            permissionService.login(email)
+            log.debug("Successful X509 authentication (user: {})", email)
+          }
+
+          id = id.withTag("success", true).withTag("fallback", "none")
+        } catch (Exception e) {
+          log.debug(
             "Unsuccessful X509 authentication (user: {}, roleCount: {}, roles: {}, legacyFallback: {})",
             email,
             roles.size(),
             roles,
             fiatClientConfigurationProperties.legacyFallback
-        )
-        id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback)
+          )
+          id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback)
+
+          if (!fiatClientConfigurationProperties.legacyFallback) {
+            throw e
+          }
+        } finally {
+          registry.counter(id).increment()
+        }
 
-        if (!fiatClientConfigurationProperties.legacyFallback) {
-          throw e
+        if (loginDebounceEnabled) {
+          loginDebounce.put(email, now)
         }
-      } finally {
-        registry.counter(id).increment()
       }
 
-      if (loginDebounceEnabled) {
-        loginDebounce.put(email, now)
+      def permission = fiatPermissionEvaluator.getPermission(email)
+      def roleNames = permission?.getRoles()?.collect { it -> it.getName() }
+      log.debug("Extracted roles from fiat permissions for user {}: {}", email, roleNames)
+      if (roleNames) {
+        roles.addAll(roleNames)
       }
-    }
 
-    def permission = fiatPermissionEvaluator.getPermission(email)
-    def roleNames = permission?.getRoles()?.collect { it -> it.getName()}
-    log.debug("Extracted roles from fiat permissions for user {}: {}", email, roleNames)
-    if (roleNames) {
-      roles.addAll(roleNames)
-    }
-    return roles.unique(/* mutate = */false)
+      return roles.unique(/* mutate = */false)
+    })
   }
 
   /**