diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java index 20303d737..989af78f6 100644 --- a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java +++ b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java @@ -76,20 +76,19 @@ public FiatService fiatService(FiatClientConfigurationProperties fiatConfigurati } @Bean - FiatWebSecurityConfigurerAdapter fiatSecurityConfig(@Value("${services.fiat.enabled:false}") boolean isFiatEnabled) { - return new FiatWebSecurityConfigurerAdapter(isFiatEnabled); + FiatWebSecurityConfigurerAdapter fiatSecurityConfig(FiatStatus fiatStatus) { + return new FiatWebSecurityConfigurerAdapter(fiatStatus); } private class FiatWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter { - private final boolean isFiatEnabled; + private final FiatStatus fiatStatus; - private FiatWebSecurityConfigurerAdapter(boolean isFiatEnabled) { - this.isFiatEnabled = isFiatEnabled; + private FiatWebSecurityConfigurerAdapter(FiatStatus fiatStatus) { + this.fiatStatus = fiatStatus; } @Override protected void configure(HttpSecurity http) throws Exception { - if (isFiatEnabled) { /* * Having `FiatAuthenticationFilter` prior to `SecurityContextPersistenceFilter` results in the * `SecurityContextHolder` being overridden with a null value. @@ -101,11 +100,7 @@ protected void configure(HttpSecurity http) throws Exception { */ http .csrf().disable() - .addFilterAfter(new FiatAuthenticationFilter(), SecurityContextPersistenceFilter.class); - } else { - log.info("Fiat service is disabled. Setting Spring Security to allow all traffic."); - http.authorizeRequests().anyRequest().permitAll().and().csrf().disable(); - } + .addFilterAfter(new FiatAuthenticationFilter(fiatStatus), SecurityContextPersistenceFilter.class); } } diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationFilter.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationFilter.java index ef91b2271..f50667d43 100644 --- a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationFilter.java +++ b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationFilter.java @@ -32,13 +32,27 @@ @Slf4j public class FiatAuthenticationFilter implements Filter { + private final FiatStatus fiatStatus; + + public FiatAuthenticationFilter(FiatStatus fiatStatus) { + this.fiatStatus = fiatStatus; + } + @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + public void doFilter(ServletRequest request, + ServletResponse response, + FilterChain chain) throws IOException, ServletException { + if (!fiatStatus.isEnabled()) { + chain.doFilter(request, response); + return; + } + Authentication auth = AuthenticatedRequest .getSpinnakerUser() - .map(username -> (Authentication) new PreAuthenticatedAuthenticationToken(username, - null, - new ArrayList<>())) + .map(username -> (Authentication) new PreAuthenticatedAuthenticationToken( + username, + null, + new ArrayList<>())) .orElseGet(() -> new AnonymousAuthenticationToken( "anonymous", "anonymous", diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java index 3fbcb2187..177738979 100644 --- a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java +++ b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java @@ -49,24 +49,22 @@ @Component @Slf4j public class FiatPermissionEvaluator implements PermissionEvaluator { - private final DynamicConfigService dynamicConfigService; private final Registry registry; private final FiatService fiatService; - private final FiatClientConfigurationProperties configProps; + private final FiatStatus fiatStatus; private final Cache permissionsCache; private final Id getPermissionCounterId; @Autowired - public FiatPermissionEvaluator(DynamicConfigService dynamicConfigService, - Registry registry, + public FiatPermissionEvaluator(Registry registry, FiatService fiatService, - FiatClientConfigurationProperties configProps) { - this.dynamicConfigService = dynamicConfigService; + FiatClientConfigurationProperties configProps, + FiatStatus fiatStatus) { this.registry = registry; this.fiatService = fiatService; - this.configProps = configProps; + this.fiatStatus = fiatStatus; this.permissionsCache = CacheBuilder .newBuilder() @@ -90,7 +88,7 @@ public boolean hasPermission(Authentication authentication, Serializable resourceName, String resourceType, Object authorization) { - if (!isEnabled()) { + if (!fiatStatus.isEnabled()) { return true; } if (resourceName == null || resourceType == null || authorization == null) { @@ -172,7 +170,7 @@ public UserPermission.View getPermission(String username) { @SuppressWarnings("unused") @Deprecated public boolean storeWholePermission() { - if (!isEnabled()) { + if (!fiatStatus.isEnabled()) { return true; } @@ -214,8 +212,4 @@ private boolean permissionContains(UserPermission.View permission, public boolean isAdmin() { return true; // TODO(ttomsu): Chosen by fair dice roll. Guaranteed to be random. } - - private boolean isEnabled() { - return dynamicConfigService.isEnabled("fiat", configProps.isEnabled()); - } } diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatStatus.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatStatus.java new file mode 100644 index 000000000..f21ca9bba --- /dev/null +++ b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatStatus.java @@ -0,0 +1,60 @@ +/* + * Copyright 2018 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.shared; + +import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +import java.util.concurrent.atomic.AtomicBoolean; + +@Component +public class FiatStatus { + private final Logger log = LoggerFactory.getLogger(FiatStatus.class); + + private final DynamicConfigService dynamicConfigService; + private final AtomicBoolean enabled; + + @Autowired + public FiatStatus(Registry registry, + DynamicConfigService dynamicConfigService, + FiatClientConfigurationProperties fiatClientConfigurationProperties) { + this.dynamicConfigService = dynamicConfigService; + this.enabled = new AtomicBoolean( + dynamicConfigService.isEnabled("fiat", fiatClientConfigurationProperties.isEnabled()) + ); + + registry.gauge("fiat.enabled", enabled, value -> enabled.get() ? 1 : 0); + } + + public boolean isEnabled() { + return enabled.get(); + } + + @Scheduled(fixedDelay = 30000L) + void refreshStatus() { + try { + enabled.set(dynamicConfigService.isEnabled("fiat", enabled.get())); + } catch (Exception e) { + log.warn("Unable to refresh fiat status, reason: {}", e.getMessage()); + } + } +} diff --git a/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy b/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy index 3e9c9d559..142c0eeac 100644 --- a/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy +++ b/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy @@ -37,14 +37,17 @@ class FiatPermissionEvaluatorSpec extends Specification { _ * isEnabled('fiat', true) >> { return true } } FiatService fiatService = Mock(FiatService) - Registry registry = new NoopRegistry(); + Registry registry = new NoopRegistry() + FiatStatus fiatStatus = Mock(FiatStatus) { + _ * isEnabled() >> { return true } + } @Subject FiatPermissionEvaluator evaluator = new FiatPermissionEvaluator( - dynamicConfigService, registry, fiatService, - buildConfigurationProperties() + buildConfigurationProperties(), + fiatStatus ) @Unroll