From 1257221c899cbffa3597cf04f19c3b093a337767 Mon Sep 17 00:00:00 2001 From: Cameron Fieber Date: Wed, 19 Jun 2019 11:18:36 -0700 Subject: [PATCH] feat(oauth): add retry/legacyFallback support (#831) Similar to the SAML implementation, adds retries on calls to fiat, and if legacyFallback is enabled makes a failure to login to fiat non-fatal. --- gate-oauth2/gate-oauth2.gradle | 7 ++- .../SpinnakerUserInfoTokenServices.groovy | 46 +++++++++++++++++-- .../SpinnakerUserInfoTokenServicesSpec.groovy | 2 - 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/gate-oauth2/gate-oauth2.gradle b/gate-oauth2/gate-oauth2.gradle index daa2e8164b..61905c9248 100644 --- a/gate-oauth2/gate-oauth2.gradle +++ b/gate-oauth2/gate-oauth2.gradle @@ -1,7 +1,10 @@ dependencies { implementation project(":gate-core") + implementation "com.netflix.spectator:spectator-api" + implementation "com.netflix.spinnaker.fiat:fiat-api:$fiatVersion" + implementation "com.netflix.spinnaker.kork:kork-exceptions" + implementation "com.netflix.spinnaker.kork:kork-security" + implementation "com.squareup.retrofit:converter-simplexml" implementation "org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure" implementation "org.springframework.session:spring-session-core" - implementation "com.squareup.retrofit:converter-simplexml" - implementation "com.netflix.spinnaker.kork:kork-security" } diff --git a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy b/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy index aba9a8f8e3..044a30c120 100644 --- a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy +++ b/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy @@ -16,11 +16,14 @@ package com.netflix.spinnaker.gate.security.oauth2 +import com.netflix.spectator.api.Registry +import com.netflix.spinnaker.fiat.shared.FiatClientConfigurationProperties import com.netflix.spinnaker.gate.security.AllowedAccountsSupport import com.netflix.spinnaker.gate.security.oauth2.provider.SpinnakerProviderTokenServices import com.netflix.spinnaker.gate.services.CredentialsService import com.netflix.spinnaker.gate.services.PermissionService import com.netflix.spinnaker.gate.services.internal.Front50Service +import com.netflix.spinnaker.kork.core.RetrySupport import com.netflix.spinnaker.security.User import groovy.util.logging.Slf4j import org.springframework.beans.factory.annotation.Autowired @@ -75,6 +78,14 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices { @Autowired AllowedAccountsSupport allowedAccountsSupport + @Autowired + FiatClientConfigurationProperties fiatClientConfigurationProperties + + @Autowired + Registry registry + + RetrySupport retrySupport = new RetrySupport() + @Override OAuth2Authentication loadAuthentication(String accessToken) throws AuthenticationException, InvalidTokenException { OAuth2Authentication oAuth2Authentication = userInfoTokenServices.loadAuthentication(accessToken) @@ -96,14 +107,39 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices { } def username = details[userInfoMapping.username] as String - def roles = getRoles(details) + def roles = getRoles(details) ?: [] // Service accounts are already logged in. if (!isServiceAccount) { - if (roles.isEmpty()) { - permissionService.login(username) - } else { - permissionService.loginWithRoles(username, roles) + def id = registry + .createId("fiat.login") + .withTag("type", "oauth2") + + try { + retrySupport.retry({ -> + if (roles.isEmpty()) { + permissionService.login(username) + } else { + permissionService.loginWithRoles(username, roles) + } + }, 5, 2000, false) + log.debug("Successful oauth2 authentication (user: {}, roleCount: {}, roles: {})", username, roles.size(), roles) + id = id.withTag("success", true).withTag("fallback", "none") + } catch (Exception e) { + log.debug( + "Unsuccessful oauth2 authentication (user: {}, roleCount: {}, roles: {}, legacyFallback: {})", + username, + roles.size(), + roles, + fiatClientConfigurationProperties.legacyFallback + ) + id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback) + + if (!fiatClientConfigurationProperties.legacyFallback) { + throw e + } + } finally { + registry.counter(id).increment() } } diff --git a/gate-oauth2/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy b/gate-oauth2/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy index a9f3894fd1..033859a234 100644 --- a/gate-oauth2/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy +++ b/gate-oauth2/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy @@ -78,8 +78,6 @@ class SpinnakerUserInfoTokenServicesSpec extends Specification { tokenServices.hasAllUserInfoRequirements(roles: ["foo_ADMIN"]) !tokenServices.hasAllUserInfoRequirements(roles: ["_ADMIN", "foo_USER"]) !tokenServices.hasAllUserInfoRequirements(roles: ["foo_ADMINISTRATOR", "bar_USER"]) - - } def "should extract roles from details"() {