From 085dd248752a655cd13c8b90432b5344a0b1a027 Mon Sep 17 00:00:00 2001 From: cgeorgilakis-grnet Date: Mon, 21 Nov 2022 16:53:04 +0200 Subject: [PATCH] Client registration service do not check client protocol for Bearer token Closes #15612 --- .../ClientRegistrationAuth.java | 1 - .../client/OIDCClientRegistrationTest.java | 26 -------- .../client/SAMLClientRegistrationTest.java | 60 ++++++++----------- 3 files changed, 26 insertions(+), 61 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java index 32e8467c2e..269287e0ae 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationAuth.java @@ -134,7 +134,6 @@ public class ClientRegistrationAuth { RegistrationAuth registrationAuth = RegistrationAuth.ANONYMOUS; if (isBearerToken()) { - checkClientProtocol(); if (hasRole(AdminRoles.MANAGE_CLIENTS, AdminRoles.CREATE_CLIENT)) { registrationAuth = RegistrationAuth.AUTHENTICATED; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java index 90250e164a..fe05e5c1df 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java @@ -586,32 +586,6 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { } } - @Test - public void testOIDCEndpointCreateWithSamlClient() throws Exception { - ClientsResource clientsResource = adminClient.realm(TEST).clients(); - ClientRepresentation samlClient = clientsResource.findByClientId("saml-client").get(0); - String samlClientServiceId = clientsResource.get(samlClient.getId()).getServiceAccountUser().getId(); - - String realmManagementId = clientsResource.findByClientId("realm-management").get(0).getId(); - RoleRepresentation role = clientsResource.get(realmManagementId).roles().get("create-client").toRepresentation(); - - adminClient.realm(TEST).users().get(samlClientServiceId).roles().clientLevel(realmManagementId).add(Arrays.asList(role)); - - String accessToken = oauth.clientId("saml-client").doClientCredentialsGrantAccessTokenRequest("secret").getAccessToken(); - reg.auth(Auth.token(accessToken)); - - // change client to saml - samlClient.setProtocol("saml"); - clientsResource.get(samlClient.getId()).update(samlClient); - - OIDCClientRepresentation client = createRep(); - assertCreateFail(client, 400, Errors.INVALID_CLIENT); - - // revert client - samlClient.setProtocol("openid-connect"); - clientsResource.get(samlClient.getId()).update(samlClient); - } - @Test public void testOIDCEndpointGetWithSamlClient() throws Exception { OIDCClientRepresentation response = create(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java index 449cea823a..3af8164fcc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java @@ -74,28 +74,7 @@ public class SAMLClientRegistrationTest extends AbstractClientRegistrationTest { @Test public void createClient() throws ClientRegistrationException, IOException { String entityDescriptor = IOUtils.toString(getClass().getResourceAsStream("/clientreg-test/saml-entity-descriptor.xml")); - ClientRepresentation response = reg.saml().create(entityDescriptor); - - assertThat(response.getRegistrationAccessToken(), notNullValue()); - assertThat(response.getClientId(), is("loadbalancer-9.siroe.com")); - assertThat(response.getRedirectUris(), containsInAnyOrder( - "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/post", - "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/soap", - "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/paos", - "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/redirect", - "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/artifact" - )); - - assertThat(response.getAttributes().get("saml_single_logout_service_url_redirect"), is("https://LoadBalancer-9.siroe.com:3443/federation/SPSloRedirect/metaAlias/sp")); - assertThat(response.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER), is(ArtifactBindingUtils.computeArtifactBindingIdentifierString("loadbalancer-9.siroe.com"))); - - Assert.assertNotNull(response.getProtocolMappers()); - Assert.assertEquals(1,response.getProtocolMappers().size()); - ProtocolMapperRepresentation mapper = response.getProtocolMappers().get(0); - Assert.assertEquals("saml-user-attribute-mapper",mapper.getProtocolMapper()); - Assert.assertEquals("urn:oid:2.5.4.42",mapper.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAME)); - Assert.assertEquals("givenName",mapper.getConfig().get(AttributeStatementHelper.FRIENDLY_NAME)); - Assert.assertEquals(AttributeStatementHelper.URI_REFERENCE,mapper.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAMEFORMAT)); + assertClientCreation(entityDescriptor); } @Test @@ -113,19 +92,32 @@ public class SAMLClientRegistrationTest extends AbstractClientRegistrationTest { reg.auth(Auth.token(accessToken)); String entityDescriptor = IOUtils.toString(getClass().getResourceAsStream("/clientreg-test/saml-entity-descriptor.xml")); - assertCreateFail(entityDescriptor, 400, Errors.INVALID_CLIENT); + assertClientCreation(entityDescriptor); } - private void assertCreateFail(String entityDescriptor, int expectedStatusCode, String expectedErrorContains) { - try { - reg.saml().create(entityDescriptor); - Assert.fail("Not expected to successfully register client"); - } catch (ClientRegistrationException expected) { - HttpErrorException httpEx = (HttpErrorException) expected.getCause(); - Assert.assertEquals(expectedStatusCode, httpEx.getStatusLine().getStatusCode()); - if (expectedErrorContains != null) { - assertTrue("Error response doesn't contain expected text", httpEx.getErrorResponse().contains(expectedErrorContains)); - } - } + private void assertClientCreation(String entityDescriptor) throws ClientRegistrationException { + ClientRepresentation response = reg.saml().create(entityDescriptor); + assertThat(response.getRegistrationAccessToken(), notNullValue()); + assertThat(response.getClientId(), is("loadbalancer-9.siroe.com")); + assertThat(response.getRedirectUris(), containsInAnyOrder( + "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/post", + "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/soap", + "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/paos", + "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/redirect", + "https://LoadBalancer-9.siroe.com:3443/federation/Consumer/metaAlias/sp/artifact" + )); + + assertThat(response.getAttributes().get("saml_single_logout_service_url_redirect"), is("https://LoadBalancer-9.siroe.com:3443/federation/SPSloRedirect/metaAlias/sp")); + assertThat(response.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER), is(ArtifactBindingUtils.computeArtifactBindingIdentifierString("loadbalancer-9.siroe.com"))); + + Assert.assertNotNull(response.getProtocolMappers()); + Assert.assertEquals(1,response.getProtocolMappers().size()); + ProtocolMapperRepresentation mapper = response.getProtocolMappers().get(0); + Assert.assertEquals("saml-user-attribute-mapper",mapper.getProtocolMapper()); + Assert.assertEquals("urn:oid:2.5.4.42",mapper.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAME)); + Assert.assertEquals("givenName",mapper.getConfig().get(AttributeStatementHelper.FRIENDLY_NAME)); + Assert.assertEquals(AttributeStatementHelper.URI_REFERENCE,mapper.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAMEFORMAT)); + + adminClient.realm(REALM_NAME).clients().get(response.getId()).remove(); } }