diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/IamUserApprovalHandler.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/IamUserApprovalHandler.java index 08b69c010..a67d1a480 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/IamUserApprovalHandler.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/IamUserApprovalHandler.java @@ -54,11 +54,14 @@ import it.infn.mw.iam.api.account.AccountUtils; import it.infn.mw.iam.api.client.service.ClientService; +import it.infn.mw.iam.persistence.model.IamAccount; @SuppressWarnings("deprecation") @Component("iamUserApprovalHandler") public class IamUserApprovalHandler implements UserApprovalHandler { + public static final String OIDC_AGENT_PREFIX_NAME = "oidc-agent:"; + @Autowired private ClientDetailsEntityService clientDetailsService; @@ -184,6 +187,13 @@ public AuthorizationRequest updateAfterApproval(AuthorizationRequest authorizati setAuthTime(authorizationRequest); + IamAccount account = accountUtils.getAuthenticatedUserAccount(userAuthentication).orElseThrow(); + + if (client.getClientName().startsWith(OIDC_AGENT_PREFIX_NAME) + && clientService.findClientOwners(clientId, null).isEmpty()) { + clientService.linkClientToAccount(client, account); + } + return authorizationRequest; } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/authzcode/AuthorizationCodeTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/authzcode/AuthorizationCodeTests.java index 5740c97b2..65eebbd7e 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/authzcode/AuthorizationCodeTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/authzcode/AuthorizationCodeTests.java @@ -18,6 +18,7 @@ import static java.lang.String.format; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.springframework.security.core.authority.AuthorityUtils.commaSeparatedStringToAuthorityList; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.securityContext; @@ -34,6 +35,8 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.mitre.oauth2.model.ClientDetailsEntity; +import org.mitre.oauth2.service.ClientDetailsEntityService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.mock.web.MockHttpSession; @@ -45,7 +48,9 @@ import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; +import it.infn.mw.iam.api.client.service.ClientService; import it.infn.mw.iam.persistence.model.IamAup; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; import it.infn.mw.iam.persistence.repository.IamAupRepository; import it.infn.mw.iam.persistence.repository.client.IamClientRepository; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -82,6 +87,34 @@ public class AuthorizationCodeTests { @Autowired private MockMvc mvc; + @Autowired + private ClientService clientService; + + @Autowired + private ClientDetailsEntityService clientDetailsService; + + @Autowired + IamAccountRepository accountRepo; + + @Autowired + private IamClientRepository clientRepo; + + private void removeTestClientOwners() { + + clientService.unlinkClientFromAccount(clientDetailsService.loadClientByClientId(TEST_CLIENT_ID), + accountRepo.findByUsername("test_199").get()); + clientService.unlinkClientFromAccount(clientDetailsService.loadClientByClientId(TEST_CLIENT_ID), + accountRepo.findByUsername("test_200").get()); + } + + private void setTestClientOwners() { + + clientService.linkClientToAccount(clientDetailsService.loadClientByClientId(TEST_CLIENT_ID), + accountRepo.findByUsername("test_199").get()); + clientService.linkClientToAccount(clientDetailsService.loadClientByClientId(TEST_CLIENT_ID), + accountRepo.findByUsername("test_200").get()); + } + @Test public void testOidcAuthorizationCodeFlowExternalHint() throws Exception { @@ -240,4 +273,145 @@ public void testNormalClientNotLinkedToUser() throws Exception { } + @Test + public void testOidcAgentClientNotLinkedToUserWhoNotApproved() throws Exception { + + ClientDetailsEntity entity = clientRepo.findByClientId(TEST_CLIENT_ID).orElseThrow(); + entity.setClientName("oidc-agent:test-client"); + clientRepo.save(entity); + removeTestClientOwners(); + + User testUser = new User(TEST_USER_ID, TEST_USER_PASSWORD, + commaSeparatedStringToAuthorityList("ROLE_USER")); + + MockHttpSession session = (MockHttpSession) mvc + .perform(get(AUTHORIZE_URL).param("response_type", RESPONSE_TYPE_CODE) + .param("client_id", TEST_CLIENT_ID) + .param("redirect_uri", TEST_CLIENT_REDIRECT_URI) + .param("scope", SCOPE) + .param("nonce", "1") + .param("state", "1") + .with(SecurityMockMvcRequestPostProcessors.user(testUser))) + .andExpect(status().isOk()) + .andExpect(forwardedUrl("/oauth/confirm_access")) + .andReturn() + .getRequest() + .getSession(); + + mvc + .perform(post("/authorize").session(session) + .param("user_oauth_approval", "false") + .param("scope_openid", "openid") + .param("scope_profile", "profile") + .param("authorize", "Authorize") + .param("remember", "none") + .with(csrf())) + .andExpect(status().is3xxRedirection()) + .andReturn(); + + mvc.perform(get("/iam/account/me/clients").session(session)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.Resources", is(empty()))); + + entity.setClientName("Test Client"); + clientRepo.save(entity); + setTestClientOwners(); + + } + + @Test + public void testOidcAgentClientNotAlreadyLinkedToUser() throws Exception { + + ClientDetailsEntity entity = clientRepo.findByClientId(TEST_CLIENT_ID).orElseThrow(); + entity.setClientName("oidc-agent:test-client"); + clientRepo.save(entity); + removeTestClientOwners(); + + User testUser = new User(TEST_USER_ID, TEST_USER_PASSWORD, + commaSeparatedStringToAuthorityList("ROLE_USER")); + + MockHttpSession session = (MockHttpSession) mvc + .perform(get(AUTHORIZE_URL).param("response_type", RESPONSE_TYPE_CODE) + .param("client_id", TEST_CLIENT_ID) + .param("redirect_uri", TEST_CLIENT_REDIRECT_URI) + .param("scope", SCOPE) + .param("nonce", "1") + .param("state", "1") + .with(SecurityMockMvcRequestPostProcessors.user(testUser))) + .andExpect(status().isOk()) + .andExpect(forwardedUrl("/oauth/confirm_access")) + .andReturn() + .getRequest() + .getSession(); + + mvc + .perform(post("/authorize").session(session) + .param("user_oauth_approval", "true") + .param("scope_openid", "openid") + .param("scope_profile", "profile") + .param("authorize", "Authorize") + .param("remember", "none") + .with(csrf())) + .andExpect(status().is3xxRedirection()) + .andReturn(); + + mvc.perform(get("/iam/account/me/clients").session(session)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalResults", is(1))) + .andExpect(jsonPath("$.Resources", not(empty()))) + .andExpect(jsonPath("$.Resources[0].client_id", is(TEST_CLIENT_ID))); + + entity.setClientName("Test Client"); + clientRepo.save(entity); + setTestClientOwners(); + + } + + @Test + public void testOidcAgentClientAlreadyLinkedToUser() throws Exception { + + ClientDetailsEntity entity = clientRepo.findByClientId(TEST_CLIENT_ID).orElseThrow(); + entity.setClientName("oidc-agent:test-client"); + clientRepo.save(entity); + + User testUser = new User(TEST_USER_ID, TEST_USER_PASSWORD, + commaSeparatedStringToAuthorityList("ROLE_USER")); + + MockHttpSession session = (MockHttpSession) mvc + .perform(get(AUTHORIZE_URL).param("response_type", RESPONSE_TYPE_CODE) + .param("client_id", TEST_CLIENT_ID) + .param("redirect_uri", TEST_CLIENT_REDIRECT_URI) + .param("scope", SCOPE) + .param("nonce", "1") + .param("state", "1") + .with(SecurityMockMvcRequestPostProcessors.user(testUser))) + .andExpect(status().isOk()) + .andExpect(forwardedUrl("/oauth/confirm_access")) + .andReturn() + .getRequest() + .getSession(); + + mvc + .perform(post("/authorize").session(session) + .param("user_oauth_approval", "true") + .param("scope_openid", "openid") + .param("scope_profile", "profile") + .param("authorize", "Authorize") + .param("remember", "none") + .with(csrf())) + .andExpect(status().is3xxRedirection()) + .andReturn(); + + mvc.perform(get("/iam/account/me/clients").session(session)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.Resources", is(empty()))); + + entity.setClientName("Test Client"); + clientRepo.save(entity); + + } + } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeApprovalTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeApprovalTests.java index 4e9adb959..9ad85c925 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeApprovalTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeApprovalTests.java @@ -20,6 +20,7 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; @@ -556,6 +557,83 @@ public void testNormalClientNotLinkedToUser() throws Exception { } + @Test + public void testOidcAgentClientIsLinkedToUser() throws Exception { + + ClientDetailsEntity entity = clientRepo.findByClientId(DEVICE_CODE_CLIENT_ID).orElseThrow(); + entity.setClientName("oidc-agent:device-code-client"); + clientRepo.save(entity); + + String response = mvc + .perform(post(DEVICE_CODE_ENDPOINT).contentType(APPLICATION_FORM_URLENCODED) + .with(httpBasic(DEVICE_CODE_CLIENT_ID, DEVICE_CODE_CLIENT_SECRET)) + .param("client_id", "device-code-client") + .param("scope", "openid profile offline_access")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.user_code").isString()) + .andExpect(jsonPath("$.device_code").isString()) + .andExpect(jsonPath("$.verification_uri", equalTo(DEVICE_USER_URL))) + .andReturn() + .getResponse() + .getContentAsString(); + + JsonNode responseJson = mapper.readTree(response); + String userCode = responseJson.get("user_code").asText(); + + MockHttpSession session = (MockHttpSession) mvc.perform(get(DEVICE_USER_URL)) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("http://localhost:8080/login")) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc.perform(get("http://localhost:8080/login").session(session)) + .andExpect(status().isOk()) + .andExpect(view().name("iam/login")) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc + .perform(post(LOGIN_URL).param("username", TEST_USERNAME) + .param("password", TEST_PASSWORD) + .param("submit", "Login") + .session(session)) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl(DEVICE_USER_URL)) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc + .perform(post(DEVICE_USER_VERIFY_URL).param("user_code", userCode).session(session)) + .andExpect(status().isOk()) + .andExpect(view().name("iam/approveDevice")) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc + .perform(post(DEVICE_USER_APPROVE_URL).param("user_code", userCode) + .param("user_oauth_approval", "true") + .session(session)) + .andExpect(status().isOk()) + .andExpect(view().name("deviceApproved")) + .andReturn() + .getRequest() + .getSession(); + + mvc.perform(get("/iam/account/me/clients").session(session)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalResults", is(1))) + .andExpect(jsonPath("$.Resources", not(empty()))) + .andExpect(jsonPath("$.Resources[0].client_id", is(DEVICE_CODE_CLIENT_ID))); + + entity.setClientName("Device code client"); + clientRepo.save(entity); + } + @Test public void testRememberParameterAllowsToAddAnApprovedSite() throws Exception {