From c01b38df40498b214bc71dc3a3de0241760a8ea0 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Thu, 30 Apr 2020 11:39:55 +0200 Subject: [PATCH 01/10] Fixed code smell reported by Sonar --- .../iam/core/oauth/profile/common/BaseAccessTokenBuilder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java index 7049df775..d9686be6e 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java @@ -97,7 +97,6 @@ protected void handleClientTokenExchange(JWTClaimsSet.Builder builder, Map actClaimContent = Maps.newHashMap(); actClaimContent.put("sub", authentication.getOAuth2Request().getClientId()); - Object subjectTokenActClaim = subjectToken.getJWTClaimsSet().getClaim(ACT_CLAIM_NAME); if (!isNull(subjectTokenActClaim)) { @@ -107,7 +106,7 @@ protected void handleClientTokenExchange(JWTClaimsSet.Builder builder, builder.claim(ACT_CLAIM_NAME, actClaimContent); } catch (ParseException e) { - LOG.error("Error getting claims from subject token: " + e.getMessage(), e); + LOG.error("Error getting claims from subject token: {}", e.getMessage(), e); } } From c0c91868bd5bf657c18f3f305e2d919bb4c4dc8e Mon Sep 17 00:00:00 2001 From: enricovianello Date: Thu, 11 Jun 2020 14:00:23 +0200 Subject: [PATCH 02/10] Fixed default urn namespace value for aarc profile --- .../mw/iam/core/oauth/profile/aarc/AarcClaimValueHelper.java | 2 +- iam-login-service/src/main/resources/application.yml | 5 ++++- .../mw/iam/test/oauth/profile/AarcClaimValueHelperTests.java | 2 +- .../iam/test/oauth/profile/AarcProfileIntegrationTests.java | 5 ++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/aarc/AarcClaimValueHelper.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/aarc/AarcClaimValueHelper.java index 9d14d1ef7..b7fe4a6d9 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/aarc/AarcClaimValueHelper.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/aarc/AarcClaimValueHelper.java @@ -39,7 +39,7 @@ public class AarcClaimValueHelper { @Value("${iam.organisation.name}") String organisationName; - @Value("${iam.aarcProfile.urnNamespace}") + @Value("${iam.aarc-profile.urn-namespace}") String urnNamespace; public Object getClaimValueFromUserInfo(String claim, IamUserInfo info) { diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index c2c74eab5..10b032786 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -97,7 +97,10 @@ iam: device-code: allow-complete-verification-uri: ${IAM_DEVICE_CODE_ALLOW_COMPLETE_VERIFICATION_URI:true} - + + aarc-profile: + urn-namespace: ${IAM_AARC_PROFILE_URN_NAMESPACE:example:iam} + x509: trustAnchorsDir: ${IAM_X509_TRUST_ANCHORS_DIR:/etc/grid-security/certificates} trustAnchorsRefreshMsec: ${IAM_X509_TRUST_ANCHORS_REFRESH:14400} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcClaimValueHelperTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcClaimValueHelperTests.java index 6b03bc9a1..4bf6a99ad 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcClaimValueHelperTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcClaimValueHelperTests.java @@ -47,7 +47,7 @@ // @formatter:off "iam.host=example.org", "iam.organisation.name=org", - "iam.aarcProfile.urnNamespace=example:iam:test", + "iam.aarc-profile.urn-namespace=example:iam:test", // @formatter:on }) public class AarcClaimValueHelperTests { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcProfileIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcProfileIntegrationTests.java index 897bff367..a6442906c 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcProfileIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/AarcProfileIntegrationTests.java @@ -71,7 +71,6 @@ "iam.host=example.org", "iam.jwt-profile.default-profile=aarc", "iam.organisation.name=org", - "iam.aarcProfile.urnNamespace=example:iam:test", // @formatter:on }) public class AarcProfileIntegrationTests extends EndpointsTestUtils { @@ -81,8 +80,8 @@ public class AarcProfileIntegrationTests extends EndpointsTestUtils { private static final String USERNAME = "test"; private static final String PASSWORD = "password"; - private static final String URN_GROUP_ANALYSIS = "urn:example:iam:test:group:Analysis#example.org"; - private static final String URN_GROUP_PRODUCTION = "urn:example:iam:test:group:Production#example.org"; + private static final String URN_GROUP_ANALYSIS = "urn:example:iam:group:Analysis#example.org"; + private static final String URN_GROUP_PRODUCTION = "urn:example:iam:group:Production#example.org"; protected static final Set BASE_SCOPES = Sets.newHashSet("openid", "profile"); protected static final Set EDUPERSON_AFFILIATION_SCOPE = From 0e07561e61db43910b15a37d2b8169a2e1e33736 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Sat, 13 Jun 2020 06:00:47 +0200 Subject: [PATCH 03/10] Allow hiding and disabling local authentication With this commit the local authentication can: - be hidden from the main login page - be enabled only for selected type of users (vo-admins) - be disabled This is useful to delegate user authentication to external IdPs but still keep flexibility on enabling local access (for admin duties or when external IdPs may have issues). --- .../password_reset/PasswordResetService.java | 5 +- .../it/infn/mw/iam/config/IamProperties.java | 46 ++++- .../it/infn/mw/iam/config/TaskConfig.java | 2 +- .../mw/iam/config/github/GithubClient.java | 167 ------------------ .../config/security/IamWebSecurityConfig.java | 11 +- .../core/IamLocalAuthenticationProvider.java | 63 +++++++ .../web/DefaultLoginPageConfiguration.java | 28 ++- .../mw/iam/core/web/IamRootController.java | 2 +- .../iam/core/web/LoginPageConfiguration.java | 4 + .../TransientNotificationFactory.java | 12 +- .../resources-filtered/application.properties | 3 + .../src/main/resources/application.yml | 6 +- .../src/main/resources/i18n/en/messages.json | 2 +- .../resources/templates/accountActivated.vm | 8 +- .../webapp/WEB-INF/views/iam/login-form.jsp | 51 ++++++ .../main/webapp/WEB-INF/views/iam/login.jsp | 63 +++---- .../mw/iam/test/login/LoginDisabledTests.java | 83 +++++++++ .../login/LoginEnabledOnlyForAdminsTests.java | 94 ++++++++++ .../mw/iam/test/login/LoginTestSupport.java | 28 +++ .../it/infn/mw/iam/test/login/LoginTests.java | 18 +- .../RegistrationFlowNotificationTests.java | 2 +- 21 files changed, 451 insertions(+), 247 deletions(-) delete mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/config/github/GithubClient.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java create mode 100644 iam-login-service/src/main/webapp/WEB-INF/views/iam/login-form.jsp create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginDisabledTests.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginEnabledOnlyForAdminsTests.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTestSupport.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/password_reset/PasswordResetService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/password_reset/PasswordResetService.java index 85bb66af2..6bec9e556 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/password_reset/PasswordResetService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/password_reset/PasswordResetService.java @@ -14,16 +14,13 @@ * limitations under the License. */ package it.infn.mw.iam.api.account.password_reset; -// Keep these includes, as the Javadoc refers to them... -import it.infn.mw.iam.api.account.password_reset.error.BadUserPasswordError; -import it.infn.mw.iam.api.account.password_reset.error.InvalidPasswordResetTokenError; -import it.infn.mw.iam.api.account.password_reset.error.UserNotActiveOrNotVerified; /** * * The IAM password reset service * */ +@SuppressWarnings("unused") public interface PasswordResetService { diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java index d07fc5b35..c5c1bdd78 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java @@ -30,13 +30,44 @@ @Component @ConfigurationProperties(prefix = "iam") public class IamProperties { - + public enum EditableFields { NAME, SURNAME, EMAIL, PICTURE } + + public enum LocalAuthenticationAllowedUsers { + ALL, + VO_ADMINS, + NONE + } + + public enum LocalAuthenticationLoginPageMode { + VISIBLE, + HIDDEN, + HIDDEN_WITH_LINK + } + + public static class LocalAuthenticationProperties { + + LocalAuthenticationLoginPageMode loginPageVisibility; + LocalAuthenticationAllowedUsers enabledFor; + + public LocalAuthenticationLoginPageMode getLoginPageVisibility() { + return loginPageVisibility; + } + public void setLoginPageVisibility(LocalAuthenticationLoginPageMode loginPageVisibility) { + this.loginPageVisibility = loginPageVisibility; + } + public LocalAuthenticationAllowedUsers getEnabledFor() { + return enabledFor; + } + public void setEnabledFor(LocalAuthenticationAllowedUsers enabledFor) { + this.enabledFor = enabledFor; + } + } @JsonInclude(JsonInclude.Include.NON_EMPTY) public static class UserProfileProperties { @@ -77,7 +108,7 @@ public void setExternalAuthAttribute(String externalAuthAttribute) { public static class RegistrationProperties { boolean requireExternalAuthentication = false; - + ExternalAuthenticationType authenticationType; String oidcIssuer; @@ -389,6 +420,8 @@ public void setUrnNamespace(String urnNamespace) { private UserProfileProperties userProfile = new UserProfileProperties(); private AarcProfileProperties aarcProfile = new AarcProfileProperties(); + + private LocalAuthenticationProperties localAuthn = new LocalAuthenticationProperties(); public String getBaseUrl() { return baseUrl; @@ -549,4 +582,13 @@ public AarcProfileProperties getAarcProfile() { public void setAarcProfile(AarcProfileProperties aarcProfile) { this.aarcProfile = aarcProfile; } + + public LocalAuthenticationProperties getLocalAuthn() { + return localAuthn; + } + + public void setLocalAuthn(LocalAuthenticationProperties localAuthn) { + this.localAuthn = localAuthn; + } + } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/TaskConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/TaskConfig.java index b9edece8b..224eea6d2 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/TaskConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/TaskConfig.java @@ -43,7 +43,7 @@ @Configuration @EnableScheduling -@Profile("prod") +@Profile({"prod", "dev"}) public class TaskConfig implements SchedulingConfigurer { public static final Logger LOG = LoggerFactory.getLogger(TaskConfig.class); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/github/GithubClient.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/github/GithubClient.java deleted file mode 100644 index 953d1ddb8..000000000 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/github/GithubClient.java +++ /dev/null @@ -1,167 +0,0 @@ -/** - * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 - * - * 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 it.infn.mw.iam.config.github; - -import java.io.IOException; - -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.security.oauth2.resource.ResourceServerProperties; -import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoTokenServices; -import org.springframework.boot.context.embedded.FilterRegistrationBean; -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Profile; -import org.springframework.core.annotation.Order; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; -import org.springframework.security.oauth2.client.OAuth2ClientContext; -import org.springframework.security.oauth2.client.OAuth2RestTemplate; -import org.springframework.security.oauth2.client.filter.OAuth2ClientAuthenticationProcessingFilter; -import org.springframework.security.oauth2.client.filter.OAuth2ClientContextFilter; -import org.springframework.security.oauth2.client.resource.OAuth2ProtectedResourceDetails; -import org.springframework.security.oauth2.client.token.grant.code.AuthorizationCodeResourceDetails; -import org.springframework.security.oauth2.config.annotation.web.configuration.EnableOAuth2Client; -import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint; -import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; -import org.springframework.security.web.csrf.CsrfFilter; -import org.springframework.security.web.csrf.CsrfToken; -import org.springframework.security.web.csrf.CsrfTokenRepository; -import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository; -import org.springframework.web.context.request.RequestContextListener; -import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.util.WebUtils; - -@Configuration -@Profile("github") -@EnableOAuth2Client -@Order(120) -public class GithubClient extends WebSecurityConfigurerAdapter { - - @Autowired - private OAuth2ClientContext oauth2ClientContext; - - @Override - protected void configure(HttpSecurity http) throws Exception { - - // @formatter:off - http.antMatcher("/gh_login**").authorizeRequests().antMatchers("/", "/gh_login**").permitAll() - .anyRequest().authenticated().and().exceptionHandling() - .authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/")).and().logout() - .logoutSuccessUrl("/").permitAll().and().csrf().csrfTokenRepository(csrfTokenRepository()) - .and().addFilterAfter(csrfHeaderFilter(), CsrfFilter.class) - .addFilterBefore(ssoGitHubFilter(), BasicAuthenticationFilter.class); - - // @formatter:on - } - - @Bean - public RequestContextListener requestContextListener() { - - return new RequestContextListener(); - } - - @Bean - public FilterRegistrationBean oauth2ClientFilterRegistration(OAuth2ClientContextFilter filter) { - - FilterRegistrationBean registration = new FilterRegistrationBean(); - registration.setFilter(filter); - registration.setOrder(-100); - return registration; - } - - @Bean - @ConfigurationProperties("github") - ClientResources github() { - - return new ClientResources(); - } - - @Bean(name = "socialAuthFilter") - public Filter ssoGitHubFilter() { - - return ssoFilter(github(), "/gh_login"); - } - - private Filter ssoFilter(ClientResources client, String path) { - - OAuth2ClientAuthenticationProcessingFilter oAuth2ClientAuthenticationFilter = - new OAuth2ClientAuthenticationProcessingFilter(path); - OAuth2RestTemplate oAuth2RestTemplate = - new OAuth2RestTemplate(client.getClient(), oauth2ClientContext); - oAuth2ClientAuthenticationFilter.setRestTemplate(oAuth2RestTemplate); - UserInfoTokenServices tokenServices = new UserInfoTokenServices( - client.getResource().getUserInfoUri(), client.getClient().getClientId()); - tokenServices.setRestTemplate(oAuth2RestTemplate); - oAuth2ClientAuthenticationFilter.setTokenServices(tokenServices); - return oAuth2ClientAuthenticationFilter; - } - - private Filter csrfHeaderFilter() { - - return new OncePerRequestFilter() { - - @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, - FilterChain filterChain) throws ServletException, IOException { - - CsrfToken csrf = (CsrfToken) request.getAttribute(CsrfToken.class.getName()); - if (csrf != null) { - Cookie cookie = WebUtils.getCookie(request, "XSRF-TOKEN"); - String token = csrf.getToken(); - if (cookie == null || token != null && !token.equals(cookie.getValue())) { - cookie = new Cookie("XSRF-TOKEN", token); - cookie.setPath("/"); - response.addCookie(cookie); - } - } - filterChain.doFilter(request, response); - } - }; - } - - private CsrfTokenRepository csrfTokenRepository() { - - HttpSessionCsrfTokenRepository repository = new HttpSessionCsrfTokenRepository(); - repository.setHeaderName("X-XSRF-TOKEN"); - return repository; - } - -} - - -class ClientResources { - - private OAuth2ProtectedResourceDetails client = new AuthorizationCodeResourceDetails(); - private ResourceServerProperties resource = new ResourceServerProperties(); - - public OAuth2ProtectedResourceDetails getClient() { - - return client; - } - - public ResourceServerProperties getResource() { - - return resource; - } -} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java index 6299b9ac7..1aa04905d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java @@ -62,6 +62,7 @@ import it.infn.mw.iam.authn.x509.IamX509PreauthenticationProcessingFilter; import it.infn.mw.iam.authn.x509.X509AuthenticationCredentialExtractor; import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.IamLocalAuthenticationProvider; import it.infn.mw.iam.persistence.repository.IamAccountRepository; @Configuration @@ -112,12 +113,13 @@ public static class UserLoginConfig extends WebSecurityConfigurerAdapter { @Autowired private ExternalAuthenticationHintService hintService; + @Autowired + private IamProperties iamProperties; + @Autowired public void configureGlobal(final AuthenticationManagerBuilder auth) throws Exception { // @formatter:off - auth - .userDetailsService(iamUserDetailsService) - .passwordEncoder(passwordEncoder); + auth.authenticationProvider(new IamLocalAuthenticationProvider(iamProperties, iamUserDetailsService, passwordEncoder)); // @formatter:on } @@ -205,7 +207,8 @@ public static class RegistrationConfig extends WebSecurityConfigurerAdapter { AccessDeniedHandler accessDeniedHandler() { return (request, response, authError) -> { - RequestDispatcher dispatcher = request.getRequestDispatcher("/registration/insufficient-auth"); + RequestDispatcher dispatcher = + request.getRequestDispatcher("/registration/insufficient-auth"); request.setAttribute("authError", authError); dispatcher.forward(request, response); }; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java new file mode 100644 index 000000000..cc0bf2da4 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java @@ -0,0 +1,63 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + * + * 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 it.infn.mw.iam.core; + +import java.util.function.Predicate; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.authentication.dao.DaoAuthenticationProvider; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.password.PasswordEncoder; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.config.IamProperties.LocalAuthenticationAllowedUsers; + +public class IamLocalAuthenticationProvider extends DaoAuthenticationProvider { + + public static final Logger LOG = LoggerFactory.getLogger(IamLocalAuthenticationProvider.class); + + public static final String DISABLED_AUTH_MESSAGE = "Local authentication is disabled"; + + private final LocalAuthenticationAllowedUsers allowedUsers; + + private final Predicate ADMIN_MATCHER = + a -> a.getAuthority().equals("ROLE_ADMIN"); + + public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsService uds, PasswordEncoder passwordEncoder) { + this.allowedUsers = properties.getLocalAuthn().getEnabledFor(); + setUserDetailsService(uds); + setPasswordEncoder(passwordEncoder); + } + + @Override + protected void additionalAuthenticationChecks(UserDetails userDetails, + UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { + + super.additionalAuthenticationChecks(userDetails, authentication); + if (LocalAuthenticationAllowedUsers.NONE.equals(allowedUsers) + || (LocalAuthenticationAllowedUsers.VO_ADMINS.equals(allowedUsers) + && userDetails.getAuthorities().stream().noneMatch(ADMIN_MATCHER))) { + throw new DisabledException(DISABLED_AUTH_MESSAGE); + } + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/DefaultLoginPageConfiguration.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/DefaultLoginPageConfiguration.java index 31df839c6..d305f13eb 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/DefaultLoginPageConfiguration.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/DefaultLoginPageConfiguration.java @@ -48,21 +48,23 @@ public class DefaultLoginPageConfiguration implements LoginPageConfiguration, En private boolean githubEnabled; private boolean samlEnabled; private boolean registrationEnabled; + private boolean localAuthenticationVisible; + private boolean showLinkToLocalAuthn; @Value(ACCOUNT_LINKING_DISABLE_PROPERTY) private Boolean accountLinkingDisable; - + private OidcValidatedProviders providers; private final IamProperties iamProperties; - + @Autowired public DefaultLoginPageConfiguration(OidcValidatedProviders providers, IamProperties properties) { this.providers = providers; this.iamProperties = properties; } - - + + @PostConstruct public void init() { @@ -70,6 +72,10 @@ public void init() { githubEnabled = activeProfilesContains("github"); samlEnabled = activeProfilesContains("saml"); registrationEnabled = activeProfilesContains("registration"); + localAuthenticationVisible = IamProperties.LocalAuthenticationLoginPageMode.VISIBLE + .equals(iamProperties.getLocalAuthn().getLoginPageVisibility()); + showLinkToLocalAuthn = IamProperties.LocalAuthenticationLoginPageMode.HIDDEN_WITH_LINK + .equals(iamProperties.getLocalAuthn().getLoginPageVisibility()); } private boolean activeProfilesContains(String val) { @@ -115,11 +121,11 @@ public boolean isAccountLinkingEnabled() { @Override public Optional getPrivacyPolicyUrl() { - + if (Strings.isNullOrEmpty(iamProperties.getPrivacyPolicy().getUrl())) { return Optional.empty(); } - + return Optional.of(iamProperties.getPrivacyPolicy().getUrl()); } @@ -155,4 +161,14 @@ public boolean isExternalAuthenticationEnabled() { return isOidcEnabled() || isSamlEnabled(); } + @Override + public boolean isLocalAuthenticationVisible() { + return localAuthenticationVisible; + } + + + @Override + public boolean isShowLinkToLocalAuthenticationPage() { + return showLinkToLocalAuthn; + } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/IamRootController.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/IamRootController.java index ce5d1f2f9..ebe1b7c68 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/IamRootController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/IamRootController.java @@ -67,7 +67,7 @@ public String resetSession(HttpSession session) { SecurityContextHolder.clearContext(); session.invalidate(); - + return "redirect:/"; } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/LoginPageConfiguration.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/LoginPageConfiguration.java index fe921a871..ee2c41f34 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/LoginPageConfiguration.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/LoginPageConfiguration.java @@ -23,6 +23,10 @@ public interface LoginPageConfiguration { + boolean isLocalAuthenticationVisible(); + + boolean isShowLinkToLocalAuthenticationPage(); + boolean isExternalAuthenticationEnabled(); boolean isOidcEnabled(); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index 04b7f0d11..6931642cd 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -101,10 +101,14 @@ public IamEmailNotification createAccountActivatedMessage(IamRegistrationRequest String resetPasswordUrl = String.format("%s%s/%s", baseUrl, PasswordResetController.BASE_TOKEN_URL, request.getAccount().getResetKey()); + + Map model = new HashMap<>(); model.put(RECIPIENT_FIELD, recipient); + model.put("resetPasswordUrl", resetPasswordUrl); model.put(ORGANISATION_NAME, organisationName); + model.put(USERNAME_FIELD, request.getAccount().getUsername()); IamEmailNotification notification = createMessage("accountActivated.vm", model, IamNotificationType.ACTIVATED, properties.getSubject().get("activated"), @@ -118,13 +122,14 @@ public IamEmailNotification createAccountActivatedMessage(IamRegistrationRequest } @Override - public IamEmailNotification createRequestRejectedMessage(IamRegistrationRequest request, Optional motivation) { + public IamEmailNotification createRequestRejectedMessage(IamRegistrationRequest request, + Optional motivation) { String recipient = request.getAccount().getUserInfo().getName(); Map model = new HashMap<>(); model.put(RECIPIENT_FIELD, recipient); model.put(ORGANISATION_NAME, organisationName); - + if (motivation.isPresent()) { model.put(MOTIVATION_FIELD, motivation.get()); } @@ -192,7 +197,8 @@ public IamEmailNotification createAdminHandleGroupRequestMessage(IamGroupRequest LOG.debug("Create group membership admin notification for request {}", groupRequest.getUuid()); return createMessage("adminHandleGroupRequest.vm", model, IamNotificationType.GROUP_MEMBERSHIP, - subject, groupManagerDeliveryStrategy.resolveGroupManagersEmailAddresses(groupRequest.getGroup())); + subject, + groupManagerDeliveryStrategy.resolveGroupManagersEmailAddresses(groupRequest.getGroup())); } @Override diff --git a/iam-login-service/src/main/resources-filtered/application.properties b/iam-login-service/src/main/resources-filtered/application.properties index ac11436e8..c7478293b 100644 --- a/iam-login-service/src/main/resources-filtered/application.properties +++ b/iam-login-service/src/main/resources-filtered/application.properties @@ -41,6 +41,9 @@ logging.level.org.opensaml.saml2.metadata.provider=INFO #logging.level.org.apache.jasper.servlet.TldScanner=DEBUG +# Notification service logging +#logging.level.it.infn.mw.iam.notification=DEBUG + #logging.level.org.springframework.security=DEBUG #logging.level.org.springframework.web=DEBUG diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 69755f9e0..8c84a3347 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -104,13 +104,17 @@ iam: oidc-issuer: ${IAM_REGISTRATION_OIDC_ISSUER:https://example.org} saml-entity-id: ${IAM_REGISTRATION_SAML_ENTITY_ID:urn:example} + local-authn: + login-page-visibility: ${IAM_LOCAL_AUTHN_LOGIN_PAGE_VISIBILITY:visible} + enabled-for: ${IAM_LOCAL_AUTHN_ENABLED_FOR:all} + user-profile: editable-fields: - email - name - picture - surname - + x509: trustAnchorsDir: ${IAM_X509_TRUST_ANCHORS_DIR:/etc/grid-security/certificates} trustAnchorsRefreshMsec: ${IAM_X509_TRUST_ANCHORS_REFRESH:14400} diff --git a/iam-login-service/src/main/resources/i18n/en/messages.json b/iam-login-service/src/main/resources/i18n/en/messages.json index 6f44d0ccf..355684205 100644 --- a/iam-login-service/src/main/resources/i18n/en/messages.json +++ b/iam-login-service/src/main/resources/i18n/en/messages.json @@ -467,7 +467,7 @@ "login": { "login_with_username_and_password": "Login with Username and Password", "login_with_saml": "Login with your SAML IdP", - "error": "The system was unable to log you in. Please try again." + "error": "The system was unable to log you in." }, "device": { "request_code": { diff --git a/iam-login-service/src/main/resources/templates/accountActivated.vm b/iam-login-service/src/main/resources/templates/accountActivated.vm index f21eb016f..031c7684d 100644 --- a/iam-login-service/src/main/resources/templates/accountActivated.vm +++ b/iam-login-service/src/main/resources/templates/accountActivated.vm @@ -2,9 +2,15 @@ Dear $recipient, your registration request has been approved. -You can set your password by following this link: +The username for your account is: + +$username + +You can set your local $organisationName password by following this link: $resetPasswordUrl +Note that this step is not needed if you are using an external identity provider +to log into $organisationName, your account is already active. The $organisationName registration service \ No newline at end of file diff --git a/iam-login-service/src/main/webapp/WEB-INF/views/iam/login-form.jsp b/iam-login-service/src/main/webapp/WEB-INF/views/iam/login-form.jsp new file mode 100644 index 000000000..83a52eb9f --- /dev/null +++ b/iam-login-service/src/main/webapp/WEB-INF/views/iam/login-form.jsp @@ -0,0 +1,51 @@ +<%-- + + Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + + 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. + +--%> +<%@ taglib prefix="authz" uri="http://www.springframework.org/security/tags"%> +<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core"%> +<%@ taglib prefix="t" tagdir="/WEB-INF/tags/iam"%> +<%@ taglib prefix="spring" uri="http://www.springframework.org/tags"%> +
+ +
+
+ + + + +
+
+
+
+ + + + +
+
+
+ + +
+
+ + + \ No newline at end of file diff --git a/iam-login-service/src/main/webapp/WEB-INF/views/iam/login.jsp b/iam-login-service/src/main/webapp/WEB-INF/views/iam/login.jsp index 5f0d5a475..419aa9366 100644 --- a/iam-login-service/src/main/webapp/WEB-INF/views/iam/login.jsp +++ b/iam-login-service/src/main/webapp/WEB-INF/views/iam/login.jsp @@ -54,7 +54,8 @@
- + +
${SPRING_SECURITY_LAST_EXCEPTION.message}
 @@ -70,50 +71,20 @@ -
- - - -
-
- - - - -
-
- -
-
- - - - -
-
- -
- - -
-
- - - + + - -
- + +
@@ -145,11 +116,16 @@ + btn="${ls.loginButton}" id="saml-login-${ls.name}" /> + + + +
+ @@ -163,7 +139,8 @@ diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginDisabledTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginDisabledTests.java new file mode 100644 index 000000000..711497e36 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginDisabledTests.java @@ -0,0 +1,83 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + * + * 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 it.infn.mw.iam.test.login; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated; +import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.log; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.WebAttributes; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.context.WebApplicationContext; + +import it.infn.mw.iam.IamLoginService; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = {IamLoginService.class}) +@WebAppConfiguration +@Transactional +@TestPropertySource(properties = {"iam.local-authn.enabled-for=none"}) +public class LoginDisabledTests implements LoginTestSupport { + + @Autowired + private WebApplicationContext context; + + private MockMvc mvc; + + @Before + public void setup() { + mvc = + MockMvcBuilders.webAppContextSetup(context).apply(springSecurity()).alwaysDo(log()).build(); + } + + @Test + public void testLocalLoginDisabled() throws Exception { + //@formatter:off + MockHttpSession session = (MockHttpSession) mvc + .perform( + post(LOGIN_URL) + .param("username", ADMIN_USERNAME) + .param("password", ADMIN_PASSWORD) + .param("submit", "Login")) + .andExpect(unauthenticated()) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/login?error=failure")) + .andReturn().getRequest().getSession(); + + AuthenticationException ae = (AuthenticationException) session.getAttribute(WebAttributes.AUTHENTICATION_EXCEPTION); + assertThat(ae.getMessage(), is(DISABLED_AUTH_MSG)); + + + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginEnabledOnlyForAdminsTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginEnabledOnlyForAdminsTests.java new file mode 100644 index 000000000..dcd1a9c31 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginEnabledOnlyForAdminsTests.java @@ -0,0 +1,94 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + * + * 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 it.infn.mw.iam.test.login; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated; +import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.log; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.WebAttributes; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.context.WebApplicationContext; + +import it.infn.mw.iam.IamLoginService; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = {IamLoginService.class}) +@WebAppConfiguration +@Transactional +@TestPropertySource(properties = {"iam.local-authn.enabled-for=vo-admins"}) +public class LoginEnabledOnlyForAdminsTests implements LoginTestSupport { + + + @Autowired + private WebApplicationContext context; + + private MockMvc mvc; + + @Before + public void setup() { + mvc = + MockMvcBuilders.webAppContextSetup(context).apply(springSecurity()).alwaysDo(log()).build(); + } + + + @Test + public void loginForAdminUserWorks() throws Exception { + mvc + .perform(post(LOGIN_URL).param("username", ADMIN_USERNAME) + .param("password", ADMIN_PASSWORD) + .param("submit", "Login")) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/dashboard")); + } + + @Test + public void loginForUnprivilegedUserFails() throws Exception { + MockHttpSession session = (MockHttpSession) mvc + .perform(post(LOGIN_URL).param("username", USER_USERNAME) + .param("password", USER_PASSWORD) + .param("submit", "Login")) + .andExpect(unauthenticated()) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/login?error=failure")) + .andReturn() + .getRequest() + .getSession(); + + AuthenticationException ae = + (AuthenticationException) session.getAttribute(WebAttributes.AUTHENTICATION_EXCEPTION); + + assertThat(ae.getMessage(), is(DISABLED_AUTH_MSG)); + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTestSupport.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTestSupport.java new file mode 100644 index 000000000..f1b04a8c9 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTestSupport.java @@ -0,0 +1,28 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + * + * 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 it.infn.mw.iam.test.login; + +public interface LoginTestSupport { + + public static final String LOGIN_URL = "/login"; + public static final String ADMIN_USERNAME = "admin"; + public static final String ADMIN_PASSWORD = "password"; + + public static final String USER_USERNAME = "test"; + public static final String USER_PASSWORD = "password"; + + public static final String DISABLED_AUTH_MSG = "Local authentication is disabled"; +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTests.java index 4120f8b40..6b903da12 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/login/LoginTests.java @@ -53,11 +53,7 @@ @SpringApplicationConfiguration(classes = {IamLoginService.class}) @WebAppConfiguration @Transactional -public class LoginTests { - - public static final String LOGIN_URL = "/login"; - public static final String ADMIN_USERNAME = "admin"; - public static final String ADMIN_PASSWORD = "password"; +public class LoginTests implements LoginTestSupport { @Autowired private WebApplicationContext context; @@ -72,10 +68,8 @@ public class LoginTests { @Before public void setup() { - mvc = MockMvcBuilders.webAppContextSetup(context) - .apply(springSecurity()) - .alwaysDo(log()) - .build(); + mvc = + MockMvcBuilders.webAppContextSetup(context).apply(springSecurity()).alwaysDo(log()).build(); } @Test @@ -142,9 +136,9 @@ public void loginRedirectsToSignAupPageWhenNeeded() throws Exception { aupRepo.save(aup); mvc - .perform( - post(LOGIN_URL).param("username", ADMIN_USERNAME).param("password", ADMIN_PASSWORD).param( - "submit", "Login")) + .perform(post(LOGIN_URL).param("username", ADMIN_USERNAME) + .param("password", ADMIN_PASSWORD) + .param("submit", "Login")) .andExpect(status().isFound()) .andExpect(redirectedUrl("/iam/aup/sign")); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/notification/RegistrationFlowNotificationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/notification/RegistrationFlowNotificationTests.java index 33307364a..397853d12 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/notification/RegistrationFlowNotificationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/notification/RegistrationFlowNotificationTests.java @@ -182,7 +182,7 @@ public void testApproveFlowNotifications() throws Exception { message = notificationDelivery.getDeliveredNotifications().get(0); assertThat(message.getSubject(), equalTo(properties.getSubject().get("activated"))); - + assertThat(message.getBody(), containsString(request.getUsername())); } From 44d7fcac7904c7ec7cdd56cce6067105f13ce2cb Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Sat, 13 Jun 2020 15:05:00 +0200 Subject: [PATCH 04/10] Fixes for Sonar warnings --- .../lifecycle/AccountLifecycleDTO.java | 3 - .../cern/DefaultCernHrDBApiService.java | 2 - .../it/infn/mw/iam/authn/util/JwtUtils.java | 1 - .../infn/mw/iam/config/saml/SamlConfig.java | 6 - .../config/security/IamWebSecurityConfig.java | 2 +- .../core/IamLocalAuthenticationProvider.java | 2 +- .../iam/core/lifecycle/AccountLifecycle.java | 20 ---- .../lifecycle/ExpiredAccountsHandler.java | 2 +- .../it/infn/mw/iam/test/api/TestSupport.java | 52 ++++----- .../test/lifecycle/AccountLifecycleTests.java | 13 +-- .../AccountLifecycleTestsNoGracePeriod.java | 108 ++++++++++++++++++ .../cern/CernAccountLifecycleTests.java | 5 - .../lifecycle/cern/LifecycleTestSupport.java | 5 + 13 files changed, 144 insertions(+), 77 deletions(-) delete mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/AccountLifecycle.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTestsNoGracePeriod.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lifecycle/AccountLifecycleDTO.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lifecycle/AccountLifecycleDTO.java index 5d3182895..6a92471ca 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lifecycle/AccountLifecycleDTO.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/lifecycle/AccountLifecycleDTO.java @@ -20,9 +20,6 @@ public class AccountLifecycleDTO { private Date endTime; - - public AccountLifecycleDTO() { - } public Date getEndTime() { return endTime; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/registration/cern/DefaultCernHrDBApiService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/registration/cern/DefaultCernHrDBApiService.java index c9f171898..67f657942 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/registration/cern/DefaultCernHrDBApiService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/registration/cern/DefaultCernHrDBApiService.java @@ -80,7 +80,6 @@ public boolean hasValidExperimentParticipation(String personId) { return response.getBody(); } catch (RestClientException e) { final String errorMsg = "HR db api error: " + e.getMessage(); - LOG.warn(errorMsg, e); throw new CernHrDbApiError(errorMsg, e); } } @@ -101,7 +100,6 @@ public VOPersonDTO getHrDbPersonRecord(String personId) { return response.getBody(); } catch (RestClientException e) { final String errorMsg = "HR db api error: " + e.getMessage(); - LOG.warn(errorMsg, e); throw new CernHrDbApiError(errorMsg, e); } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/util/JwtUtils.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/util/JwtUtils.java index 28818347e..4a258814a 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/util/JwtUtils.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/util/JwtUtils.java @@ -63,7 +63,6 @@ public static Map getClaimsAsMap(JWT jwt) { } else { LOG.warn("Unsupported claim type '{}' for claim '{}'... skipping it", claimValue.getClass().getName(), claimName); - continue; } } return claimsMap; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/saml/SamlConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/saml/SamlConfig.java index c243da18a..28667f28b 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/saml/SamlConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/saml/SamlConfig.java @@ -329,12 +329,6 @@ public Timer samlMetadataFetchTimer() { return new Timer("metadata-fetch", Boolean.TRUE); } -// @Bean -// public BasicParserPool samlParserPool() { -// BasicParserPool parserPool = new BasicParserPool(); -// return parserPool; -// } - @Bean public SAMLEntryPoint samlEntryPoint() { IamSamlEntryPoint ep = new IamSamlEntryPoint(optionsResolver); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java index 1aa04905d..6b627d657 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java @@ -215,7 +215,7 @@ AccessDeniedHandler accessDeniedHandler() { } AuthenticationEntryPoint entryPoint() { - String discoveryId = "/unset"; + String discoveryId; if (OIDC.equals(iamProperties.getRegistration().getAuthenticationType())) { discoveryId = String.format("/openid_connect_login?iss=%s", iamProperties.getRegistration().getOidcIssuer()); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java index cc0bf2da4..d9ef9cfbc 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java @@ -39,7 +39,7 @@ public class IamLocalAuthenticationProvider extends DaoAuthenticationProvider { private final LocalAuthenticationAllowedUsers allowedUsers; - private final Predicate ADMIN_MATCHER = + private static final Predicate ADMIN_MATCHER = a -> a.getAuthority().equals("ROLE_ADMIN"); public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsService uds, PasswordEncoder passwordEncoder) { diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/AccountLifecycle.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/AccountLifecycle.java deleted file mode 100644 index c0597350e..000000000 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/AccountLifecycle.java +++ /dev/null @@ -1,20 +0,0 @@ -/** - * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 - * - * 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 it.infn.mw.iam.core.lifecycle; - -public interface AccountLifecycle { - -} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/ExpiredAccountsHandler.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/ExpiredAccountsHandler.java index 03390c4c8..48c898e70 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/ExpiredAccountsHandler.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/ExpiredAccountsHandler.java @@ -75,7 +75,7 @@ private boolean pastGracePeriod(IamAccount expiredAccount, long gracePeriodDays) return checkTime.isAfter(endTime.plus(gracePeriodDays, ChronoUnit.DAYS)); } - return false; + return true; } private boolean pastSuspensionGracePeriod(IamAccount expiredAccount) { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/TestSupport.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/TestSupport.java index 1fcc9238b..22c35eb1d 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/TestSupport.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/TestSupport.java @@ -30,53 +30,53 @@ public class TestSupport { - protected static final ResultMatcher OK = status().isOk(); - protected static final ResultMatcher NO_CONTENT = status().isNoContent(); - protected static final ResultMatcher BAD_REQUEST = status().isBadRequest(); - protected static final ResultMatcher UNAUTHORIZED = status().isUnauthorized(); - protected static final ResultMatcher FORBIDDEN = status().isForbidden(); - protected static final ResultMatcher NOT_FOUND = status().isNotFound(); + public static final ResultMatcher OK = status().isOk(); + public static final ResultMatcher NO_CONTENT = status().isNoContent(); + public static final ResultMatcher BAD_REQUEST = status().isBadRequest(); + public static final ResultMatcher UNAUTHORIZED = status().isUnauthorized(); + public static final ResultMatcher FORBIDDEN = status().isForbidden(); + public static final ResultMatcher NOT_FOUND = status().isNotFound(); - protected static final String RANDOM_UUID = UUID.randomUUID().toString(); + public static final String RANDOM_UUID = UUID.randomUUID().toString(); - protected static final String TEST_001_GROUP_UUID = "c617d586-54e6-411d-8e38-649677980001"; - protected static final String TEST_002_GROUP_UUID = "c617d586-54e6-411d-8e38-649677980002"; + public static final String TEST_001_GROUP_UUID = "c617d586-54e6-411d-8e38-649677980001"; + public static final String TEST_002_GROUP_UUID = "c617d586-54e6-411d-8e38-649677980002"; - protected static final String TEST_USER = "test"; - protected static final String TEST_USER_UUID = "80e5fb8d-b7c8-451a-89ba-346ae278a66f"; - protected static final String TEST_100_USER = "test_100"; - protected static final String TEST_100_USER_UUID = "f2ce8cb2-a1db-4884-9ef0-d8842cc02b4a"; + public static final String TEST_USER = "test"; + public static final String TEST_USER_UUID = "80e5fb8d-b7c8-451a-89ba-346ae278a66f"; + public static final String TEST_100_USER = "test_100"; + public static final String TEST_100_USER_UUID = "f2ce8cb2-a1db-4884-9ef0-d8842cc02b4a"; - protected static final String EXPECTED_ACCOUNT_NOT_FOUND = "Expected account not found"; - protected static final String EXPECTED_GROUP_NOT_FOUND = "Expected group not found"; + public static final String EXPECTED_ACCOUNT_NOT_FOUND = "Expected account not found"; + public static final String EXPECTED_GROUP_NOT_FOUND = "Expected group not found"; - protected static String LABEL_PREFIX = "indigo-iam.github.io"; - protected static String LABEL_NAME = "example.label"; - protected static String LABEL_VALUE = "example-label-value"; + public static String LABEL_PREFIX = "indigo-iam.github.io"; + public static String LABEL_NAME = "example.label"; + public static String LABEL_VALUE = "example-label-value"; - protected static LabelDTO TEST_LABEL = + public static LabelDTO TEST_LABEL = LabelDTO.builder().prefix(LABEL_PREFIX).name(LABEL_NAME).value(LABEL_VALUE).build(); - protected static final TypeReference> LIST_OF_LABEL_DTO = + public static final TypeReference> LIST_OF_LABEL_DTO = new TypeReference>() {}; - protected static final ResultMatcher INVALID_PREFIX_ERROR_MESSAGE = + public static final ResultMatcher INVALID_PREFIX_ERROR_MESSAGE = jsonPath("$.error", containsString("invalid prefix (does not match")); - protected static final ResultMatcher PREFIX_TOO_LONG_ERROR_MESSAGE = + public static final ResultMatcher PREFIX_TOO_LONG_ERROR_MESSAGE = jsonPath("$.error", containsString("invalid prefix length")); - protected static final ResultMatcher NAME_REQUIRED_ERROR_MESSAGE = + public static final ResultMatcher NAME_REQUIRED_ERROR_MESSAGE = jsonPath("$.error", containsString("name is required")); - protected static final ResultMatcher INVALID_NAME_ERROR_MESSAGE = + public static final ResultMatcher INVALID_NAME_ERROR_MESSAGE = jsonPath("$.error", containsString("invalid name (does not match")); - protected static final ResultMatcher NAME_TOO_LONG_ERROR_MESSAGE = + public static final ResultMatcher NAME_TOO_LONG_ERROR_MESSAGE = jsonPath("$.error", containsString("invalid name length")); - protected static final ResultMatcher VALUE_TOO_LONG_ERROR_MESSAGE = + public static final ResultMatcher VALUE_TOO_LONG_ERROR_MESSAGE = jsonPath("$.error", containsString("invalid value length")); } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTests.java index 22de68eba..b7ce50aa2 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTests.java @@ -25,7 +25,6 @@ import java.time.ZoneId; import java.util.Date; import java.util.Optional; -import java.util.function.Supplier; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,7 +40,6 @@ import it.infn.mw.iam.IamLoginService; import it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler; -import it.infn.mw.iam.core.user.IamAccountService; import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamLabel; import it.infn.mw.iam.persistence.repository.IamAccountRepository; @@ -69,18 +67,11 @@ Clock mockClock() { } } - private Supplier assertionError(String message) { - return () -> new AssertionError(message); - } - - @Autowired - IamAccountRepository repo; - @Autowired - IamAccountService service; + private IamAccountRepository repo; @Autowired - ExpiredAccountsHandler handler; + private ExpiredAccountsHandler handler; @Test public void testSuspensionGracePeriodWorks() { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTestsNoGracePeriod.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTestsNoGracePeriod.java new file mode 100644 index 000000000..61dd6a2bb --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/AccountLifecycleTestsNoGracePeriod.java @@ -0,0 +1,108 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2019 + * + * 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 it.infn.mw.iam.test.lifecycle; + +import static it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler.LIFECYCLE_STATUS_LABEL; +import static it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler.LIFECYCLE_TIMESTAMP_LABEL; +import static it.infn.mw.iam.test.api.TestSupport.EXPECTED_ACCOUNT_NOT_FOUND; +import static it.infn.mw.iam.test.api.TestSupport.TEST_USER_UUID; +import static java.lang.String.valueOf; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import java.time.Clock; +import java.time.ZoneId; +import java.util.Date; +import java.util.Optional; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.transaction.annotation.Transactional; + +import it.infn.mw.iam.IamLoginService; +import it.infn.mw.iam.core.lifecycle.ExpiredAccountsHandler; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamLabel; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.test.api.TestSupport; +import it.infn.mw.iam.test.core.CoreControllerTestSupport; +import it.infn.mw.iam.test.lifecycle.AccountLifecycleTests.TestConfig; +import it.infn.mw.iam.test.lifecycle.cern.LifecycleTestSupport; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration( + classes = {IamLoginService.class, CoreControllerTestSupport.class, TestConfig.class}) +@WebAppConfiguration +@Transactional +@TestPropertySource( + properties = {"lifecycle.account.expiredAccountPolicy.suspensionGracePeriodDays=0", + "lifecycle.account.expiredAccountPolicy.removalGracePeriodDays=30"}) +public class AccountLifecycleTestsNoGracePeriod implements LifecycleTestSupport { + + @Configuration + public static class TestConfig { + @Bean + @Primary + Clock mockClock() { + return Clock.fixed(NOW, ZoneId.systemDefault()); + } + } + + + + @Autowired + private IamAccountRepository repo; + + @Autowired + private ExpiredAccountsHandler handler; + + @Test + public void testZeroDaysSuspensionGracePeriod() { + IamAccount testAccount = + repo.findByUuid(TestSupport.TEST_USER_UUID).orElseThrow(assertionError(TestSupport.EXPECTED_ACCOUNT_NOT_FOUND)); + + assertThat(testAccount.isActive(), is(true)); + + testAccount.setEndTime(Date.from(FOUR_DAYS_AGO)); + repo.save(testAccount); + + handler.handleExpiredAccounts(); + + testAccount = + repo.findByUuid(TEST_USER_UUID).orElseThrow(assertionError(EXPECTED_ACCOUNT_NOT_FOUND)); + + assertThat(testAccount.isActive(), is(false)); + + Optional timestampLabel = testAccount.getLabelByName(LIFECYCLE_TIMESTAMP_LABEL); + + assertThat(timestampLabel.isPresent(), is(true)); + assertThat(timestampLabel.get().getValue(), is(valueOf(NOW.toEpochMilli()))); + + Optional statusLabel = testAccount.getLabelByName(LIFECYCLE_STATUS_LABEL); + assertThat(statusLabel.isPresent(), is(true)); + assertThat(statusLabel.get().getValue(), + is(ExpiredAccountsHandler.AccountLifecycleStatus.PENDING_REMOVAL.toString())); + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java index 67a39ea20..cc8408f94 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java @@ -35,7 +35,6 @@ import java.time.ZoneId; import java.util.Optional; import java.util.UUID; -import java.util.function.Supplier; import org.junit.After; import org.junit.Test; @@ -98,10 +97,6 @@ CernHrDBApiService hrDb() { } } - private Supplier assertionError(String message) { - return () -> new AssertionError(message); - } - @Autowired IamAccountRepository repo; diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/LifecycleTestSupport.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/LifecycleTestSupport.java index e636935f0..672ce91ce 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/LifecycleTestSupport.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/LifecycleTestSupport.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Date; +import java.util.function.Supplier; import com.google.common.collect.Sets; @@ -94,5 +95,9 @@ default VOPersonDTO voPerson(String personId) { return dto; } + + default Supplier assertionError(String message) { + return () -> new AssertionError(message); + } } From 158b7f8b817a34902054b2230b0ba51510c0e749 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 13:05:30 +0200 Subject: [PATCH 05/10] Fix sonarlint warnings --- iam-login-service/pom.xml | 3 +-- .../x509/PEMX509CertificateChainParser.java | 6 ------ .../config/security/IamWebSecurityConfig.java | 10 +++++++--- .../core/IamLocalAuthenticationProvider.java | 2 +- .../oauth/exchange/ClientMatcherFactory.java | 4 ++++ .../profile/common/BaseAccessTokenBuilder.java | 6 +++--- .../util/test/OidcSecurityContextBuilder.java | 17 +++++++++-------- 7 files changed, 25 insertions(+), 23 deletions(-) diff --git a/iam-login-service/pom.xml b/iam-login-service/pom.xml index 4a179f3d7..a37a8dc35 100644 --- a/iam-login-service/pom.xml +++ b/iam-login-service/pom.xml @@ -353,9 +353,8 @@ jacoco-maven-plugin - it/infn/mw/iam/github/**/*.class - it/infn/mw/iam/config/github/**/*.class it/infn/mw/iam/api/registration/cern/mock/*.class + it/infn/mw/iam/api/registration/cern/dto/*.class diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/x509/PEMX509CertificateChainParser.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/x509/PEMX509CertificateChainParser.java index fc8593b15..e4ce651ce 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/x509/PEMX509CertificateChainParser.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/x509/PEMX509CertificateChainParser.java @@ -23,8 +23,6 @@ import java.nio.charset.StandardCharsets; import java.security.cert.X509Certificate; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; import eu.emi.security.authn.x509.impl.CertificateUtils; @@ -32,8 +30,6 @@ @Component public class PEMX509CertificateChainParser implements X509CertificateChainParser { - - public static final Logger LOG = LoggerFactory.getLogger(PEMX509CertificateChainParser.class); public PEMX509CertificateChainParser() { configureSecProvider(); @@ -54,8 +50,6 @@ public X509CertificateChainParsingResult parseChainFromString(String pemString) final String errorMessage = String.format("Error parsing certificate chain: %s", e.getMessage()); - LOG.error(errorMessage, e); - throw new CertificateParsingError(errorMessage, e); } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java index 6b627d657..3979d3f38 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java @@ -68,6 +68,8 @@ @Configuration @EnableWebSecurity public class IamWebSecurityConfig { + + @Bean public SecurityEvaluationContextExtension contextExtension() { @@ -201,6 +203,8 @@ public AuthenticationSuccessHandler successHandler() { @Configuration @Order(101) public static class RegistrationConfig extends WebSecurityConfigurerAdapter { + + public static final String START_REGISTRATION_ENDPOINT = "/start-registration"; @Autowired IamProperties iamProperties; @@ -230,21 +234,21 @@ AuthenticationEntryPoint entryPoint() { protected void configure(HttpSecurity http) throws Exception { http.requestMatchers() - .antMatchers("/start-registration") + .antMatchers(START_REGISTRATION_ENDPOINT) .and() .sessionManagement() .enableSessionUrlRewriting(false); if (iamProperties.getRegistration().isRequireExternalAuthentication()) { http.authorizeRequests() - .antMatchers("/start-registration") + .antMatchers(START_REGISTRATION_ENDPOINT) .hasAuthority(EXT_AUTHN_UNREGISTERED_USER_AUTH.getAuthority()) .and() .exceptionHandling() .accessDeniedHandler(accessDeniedHandler()) .authenticationEntryPoint(entryPoint()); } else { - http.authorizeRequests().antMatchers("/start-registration").permitAll(); + http.authorizeRequests().antMatchers(START_REGISTRATION_ENDPOINT).permitAll(); } } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java index d9ef9cfbc..b073b844f 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java @@ -50,7 +50,7 @@ public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsServi @Override protected void additionalAuthenticationChecks(UserDetails userDetails, - UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { + UsernamePasswordAuthenticationToken authentication) { super.additionalAuthenticationChecks(userDetails, authentication); if (LocalAuthenticationAllowedUsers.NONE.equals(allowedUsers) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/exchange/ClientMatcherFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/exchange/ClientMatcherFactory.java index 12b825210..d0a63ed0a 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/exchange/ClientMatcherFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/exchange/ClientMatcherFactory.java @@ -20,6 +20,10 @@ public class ClientMatcherFactory { + private ClientMatcherFactory() { + // empty on purpose + } + public static ClientMatcher newClientMatcher(IamClientMatchingPolicy clientMatchingPolicy) { switch (clientMatchingPolicy.getType()) { case ANY: diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java index 7049df775..ccfd6555c 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/common/BaseAccessTokenBuilder.java @@ -58,7 +58,7 @@ public abstract class BaseAccessTokenBuilder implements JWTAccessTokenBuilder { protected final IamProperties properties; - protected final Splitter SPLITTER = Splitter.on(' ').trimResults().omitEmptyStrings(); + protected final Splitter splitter = Splitter.on(' ').trimResults().omitEmptyStrings(); public BaseAccessTokenBuilder(IamProperties properties) { this.properties = properties; @@ -107,7 +107,7 @@ protected void handleClientTokenExchange(JWTClaimsSet.Builder builder, builder.claim(ACT_CLAIM_NAME, actClaimContent); } catch (ParseException e) { - LOG.error("Error getting claims from subject token: " + e.getMessage(), e); + LOG.error("Error getting claims from subject token: {}", e.getMessage(), e); } } @@ -162,7 +162,7 @@ protected JWTClaimsSet.Builder baseJWTSetup(OAuth2AccessTokenEntity token, } if (!isNullOrEmpty(audience)) { - builder.audience(SPLITTER.splitToList(audience)); + builder.audience(splitter.splitToList(audience)); } if (isTokenExchangeRequest(authentication)) { diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/util/test/OidcSecurityContextBuilder.java b/iam-login-service/src/main/java/it/infn/mw/iam/util/test/OidcSecurityContextBuilder.java index f6615da40..2022b24d3 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/util/test/OidcSecurityContextBuilder.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/util/test/OidcSecurityContextBuilder.java @@ -37,7 +37,7 @@ public class OidcSecurityContextBuilder extends SecurityContextBuilderSupport { - UserInfo userInfo = null; + private UserInfo userInfo = null; private Map stringClaims = Maps.newHashMap(); public OidcSecurityContextBuilder() { @@ -54,8 +54,8 @@ public OidcSecurityContextBuilder() { public SecurityContext buildSecurityContext() { OIDCAuthenticationToken authToken = mock(OIDCAuthenticationToken.class); - UserInfo userInfo = mock(UserInfo.class); - when(authToken.getUserInfo()).thenReturn(userInfo); + UserInfo ui = mock(UserInfo.class); + when(authToken.getUserInfo()).thenReturn(ui); JWTClaimsSet.Builder builder = new JWTClaimsSet.Builder(); @@ -81,11 +81,11 @@ public SecurityContext buildSecurityContext() { expirationTime, authToken.getPrincipal(), "", authorities); - when(userInfo.getGivenName()).thenReturn(stringClaims.get("given_name")); - when(userInfo.getFamilyName()).thenReturn(stringClaims.get("family_name")); - when(userInfo.getName()).thenReturn(stringClaims.get("name")); - when(userInfo.getEmail()).thenReturn(stringClaims.get("email")); - when(userInfo.getPreferredUsername()).thenReturn(username); + when(ui.getGivenName()).thenReturn(stringClaims.get("given_name")); + when(ui.getFamilyName()).thenReturn(stringClaims.get("family_name")); + when(ui.getName()).thenReturn(stringClaims.get("name")); + when(ui.getEmail()).thenReturn(stringClaims.get("email")); + when(ui.getPreferredUsername()).thenReturn(username); @@ -120,6 +120,7 @@ public SecurityContextBuilderSupport email(String email) { } + @Override public OidcSecurityContextBuilder expirationTime(long unixTime) { if (unixTime > 0) { this.expirationTime = new Date(unixTime); From 5bcfb1eb2d58e0067c60857e1ea190fd83b2c259 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 13:10:17 +0200 Subject: [PATCH 06/10] Bump guava version to 29.0-jre --- iam-persistence/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-persistence/pom.xml b/iam-persistence/pom.xml index 8a4469c01..a5d3dbb0c 100644 --- a/iam-persistence/pom.xml +++ b/iam-persistence/pom.xml @@ -60,7 +60,7 @@ com.google.guava guava - 20.0 + 29.0-jre From 9939e6f94131678cc364d7e00cac3273e419282f Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 13:56:41 +0200 Subject: [PATCH 07/10] First attempt at running sonar on GH actions --- .github/workflows/maven.yml | 3 --- .github/workflows/sonar.yml | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/sonar.yml diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e2c64d832..4cec2d38f 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,6 +1,3 @@ -# This workflow will build a Java project with Maven -# For more information see: https://help.github.com/actions/language-and-framework-guides/building-and-testing-java-with-maven - name: build & packaging on: [push, pull_request] diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml new file mode 100644 index 000000000..a3f5ca468 --- /dev/null +++ b/.github/workflows/sonar.yml @@ -0,0 +1,38 @@ +name: Sonar analysis + +on: + + pull_request: + types: [opened, edited, reopened, synchronize] + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Set up JDK 1.8 + uses: actions/setup-java@v1 + with: + java-version: 1.8 + - name: Cache Maven packages + uses: actions/cache@v1 + with: + path: ~/.m2 + key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-m2 + - name: Sonar analysis + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: mvn -B -U install sonar:sonar + -Dsonar.projectKey=indigo-iam_iam + -Dsonar.organization=indigo-iam + -Dsonar.login=$SONAR_TOKEN + -Dsonar.pullrequest.key=${{ github.event.number }} + -Dsonar.pullrequest.branch=${{ github.HEAD_REF }} + -Dsonar.pullrequest.base=${{ github.BASE_REF }} + -Dsonar.pullrequest.github.repository=${{ github.repository }} + -Dsonar.scm.provider=git From 451defa34ea13ab41e92b6a94fcf7a7bdafd5e78 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 14:49:56 +0200 Subject: [PATCH 08/10] Remove unused import --- .../it/infn/mw/iam/core/IamLocalAuthenticationProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java index b073b844f..23f6bc9eb 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/IamLocalAuthenticationProvider.java @@ -22,7 +22,6 @@ import org.springframework.security.authentication.DisabledException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; -import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsService; @@ -42,7 +41,8 @@ public class IamLocalAuthenticationProvider extends DaoAuthenticationProvider { private static final Predicate ADMIN_MATCHER = a -> a.getAuthority().equals("ROLE_ADMIN"); - public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsService uds, PasswordEncoder passwordEncoder) { + public IamLocalAuthenticationProvider(IamProperties properties, UserDetailsService uds, + PasswordEncoder passwordEncoder) { this.allowedUsers = properties.getLocalAuthn().getEnabledFor(); setUserDetailsService(uds); setPasswordEncoder(passwordEncoder); From 94b15c5303b7d0bd64432d2742902984705043ba Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 14:53:25 +0200 Subject: [PATCH 09/10] Set sonar.host for GH actions sonar workflow --- .github/workflows/sonar.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index a3f5ca468..14a76b31a 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -36,3 +36,4 @@ jobs: -Dsonar.pullrequest.base=${{ github.BASE_REF }} -Dsonar.pullrequest.github.repository=${{ github.repository }} -Dsonar.scm.provider=git + -Dsonar.host.url=https://sonarcloud.io From b1f7ff3ec34a68bff6f62d1f2e5fb4d8d75bba83 Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 7 Jul 2020 15:43:13 +0200 Subject: [PATCH 10/10] Fix SONAR TOKEN inclusion --- .github/workflows/sonar.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 14a76b31a..fc500d087 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -27,6 +27,7 @@ jobs: - name: Sonar analysis env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: mvn -B -U install sonar:sonar -Dsonar.projectKey=indigo-iam_iam -Dsonar.organization=indigo-iam