From 787cd9cf15ee501940d4bdb6b592c6aaeadafba2 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Wed, 24 Jan 2024 12:23:18 +0100 Subject: [PATCH] Only registered users can get client credentials grant type (#683) * Fix remaining broken unit tests * Increase coverage * Try to test uncovered condition to achieve 100% coverage --------- Co-authored-by: rmiccoli --- .../DefaultClientRegistrationService.java | 45 +++-- ...ClientRegistrationAPIIntegrationTests.java | 9 +- .../client/RegistrationAccessTokenTests.java | 185 ++++++++++-------- .../ClientRegistrationAPIControllerTests.java | 49 ++++- ...ClientRegistrationScopeFilteringTests.java | 9 + .../ClientRegistrationTests.java | 47 +++-- .../ClientRegistrationServiceTests.java | 158 ++++++++++++--- 7 files changed, 358 insertions(+), 144 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java index adb68cfa6..09bc73fb8 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java @@ -81,8 +81,10 @@ public class DefaultClientRegistrationService implements ClientRegistrationServi public static final String GRANT_TYPE_NOT_ALLOWED_ERROR_STR = "Grant type not allowed: %s"; - private static final EnumSet PRIVILEGED_ALLOWED_GRANT_TYPES = + private static final EnumSet FORBIDDEN_GRANT_TYPES_FOR_USER = EnumSet.of(AuthorizationGrantType.PASSWORD, AuthorizationGrantType.TOKEN_EXCHANGE); + private static final EnumSet FORBIDDEN_GRANT_TYPES_FOR_ANONYMOUS = + EnumSet.of(AuthorizationGrantType.PASSWORD, AuthorizationGrantType.TOKEN_EXCHANGE, AuthorizationGrantType.CLIENT_CREDENTIALS); private final Clock clock; private final ClientService clientService; @@ -136,25 +138,44 @@ protected boolean isAnonymous(Authentication authentication) { private void checkAllowedGrantTypes(RegisteredClientDTO request, Authentication authentication) { - if (!accountUtils.isAdmin(authentication)) { + if (accountUtils.isAdmin(authentication)) { + return; + } + if (accountUtils.isRegisteredUser(authentication)) { request.getGrantTypes() - .stream() - .filter(PRIVILEGED_ALLOWED_GRANT_TYPES::contains) - .findFirst() - .ifPresent(this::throwGrantTypeNotAllowed); + .stream() + .filter(FORBIDDEN_GRANT_TYPES_FOR_USER::contains) + .findFirst() + .ifPresent(this::throwGrantTypeNotAllowed); + } else { + request.getGrantTypes() + .stream() + .filter(FORBIDDEN_GRANT_TYPES_FOR_ANONYMOUS::contains) + .findFirst() + .ifPresent(this::throwGrantTypeNotAllowed); } } private void checkAllowedGrantTypesOnUpdate(RegisteredClientDTO request, Authentication authentication, ClientDetailsEntity oldClient) { - if (!accountUtils.isAdmin(authentication)) { + if (accountUtils.isAdmin(authentication)) { + return; + } + if (accountUtils.isRegisteredUser(authentication)) { request.getGrantTypes() - .stream() - .filter(s -> !oldClient.getGrantTypes().contains(s.getGrantType())) - .filter(PRIVILEGED_ALLOWED_GRANT_TYPES::contains) - .findFirst() - .ifPresent(this::throwGrantTypeNotAllowed); + .stream() + .filter(s -> !oldClient.getGrantTypes().contains(s.getGrantType())) + .filter(FORBIDDEN_GRANT_TYPES_FOR_USER::contains) + .findFirst() + .ifPresent(this::throwGrantTypeNotAllowed); + } else { + request.getGrantTypes() + .stream() + .filter(s -> !oldClient.getGrantTypes().contains(s.getGrantType())) + .filter(FORBIDDEN_GRANT_TYPES_FOR_ANONYMOUS::contains) + .findFirst() + .ifPresent(this::throwGrantTypeNotAllowed); } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java index a2a3472ac..71aac3858 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java @@ -53,7 +53,7 @@ public class ClientRegistrationAPIIntegrationTests extends TestSupport { @WithAnonymousUser public void dynamicRegistrationWorksForAnonymousUser() throws Exception { - String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); + String clientJson = ClientJsonStringBuilder.builder().scopes("openid").grantTypes("authorization_code").build(); mvc .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) @@ -67,6 +67,13 @@ public void dynamicRegistrationWorksForAnonymousUser() throws Exception { .andExpect(jsonPath("$.dynamically_registered").value(true)) .andExpect(jsonPath("$.registration_access_token").exists()); + clientJson = ClientJsonStringBuilder.builder().scopes("openid").grantTypes("client_credentials").build(); + + mvc + .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(BAD_REQUEST); + } @Test diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/RegistrationAccessTokenTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/RegistrationAccessTokenTests.java index b3a447048..331a82193 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/RegistrationAccessTokenTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/RegistrationAccessTokenTests.java @@ -76,8 +76,9 @@ public void setup() { @Test public void testRatWorkAsExpected() throws ParseException { - String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); - + String clientJson = + ClientJsonStringBuilder.builder().scopes("openid").grantTypes("authorization_code").build(); + // @formatter:off RegisteredClientDTO registerResponse = RestAssured .given() @@ -96,47 +97,62 @@ public void testRatWorkAsExpected() throws ParseException { assertThat(jwt.getJWTClaimsSet().getExpirationTime(), nullValue()); assertThat(registerResponse.getScope(), not(empty())); - RegisteredClientDTO getResponse = - RestAssured.given() + RegisteredClientDTO getResponse = RestAssured.given() .auth() - .oauth2(registerResponse.getRegistrationAccessToken()) + .oauth2(registerResponse.getRegistrationAccessToken()) .when() - .get(registerUrl + "/" + registerResponse.getClientId()) + .get(registerUrl + "/" + registerResponse.getClientId()) .then() .statusCode(HttpStatus.OK.value()) .log() - .all() - .extract() - .body() - .as(RegisteredClientDTO.class); + .all() + .extract() + .body() + .as(RegisteredClientDTO.class); assertThat(getResponse.getClientSecret(), is(registerResponse.getClientSecret())); assertThat(getResponse.getRegistrationAccessToken(), nullValue()); - + RegisteredClientDTO rotatedRatClient = - managementService.rotateRegistrationAccessToken(getResponse.getClientId()); + managementService.rotateRegistrationAccessToken(getResponse.getClientId()); assertThat(rotatedRatClient.getRegistrationAccessToken(), notNullValue()); - + try { - - RestAssured.given() - .auth() - .oauth2(registerResponse.getRegistrationAccessToken()) - .when() - .get(registerUrl + "/" + registerResponse.getClientId()) - .then() - .statusCode(HttpStatus.UNAUTHORIZED.value()) - .log() - .ifError(); - RestAssured.given() - .auth() - .oauth2(rotatedRatClient.getRegistrationAccessToken()) + RestAssured.given() + .auth() + .oauth2(registerResponse.getRegistrationAccessToken()) + .when() + .get(registerUrl + "/" + registerResponse.getClientId()) + .then() + .statusCode(HttpStatus.UNAUTHORIZED.value()) + .log() + .ifError(); + + RestAssured.given() + .auth() + .oauth2(rotatedRatClient.getRegistrationAccessToken()) + .when() + .get(registerUrl + "/" + registerResponse.getClientId()) + .then() + .statusCode(HttpStatus.OK.value()); + + clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .grantTypes("client_credentials") + .build(); + + // @formatter:off + RestAssured + .given() + .body(clientJson) + .contentType(APPLICATION_JSON_VALUE) .when() - .get(registerUrl + "/" + registerResponse.getClientId()) + .post(registerUrl) .then() - .statusCode(HttpStatus.OK.value()); + .log().all() + .statusCode(HttpStatus.BAD_REQUEST.value()); } finally { @@ -149,7 +165,7 @@ public void testRatWorkAsExpected() throws ParseException { public void testRedeemClientFlow() { // 1. Register a client - String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); + String clientJson = ClientJsonStringBuilder.builder().scopes("openid").grantTypes("authorization_code").build(); // @formatter:off RegisteredClientDTO registerResponse = RestAssured @@ -164,60 +180,75 @@ public void testRedeemClientFlow() { .extract().body().as(RegisteredClientDTO.class); // @formatter:on try { - assertThat(registerResponse.getRegistrationAccessToken(), notNullValue()); - assertThat(registerResponse.getScope(), not(empty())); + assertThat(registerResponse.getRegistrationAccessToken(), notNullValue()); + assertThat(registerResponse.getScope(), not(empty())); - // 2. Get an access token for the 'test' account - String testAt = - TestUtils.passwordTokenGetter() - .username("test") - .password("password") - .port(iamPort) - .getAccessToken(); + // 2. Get an access token for the 'test' account + String testAt = TestUtils.passwordTokenGetter() + .username("test") + .password("password") + .port(iamPort) + .getAccessToken(); - // 3. Check that test account doesn't own any client - RestAssured.given() - .auth() - .oauth2(testAt) - .log() - .all() - .when() - .get(ownedClientsUrl) - .then() - .log() - .all() - .statusCode(HttpStatus.OK.value()) - .body("totalResults", equalTo(0)); + // 3. Check that test account doesn't own any client + RestAssured.given() + .auth() + .oauth2(testAt) + .log() + .all() + .when() + .get(ownedClientsUrl) + .then() + .log() + .all() + .statusCode(HttpStatus.OK.value()) + .body("totalResults", equalTo(0)); - // 4. Redeem just-registered client - RestAssured.given() - .contentType(MediaType.APPLICATION_JSON_VALUE) - .body(registerResponse.getRegistrationAccessToken()) - .auth() - .oauth2(testAt) - .log() - .all() - .when() - .post(registerUrl + "/" + registerResponse.getClientId() + "/redeem") - .then() - .log() - .all() - .statusCode(HttpStatus.OK.value()); + // 4. Redeem just-registered client + RestAssured.given() + .contentType(MediaType.APPLICATION_JSON_VALUE) + .body(registerResponse.getRegistrationAccessToken()) + .auth() + .oauth2(testAt) + .log() + .all() + .when() + .post(registerUrl + "/" + registerResponse.getClientId() + "/redeem") + .then() + .log() + .all() + .statusCode(HttpStatus.OK.value()); - // 5. Test account own registered client - RestAssured.given() - .auth() - .oauth2(testAt) - .log() - .all() + // 5. Test account own registered client + RestAssured.given() + .auth() + .oauth2(testAt) + .log() + .all() + .when() + .get(ownedClientsUrl) + .then() + .log() + .all() + .statusCode(HttpStatus.OK.value()) + .body("totalResults", equalTo(1)) + .body("Resources[0].client_id", equalTo(registerResponse.getClientId())); + + clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .grantTypes("client_credentials") + .build(); + + // @formatter:off + RestAssured + .given() + .body(clientJson) + .contentType(APPLICATION_JSON_VALUE) .when() - .get(ownedClientsUrl) + .post(registerUrl) .then() - .log() - .all() - .statusCode(HttpStatus.OK.value()) - .body("totalResults", equalTo(1)) - .body("Resources[0].client_id", equalTo(registerResponse.getClientId())); + .log().all() + .statusCode(HttpStatus.BAD_REQUEST.value()); } finally { managementService.deleteClientByClientId(registerResponse.getClientId()); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/client/registration/ClientRegistrationAPIControllerTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/client/registration/ClientRegistrationAPIControllerTests.java index 0fd8c5893..eb8ed59cd 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/client/registration/ClientRegistrationAPIControllerTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/client/registration/ClientRegistrationAPIControllerTests.java @@ -87,7 +87,7 @@ public void registerClientWithNullValuesAndCheckDefaultValues() RegisteredClientDTO client = new RegisteredClientDTO(); client.setClientName("test-client-creation"); - client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); client.setScope(Sets.newHashSet("test")); client.setAccessTokenValiditySeconds(null); client.setRefreshTokenValiditySeconds(null); @@ -102,7 +102,7 @@ public void registerClientWithNullValuesAndCheckDefaultValues() .andExpect(jsonPath("$.client_name", is("test-client-creation"))) .andExpect(jsonPath("$.scope", is("test"))) .andExpect(jsonPath("$.grant_types").isArray()) - .andExpect(jsonPath("$.grant_types", hasItem("client_credentials"))) + .andExpect(jsonPath("$.grant_types", hasItem("urn:ietf:params:oauth:grant-type:device_code"))) .andExpect(jsonPath("$.token_endpoint_auth_method", is("client_secret_basic"))) .andExpect(jsonPath("$.dynamically_registered", is(true))) .andExpect(jsonPath("$.registration_access_token").exists()) @@ -115,10 +115,24 @@ public void registerClientWithNullValuesAndCheckDefaultValues() .getResponse() .getContentAsString(), RegisteredClientDTO.class); - ClientDetailsEntity createdClient = clientRepository.findByClientId(response.getClientId()).get(); + ClientDetailsEntity createdClient = + clientRepository.findByClientId(response.getClientId()).get(); assertEquals(3600, createdClient.getAccessTokenValiditySeconds()); assertEquals(2592000, createdClient.getRefreshTokenValiditySeconds()); + + client = new RegisteredClientDTO(); + client.setClientName("test-client-creation"); + client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + client.setScope(Sets.newHashSet("test")); + client.setAccessTokenValiditySeconds(null); + client.setRefreshTokenValiditySeconds(null); + client.setTokenEndpointAuthMethod(null); + + mvc + .perform(post(IAM_CLIENT_REGISTRATION_API_URL).contentType(APPLICATION_JSON) + .content(mapper.writeValueAsString(client))) + .andExpect(BAD_REQUEST); } @Test @@ -179,7 +193,7 @@ public void registerClientPrivateJwtValidationException() RegisteredClientDTO client = new RegisteredClientDTO(); client.setClientName("test-client-creation"); - client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); client.setScope(Sets.newHashSet("test")); client.setTokenEndpointAuthMethod(TokenEndpointAuthenticationMethod.private_key_jwt); @@ -204,6 +218,31 @@ public void registerClientPrivateJwtValidationException() mvc.perform(delete(IAM_CLIENT_REGISTRATION_API_URL + client.getClientId())); } + client = new RegisteredClientDTO(); + client.setClientName("test-client-creation"); + client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + client.setScope(Sets.newHashSet("test")); + client.setTokenEndpointAuthMethod(TokenEndpointAuthenticationMethod.private_key_jwt); + + expectedMessage = "registerClient.request: private_key_jwt requires a jwks uri or a jwk value"; + + mvc + .perform(post(IAM_CLIENT_REGISTRATION_API_URL).contentType(APPLICATION_JSON) + .content(mapper.writeValueAsString(client))) + .andExpect(BAD_REQUEST) + .andExpect(jsonPath("$.error", is(expectedMessage))); + + client.setJwk(NOT_A_JSON_STRING); + client.setJwksUri(URI_STRING); + + expectedMessage = "Grant type not allowed: client_credentials"; + + mvc + .perform(post(IAM_CLIENT_REGISTRATION_API_URL).contentType(APPLICATION_JSON) + .content(mapper.writeValueAsString(client))) + .andExpect(BAD_REQUEST) + .andExpect(jsonPath("$.error", is(expectedMessage))); + } @Test @@ -213,7 +252,7 @@ public void updateClientPrivateJwtValidationException() RegisteredClientDTO client = new RegisteredClientDTO(); client.setClientName("test-client-creation"); - client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + client.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); client.setScope(Sets.newHashSet("test")); mvc diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationScopeFilteringTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationScopeFilteringTests.java index f25f4eaa0..d409d6f70 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationScopeFilteringTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationScopeFilteringTests.java @@ -56,12 +56,21 @@ public void testClientRegistrationScopesAreFiltered() throws Exception { String jsonInString = ClientJsonStringBuilder.builder() .scopes("example", "storage.read:/", "storage.read:/sub/path") + .grantTypes("authorization_code") .build(); mvc.perform(post(REGISTER_ENDPOINT).contentType(APPLICATION_JSON).content(jsonInString)) .andExpect(status().isCreated()) .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(jsonPath("$.scope", is("example"))); + + jsonInString = ClientJsonStringBuilder.builder() + .scopes("example", "storage.read:/", "storage.read:/sub/path") + .grantTypes("client_credentials") + .build(); + + mvc.perform(post(REGISTER_ENDPOINT).contentType(APPLICATION_JSON).content(jsonInString)) + .andExpect(status().isBadRequest()); } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java index e7f624966..ab1bbf9e2 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java @@ -89,7 +89,8 @@ public void testClientRegistrationWorksForLegacyEndpoint() throws Exception { @Test public void testClientRegistrationAccessTokenWorks() throws Exception { - String jsonInString = ClientJsonStringBuilder.builder().scopes("test").build(); + String jsonInString = + ClientJsonStringBuilder.builder().scopes("test").grantTypes("authorization_code").build(); // @formatter:off String response = @@ -112,15 +113,15 @@ public void testClientRegistrationAccessTokenWorks() throws Exception { assertThat(rat, notNullValue()); assertThat(registrationUri, notNullValue()); - mvc.perform(get(registrationUri) - .contentType(APPLICATION_JSON) + mvc + .perform(get(registrationUri).contentType(APPLICATION_JSON) .header("Authorization", "Bearer " + rat)) .andExpect(status().isOk()) .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(jsonPath("$.client_id", is(clientId))); - - mvc.perform(delete(registrationUri) - .contentType(APPLICATION_JSON) + + mvc + .perform(delete(registrationUri).contentType(APPLICATION_JSON) .header("Authorization", "Bearer " + rat)) .andExpect(status().isNoContent()); @@ -128,6 +129,15 @@ public void testClientRegistrationAccessTokenWorks() throws Exception { .perform(get(registrationUri).contentType(APPLICATION_JSON) .header("Authorization", "Bearer " + rat)) .andExpect(status().isUnauthorized()); + + jsonInString = + ClientJsonStringBuilder.builder().scopes("test").grantTypes("client_credentilas").build(); + + // @formatter:off + mvc.perform(post(REGISTER_ENDPOINT) + .contentType(APPLICATION_JSON) + .content(jsonInString)) + .andExpect(status().isBadRequest()); } @Test @@ -136,8 +146,7 @@ public void testCreateClientWithRegistrationReservedScopes() throws Exception { String[] scopes = {"registration:read", "registration:write", "scim:read", "scim:write", "proxy:generate"}; - String jsonInString = ClientJsonStringBuilder.builder().scopes(scopes).build(); - + String jsonInString = ClientJsonStringBuilder.builder().scopes(scopes).grantTypes("authorization_code").build(); // @formatter:off String response = @@ -164,7 +173,8 @@ public void testGetTokenWithScimReservedScopesFailure() throws Exception { String[] scopes = {"scim:read", "scim:write", "registration:read", "registration:write"}; - String jsonInString = ClientJsonStringBuilder.builder().scopes(scopes).build(); + String jsonInString = + ClientJsonStringBuilder.builder().scopes(scopes).grantTypes("authorization_code").build(); // @formatter:off String response = @@ -195,11 +205,10 @@ public void testGetTokenWithScimReservedScopesFailure() throws Exception { @Test public void passwordGrantTypeNotAllowedWhenRegisteringNewClient() throws Exception { - String jsonInString = - ClientJsonStringBuilder.builder() - .grantTypes("authorization_code", "password") - .scopes("openid") - .build(); + String jsonInString = ClientJsonStringBuilder.builder() + .grantTypes("authorization_code", "password") + .scopes("openid") + .build(); mvc.perform(post(REGISTER_ENDPOINT).contentType(APPLICATION_JSON).content(jsonInString)) .andExpect(status().isBadRequest()) @@ -255,8 +264,8 @@ public void additionalGrantTypesAreNotLostWhenUpdatingClient() throws Exception .header("Authorization", "Bearer " + rat)) .andExpect(status().isOk()) .andExpect(content().contentType(APPLICATION_JSON)) - .andExpect( - jsonPath("$.grant_types", hasItems("password", TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE))) + .andExpect(jsonPath("$.grant_types", + hasItems("password", TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE))) .andReturn() .getResponse() .getContentAsString(); @@ -267,7 +276,8 @@ public void additionalGrantTypesAreNotLostWhenUpdatingClient() throws Exception .content(clientJson)) .andExpect(status().isOk()) .andExpect(jsonPath("$.grant_types", hasItem("password"))) - .andExpect(jsonPath("$.grant_types", hasItem(TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE))); + .andExpect( + jsonPath("$.grant_types", hasItem(TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE))); clientModel = clientService.loadClientByClientId(clientId); @@ -281,7 +291,8 @@ public void additionalGrantTypesAreNotLostWhenUpdatingClient() throws Exception .andExpect(status().isOk()) .andExpect(content().contentType(APPLICATION_JSON)) .andExpect(jsonPath("$.grant_types", not(hasItem("password")))) - .andExpect(jsonPath("$.grant_types", not(hasItem(TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE)))) + .andExpect(jsonPath("$.grant_types", + not(hasItem(TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE)))) .andReturn() .getResponse() .getContentAsString(); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java index 2b1854964..89df29857 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java @@ -167,7 +167,7 @@ public void beforeEach() { doReturn(Optional.of(test100Account)).when(accountUtils) .getAuthenticatedUserAccount(anotherUserAuth); doReturn(Optional.of(adminAccount)).when(accountUtils).getAuthenticatedUserAccount(adminAuth); - + ratAuth = Mockito.mock(OAuth2Authentication.class); oauthRequest = Mockito.mock(OAuth2Request.class); oauthDetails = Mockito.mock(OAuth2AuthenticationDetails.class); @@ -304,6 +304,7 @@ public void testBlacklistedUriValidation() { @Test public void testAllowedGrantTypeChecks() throws ParseException { + // ask exchange grant type as user InvalidClientRegistrationRequest exception = Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { RegisteredClientDTO request = new RegisteredClientDTO(); @@ -317,6 +318,21 @@ public void testAllowedGrantTypeChecks() throws ParseException { assertThat(exception.getMessage(), containsString( "Grant type not allowed: " + AuthorizationGrantType.TOKEN_EXCHANGE.getGrantType())); + // ask exchange grant type as anonymous + exception = + Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { + RegisteredClientDTO request = new RegisteredClientDTO(); + request.setClientName("example"); + request.setGrantTypes( + Sets.newHashSet(AuthorizationGrantType.CODE, AuthorizationGrantType.TOKEN_EXCHANGE)); + request.setRedirectUris(Sets.newHashSet("https://example/cb")); + service.registerClient(request, noAuth); + }); + + assertThat(exception.getMessage(), containsString( + "Grant type not allowed: " + AuthorizationGrantType.TOKEN_EXCHANGE.getGrantType())); + + // ask password grant type as user exception = Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); @@ -329,20 +345,68 @@ public void testAllowedGrantTypeChecks() throws ParseException { assertThat(exception.getMessage(), containsString( "Grant type not allowed: " + AuthorizationGrantType.PASSWORD.getGrantType())); + // ask password grant type as anonymous + exception = Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { + RegisteredClientDTO request = new RegisteredClientDTO(); + request.setClientName("example"); + request.setGrantTypes( + Sets.newHashSet(AuthorizationGrantType.CODE, AuthorizationGrantType.PASSWORD)); + request.setRedirectUris(Sets.newHashSet("https://example/cb")); + service.registerClient(request, noAuth); + }); + + assertThat(exception.getMessage(), containsString( + "Grant type not allowed: " + AuthorizationGrantType.PASSWORD.getGrantType())); + + // ask client credentials grant type as anonymous + exception = Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { + RegisteredClientDTO request = new RegisteredClientDTO(); + request.setClientName("example"); + request.setGrantTypes( + Sets.newHashSet(AuthorizationGrantType.CODE, AuthorizationGrantType.CLIENT_CREDENTIALS)); + request.setRedirectUris(Sets.newHashSet("https://example/cb")); + service.registerClient(request, noAuth); + }); + + assertThat(exception.getMessage(), containsString( + "Grant type not allowed: " + AuthorizationGrantType.CLIENT_CREDENTIALS.getGrantType())); + // ask client credentials grant type as user RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); - request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CODE, - AuthorizationGrantType.PASSWORD, AuthorizationGrantType.TOKEN_EXCHANGE)); + request.setGrantTypes( + Sets.newHashSet(AuthorizationGrantType.CODE, AuthorizationGrantType.CLIENT_CREDENTIALS)); request.setRedirectUris(Sets.newHashSet("https://example/cb")); + RegisteredClientDTO response = service.registerClient(request, userAuth); - RegisteredClientDTO response = service.registerClient(request, adminAuth); + assertThat(exception.getMessage(), containsString( + "Grant type not allowed: " + AuthorizationGrantType.CLIENT_CREDENTIALS.getGrantType())); assertThat(response.getClientName(), is("example")); assertThat(response.getClientId(), notNullValue()); assertThat(response.getTokenEndpointAuthMethod(), is(TokenEndpointAuthenticationMethod.client_secret_basic)); - assertThat(response.getGrantTypes(), hasItems(AuthorizationGrantType.CODE, - AuthorizationGrantType.TOKEN_EXCHANGE, AuthorizationGrantType.PASSWORD)); + assertThat(response.getGrantTypes(), + hasItems(AuthorizationGrantType.CODE, AuthorizationGrantType.CLIENT_CREDENTIALS)); + + assertThat(response.getRegistrationAccessToken(), nullValue()); + assertThat(response.getClientSecret(), notNullValue()); + + // ask password, client credentials and exchange grant types as admin + request = new RegisteredClientDTO(); + request.setClientName("example"); + request + .setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CODE, AuthorizationGrantType.PASSWORD, + AuthorizationGrantType.TOKEN_EXCHANGE, AuthorizationGrantType.CLIENT_CREDENTIALS)); + request.setRedirectUris(Sets.newHashSet("https://example/cb")); + + response = service.registerClient(request, adminAuth); + assertThat(response.getClientName(), is("example")); + assertThat(response.getClientId(), notNullValue()); + assertThat(response.getTokenEndpointAuthMethod(), + is(TokenEndpointAuthenticationMethod.client_secret_basic)); + assertThat(response.getGrantTypes(), + hasItems(AuthorizationGrantType.CODE, AuthorizationGrantType.TOKEN_EXCHANGE, + AuthorizationGrantType.PASSWORD, AuthorizationGrantType.CLIENT_CREDENTIALS)); assertThat(response.getRegistrationAccessToken(), nullValue()); assertThat(response.getClientSecret(), notNullValue()); @@ -388,8 +452,7 @@ public void testRestrictedScopesAreFilteredOut() { } @Test - public void testRestrictedScopesAreFilteredOutWithMatchers() - throws ParseException { + public void testRestrictedScopesAreFilteredOutWithMatchers() throws ParseException { String restrictedScope1 = "storage.read:/whatever"; String restrictedScope2 = "storage.read:/"; @@ -476,19 +539,18 @@ public void testAdminCanRegisterClientWithRestrictedScope() { } @Test - public void testAnonymousRequestYeldsRegistrationAccessToken() - throws ParseException { + public void testAnonymousRequestYeldsRegistrationAccessToken() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); - request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); RegisteredClientDTO response = service.registerClient(request, noAuth); assertThat(response.getClientName(), is("example")); assertThat(response.getClientId(), notNullValue()); assertThat(response.getTokenEndpointAuthMethod(), is(TokenEndpointAuthenticationMethod.client_secret_basic)); - assertThat(response.getGrantTypes(), hasItem(AuthorizationGrantType.CLIENT_CREDENTIALS)); + assertThat(response.getGrantTypes(), hasItem(AuthorizationGrantType.DEVICE_CODE)); assertThat(response.getRegistrationAccessToken(), notNullValue()); assertThat(response.getRegistrationClientUri(), is("http://localhost:8080/iam/api/client-registration/" + response.getClientId())); @@ -596,8 +658,7 @@ public void testAuthzComesBeforeLookupForRetrieveClient() { } @Test - public void testRegisterAndRetrieveWorksForUser() - throws ParseException { + public void testRegisterAndRetrieveWorksForUser() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); @@ -621,12 +682,11 @@ public void testRegisterAndRetrieveWorksForUser() @Test - public void testRegisterAndRetrieveWorksForAnonymousUser() - throws ParseException { + public void testRegisterAndRetrieveWorksForAnonymousUser() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); - request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); RegisteredClientDTO registerResponse = service.registerClient(request, noAuth); assertThat(registerResponse.getRegistrationAccessToken(), notNullValue()); @@ -640,7 +700,7 @@ public void testRegisterAndRetrieveWorksForAnonymousUser() assertThat(response.getClientId(), is(registerResponse.getClientId())); assertThat(response.getTokenEndpointAuthMethod(), is(TokenEndpointAuthenticationMethod.client_secret_basic)); - assertThat(response.getGrantTypes(), hasItem(AuthorizationGrantType.CLIENT_CREDENTIALS)); + assertThat(response.getGrantTypes(), hasItem(AuthorizationGrantType.DEVICE_CODE)); assertThat(response.getClientSecret(), notNullValue()); assertThat(response.getRegistrationClientUri(), is("http://localhost:8080/iam/api/client-registration/" + response.getClientId())); @@ -649,12 +709,11 @@ public void testRegisterAndRetrieveWorksForAnonymousUser() @Test - public void testRatClientIdAndScopesAreChecked() - throws ParseException { + public void testRatClientIdAndScopesAreChecked() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); - request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.DEVICE_CODE)); RegisteredClientDTO registerResponse = service.registerClient(request, noAuth); assertThat(registerResponse.getRegistrationAccessToken(), notNullValue()); @@ -707,8 +766,7 @@ public void testSuccesfullDelete() throws ParseException { } @Test - public void testAccountAuthzForClientManagement() - throws ParseException { + public void testAccountAuthzForClientManagement() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); @@ -728,8 +786,7 @@ public void testAccountAuthzForClientManagement() } @Test - public void testGranTypesAreCheckedOnUpdate() - throws ParseException { + public void testGranTypesAreCheckedOnUpdate() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); @@ -746,11 +803,26 @@ public void testGranTypesAreCheckedOnUpdate() }); assertThat(exception.getMessage(), containsString("Grant type not allowed")); + + // update client grant types as admin + + RegisteredClientDTO reqClient = new RegisteredClientDTO(); + reqClient.setClientName("example2"); + reqClient.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + RegisteredClientDTO respClient = service.registerClient(reqClient, adminAuth); + + RegisteredClientDTO updateReq = respClient; + updateReq.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS, + AuthorizationGrantType.TOKEN_EXCHANGE)); + RegisteredClientDTO updateResponse = service.updateClient(respClient.getClientId(), updateReq, adminAuth); + + assertThat(updateResponse.getGrantTypes(), hasItems(AuthorizationGrantType.CLIENT_CREDENTIALS, + AuthorizationGrantType.TOKEN_EXCHANGE)); + } @Test - public void testRedirectUrisAreCheckedOnUpdate() - throws ParseException { + public void testRedirectUrisAreCheckedOnUpdate() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); @@ -808,8 +880,7 @@ public void testRatIsUpdated() throws ParseException { } @Test - public void testPrivilegedGrantTypesArePreservedOnUpdate() - throws ParseException { + public void testPrivilegedGrantTypesArePreservedOnUpdate() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); @@ -841,8 +912,33 @@ public void testPrivilegedGrantTypesArePreservedOnUpdate() } @Test - public void testRestrictedScopesArePreserved() - throws ParseException { + public void testPrivilegedGrantTypesAreCheckedOnUpdateForAnonymousUser() throws ParseException { + + RegisteredClientDTO request = new RegisteredClientDTO(); + request.setClientName("example"); + request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CODE)); + request.setRedirectUris(Sets.newHashSet("https://test.example/cb")); + + RegisteredClientDTO response = service.registerClient(request, noAuth); + + when(oauthDetails.getTokenValue()).thenReturn(response.getRegistrationAccessToken()); + when(oauthRequest.getClientId()).thenReturn(response.getClientId()); + when(oauthRequest.getScope()) + .thenReturn(Sets.newHashSet(SystemScopeService.REGISTRATION_TOKEN_SCOPE)); + + InvalidClientRegistrationRequest exception = + Assertions.assertThrows(InvalidClientRegistrationRequest.class, () -> { + RegisteredClientDTO updateRequest = response; + updateRequest.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); + service.updateClient(response.getClientId(), updateRequest, ratAuth); + + }); + + assertThat(exception.getMessage(), containsString("Grant type not allowed")); + } + + @Test + public void testRestrictedScopesArePreserved() throws ParseException { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("restricted-scopes-preserved");