From c3b028f12860b8a62269324aea5b8842e489bdf1 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Tue, 29 Oct 2024 17:54:32 +0100 Subject: [PATCH 1/2] Check if oidc-agent client is already linked --- .../core/oauth/IamUserApprovalHandler.java | 3 +- .../authzcode/AuthorizationCodeTests.java | 81 ++++++++++++++++++- 2 files changed, 81 insertions(+), 3 deletions(-) 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 ad47ad70e..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 @@ -189,7 +189,8 @@ public AuthorizationRequest updateAfterApproval(AuthorizationRequest authorizati IamAccount account = accountUtils.getAuthenticatedUserAccount(userAuthentication).orElseThrow(); - if (client.getClientName().startsWith(OIDC_AGENT_PREFIX_NAME)) { + if (client.getClientName().startsWith(OIDC_AGENT_PREFIX_NAME) + && clientService.findClientOwners(clientId, null).isEmpty()) { clientService.linkClientToAccount(client, account); } 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 35aeff342..419824000 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 @@ -36,6 +36,7 @@ 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; @@ -47,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; @@ -87,6 +90,31 @@ public class AuthorizationCodeTests { @Autowired private MockMvc mvc; + @Autowired + private ClientService clientService; + + @Autowired + private ClientDetailsEntityService clientDetailsService; + + @Autowired + IamAccountRepository accountRepo; + + 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 { @@ -251,6 +279,7 @@ 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")); @@ -287,15 +316,17 @@ public void testOidcAgentClientNotLinkedToUserWhoNotApproved() throws Exception entity.setClientName("Test Client"); clientRepo.save(entity); + setTestClientOwners(); } - + @Test - public void testOidcAgentClientIsLinkedToUser() throws Exception { + 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")); @@ -334,6 +365,52 @@ public void testOidcAgentClientIsLinkedToUser() throws Exception { 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); } From bac35105c479bd58e4663e9575a184e46c7b4013 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Tue, 29 Oct 2024 18:36:43 +0100 Subject: [PATCH 2/2] Small fixes --- .../core/oauth/IamUserApprovalHandler.java | 5 +- .../authzcode/AuthorizationCodeTests.java | 4 + .../devicecode/DeviceCodeApprovalTests.java | 78 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) 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 8334fc1ec..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; @@ -190,7 +193,7 @@ public AuthorizationRequest updateAfterApproval(AuthorizationRequest authorizati && 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 0a8f0ad7e..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; @@ -95,6 +96,9 @@ public class AuthorizationCodeTests { @Autowired IamAccountRepository accountRepo; + @Autowired + private IamClientRepository clientRepo; + private void removeTestClientOwners() { clientService.unlinkClientFromAccount(clientDetailsService.loadClientByClientId(TEST_CLIENT_ID), 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 {