diff --git a/gate-oauth2/gate-oauth2.gradle b/gate-oauth2/gate-oauth2.gradle index a6b0ccfcc..258fba62e 100644 --- a/gate-oauth2/gate-oauth2.gradle +++ b/gate-oauth2/gate-oauth2.gradle @@ -7,7 +7,7 @@ dependencies { implementation "io.spinnaker.kork:kork-retrofit" implementation "io.spinnaker.kork:kork-security" implementation "org.apache.groovy:groovy-json" - implementation "org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure" + implementation "org.springframework.boot:spring-boot-starter-oauth2-client" implementation "org.springframework.session:spring-session-core" testImplementation "com.squareup.retrofit2:retrofit-mock" diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.java deleted file mode 100644 index dc1bf54e4..000000000 --- a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright 2025 OpsMx, 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.gate.security.oauth2; - -import java.io.IOException; -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 org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoRestTemplateFactory; -import org.springframework.security.core.Authentication; -import org.springframework.security.oauth2.client.OAuth2ClientContext; -import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken; -import org.springframework.security.oauth2.common.OAuth2AccessToken; -import org.springframework.security.oauth2.provider.authentication.BearerTokenExtractor; -import org.springframework.stereotype.Component; - -/** - * This class supports the use case of an externally provided OAuth access token, for example, a - * Github-issued personal access token. - */ -@Component -public class ExternalAuthTokenFilter implements Filter { - - @Autowired(required = false) - private UserInfoRestTemplateFactory userInfoRestTemplateFactory; - - private BearerTokenExtractor extractor = new BearerTokenExtractor(); - - @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) - throws IOException, ServletException { - HttpServletRequest httpServletRequest = (HttpServletRequest) request; - Authentication auth = extractor.extract(httpServletRequest); - if (auth != null && auth.getPrincipal() != null && !auth.getPrincipal().toString().isEmpty()) { - DefaultOAuth2AccessToken token = new DefaultOAuth2AccessToken(auth.getPrincipal().toString()); - // Reassign token type to be capitalized "Bearer", - // see https://github.com/spinnaker/spinnaker/issues/2074 - token.setTokenType(OAuth2AccessToken.BEARER_TYPE); - if (userInfoRestTemplateFactory != null) { - OAuth2ClientContext ctx = - userInfoRestTemplateFactory.getUserInfoRestTemplate().getOAuth2ClientContext(); - ctx.setAccessToken(token); - } - } - chain.doFilter(request, response); - } - - @Override - public void init(FilterConfig filterConfig) {} - - @Override - public void destroy() {} -} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.java index 159b7b67d..a08abaee6 100644 --- a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.java +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.java @@ -15,102 +15,44 @@ */ 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.config.AuthConfig; -import com.netflix.spinnaker.gate.security.AllowedAccountsSupport; import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig; -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 java.util.HashMap; -import java.util.Optional; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import lombok.Data; -import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.autoconfigure.security.oauth2.client.EnableOAuth2Sso; -import org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2SsoProperties; -import org.springframework.boot.autoconfigure.security.oauth2.resource.ResourceServerProperties; -import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoTokenServices; import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Primary; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.oauth2.client.token.grant.code.AuthorizationCodeResourceDetails; -import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices; -import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint; -import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; -import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter; -import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.session.web.http.DefaultCookieSerializer; import org.springframework.stereotype.Component; @Configuration -@SpinnakerAuthConfig @EnableWebSecurity -@EnableOAuth2Sso -@EnableConfigurationProperties -@ConditionalOnProperty(name = "security.oauth2.client.clientId") -@Slf4j +@SpinnakerAuthConfig +@Conditional(OAuthConfigEnabled.class) public class OAuth2SsoConfig extends WebSecurityConfigurerAdapter { @Autowired private AuthConfig authConfig; - @Autowired private ExternalAuthTokenFilter externalAuthTokenFilter; - @Autowired private ExternalSslAwareEntryPoint entryPoint; + @Autowired private SpinnakerOAuth2UserInfoService customOAuth2UserService; + @Autowired private SpinnakerOIDCUserInfoService oidcUserInfoService; @Autowired private DefaultCookieSerializer defaultCookieSerializer; - @Primary - @Bean - @ConditionalOnProperty( - prefix = "security.oauth2.resource.spinnaker-user-info-token-services", - name = "enabled", - havingValue = "true", - matchIfMissing = true) - public ResourceServerTokenServices spinnakerUserInfoTokenServices( - ResourceServerProperties sso, - UserInfoTokenServices userInfoTokenServices, - CredentialsService credentialsService, - OAuth2SsoConfig.UserInfoMapping userInfoMapping, - OAuth2SsoConfig.UserInfoRequirements userInfoRequirements, - PermissionService permissionService, - Front50Service front50Service, - Optional providerTokenServices, - AllowedAccountsSupport allowedAccountsSupport, - FiatClientConfigurationProperties fiatClientConfigurationProperties, - Registry registry) { - return new SpinnakerUserInfoTokenServices( - sso, - userInfoTokenServices, - credentialsService, - userInfoMapping, - userInfoRequirements, - permissionService, - front50Service, - providerTokenServices, - allowedAccountsSupport, - fiatClientConfigurationProperties, - registry); - } - @Override - public void configure(HttpSecurity http) throws Exception { + public void configure(HttpSecurity httpSecurity) throws Exception { defaultCookieSerializer.setSameSite(null); - authConfig.configure(http); - - http.exceptionHandling().authenticationEntryPoint(entryPoint); - http.addFilterBefore( - new BasicAuthenticationFilter(authenticationManager()), - UsernamePasswordAuthenticationFilter.class); - http.addFilterBefore(externalAuthTokenFilter, AbstractPreAuthenticatedProcessingFilter.class); + authConfig.configure(httpSecurity); + httpSecurity + .authorizeRequests(auth -> auth.anyRequest().authenticated()) + .oauth2Login( + oauth2 -> + oauth2.userInfoEndpoint( + userInfo -> + userInfo + .userService(customOAuth2UserService) + .oidcUserService(oidcUserInfoService))); } /** @@ -118,7 +60,7 @@ public void configure(HttpSecurity http) throws Exception { * be in the User. */ @Component - @ConfigurationProperties("security.oauth2.user-info-mapping") + @ConfigurationProperties("spring.security.oauth2.client.registration.user-info-mapping") @Data public static class UserInfoMapping { private String email = "email"; @@ -130,32 +72,6 @@ public static class UserInfoMapping { } @Component - @ConfigurationProperties("security.oauth2.user-info-requirements") + @ConfigurationProperties("spring.security.oauth2.client.registration.user-info-requirements") public static class UserInfoRequirements extends HashMap {} - - /** - * This class exists to change the login redirect (to /login) to the same URL as the - * preEstablishedRedirectUri, if set, where the SSL is terminated outside of this server. - */ - @Component - @ConditionalOnProperty(name = "security.oauth2.client.client-id") - public static class ExternalSslAwareEntryPoint extends LoginUrlAuthenticationEntryPoint { - @Autowired private AuthorizationCodeResourceDetails details; - - @Autowired - public ExternalSslAwareEntryPoint(OAuth2SsoProperties sso) { - super(sso.getLoginPath()); - } - - @Override - protected String determineUrlToUseForThisRequest( - HttpServletRequest request, - HttpServletResponse response, - AuthenticationException exception) { - final String uri = details.getPreEstablishedRedirectUri(); - return uri != null - ? uri - : super.determineUrlToUseForThisRequest(request, response, exception); - } - } } diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabled.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabled.java new file mode 100644 index 000000000..bb58ae74f --- /dev/null +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabled.java @@ -0,0 +1,101 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import java.util.regex.Pattern; +import lombok.extern.slf4j.Slf4j; +import org.springframework.context.annotation.Condition; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.EnumerablePropertySource; +import org.springframework.core.env.PropertySource; +import org.springframework.core.type.AnnotatedTypeMetadata; + +/** + * This class implements the {@link Condition} interface to check if OAuth2 configuration is enabled + * in the Spring environment based on the presence of a client ID property. It inspects the + * environment's property sources for any property matching the regular expression for OAuth2 client + * registration. + * + *

The condition matches if there is any property key that matches the pattern: + * "spring.security.oauth2.client.registration..client-id". If such a property exists, + * the condition returns {@code true}, indicating that OAuth2 configuration is enabled. Otherwise, + * it returns {@code false}. + * + *

This condition can be used in Spring's {@link + * org.springframework.context.annotation.Conditional} annotations to conditionally enable or + * disable beans based on the presence of OAuth2 client properties in the application's + * configuration. + * + *

Example: + * + * 

+ * @Configuration
+ * @Conditional(OAuthConfigEnabled.class)
+ * public class OAuth2SsoConfig {
+ *     // Bean definitions for OAuth2 configuration
+ * }
+ *
+ * + *

Note: The condition looks for the client ID property in the environment, which is a standard + * property for configuring OAuth2 client registration in Spring Security. + */ +@Slf4j +public class OAuthConfigEnabled implements Condition { + private static final String SPRING_SECURITY_OAUTH2_REGEX = + "spring\\.security\\.oauth2\\.client\\.registration\\..*\\.client-id"; + private static final Pattern SPRING_SECURITY_OAUTH2_PATTERN = + Pattern.compile(SPRING_SECURITY_OAUTH2_REGEX); + + /** + * Evaluates whether the condition matches based on the presence of OAuth2 client registration + * properties in the Spring environment. + * + *

This method checks if the application's {@link ConfigurableEnvironment} contains any + * property names that match the pattern + * spring.security.oauth2.client.registration.<client-name>.client-id. If at least + * one such property exists, the method returns {@code true}, indicating that OAuth2 configuration + * is enabled. Otherwise, it returns {@code false}. + * + * @param context The {@link ConditionContext}, which provides access to the Spring environment + * and application context. + * @param metadata The {@link AnnotatedTypeMetadata} of the annotated component. (Not used in this + * implementation.) + * @return {@code true} if at least one OAuth2 client registration property is found, {@code + * false} otherwise. + */ + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + if (!(context.getEnvironment() instanceof ConfigurableEnvironment)) { + return false; + } + + ConfigurableEnvironment env = (ConfigurableEnvironment) context.getEnvironment(); + + for (PropertySource propertySource : env.getPropertySources()) { + if (propertySource instanceof EnumerablePropertySource) { + for (String propertyName : + ((EnumerablePropertySource) propertySource).getPropertyNames()) { + if (SPRING_SECURITY_OAUTH2_PATTERN.matcher(propertyName).matches()) { + return true; // If any property matches, load the configuration + } + } + } + } + return false; // Skip configuration if no matching properties found + } +} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelper.java similarity index 67% rename from gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.java rename to gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelper.java index 5fc4bd07a..a5c2da238 100644 --- a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.java +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelper.java @@ -1,7 +1,7 @@ /* * Copyright 2025 OpsMx, Inc. * - * Licensed under the Apache License, Version 2.0 (the "License") + * 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 * @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package com.netflix.spinnaker.gate.security.oauth2; import static net.logstash.logback.argument.StructuredArguments.entries; @@ -23,13 +24,11 @@ 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.kork.retrofit.Retrofit2SyncCall; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; -import com.netflix.spinnaker.security.User; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -37,6 +36,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.function.BiFunction; import java.util.regex.Pattern; @@ -45,90 +45,79 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.boot.autoconfigure.security.oauth2.resource.ResourceServerProperties; -import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoTokenServices; import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.oauth2.common.OAuth2AccessToken; -import org.springframework.security.oauth2.common.exceptions.InvalidTokenException; -import org.springframework.security.oauth2.provider.OAuth2Authentication; -import org.springframework.security.oauth2.provider.OAuth2Request; -import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices; -import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.security.oauth2.core.user.OAuth2User; +import org.springframework.stereotype.Component; /** - * ResourceServerTokenServices is an interface used to manage access tokens. The - * UserInfoTokenService object is an implementation of that interface that uses an access token to - * get the logged in user's data (such as email or profile). We want to customize the Authentication - * object that is returned to include our custom (Kork) User. + * A helper class to handle common user loading logic for both OAuth2 and OIDC authentication. This + * class extracts shared code to avoid duplication in OAuth2 and OIDC user loading processes. */ +@Component @Slf4j -public class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices { +public class OAuthUserInfoServiceHelper { - private final ResourceServerProperties sso; - private final UserInfoTokenServices userInfoTokenServices; - private final CredentialsService credentialsService; private final OAuth2SsoConfig.UserInfoMapping userInfoMapping; + private final OAuth2SsoConfig.UserInfoRequirements userInfoRequirements; + private final PermissionService permissionService; + private final Front50Service front50Service; - private final SpinnakerProviderTokenServices providerTokenServices; private final AllowedAccountsSupport allowedAccountsSupport; + private final FiatClientConfigurationProperties fiatClientConfigurationProperties; + private final Registry registry; + private SpinnakerProviderTokenServices providerTokenServices; + @Autowired(required = false) @Qualifier("spinnaker-oauth2-group-extractor") - private BiFunction> groupExtractor; + private BiFunction, List> groupExtractor; - private RetrySupport retrySupport = new RetrySupport(); + private final RetrySupport retrySupport = new RetrySupport(); @Autowired - public SpinnakerUserInfoTokenServices( - ResourceServerProperties sso, - UserInfoTokenServices userInfoTokenServices, - CredentialsService credentialsService, + public OAuthUserInfoServiceHelper( OAuth2SsoConfig.UserInfoMapping userInfoMapping, OAuth2SsoConfig.UserInfoRequirements userInfoRequirements, PermissionService permissionService, Front50Service front50Service, - Optional providerTokenServices, AllowedAccountsSupport allowedAccountsSupport, FiatClientConfigurationProperties fiatClientConfigurationProperties, - Registry registry) { - this.sso = sso; - this.userInfoTokenServices = userInfoTokenServices; - this.credentialsService = credentialsService; + Registry registry, + Optional providerTokenServices) { this.userInfoMapping = userInfoMapping; this.userInfoRequirements = userInfoRequirements; this.permissionService = permissionService; this.front50Service = front50Service; - this.providerTokenServices = providerTokenServices.orElse(null); this.allowedAccountsSupport = allowedAccountsSupport; this.fiatClientConfigurationProperties = fiatClientConfigurationProperties; this.registry = registry; - } - @Override - public OAuth2Authentication loadAuthentication(final String accessToken) - throws AuthenticationException, InvalidTokenException { - OAuth2Authentication oAuth2Authentication = - userInfoTokenServices.loadAuthentication(accessToken); + if (providerTokenServices.isPresent()) { + this.providerTokenServices = providerTokenServices.get(); + } + } - final Map details = - (Map) oAuth2Authentication.getUserAuthentication().getDetails(); + T getOAuthSpinnakerUser(T oAuth2User, OAuth2UserRequest userRequest) { + Map details = oAuth2User.getAttributes(); if (log.isDebugEnabled()) { log.debug("UserInfo details: " + entries(details)); } boolean isServiceAccount = isServiceAccount(details); + String accessToken = userRequest.getAccessToken().getTokenValue(); + if (!isServiceAccount) { if (!hasAllUserInfoRequirements(details)) { throw new BadCredentialsException("User's info does not have all required fields."); } - if (providerTokenServices != null && !providerTokenServices.hasAllProviderRequirements(accessToken, details)) { throw new BadCredentialsException( @@ -136,7 +125,7 @@ public OAuth2Authentication loadAuthentication(final String accessToken) } } - final String username = toStringOrNull(details.get(userInfoMapping.getUsername())); + final String username = Objects.toString(details.get(userInfoMapping.getUsername()), null); List roles = Optional.ofNullable(groupExtractor) .map(extractor -> extractor.apply(accessToken, details)) @@ -185,45 +174,37 @@ public OAuth2Authentication loadAuthentication(final String accessToken) } } - User spinnakerUser = new User(); - spinnakerUser.setEmail(toStringOrNull(details.get(userInfoMapping.getEmail()))); - spinnakerUser.setFirstName(toStringOrNull(details.get(userInfoMapping.getFirstName()))); - spinnakerUser.setLastName(toStringOrNull(details.get(userInfoMapping.getLastName()))); - spinnakerUser.setAllowedAccounts(allowedAccountsSupport.filterAllowedAccounts(username, roles)); - spinnakerUser.setRoles(roles); - spinnakerUser.setUsername(username); - - PreAuthenticatedAuthenticationToken authentication = - new PreAuthenticatedAuthenticationToken( - spinnakerUser, null, spinnakerUser.getAuthorities()); - - // impl copied from UserInfoTokenServices - OAuth2Request storedRequest = - new OAuth2Request(null, sso.getClientId(), null, true, null, null, null, null, null); - - return new OAuth2Authentication(storedRequest, authentication); - } - - /** - * Safely converts an object to a string representation. - * - *

This method checks if the provided object is non-null before calling {@code toString()}. If - * the object is {@code null}, it returns {@code null} instead of throwing a {@code - * NullPointerException}. - * - * @param o the object to convert to a string, may be {@code null} - * @return the string representation of the object, or {@code null} if the object is {@code null} - */ - private String toStringOrNull(Object o) { - return o != null ? o.toString() : null; - } - - @Override - public OAuth2AccessToken readAccessToken(String accessToken) { - return userInfoTokenServices.readAccessToken(accessToken); + if (oAuth2User instanceof OidcUser oidcUser) { + SpinnakerOIDCUser spinnakerUser = + new SpinnakerOIDCUser( + Objects.toString(details.get(userInfoMapping.getEmail()), null), + Objects.toString(details.get(userInfoMapping.getFirstName()), null), + Objects.toString(details.get(userInfoMapping.getLastName()), null), + allowedAccountsSupport.filterAllowedAccounts(username, roles), + roles, + username, + oidcUser.getIdToken(), + oidcUser.getUserInfo(), + details, + oidcUser.getAuthorities()); + + return (T) spinnakerUser; + } else { + SpinnakerOAuth2User spinnakerUser = + new SpinnakerOAuth2User( + Objects.toString(details.get(userInfoMapping.getEmail()), null), + Objects.toString(details.get(userInfoMapping.getFirstName()), null), + Objects.toString(details.get(userInfoMapping.getLastName()), null), + allowedAccountsSupport.filterAllowedAccounts(username, roles), + roles, + username, + details, + oAuth2User.getAuthorities()); + return (T) spinnakerUser; + } } - protected boolean isServiceAccount(Map details) { + boolean isServiceAccount(Map details) { String email = (String) details.get(userInfoMapping.getServiceAccountEmail()); if (email == null || !permissionService.isEnabled()) { return false; @@ -251,7 +232,7 @@ private static boolean valueMatchesConstraint(Object value, String requiredVal) return value.equals(requiredVal); } - public boolean hasAllUserInfoRequirements(Map details) { + boolean hasAllUserInfoRequirements(Map details) { if (userInfoRequirements == null || userInfoRequirements.isEmpty()) { return true; } @@ -290,7 +271,7 @@ public boolean hasAllUserInfoRequirements(Map details) { return invalidFields.isEmpty(); } - public static boolean isRegexExpression(String val) { + private static boolean isRegexExpression(String val) { if (val.startsWith("/") && val.endsWith("/")) { try { Pattern.compile(val); @@ -303,12 +284,12 @@ public static boolean isRegexExpression(String val) { return false; } - public static String mutateRegexPattern(String val) { + private static String mutateRegexPattern(String val) { // "/expr/" -> "expr" return val.substring(1, val.length() - 1); } - protected List getRoles(Map details) { + List getRoles(Map details) { if (userInfoMapping == null || userInfoMapping.getRoles() == null) { return List.of(); } diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2User.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2User.java new file mode 100644 index 000000000..d46bd2405 --- /dev/null +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2User.java @@ -0,0 +1,91 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import com.netflix.spinnaker.security.User; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.oauth2.core.user.OAuth2User; + +/** + * Custom implementation of {@link OAuth2User} that integrates with Spinnaker's {@link User} model. + * This class holds OAuth2-related user details and provides attributes such as roles and allowed + * accounts. + * + *

It extends {@link User} from Netflix Spinnaker to include Spinnaker-specific fields. + * + *

Usage: This class is used in OAuth2 authentication flows where user details are retrieved from + * an OAuth2 provider. + * + * @author rahul-chekuri + * @see User + */ +public class SpinnakerOAuth2User extends User implements OAuth2User { + /** + * Attributes containing user details, retrieved from the OIDC provider. These attributes + * typically include user profile information such as name, email, and roles. + */ + private final Map attributes; + + /** Authorities assigned to the user, used for authorization in Spring Security. */ + private final List authorities; + + public SpinnakerOAuth2User( + String email, + String firstName, + String lastName, + Collection allowedAccounts, + List roles, + String username, + Map attributes, + Collection authorities) { + this.email = email; + this.firstName = firstName; + this.lastName = lastName; + this.allowedAccounts = allowedAccounts; + this.roles = roles; + this.username = username; + this.attributes = + attributes != null + ? Collections.unmodifiableMap(new HashMap<>(attributes)) + : Collections.emptyMap(); + this.authorities = + authorities != null + ? Collections.unmodifiableList(new ArrayList<>(authorities)) + : Collections.emptyList(); + } + + @Override + public Map getAttributes() { + return attributes; + } + + @Override + public List getAuthorities() { + return authorities; + } + + @Override + public String getName() { + return super.getUsername(); + } +} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2UserInfoService.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2UserInfoService.java new file mode 100644 index 000000000..0d5836420 --- /dev/null +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOAuth2UserInfoService.java @@ -0,0 +1,55 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.user.OAuth2User; +import org.springframework.stereotype.Service; + +/** + * Custom OAuth2 user service that extends {@link DefaultOAuth2UserService} to load and process + * OAuth2 user details. This service integrates with {@link OAuthUserInfoServiceHelper} to transform + * and return the Spinnaker-specific OAuth2 user object. + * + *

Overrides the {@link #loadUser(OAuth2UserRequest)} method to modify user details after + * retrieving them from the OAuth2 provider. + * + *

Usage: This service is automatically registered as a Spring Bean and is used during OAuth2 + * authentication. + * + * @author rahul-chekuri + * @see OAuthUserInfoServiceHelper + */ +@Service +@Slf4j +public class SpinnakerOAuth2UserInfoService extends DefaultOAuth2UserService { + private OAuthUserInfoServiceHelper userInfoService; + + @Autowired + public SpinnakerOAuth2UserInfoService(OAuthUserInfoServiceHelper userInfoService) { + this.userInfoService = userInfoService; + } + + @Override + public OAuth2User loadUser(OAuth2UserRequest userRequest) { + OAuth2User oAuth2User = super.loadUser(userRequest); + return userInfoService.getOAuthSpinnakerUser(oAuth2User, userRequest); + } +} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUser.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUser.java new file mode 100644 index 000000000..2c6e72c09 --- /dev/null +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUser.java @@ -0,0 +1,131 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import com.netflix.spinnaker.security.User; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; + +/** + * Custom implementation of {@link OidcUser} that integrates with Spinnaker's {@link User} model. + * This class holds OIDC-related user details such as ID token, user info, and additional + * attributes. + * + *

It extends {@link User} from kork to include Spinnaker-specific fields like allowed accounts + * and roles. + * + *

Usage: This class is used in OIDC authentication flows where user details are retrieved from + * an OIDC provider. + * + * @author rahul-chekuri + * @see User + */ +public class SpinnakerOIDCUser extends User implements OidcUser { + /** + * Attributes containing user details, retrieved from the OIDC provider. These attributes + * typically include user profile information such as name, email, and roles. + */ + private final Map attributes; + + /** Authorities assigned to the user, used for authorization in Spring Security. */ + private final List authorities; + + private final OidcIdToken idToken; + private final OidcUserInfo userInfo; + + public SpinnakerOIDCUser( + String email, + String firstName, + String lastName, + Collection allowedAccounts, + List roles, + String username, + OidcIdToken idToken, + OidcUserInfo userInfo, + Map attributes, + Collection authorities) { + this.idToken = idToken; + this.userInfo = userInfo; + this.email = email; + this.firstName = firstName; + this.lastName = lastName; + this.allowedAccounts = allowedAccounts; + this.roles = roles; + this.username = username; + this.attributes = + attributes != null + ? Collections.unmodifiableMap(new HashMap<>(attributes)) + : Collections.emptyMap(); + this.authorities = + authorities != null + ? Collections.unmodifiableList(new ArrayList<>(authorities)) + : Collections.emptyList(); + } + + @Override + public Map getAttributes() { + return attributes; + } + + @Override + public List getAuthorities() { + return authorities; + } + + @Override + public String getName() { + return super.getUsername(); + } + + /** + * Returns the claims associated with the authenticated user. + * + *

This method returns the same attributes map as {@link #getAttributes()} because, in OIDC, + * claims typically refer to user attributes retrieved from the identity provider. This approach + * aligns with the implementation in {@link + * org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser}, which also returns the + * attributes map for claims. + * + *

By returning attributes as claims, we ensure consistency with how Spring Security processes + * OIDC user details and maintains compatibility with components expecting claims to be retrieved + * from this method. + * + * @return a map containing the user's claims + */ + @Override + public Map getClaims() { + return this.attributes; + } + + @Override + public OidcUserInfo getUserInfo() { + return this.userInfo; + } + + @Override + public OidcIdToken getIdToken() { + return this.idToken; + } +} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUserInfoService.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUserInfoService.java new file mode 100644 index 000000000..d04b8a224 --- /dev/null +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerOIDCUserInfoService.java @@ -0,0 +1,52 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest; +import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.stereotype.Service; + +/** + * Custom OIDC user service that extends {@link OidcUserService} to load and process OIDC user + * details. This service integrates with {@link OAuthUserInfoServiceHelper} to transform and return + * the Spinnaker-specific OIDC user object. + * + *

Overrides the {@link #loadUser(OidcUserRequest)} method to modify user details after + * retrieving them from the OpenID Connect (OIDC) provider. + * + * @author rahul-chekuri + * @see OAuthUserInfoServiceHelper + */ +@Service +@Slf4j +public class SpinnakerOIDCUserInfoService extends OidcUserService { + private OAuthUserInfoServiceHelper userInfoService; + + @Autowired + public SpinnakerOIDCUserInfoService(OAuthUserInfoServiceHelper userInfoService) { + this.userInfoService = userInfoService; + } + + @Override + public OidcUser loadUser(OidcUserRequest userRequest) { + OidcUser oidcUser = super.loadUser(userRequest); + return userInfoService.getOAuthSpinnakerUser(oidcUser, userRequest); + } +} diff --git a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.java b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.java index fcde8ae2d..be33c960d 100644 --- a/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.java +++ b/gate-oauth2/src/main/java/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.java @@ -17,32 +17,36 @@ import java.util.List; import java.util.Map; +import lombok.Data; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.autoconfigure.security.oauth2.resource.ResourceServerProperties; import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.security.oauth2.client.OAuth2RestOperations; -import org.springframework.security.oauth2.client.OAuth2RestTemplate; -import org.springframework.security.oauth2.client.resource.BaseOAuth2ProtectedResourceDetails; -import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken; -import org.springframework.security.oauth2.common.OAuth2AccessToken; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Component; +import org.springframework.web.client.RestTemplate; +/** + * This class implements {@link SpinnakerProviderTokenServices} to verify if a user meets the + * provider-specific authentication requirements for GitHub. + * + *

It checks if a user is a member of a required GitHub organization before granting access. + */ @Slf4j @Component -@ConditionalOnProperty(name = "security.oauth2.provider-requirements.type", havingValue = "github") +@ConditionalOnProperty(name = "spring.security.oauth2.client.registration.github.client-id") public class GithubProviderTokenServices implements SpinnakerProviderTokenServices { - @Autowired private ResourceServerProperties sso; @Autowired private GithubRequirements requirements; - private String tokenType = DefaultOAuth2AccessToken.BEARER_TYPE; - private OAuth2RestOperations restTemplate; private boolean githubOrganizationMember( String organization, List> organizations) { - for (int i = 0; i < organizations.size(); i++) { - if (organization.equals(organizations.get(i).get("login"))) { + for (Map org : organizations) { + if (organization.equals(org.get("login"))) { return true; } } @@ -53,22 +57,17 @@ private boolean checkOrganization( String accessToken, String organizationsUrl, String organization) { try { log.debug("Getting user organizations from URL {}", organizationsUrl); - OAuth2RestOperations restTemplate = this.restTemplate; - if (restTemplate == null) { - BaseOAuth2ProtectedResourceDetails resource = new BaseOAuth2ProtectedResourceDetails(); - resource.setClientId(sso.getClientId()); - restTemplate = new OAuth2RestTemplate(resource); - } + RestTemplate restTemplate = new RestTemplate(); - OAuth2AccessToken existingToken = restTemplate.getOAuth2ClientContext().getAccessToken(); - if (existingToken == null || !accessToken.equals(existingToken.getValue())) { - DefaultOAuth2AccessToken token = new DefaultOAuth2AccessToken(accessToken); - token.setTokenType(this.tokenType); - restTemplate.getOAuth2ClientContext().setAccessToken(token); - } + HttpHeaders headers = new HttpHeaders(); + headers.set("Authorization", "Bearer " + accessToken); + headers.set("Accept", MediaType.APPLICATION_JSON_VALUE); + + ResponseEntity response = + restTemplate.exchange( + organizationsUrl, HttpMethod.GET, new HttpEntity<>(headers), List.class); - List> organizations = - restTemplate.getForEntity(organizationsUrl, List.class).getBody(); + List> organizations = response.getBody(); return githubOrganizationMember(organization, organizations); } catch (Exception e) { log.warn("Could not fetch user organizations", e); @@ -91,16 +90,10 @@ public boolean hasAllProviderRequirements(String token, Map details) { } @Component - @ConfigurationProperties("security.oauth2.provider-requirements") + @ConfigurationProperties( + "spring.security.oauth2.client.registration.github.provider-requirements") + @Data public static class GithubRequirements { - public String getOrganization() { - return organization; - } - - public void setOrganization(String organization) { - this.organization = organization; - } - private String organization; } } diff --git a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilterTest.java b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilterTest.java deleted file mode 100644 index f563ff7e4..000000000 --- a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilterTest.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright 2025 OpsMx, 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.gate.security.oauth2; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import javax.servlet.FilterChain; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoRestTemplateFactory; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.security.oauth2.client.OAuth2ClientContext; -import org.springframework.security.oauth2.client.OAuth2RestTemplate; -import org.springframework.security.oauth2.common.OAuth2AccessToken; - -public class ExternalAuthTokenFilterTest { - - @Mock private UserInfoRestTemplateFactory restTemplateFactory; - - @Mock private OAuth2RestTemplate restTemplate; - - @Mock private OAuth2ClientContext oauth2ClientContext; - - @InjectMocks private ExternalAuthTokenFilter filter; - - private MockHttpServletRequest request; - private MockHttpServletResponse response; - private FilterChain chain; - - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - request = new MockHttpServletRequest(); - response = new MockHttpServletResponse(); - chain = mock(FilterChain.class); - } - - @Test - void shouldEnsureBearerTokenIsForwardedProperly() - throws javax.servlet.ServletException, IOException { - // Arrange - request.addHeader("Authorization", "bearer foo"); - - OAuth2AccessToken token = mock(OAuth2AccessToken.class); - when(token.getTokenType()).thenReturn("Bearer"); - when(token.getValue()).thenReturn("foo"); - - when(restTemplateFactory.getUserInfoRestTemplate()).thenReturn(restTemplate); - when(restTemplate.getOAuth2ClientContext()).thenReturn(oauth2ClientContext); - when(oauth2ClientContext.getAccessToken()).thenReturn(token); - - // Act - filter.doFilter(request, response, chain); - - // Assert - verify(chain).doFilter(request, response); - assertThat(token.getTokenType()).isEqualTo("Bearer"); - assertThat(token.getValue()).isEqualTo("foo"); - } -} diff --git a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabledTest.java b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabledTest.java new file mode 100644 index 000000000..c9f76ec16 --- /dev/null +++ b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthConfigEnabledTest.java @@ -0,0 +1,120 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.EnumerablePropertySource; +import org.springframework.core.env.PropertySource; +import org.springframework.core.type.AnnotatedTypeMetadata; +import org.springframework.mock.env.MockPropertySource; + +class OAuthConfigEnabledTest { + + private OAuthConfigEnabled condition; + + @Mock private ConfigurableEnvironment environment; + @Mock private AnnotatedTypeMetadata metadata; + @Mock private ConditionContext context; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + condition = new OAuthConfigEnabled(); + when(context.getEnvironment()).thenReturn(environment); + } + + @Test + void testMatches_WhenOAuth2ConfigExists_ShouldReturnTrue() { + MockPropertySource propertySource = new MockPropertySource(); + propertySource.setProperty( + "spring.security.oauth2.client.registration.test-client.client-id", "test-client-id"); + + when(environment.getPropertySources()).thenReturn(new MockPropertySources(propertySource)); + + boolean result = condition.matches(context, metadata); + assertThat(result).isTrue(); + } + + @Test + void testMatches_WhenNoOAuth2Config_ShouldReturnFalse() { + MockPropertySource propertySource = new MockPropertySource(); + propertySource.setProperty("some.other.property", "value"); + when(environment.getPropertySources()).thenReturn(new MockPropertySources(propertySource)); + + boolean result = condition.matches(context, metadata); + assertThat(result).isFalse(); + } + + @Test + void testMatches_WhenEnvironmentNotConfigurable_ShouldReturnFalse() { + var nonConfigurableEnv = mock(org.springframework.core.env.Environment.class); + when(context.getEnvironment()).thenReturn(nonConfigurableEnv); + + boolean result = condition.matches(context, metadata); + assertThat(result).isFalse(); + } + + @Test + void testMatches_WithEnumerablePropertySource_ShouldReturnTrue() { + + @SuppressWarnings("unchecked") + EnumerablePropertySource> propertySource = + mock(EnumerablePropertySource.class); + when(propertySource.getPropertyNames()) + .thenReturn( + new String[] {"spring.security.oauth2.client.registration.test-client.client-id"}); + + when(environment.getPropertySources()).thenReturn(new MockPropertySources(propertySource)); + + boolean result = condition.matches(context, metadata); + assertThat(result).isTrue(); + } + + @Test + void testMatches_WithEnumerablePropertySourceButNoMatchingProperties_ShouldReturnFalse() { + + @SuppressWarnings("unchecked") + EnumerablePropertySource> propertySource = + mock(EnumerablePropertySource.class); + when(propertySource.getPropertyNames()).thenReturn(new String[] {"some.unrelated.property"}); + + when(environment.getPropertySources()).thenReturn(new MockPropertySources(propertySource)); + + boolean result = condition.matches(context, metadata); + assertThat(result).isFalse(); + } + + // Helper class to mock property sources + private static class MockPropertySources + extends org.springframework.core.env.MutablePropertySources { + MockPropertySources(PropertySource... propertySources) { + for (PropertySource ps : propertySources) { + addLast(ps); + } + } + } +} diff --git a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelperTest.java b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelperTest.java new file mode 100644 index 000000000..88d322cbc --- /dev/null +++ b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/OAuthUserInfoServiceHelperTest.java @@ -0,0 +1,197 @@ +/* + * Copyright 2025 OpsMx, 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.gate.security.oauth2; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import com.netflix.spinnaker.fiat.model.resources.ServiceAccount; +import com.netflix.spinnaker.gate.services.PermissionService; +import com.netflix.spinnaker.gate.services.internal.Front50Service; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import retrofit2.mock.Calls; + +@ExtendWith(MockitoExtension.class) +public class OAuthUserInfoServiceHelperTest { + + @Mock private OAuth2SsoConfig.UserInfoMapping userInfoMapping; + + @Mock private OAuth2SsoConfig.UserInfoRequirements userInfoRequirements; + + @Mock private PermissionService permissionService; + + @Mock private Front50Service front50Service; + + private OAuthUserInfoServiceHelper userInfoService; + + @BeforeEach + void setUp() { + // Manually instantiate the class to ensure correct injection + userInfoService = + new OAuthUserInfoServiceHelper( + userInfoMapping, + userInfoRequirements, + permissionService, + front50Service, + null, + null, + null, + Optional.empty()); + } + + @Test + void shouldEvaluateUserInfoRequirementsAgainstAuthenticationDetails() { + + // No domain restriction, everything should match + Map requirements = new HashMap<>(); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of())).isTrue(); + requirements = Map.of("hd", "foo.com"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); + requirements = Map.of("bar", "foo.com"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isTrue(); + requirements = Map.of("bar", "bar.com"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isTrue(); + + // Domain restricted but not found + requirements = Map.of("hd", "foo.com"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of())).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isFalse(); + + // Domain restricted by regex + requirements = Map.of("hd", "/foo\\.com|bar\\.com/"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of())).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "bar.com"))).isTrue(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "baz.com"))).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isFalse(); + + // Multiple restriction values + requirements = Map.of("bar", "bar.com"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isTrue(); + assertThat( + userInfoService.hasAllUserInfoRequirements(Map.of("hd", "foo.com", "bar", "bar.com"))) + .isTrue(); + + // Evaluating a list + requirements = Map.of("roles", "expected-role"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("roles", "expected-role"))) + .isTrue(); + assertThat( + userInfoService.hasAllUserInfoRequirements( + Map.of("roles", List.of("expected-role", "unexpected-role")))) + .isTrue(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of())).isFalse(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("roles", "unexpected-role"))) + .isFalse(); + assertThat( + userInfoService.hasAllUserInfoRequirements(Map.of("roles", List.of("unexpected-role")))) + .isFalse(); + + // Evaluating a regex in a list + requirements = Map.of("roles", "/^.+_ADMIN$/"); + when(userInfoRequirements.entrySet()).thenReturn(requirements.entrySet()); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("roles", "foo_ADMIN"))).isTrue(); + assertThat(userInfoService.hasAllUserInfoRequirements(Map.of("roles", List.of("foo_ADMIN")))) + .isTrue(); + assertThat( + userInfoService.hasAllUserInfoRequirements( + Map.of("roles", List.of("_ADMIN", "foo_USER")))) + .isFalse(); + assertThat( + userInfoService.hasAllUserInfoRequirements( + Map.of("roles", List.of("foo_ADMINISTRATOR", "bar_USER")))) + .isFalse(); + } + + @Test + void testIsServiceAccountValidServiceAccount() { + Map details = new HashMap<>(); + details.put("email", "service@example.com"); + + when(userInfoMapping.getServiceAccountEmail()).thenReturn("email"); + when(permissionService.isEnabled()).thenReturn(true); + ServiceAccount serviceAccount = new ServiceAccount(); + serviceAccount = serviceAccount.setName("service@example.com"); + when(front50Service.getServiceAccounts()).thenReturn(Calls.response(List.of(serviceAccount))); + + assertThat(userInfoService.isServiceAccount(details)).isTrue(); + } + + @Test + void testIsServiceAccountNotAServiceAccount() { + Map details = new HashMap<>(); + details.put("email", "user@example.com"); + + when(userInfoMapping.getServiceAccountEmail()).thenReturn("email"); + when(permissionService.isEnabled()).thenReturn(true); + ServiceAccount serviceAccount = new ServiceAccount(); + serviceAccount = serviceAccount.setName("service@example.com"); + when(front50Service.getServiceAccounts()).thenReturn(Calls.response(List.of(serviceAccount))); + + assertThat(userInfoService.isServiceAccount(details)).isFalse(); + } + + @ParameterizedTest + @MethodSource("provideRoleData") + public void shouldExtractRolesFromDetails(Object rolesValue, List expectedRoles) { + Map details = new HashMap<>(); + details.put("roles", rolesValue); + when(userInfoMapping.getRoles()).thenReturn("roles"); + assertThat(userInfoService.getRoles(details)).isEqualTo(expectedRoles); + } + + private static Stream provideRoleData() { + return Stream.of( + Arguments.of(null, List.of()), + Arguments.of("", List.of()), + Arguments.of(List.of("foo", "bar"), List.of("foo", "bar")), + Arguments.of("foo,bar", List.of("foo", "bar")), + Arguments.of("foo bar", List.of("foo", "bar")), + Arguments.of("foo", List.of("foo")), + Arguments.of("foo bar", List.of("foo", "bar")), + Arguments.of("foo,,,bar", List.of("foo", "bar")), + Arguments.of("foo, bar", List.of("foo", "bar")), + Arguments.of(List.of("[]"), List.of()), + Arguments.of(List.of("[\"foo\"]"), List.of("foo")), + Arguments.of(List.of("[\"foo\", \"bar\"]"), List.of("foo", "bar")), + Arguments.of(1, List.of()), + Arguments.of(Map.of("blergh", "blarg"), List.of())); + } +} diff --git a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesTest.java b/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesTest.java deleted file mode 100644 index 51b48f887..000000000 --- a/gate-oauth2/src/test/java/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesTest.java +++ /dev/null @@ -1,195 +0,0 @@ -/* - * Copyright 2025 OpsMx, 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.gate.security.oauth2; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.netflix.spinnaker.fiat.model.resources.ServiceAccount; -import com.netflix.spinnaker.gate.services.PermissionService; -import com.netflix.spinnaker.gate.services.internal.Front50Service; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Stream; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import retrofit2.mock.Calls; - -public class SpinnakerUserInfoTokenServicesTest { - - private SpinnakerUserInfoTokenServices tokenServices; - private OAuth2SsoConfig.UserInfoRequirements userInfoRequirements; - - @BeforeEach - public void setUp() { - userInfoRequirements = new OAuth2SsoConfig.UserInfoRequirements(); - tokenServices = - new SpinnakerUserInfoTokenServices( - null, - null, - null, - null, - userInfoRequirements, - null, - null, - Optional.empty(), - null, - null, - null); - } - - @Test - public void shouldEvaluateUserInfoRequirementsAgainstAuthenticationDetails() { - // No domain restriction, everything should match - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of())).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isTrue(); - - // Domain restricted but not found - userInfoRequirements.put("hd", "foo.com"); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of())).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isFalse(); - - // Domain restricted by regex - userInfoRequirements.put("hd", "/foo\\.com|bar\\.com/"); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of())).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "bar.com"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "baz.com"))).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "foo.com"))).isFalse(); - - // Multiple restriction values - userInfoRequirements.put("bar", "bar.com"); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "foo.com"))).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("bar", "bar.com"))).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("hd", "foo.com", "bar", "bar.com"))) - .isTrue(); - - // Evaluating a list - userInfoRequirements.clear(); - userInfoRequirements.put("roles", "expected-role"); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("roles", "expected-role"))).isTrue(); - assertThat( - tokenServices.hasAllUserInfoRequirements( - Map.of("roles", List.of("expected-role", "unexpected-role")))) - .isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of())).isFalse(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("roles", "unexpected-role"))) - .isFalse(); - assertThat( - tokenServices.hasAllUserInfoRequirements(Map.of("roles", List.of("unexpected-role")))) - .isFalse(); - - // Evaluating a regex in a list - userInfoRequirements.put("roles", "/^.+_ADMIN$/"); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("roles", "foo_ADMIN"))).isTrue(); - assertThat(tokenServices.hasAllUserInfoRequirements(Map.of("roles", List.of("foo_ADMIN")))) - .isTrue(); - assertThat( - tokenServices.hasAllUserInfoRequirements( - Map.of("roles", List.of("_ADMIN", "foo_USER")))) - .isFalse(); - assertThat( - tokenServices.hasAllUserInfoRequirements( - Map.of("roles", List.of("foo_ADMINISTRATOR", "bar_USER")))) - .isFalse(); - } - - @Test - public void verifyIsServiceAccount() { - PermissionService permissionService = mock(PermissionService.class); - when(permissionService.isEnabled()).thenReturn(true); - - Front50Service front50Service = mock(Front50Service.class); - ServiceAccount serviceAccount = new ServiceAccount(); - serviceAccount = serviceAccount.setName("ex@foo.com"); - when(front50Service.getServiceAccounts()).thenReturn(Calls.response(List.of(serviceAccount))); - - SpinnakerUserInfoTokenServices tokenServices = - new SpinnakerUserInfoTokenServices( - null, - null, - null, - new OAuth2SsoConfig.UserInfoMapping(), - null, - permissionService, - front50Service, - Optional.empty(), - null, - null, - null); - - Map details = Map.of("client_email", "ex@foo.com"); - boolean isServiceAccount = tokenServices.isServiceAccount(details); - - assertThat(isServiceAccount).isTrue(); - verify(permissionService).isEnabled(); - verify(front50Service).getServiceAccounts(); - } - - @ParameterizedTest - @MethodSource("provideRoleData") - public void shouldExtractRolesFromDetails(Object rolesValue, List expectedRoles) { - OAuth2SsoConfig.UserInfoMapping userInfoMapping = new OAuth2SsoConfig.UserInfoMapping(); - userInfoMapping.setRoles("roles"); - SpinnakerUserInfoTokenServices tokenServices = - new SpinnakerUserInfoTokenServices( - null, - null, - null, - userInfoMapping, - null, - null, - null, - Optional.empty(), - null, - null, - null); - - Map details = new HashMap<>(); - details.put("roles", rolesValue); - assertThat(tokenServices.getRoles(details)).isEqualTo(expectedRoles); - } - - private static Stream provideRoleData() { - return Stream.of( - Arguments.of(null, List.of()), - Arguments.of("", List.of()), - Arguments.of(List.of("foo", "bar"), List.of("foo", "bar")), - Arguments.of("foo,bar", List.of("foo", "bar")), - Arguments.of("foo bar", List.of("foo", "bar")), - Arguments.of("foo", List.of("foo")), - Arguments.of("foo bar", List.of("foo", "bar")), - Arguments.of("foo,,,bar", List.of("foo", "bar")), - Arguments.of("foo, bar", List.of("foo", "bar")), - Arguments.of(List.of("[]"), List.of()), - Arguments.of(List.of("[\"foo\"]"), List.of("foo")), - Arguments.of(List.of("[\"foo\", \"bar\"]"), List.of("foo", "bar")), - Arguments.of(1, List.of()), - Arguments.of(Map.of("blergh", "blarg"), List.of())); - } -} diff --git a/gate-web/src/test/java/com/netflix/spinnaker/gate/security/oauth/OAuth2Test.java b/gate-web/src/test/java/com/netflix/spinnaker/gate/security/oauth/OAuth2Test.java index 414465357..0f7be90cb 100644 --- a/gate-web/src/test/java/com/netflix/spinnaker/gate/security/oauth/OAuth2Test.java +++ b/gate-web/src/test/java/com/netflix/spinnaker/gate/security/oauth/OAuth2Test.java @@ -22,6 +22,8 @@ import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import com.netflix.spinnaker.gate.security.oauth2.provider.SpinnakerProviderTokenServices; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -34,14 +36,21 @@ @AutoConfigureMockMvc @SpringBootTest( properties = { - "security.oauth2.client.clientId=Spinnaker-Client", - "security.oauth2.resource.userInfoUri=http://localhost/userinfo" + "spring.security.oauth2.client.registration.github.client-id=client-id", + "spring.security.oauth2.client.registration.github.client-secret=client-secret" }) @TestPropertySource(properties = {"spring.config.location=classpath:gate-test.yml"}) public class OAuth2Test { @Autowired private MockMvc mockMvc; + /** + * This property is used to test the creation of the `GithubProviderTokenServices` bean when the + * `spring.security.oauth2.client.registration.github.client-id` property is present in the + * configuration. fails if GithubProviderTokenServices bean is unavailable in the context + */ + @Autowired private SpinnakerProviderTokenServices providerTokenServices; + @Test void shouldRedirectOnOauth2Authentication() throws Exception { MvcResult result = @@ -51,6 +60,7 @@ void shouldRedirectOnOauth2Authentication() throws Exception { .andReturn(); assertEquals(302, result.getResponse().getStatus()); + Assertions.assertThat(providerTokenServices).isNotNull(); } /** Test: Public endpoint should be accessible without authentication */