Skip to content

Commit

Permalink
Only registered users can get client credentials grant type (#683)
Browse files Browse the repository at this point in the history 

* Fix remaining broken unit tests

* Increase coverage

* Try to test uncovered condition

to achieve 100% coverage

---------

Co-authored-by: rmiccoli <roberta.miccoli@cnaf.infn.it>
  • Loading branch information
@enricovianello @rmiccoli
enricovianello and rmiccoli committed Jan 24, 2024
1 parent b0e2870 commit 787cd9c
Show file tree
Hide file tree
Showing 7 changed files with 358 additions and 144 deletions.
45 changes: 33 additions & 12 deletions ...java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<AuthorizationGrantType> PRIVILEGED_ALLOWED_GRANT_TYPES =
private static final EnumSet<AuthorizationGrantType> FORBIDDEN_GRANT_TYPES_FOR_USER =
EnumSet.of(AuthorizationGrantType.PASSWORD, AuthorizationGrantType.TOKEN_EXCHANGE);
private static final EnumSet<AuthorizationGrantType> FORBIDDEN_GRANT_TYPES_FOR_ANONYMOUS =
EnumSet.of(AuthorizationGrantType.PASSWORD, AuthorizationGrantType.TOKEN_EXCHANGE, AuthorizationGrantType.CLIENT_CREDENTIALS);

private final Clock clock;
private final ClientService clientService;
Expand Down Expand Up @@ -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);
}
}

Expand Down
9 changes: 8 additions & 1 deletion ...e/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
185 changes: 108 additions & 77 deletions ...in-service/src/test/java/it/infn/mw/iam/test/api/client/RegistrationAccessTokenTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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());
Expand Down
Loading

0 comments on commit 787cd9c

Please sign in to comment.