diff --git a/src/main/java/org/opendevstack/component_catalog/config/SecurityConfiguration.java b/src/main/java/org/opendevstack/component_catalog/config/SecurityConfiguration.java index 5ea0a66..9f6c77e 100644 --- a/src/main/java/org/opendevstack/component_catalog/config/SecurityConfiguration.java +++ b/src/main/java/org/opendevstack/component_catalog/config/SecurityConfiguration.java @@ -2,6 +2,7 @@ import com.azure.spring.cloud.autoconfigure.implementation.aad.filter.AadAppRoleStatelessAuthenticationFilter; import lombok.RequiredArgsConstructor; +import org.opendevstack.component_catalog.config.azure.ConditionalAadFilter; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; @@ -9,7 +10,6 @@ import org.springframework.http.HttpMethod; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.WebSecurityCustomizer; import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.config.web.PathPatternRequestMatcherBuilderFactoryBean; @@ -17,6 +17,8 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint; import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.cors.CorsConfiguration; @Configuration @@ -35,19 +37,6 @@ PathPatternRequestMatcherBuilderFactoryBean pathPatternRequestMatcherBuilderFact return new PathPatternRequestMatcherBuilderFactoryBean(); } - @Bean - public WebSecurityCustomizer webSecurityCustomizer() { - return web -> web.ignoring().requestMatchers( - "/swagger-ui/**", - "/v3/api-docs/**", - "/v1/catalog-items/*/user-actions/**", - "/v1/user-actions/**", - "/v1/provision/*/*", - "/v1/schema-validation/**", - "/actuator/health" - ); - } - /** * Chain #1: ONLY for DELETE /v1/provision/* -> HTTP Basic with role PROVISIONER */ @@ -79,10 +68,28 @@ SecurityFilterChain basicForProvisionDelete(HttpSecurity http) throws Exception @Order(2) public SecurityFilterChain aadForEverythingElse(HttpSecurity http) throws Exception { + RequestMatcher protectedEndpoints = new OrRequestMatcher( + PathPatternRequestMatcher.withDefaults().matcher("/v1/**"), + PathPatternRequestMatcher.withDefaults().matcher("/actuator/**") + ); + + RequestMatcher whitelistedEndpoints = new OrRequestMatcher( + PathPatternRequestMatcher.withDefaults().matcher("/v1/catalog-items/*/user-actions/**"), + PathPatternRequestMatcher.withDefaults().matcher("/v1/user-actions/**"), + PathPatternRequestMatcher.withDefaults().matcher("/v1/schema-validation/**"), + PathPatternRequestMatcher.withDefaults().matcher("/swagger-ui/**"), + PathPatternRequestMatcher.withDefaults().matcher("/v3/api-docs/**"), + PathPatternRequestMatcher.withDefaults().matcher("/v1/user-actions/**"), + PathPatternRequestMatcher.withDefaults().matcher("/v1/provision/*/*"), + PathPatternRequestMatcher.withDefaults().matcher("/actuator/health") + ); + http - .authorizeHttpRequests(request -> request - .requestMatchers("/v1/**", "/actuator/**") - .hasAuthority("ROLE_USER") // If required, change or add proper roles set by AAD + .authorizeHttpRequests(req -> req + .requestMatchers( + whitelistedEndpoints + ).permitAll() + .anyRequest().hasAuthority("ROLE_USER") ) .csrf(CsrfConfigurer::disable) //NOSONAR required for /actuator endpoints, STATELESS prevents CSRF .cors(c -> c.configurationSource(request -> @@ -90,9 +97,10 @@ public SecurityFilterChain aadForEverythingElse(HttpSecurity http) throws Except .sessionManagement(configurer -> // Avoid session caching and validation e.g. via JSESSIONID cookie, as we are stateless configurer.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) - - // 2) Azure AD (bearer/JWT) for the rest - .addFilterBefore(aadAuthFilter, UsernamePasswordAuthenticationFilter.class); + .addFilterBefore( + new ConditionalAadFilter(aadAuthFilter, protectedEndpoints, whitelistedEndpoints), + UsernamePasswordAuthenticationFilter.class + ); return http.build(); } diff --git a/src/main/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilter.java b/src/main/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilter.java new file mode 100644 index 0000000..d88abb1 --- /dev/null +++ b/src/main/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilter.java @@ -0,0 +1,55 @@ +package org.opendevstack.component_catalog.config.azure; + +import com.azure.spring.cloud.autoconfigure.implementation.aad.filter.AadAppRoleStatelessAuthenticationFilter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import lombok.extern.slf4j.Slf4j; +import org.jspecify.annotations.NonNull; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; + +@Slf4j +public class ConditionalAadFilter extends OncePerRequestFilter { + + // Ideally, we should create different filters and order them in the security configuration. + // The thing is, AadAppRoleStatelessAuthenticationFilter is not ordered, so if we try to do so, spring will complain. + // The solution would be, wrap it and add an @order annotation, or wrap it in the conditional filter itself, as we do here. + private final AadAppRoleStatelessAuthenticationFilter delegate; + private final RequestMatcher protectedEndpoints; + private final RequestMatcher whitelistedEndpoints; + + public ConditionalAadFilter( + AadAppRoleStatelessAuthenticationFilter delegate, + RequestMatcher protectedEndpoints, + RequestMatcher whitelistedEndpoints) { + this.delegate = delegate; + this.protectedEndpoints = protectedEndpoints; + this.whitelistedEndpoints = whitelistedEndpoints; + } + + @Override + protected boolean shouldNotFilter(@NonNull HttpServletRequest request) { + var shouldNotFilter = whitelistedEndpoints.matches(request) || !protectedEndpoints.matches(request); + + log.debug("Validating url {}: protectedEndpoints matches? {}, whitelistedEndpoints matches? {}, shouldNotFilter? {}", + request.getRequestURI(), + protectedEndpoints.matches(request), + whitelistedEndpoints.matches(request), + shouldNotFilter); + + return shouldNotFilter; + } + + @Override + protected void doFilterInternal( + @NonNull HttpServletRequest request, + @NonNull HttpServletResponse response, + @NonNull FilterChain filterChain) throws IOException, ServletException { + + delegate.doFilter(request, response, filterChain); + } +} diff --git a/src/main/resources/application-local.yml b/src/main/resources/application-local.yml index 9f3a344..e0c2ce6 100644 --- a/src/main/resources/application-local.yml +++ b/src/main/resources/application-local.yml @@ -4,6 +4,7 @@ logging: org.springframework: INFO org.springframework.security: INFO com.azure.spring.cloud.autoconfigure: INFO + org.opendevstack.component_catalog.config.azure: INFO org.opendevstack.component_catalog.config: INFO org.opendevstack.component_catalog.server.controllers: INFO org.opendevstack.component_catalog.server.api: INFO diff --git a/src/test/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilterTest.java b/src/test/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilterTest.java new file mode 100644 index 0000000..b5bd704 --- /dev/null +++ b/src/test/java/org/opendevstack/component_catalog/config/azure/ConditionalAadFilterTest.java @@ -0,0 +1,85 @@ +package org.opendevstack.component_catalog.config.azure; + +import com.azure.spring.cloud.autoconfigure.implementation.aad.filter.AadAppRoleStatelessAuthenticationFilter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +class ConditionalAadFilterTest { + + private AadAppRoleStatelessAuthenticationFilter delegate; + private RequestMatcher protectedEndpoints; + private RequestMatcher whitelistedEndpoints; + + private ConditionalAadFilter filter; + + @BeforeEach + void setup() { + delegate = mock(AadAppRoleStatelessAuthenticationFilter.class); + protectedEndpoints = mock(RequestMatcher.class); + whitelistedEndpoints = mock(RequestMatcher.class); + + filter = new ConditionalAadFilter(delegate, protectedEndpoints, whitelistedEndpoints); + } + + @Test + void givenWhitelistedPath_whenShouldNotFilter_thenTrue() { + // given + HttpServletRequest request = mock(HttpServletRequest.class); + when(whitelistedEndpoints.matches(request)).thenReturn(true); + + // when + boolean result = filter.shouldNotFilter(request); + + // then + assertThat(result).isTrue(); + } + + @Test + void givenProtectedAndNotWhitelisted_whenShouldNotFilter_thenFalse() { + // given + HttpServletRequest request = mock(HttpServletRequest.class); + when(whitelistedEndpoints.matches(request)).thenReturn(false); + when(protectedEndpoints.matches(request)).thenReturn(true); + + // when + boolean result = filter.shouldNotFilter(request); + + // then + assertThat(result).isFalse(); + } + + @Test + void givenNotProtectedAndNotWhitelisted_whenShouldNotFilter_thenTrue() { + // given + HttpServletRequest request = mock(HttpServletRequest.class); + when(whitelistedEndpoints.matches(request)).thenReturn(false); + when(protectedEndpoints.matches(request)).thenReturn(false); + + // when + boolean result = filter.shouldNotFilter(request); + + // then + assertThat(result).isTrue(); + } + + @Test + void givenAnyRequest_whenDoFilterInternal_thenDelegateIsCalled() throws Exception { + // given + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + FilterChain filterChain = mock(FilterChain.class); + + // when + filter.doFilterInternal(request, response, filterChain); + + // then + verify(delegate).doFilter(request, response, filterChain); + } +}