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 8e40eb190..95edad4b1 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 @@ -221,6 +221,8 @@ public static class RegistrationProperties { boolean showRegistrationButtonInLoginPage = true; boolean requireExternalAuthentication = false; + + boolean addNicknameAsAttribute = false; ExternalAuthenticationType authenticationType; @@ -245,6 +247,14 @@ public boolean isRequireExternalAuthentication() { public void setRequireExternalAuthentication(boolean requireExternalAuthentication) { this.requireExternalAuthentication = requireExternalAuthentication; } + + public boolean isAddNicknameAsAttribute() { + return addNicknameAsAttribute; + } + + public void setAddNicknameAsAttribute(boolean addNicknameAsAttribute) { + this.addNicknameAsAttribute = addNicknameAsAttribute; + } public ExternalAuthenticationType getAuthenticationType() { return authenticationType; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/registration/DefaultRegistrationRequestService.java b/iam-login-service/src/main/java/it/infn/mw/iam/registration/DefaultRegistrationRequestService.java index 79c7d8b2a..5c476b346 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/registration/DefaultRegistrationRequestService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/registration/DefaultRegistrationRequestService.java @@ -59,11 +59,13 @@ import it.infn.mw.iam.audit.events.registration.RegistrationRequestEvent; import it.infn.mw.iam.authn.ExternalAuthenticationRegistrationInfo; import it.infn.mw.iam.authn.ExternalAuthenticationRegistrationInfo.ExternalAuthenticationType; +import it.infn.mw.iam.config.IamProperties; import it.infn.mw.iam.config.lifecycle.LifecycleProperties; import it.infn.mw.iam.core.IamRegistrationRequestStatus; import it.infn.mw.iam.core.user.IamAccountService; import it.infn.mw.iam.notification.NotificationFactory; import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamAttribute; import it.infn.mw.iam.persistence.model.IamAup; import it.infn.mw.iam.persistence.model.IamAupSignature; import it.infn.mw.iam.persistence.model.IamLabel; @@ -121,8 +123,13 @@ public class DefaultRegistrationRequestService @Autowired private Clock clock; + @Autowired + private IamProperties iamProperties; + private ApplicationEventPublisher eventPublisher; + public static final String NICKNAME_ATTRIBUTE_KEY = "nickname"; + private IamRegistrationRequest findRequestById(String requestUuid) { return requestRepository.findByUuid(requestUuid) .orElseThrow(() -> new ScimResourceNotFoundException( @@ -165,7 +172,8 @@ private void addExternalAuthnInfo(ScimUser.Builder user, private void createAupSignatureForAccountIfNeeded(IamAccount account) { Optional aup = iamAupRepo.findDefaultAup(); if (aup.isPresent()) { - IamAupSignature signature = iamAupSignatureRepo.createSignatureForAccount(aup.get(), account, Date.from(clock.instant())); + IamAupSignature signature = iamAupSignatureRepo.createSignatureForAccount(aup.get(), account, + Date.from(clock.instant())); eventPublisher.publishEvent(new AupSignedEvent(this, signature)); } } @@ -200,6 +208,11 @@ public RegistrationRequestDto createRequest(RegistrationRequestDto dto, accountEntity.setConfirmationKey(tokenGenerator.generateToken()); accountEntity.setActive(false); + if (iamProperties.getRegistration().isAddNicknameAsAttribute()) { + accountEntity.getAttributes() + .add(IamAttribute.newInstance(NICKNAME_ATTRIBUTE_KEY, dto.getUsername())); + } + createAupSignatureForAccountIfNeeded(accountEntity); IamRegistrationRequest requestEntity = new IamRegistrationRequest(); @@ -399,12 +412,4 @@ public RegistrationRequestDto approveRequest(String requestUuid) { return handleApprove(request); } - public void setValidationService(RegistrationRequestValidationService validationService) { - this.validationService = validationService; - } - - public RegistrationRequestValidationService getValidationService() { - return validationService; - } - } diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 24cd9ec8a..67f4baa26 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -153,6 +153,7 @@ iam: authentication-type: ${IAM_REGISTRATION_AUTHENTICATION_TYPE:oidc} oidc-issuer: ${IAM_REGISTRATION_OIDC_ISSUER:https://example.org} saml-entity-id: ${IAM_REGISTRATION_SAML_ENTITY_ID:urn:example} + add-nickname-as-attribute: ${IAM_ADD_NICKNAME_AS_ATTRIBUTE:false} local-authn: login-page-visibility: ${IAM_LOCAL_AUTHN_LOGIN_PAGE_VISIBILITY:visible} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/OidcExtAuthRegistrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/OidcExtAuthRegistrationTests.java index a9caaddb4..77a4e629e 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/OidcExtAuthRegistrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/OidcExtAuthRegistrationTests.java @@ -16,11 +16,13 @@ package it.infn.mw.iam.test.registration; import static it.infn.mw.iam.authn.ExternalAuthenticationHandlerSupport.EXT_AUTH_ERROR_KEY; +import static it.infn.mw.iam.registration.DefaultRegistrationRequestService.NICKNAME_ATTRIBUTE_KEY; import static it.infn.mw.iam.test.ext_authn.oidc.OidcTestConfig.TEST_OIDC_CLIENT_ID; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.request; @@ -114,34 +116,31 @@ public void externalOidcRegistrationCreatesDisabledAccount() throws Exception { // If the user tries to authenticate with his external account, he's redirected to the // login page with an account disabled error - MockHttpSession session = - (MockHttpSession) mvc - .perform(get("/openid_connect_login").with( - SecurityMockMvcRequestPostProcessors.authentication(anonymousAuthenticationToken()))) - .andExpect(status().isFound()) - .andExpect(MockMvcResultMatchers - .redirectedUrlPattern(OidcTestConfig.TEST_OIDC_AUTHORIZATION_ENDPOINT_URI + "**")) - .andReturn() - .getRequest() - .getSession(); + MockHttpSession session = (MockHttpSession) mvc + .perform(get("/openid_connect_login") + .with(SecurityMockMvcRequestPostProcessors.authentication(anonymousAuthenticationToken()))) + .andExpect(status().isFound()) + .andExpect(MockMvcResultMatchers + .redirectedUrlPattern(OidcTestConfig.TEST_OIDC_AUTHORIZATION_ENDPOINT_URI + "**")) + .andReturn() + .getRequest() + .getSession(); String state = (String) session.getAttribute("state"); String nonce = (String) session.getAttribute("nonce"); oidcProvider.prepareTokenResponse(TEST_OIDC_CLIENT_ID, TEST_100_USER, nonce); - session = - (MockHttpSession) mvc - .perform(get("/openid_connect_login").param("state", state) - .param("code", "1234") - .session(session)) - .andExpect(status().isFound()) - .andExpect(MockMvcResultMatchers.redirectedUrlPattern("/login**")) - .andExpect( - MockMvcResultMatchers.request().sessionAttribute(EXT_AUTH_ERROR_KEY, notNullValue())) - .andReturn() - .getRequest() - .getSession(); + session = (MockHttpSession) mvc + .perform( + get("/openid_connect_login").param("state", state).param("code", "1234").session(session)) + .andExpect(status().isFound()) + .andExpect(MockMvcResultMatchers.redirectedUrlPattern("/login**")) + .andExpect( + MockMvcResultMatchers.request().sessionAttribute(EXT_AUTH_ERROR_KEY, notNullValue())) + .andReturn() + .getRequest() + .getSession(); assertThat(session.getAttribute(EXT_AUTH_ERROR_KEY), instanceOf(DisabledException.class)); @@ -152,33 +151,30 @@ public void externalOidcRegistrationCreatesDisabledAccount() throws Exception { // the same happens after having confirmed the request mvc.perform(get("/registration/confirm/{token}", token)).andExpect(status().isOk()); - session = - (MockHttpSession) mvc - .perform(get("/openid_connect_login").with( - SecurityMockMvcRequestPostProcessors.authentication(anonymousAuthenticationToken()))) - .andExpect(status().isFound()) - .andExpect(MockMvcResultMatchers - .redirectedUrlPattern(OidcTestConfig.TEST_OIDC_AUTHORIZATION_ENDPOINT_URI + "**")) - .andReturn() - .getRequest() - .getSession(); + session = (MockHttpSession) mvc + .perform(get("/openid_connect_login") + .with(SecurityMockMvcRequestPostProcessors.authentication(anonymousAuthenticationToken()))) + .andExpect(status().isFound()) + .andExpect(MockMvcResultMatchers + .redirectedUrlPattern(OidcTestConfig.TEST_OIDC_AUTHORIZATION_ENDPOINT_URI + "**")) + .andReturn() + .getRequest() + .getSession(); state = (String) session.getAttribute("state"); nonce = (String) session.getAttribute("nonce"); oidcProvider.prepareTokenResponse(TEST_OIDC_CLIENT_ID, TEST_100_USER, nonce); - session = - (MockHttpSession) mvc - .perform(get("/openid_connect_login").param("state", state) - .param("code", "1234") - .session(session)) - .andExpect(status().isFound()) - .andExpect(MockMvcResultMatchers.redirectedUrlPattern("/login**")) - .andExpect(request().sessionAttribute(EXT_AUTH_ERROR_KEY, notNullValue())) - .andReturn() - .getRequest() - .getSession(); + session = (MockHttpSession) mvc + .perform( + get("/openid_connect_login").param("state", state).param("code", "1234").session(session)) + .andExpect(status().isFound()) + .andExpect(MockMvcResultMatchers.redirectedUrlPattern("/login**")) + .andExpect(request().sessionAttribute(EXT_AUTH_ERROR_KEY, notNullValue())) + .andReturn() + .getRequest() + .getSession(); assertThat(session.getAttribute(EXT_AUTH_ERROR_KEY), instanceOf(DisabledException.class)); err = (DisabledException) session.getAttribute(EXT_AUTH_ERROR_KEY); @@ -187,6 +183,7 @@ public void externalOidcRegistrationCreatesDisabledAccount() throws Exception { IamAccount account = iamAccountRepo.findByOidcId(OidcTestConfig.TEST_OIDC_ISSUER, TEST_100_USER) .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue(account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY).isEmpty()); iamAccountRepo.delete(account); } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationAttributesTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationAttributesTests.java new file mode 100644 index 000000000..5d66cd0b0 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationAttributesTests.java @@ -0,0 +1,169 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * 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.registration; + +import static it.infn.mw.iam.registration.DefaultRegistrationRequestService.NICKNAME_ATTRIBUTE_KEY; +import static it.infn.mw.iam.test.ext_authn.saml.SamlAuthenticationTestSupport.DEFAULT_IDP_ID; +import static it.infn.mw.iam.test.ext_authn.saml.SamlAuthenticationTestSupport.T1_EPUID; +import static org.junit.Assert.assertTrue; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.web.servlet.MockMvc; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import it.infn.mw.iam.authn.saml.util.Saml2Attribute; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamSamlId; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; +import it.infn.mw.iam.registration.RegistrationRequestDto; +import it.infn.mw.iam.test.api.TestSupport; +import it.infn.mw.iam.test.ext_authn.oidc.OidcTestConfig; +import it.infn.mw.iam.test.util.WithMockOIDCUser; +import it.infn.mw.iam.test.util.WithMockSAMLUser; +import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; +import it.infn.mw.iam.test.util.oauth.MockOAuth2Filter; + + +@RunWith(SpringRunner.class) +@IamMockMvcIntegrationTest +@TestPropertySource(properties = {"iam.registration.add-nickname-as-attribute=true"}) +public class RegistrationAttributesTests extends TestSupport { + + @Autowired + private ObjectMapper objectMapper; + + @Autowired + private MockOAuth2Filter oauth2Filter; + + @Autowired + private MockMvc mvc; + + @Autowired + private IamAccountRepository iamAccountRepo; + + @Before + public void setup() { + oauth2Filter.cleanupSecurityContext(); + } + + @After + public void teardown() { + oauth2Filter.cleanupSecurityContext(); + } + + private RegistrationRequestDto createRegistrationRequest() { + + String username = "test-attributes"; + + String email = username + "@example.org"; + RegistrationRequestDto request = new RegistrationRequestDto(); + request.setGivenname("Test"); + request.setFamilyname("User"); + request.setEmail(email); + request.setUsername(username); + request.setNotes("Some short notes..."); + request.setPassword("password"); + + return request; + } + + @Test + public void linkAttributesFromLocalRegistrationRequest() throws Exception { + + RegistrationRequestDto r = createRegistrationRequest(); + mvc + .perform(post("/registration/create").contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(r))) + .andExpect(status().isOk()); + + IamAccount account = iamAccountRepo.findByEmail("test-attributes@example.org") + .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue(account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY) + .get() + .getValue() + .equals("test-attributes")); + + iamAccountRepo.delete(account); + + } + + @Test + @WithMockOIDCUser(subject = TEST_100_USER, issuer = OidcTestConfig.TEST_OIDC_ISSUER) + public void linkAttributesFromOidcRegistrationRequest() throws Exception { + + String username = "test-oidc-subject"; + + String email = username + "@example.org"; + RegistrationRequestDto request = new RegistrationRequestDto(); + request.setGivenname("Test"); + request.setFamilyname("User"); + request.setEmail(email); + request.setUsername(username); + request.setNotes("Some short notes..."); + + mvc + .perform(post("/registration/create").contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsBytes(request))) + .andExpect(status().isOk()); + + IamAccount account = iamAccountRepo.findByOidcId(OidcTestConfig.TEST_OIDC_ISSUER, TEST_100_USER) + .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue( + account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY).get().getValue().equals(username)); + + iamAccountRepo.delete(account); + } + + @Test + @WithMockSAMLUser(issuer = DEFAULT_IDP_ID, subject = T1_EPUID) + public void linkAttributesFromSamlRegistrationRequest() throws Throwable { + + String username = "test-saml-ext-reg"; + + String email = username + "@example.org"; + RegistrationRequestDto request = new RegistrationRequestDto(); + request.setGivenname("Test"); + request.setFamilyname("Saml User"); + request.setEmail(email); + request.setUsername(username); + request.setNotes("Some short notes..."); + + mvc + .perform(post("/registration/create").contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsBytes(request))) + .andExpect(status().isOk()); + + IamSamlId id = new IamSamlId(DEFAULT_IDP_ID, Saml2Attribute.EPUID.getAttributeName(), T1_EPUID); + + IamAccount account = iamAccountRepo.findBySamlId(id) + .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue( + account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY).get().getValue().equals(username)); + + iamAccountRepo.delete(account); + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationUsernameTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationUsernameTests.java index 9eabe8f85..af9c23eb8 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationUsernameTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/RegistrationUsernameTests.java @@ -15,6 +15,8 @@ */ package it.infn.mw.iam.test.registration; +import static it.infn.mw.iam.registration.DefaultRegistrationRequestService.NICKNAME_ATTRIBUTE_KEY; +import static org.junit.Assert.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -29,6 +31,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; import it.infn.mw.iam.registration.RegistrationRequestDto; import it.infn.mw.iam.test.api.TestSupport; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -48,6 +52,9 @@ public class RegistrationUsernameTests extends TestSupport { @Autowired private MockMvc mvc; + @Autowired + private IamAccountRepository iamAccountRepo; + @Before public void setup() { oauth2Filter.cleanupSecurityContext(); @@ -85,6 +92,12 @@ public void validUsernames() throws Exception { .andExpect(status().isOk()); } + IamAccount account = iamAccountRepo.findByUsername("bob") + .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue(account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY).isEmpty()); + + iamAccountRepo.delete(account); + } @Test diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/SamlExtAuthRegistrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/SamlExtAuthRegistrationTests.java index 7c378c6be..b0f4fff86 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/SamlExtAuthRegistrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/registration/SamlExtAuthRegistrationTests.java @@ -16,10 +16,12 @@ package it.infn.mw.iam.test.registration; import static it.infn.mw.iam.authn.ExternalAuthenticationHandlerSupport.EXT_AUTH_ERROR_KEY; +import static it.infn.mw.iam.registration.DefaultRegistrationRequestService.NICKNAME_ATTRIBUTE_KEY; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern; @@ -148,6 +150,8 @@ public void externalSamlRegistrationCreatesDisabledAccount() throws Throwable { IamAccount account = iamAccountRepo.findBySamlId(id) .orElseThrow(() -> new AssertionError("Expected account not found")); + assertTrue(account.getAttributeByName(NICKNAME_ATTRIBUTE_KEY).isEmpty()); + iamAccountRepo.delete(account); }