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 08581ff19..e21ae9605 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 @@ -16,7 +16,6 @@ package it.infn.mw.iam.core.oauth; import static it.infn.mw.iam.core.oauth.IamOauthRequestParameters.REMEMBER_PARAMETER_KEY; - import static java.lang.String.valueOf; import static org.mitre.openid.connect.request.ConnectRequestParameters.APPROVED_SITE; import static org.mitre.openid.connect.request.ConnectRequestParameters.PROMPT; @@ -53,6 +52,10 @@ import com.google.common.base.Strings; import com.google.common.collect.Sets; +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 { @@ -61,14 +64,21 @@ public class IamUserApprovalHandler implements UserApprovalHandler { private final ApprovedSiteService approvedSiteService; private final WhitelistedSiteService whitelistedSiteService; private final SystemScopeService systemScopeService; + private final AccountUtils accountUtils; + private final ClientService clientService; + + public static final String OIDC_AGENT_PREFIX_NAME = "oidc-agent:"; public IamUserApprovalHandler(ClientDetailsEntityService clientDetailsService, ApprovedSiteService approvedSiteService, WhitelistedSiteService whitelistedSiteService, - SystemScopeService systemScopeService) { + SystemScopeService systemScopeService, AccountUtils accountUtils, + ClientService clientService) { this.clientDetailsService = clientDetailsService; this.approvedSiteService = approvedSiteService; this.whitelistedSiteService = whitelistedSiteService; this.systemScopeService = systemScopeService; + this.accountUtils = accountUtils; + this.clientService = clientService; } @Override @@ -176,6 +186,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 ad5175ee8..066bd0cb0 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,8 +48,11 @@ 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; @@ -81,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 { @@ -239,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", "true") + .param("scope.profile", "true") + .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("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 f0f911e99..6bc43533f 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 @@ -21,6 +21,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.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; @@ -557,6 +558,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 {