Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only registered users can get client credentials grant type #683

Merged
merged 6 commits into from
Jan 24, 2024
Merged

Only registered users can get client credentials grant type #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3e84d5d
Only registered users can get client credentials grant type
enricovianello Jan 9, 2024
727ce35
WIP fixing broken unit tests
enricovianello Jan 9, 2024
af92d79
Change variable names
rmiccoli Jan 10, 2024
4fb7c2e
Fix remaining broken unit tests
rmiccoli Jan 10, 2024
d18d6f5
Increase coverage
rmiccoli Jan 22, 2024
9dc383e
Try to test uncovered condition
rmiccoli Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 33 additions & 12 deletions ...java/it/infn/mw/iam/api/client/registration/service/DefaultClientRegistrationService.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading