From ec327c99f4dc60f15167b1d04fab15b86edf9bbe Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 30 Nov 2015 12:40:04 +0100 Subject: [PATCH] KEYCLOAK-2152 KEYCLOAK-2061 Client switches changes. Support for response_types and grant_types in OIDC Client registration --- .../keycloak/common/util/CollectionUtil.java | 15 +++++ .../META-INF/jpa-changelog-1.7.0.xml | 2 +- .../java/org/keycloak/OAuth2Constants.java | 2 + .../idm/ClientRepresentation.java | 2 + .../oidc/OIDCClientRepresentation.java | 12 ++-- .../java/org/keycloak/events/Details.java | 1 + .../admin/resources/js/controllers/clients.js | 1 - .../models/entities/ClientEntity.java | 9 --- .../models/utils/RepresentationToModel.java | 10 +-- .../org/keycloak/models/jpa/RealmAdapter.java | 1 - .../mongo/keycloak/adapters/RealmAdapter.java | 1 - ...yDescriptorClientRegistrationProvider.java | 1 + .../protocol/oidc/OIDCWellKnownProvider.java | 7 +- .../oidc/endpoints/TokenEndpoint.java | 6 +- .../oidc/utils/OIDCRedirectUriBuilder.java | 6 +- .../protocol/oidc/utils/OIDCResponseType.java | 15 +++-- .../ClientRegistrationException.java | 23 +++++++ .../oidc/DescriptionConverter.java | 67 ++++++++++++++++++- .../oidc/OIDCClientRegistrationProvider.java | 36 +++++++--- .../resources/admin/RealmAdminResource.java | 1 + .../org/keycloak/test/ResponseTypeTest.java | 37 +++++++++- .../client/OIDCClientRegistrationTest.java | 27 +++++++- .../org/keycloak/testsuite/AssertEvents.java | 2 +- .../testsuite/forms/CustomFlowTest.java | 2 +- .../keycloak/testsuite/model/GroupTest.java | 2 +- .../oauth/ClientAuthSignedJWTTest.java | 2 +- .../testsuite/oauth/OfflineTokenTest.java | 4 +- ...urceOwnerPasswordCredentialsGrantTest.java | 8 +-- 28 files changed, 241 insertions(+), 61 deletions(-) create mode 100644 services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationException.java diff --git a/common/src/main/java/org/keycloak/common/util/CollectionUtil.java b/common/src/main/java/org/keycloak/common/util/CollectionUtil.java index 747b0bb567..f0fda16ecc 100644 --- a/common/src/main/java/org/keycloak/common/util/CollectionUtil.java +++ b/common/src/main/java/org/keycloak/common/util/CollectionUtil.java @@ -23,4 +23,19 @@ public class CollectionUtil { } return sb.toString(); } + + // Return true if all items from col1 are in col2 and viceversa. Order is not taken into account + public static boolean collectionEquals(Collection col1, Collection col2) { + if (col1.size() != col2.size()) { + return false; + } + + for (T item : col1) { + if (!col2.contains(item)) { + return false; + } + } + + return true; + } } diff --git a/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.7.0.xml b/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.7.0.xml index eadbdcf428..121d933a80 100755 --- a/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.7.0.xml +++ b/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.7.0.xml @@ -83,7 +83,7 @@ - + diff --git a/core/src/main/java/org/keycloak/OAuth2Constants.java b/core/src/main/java/org/keycloak/OAuth2Constants.java index b552d1bd89..4b38a76430 100644 --- a/core/src/main/java/org/keycloak/OAuth2Constants.java +++ b/core/src/main/java/org/keycloak/OAuth2Constants.java @@ -27,6 +27,8 @@ public interface OAuth2Constants { String AUTHORIZATION_CODE = "authorization_code"; + String IMPLICIT = "implicit"; + String PASSWORD = "password"; String CLIENT_CREDENTIALS = "client_credentials"; diff --git a/core/src/main/java/org/keycloak/representations/idm/ClientRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/ClientRepresentation.java index aef643c180..a648f12a22 100755 --- a/core/src/main/java/org/keycloak/representations/idm/ClientRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/ClientRepresentation.java @@ -30,6 +30,7 @@ public class ClientRepresentation { protected Boolean implicitFlowEnabled; protected Boolean directAccessGrantsEnabled; protected Boolean serviceAccountsEnabled; + @Deprecated protected Boolean directGrantsOnly; protected Boolean publicClient; protected Boolean frontchannelLogout; @@ -216,6 +217,7 @@ public class ClientRepresentation { this.serviceAccountsEnabled = serviceAccountsEnabled; } + @Deprecated public Boolean isDirectGrantsOnly() { return directGrantsOnly; } diff --git a/core/src/main/java/org/keycloak/representations/oidc/OIDCClientRepresentation.java b/core/src/main/java/org/keycloak/representations/oidc/OIDCClientRepresentation.java index 1ff32fc096..c76f5c7086 100644 --- a/core/src/main/java/org/keycloak/representations/oidc/OIDCClientRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/oidc/OIDCClientRepresentation.java @@ -14,9 +14,9 @@ public class OIDCClientRepresentation { private String token_endpoint_auth_method; - private String grant_types; + private List grant_types; - private String response_types; + private List response_types; private String client_id; @@ -68,19 +68,19 @@ public class OIDCClientRepresentation { this.token_endpoint_auth_method = token_endpoint_auth_method; } - public String getGrantTypes() { + public List getGrantTypes() { return grant_types; } - public void setGrantTypes(String grantTypes) { + public void setGrantTypes(List grantTypes) { this.grant_types = grantTypes; } - public String getResponseTypes() { + public List getResponseTypes() { return response_types; } - public void setResponseTypes(String responseTypes) { + public void setResponseTypes(List responseTypes) { this.response_types = responseTypes; } diff --git a/events/api/src/main/java/org/keycloak/events/Details.java b/events/api/src/main/java/org/keycloak/events/Details.java index a995d7ba3d..ef7f004c8b 100755 --- a/events/api/src/main/java/org/keycloak/events/Details.java +++ b/events/api/src/main/java/org/keycloak/events/Details.java @@ -12,6 +12,7 @@ public interface Details { String REDIRECT_URI = "redirect_uri"; String RESPONSE_TYPE = "response_type"; String RESPONSE_MODE = "response_mode"; + String GRANT_TYPE = "grant_type"; String AUTH_TYPE = "auth_type"; String AUTH_METHOD = "auth_method"; String IDENTITY_PROVIDER = "identity_provider"; diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js index 11bb11b3b7..4e81a43035 100755 --- a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js +++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js @@ -865,7 +865,6 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, $route, se $scope.client = { enabled: true, standardFlowEnabled: true, - directAccessGrantsEnabled: true, attributes: {} }; $scope.client.attributes['saml_signature_canonicalization_method'] = $scope.canonicalization[0].value; diff --git a/model/api/src/main/java/org/keycloak/models/entities/ClientEntity.java b/model/api/src/main/java/org/keycloak/models/entities/ClientEntity.java index b5edbaf9ee..46259c9198 100755 --- a/model/api/src/main/java/org/keycloak/models/entities/ClientEntity.java +++ b/model/api/src/main/java/org/keycloak/models/entities/ClientEntity.java @@ -34,7 +34,6 @@ public class ClientEntity extends AbstractIdentifiableEntity { private boolean implicitFlowEnabled; private boolean directAccessGrantsEnabled; private boolean serviceAccountsEnabled; - private boolean directGrantsOnly; private int nodeReRegistrationTimeout; // We are using names of defaultRoles (not ids) @@ -278,14 +277,6 @@ public class ClientEntity extends AbstractIdentifiableEntity { this.serviceAccountsEnabled = serviceAccountsEnabled; } - public boolean isDirectGrantsOnly() { - return directGrantsOnly; - } - - public void setDirectGrantsOnly(boolean directGrantsOnly) { - this.directGrantsOnly = directGrantsOnly; - } - public List getDefaultRoles() { return defaultRoles; } diff --git a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 8360e48731..24db3a6901 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -776,17 +776,19 @@ public class RepresentationToModel { if (resourceRep.getBaseUrl() != null) client.setBaseUrl(resourceRep.getBaseUrl()); if (resourceRep.isBearerOnly() != null) client.setBearerOnly(resourceRep.isBearerOnly()); if (resourceRep.isConsentRequired() != null) client.setConsentRequired(resourceRep.isConsentRequired()); - if (resourceRep.isStandardFlowEnabled() != null) client.setStandardFlowEnabled(resourceRep.isStandardFlowEnabled()); - if (resourceRep.isImplicitFlowEnabled() != null) client.setImplicitFlowEnabled(resourceRep.isImplicitFlowEnabled()); - if (resourceRep.isDirectAccessGrantsEnabled() != null) client.setDirectAccessGrantsEnabled(resourceRep.isDirectAccessGrantsEnabled()); - if (resourceRep.isServiceAccountsEnabled() != null) client.setServiceAccountsEnabled(resourceRep.isServiceAccountsEnabled()); // Backwards compatibility only if (resourceRep.isDirectGrantsOnly() != null) { logger.warn("Using deprecated 'directGrantsOnly' configuration in JSON representation. It will be removed in future versions"); client.setStandardFlowEnabled(!resourceRep.isDirectGrantsOnly()); + client.setDirectAccessGrantsEnabled(resourceRep.isDirectGrantsOnly()); } + if (resourceRep.isStandardFlowEnabled() != null) client.setStandardFlowEnabled(resourceRep.isStandardFlowEnabled()); + if (resourceRep.isImplicitFlowEnabled() != null) client.setImplicitFlowEnabled(resourceRep.isImplicitFlowEnabled()); + if (resourceRep.isDirectAccessGrantsEnabled() != null) client.setDirectAccessGrantsEnabled(resourceRep.isDirectAccessGrantsEnabled()); + if (resourceRep.isServiceAccountsEnabled() != null) client.setServiceAccountsEnabled(resourceRep.isServiceAccountsEnabled()); + if (resourceRep.isPublicClient() != null) client.setPublicClient(resourceRep.isPublicClient()); if (resourceRep.isFrontchannelLogout() != null) client.setFrontchannelLogout(resourceRep.isFrontchannelLogout()); if (resourceRep.getProtocol() != null) client.setProtocol(resourceRep.getProtocol()); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index cb69f14d62..3f5817956b 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -726,7 +726,6 @@ public class RealmAdapter implements RealmModel { entity.setClientId(clientId); entity.setEnabled(true); entity.setStandardFlowEnabled(true); - entity.setDirectAccessGrantsEnabled(true); entity.setRealm(realm); realm.getClients().add(entity); em.persist(entity); diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index c8fc1f0b95..fc840ba534 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -811,7 +811,6 @@ public class RealmAdapter extends AbstractMongoAdapter impleme clientEntity.setRealmId(getId()); clientEntity.setEnabled(true); clientEntity.setStandardFlowEnabled(true); - clientEntity.setDirectAccessGrantsEnabled(true); getMongoStore().insertEntity(clientEntity, invocationContext); final ClientModel model = new ClientAdapter(session, this, clientEntity, invocationContext); diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/clientregistration/EntityDescriptorClientRegistrationProvider.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/clientregistration/EntityDescriptorClientRegistrationProvider.java index 1ee3cd2bff..10ef0191c4 100644 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/clientregistration/EntityDescriptorClientRegistrationProvider.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/clientregistration/EntityDescriptorClientRegistrationProvider.java @@ -5,6 +5,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.protocol.saml.EntityDescriptorDescriptionConverter; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.services.clientregistration.AbstractClientRegistrationProvider; +import org.keycloak.services.clientregistration.ClientRegistrationException; import javax.ws.rs.Consumes; import javax.ws.rs.POST; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java index 60e9f9d36a..98fb49e08d 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCWellKnownProvider.java @@ -4,6 +4,7 @@ import org.keycloak.OAuth2Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.protocol.oidc.representations.OIDCConfigurationRepresentation; +import org.keycloak.protocol.oidc.utils.OIDCResponseType; import org.keycloak.services.clientregistration.ClientRegistrationService; import org.keycloak.services.clientregistration.oidc.OIDCClientRegistrationProviderFactory; import org.keycloak.services.resources.RealmsResource; @@ -22,13 +23,13 @@ public class OIDCWellKnownProvider implements WellKnownProvider { public static final List DEFAULT_ID_TOKEN_SIGNING_ALG_VALUES_SUPPORTED = list("RS256"); - public static final List DEFAULT_GRANT_TYPES_SUPPORTED = list(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD, OAuth2Constants.CLIENT_CREDENTIALS); + public static final List DEFAULT_GRANT_TYPES_SUPPORTED = list(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.IMPLICIT, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD, OAuth2Constants.CLIENT_CREDENTIALS); - public static final List DEFAULT_RESPONSE_TYPES_SUPPORTED = list(OAuth2Constants.CODE); + public static final List DEFAULT_RESPONSE_TYPES_SUPPORTED = list(OAuth2Constants.CODE, OIDCResponseType.NONE, OIDCResponseType.ID_TOKEN, "id_token token", "code id_token", "code token", "code id_token token"); public static final List DEFAULT_SUBJECT_TYPES_SUPPORTED = list("public"); - public static final List DEFAULT_RESPONSE_MODES_SUPPORTED = list("query"); + public static final List DEFAULT_RESPONSE_MODES_SUPPORTED = list("query", "fragment", "form_post"); private KeycloakSession session; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index fe358e0ba4..96f6ccd96c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -178,6 +178,8 @@ public class TokenEndpoint { } else { throw new ErrorResponseException(Errors.INVALID_REQUEST, "Invalid " + OIDCLoginProtocol.GRANT_TYPE_PARAM, Response.Status.BAD_REQUEST); } + + event.detail(Details.GRANT_TYPE, grantType); } public Response buildAuthorizationCodeAccessTokenResponse() { @@ -327,7 +329,7 @@ public class TokenEndpoint { } public Response buildResourceOwnerPasswordCredentialsGrant() { - event.detail(Details.AUTH_METHOD, "oauth_credentials").detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD); + event.detail(Details.AUTH_METHOD, "oauth_credentials"); if (client.isConsentRequired()) { event.error(Errors.CONSENT_DENIED); @@ -393,8 +395,6 @@ public class TokenEndpoint { throw new ErrorResponseException("unauthorized_client", "Client not enabled to retrieve service account", Response.Status.UNAUTHORIZED); } - event.detail(Details.RESPONSE_TYPE, OAuth2Constants.CLIENT_CREDENTIALS); - UserModel clientUser = session.users().getUserByServiceAccountClient(client); if (clientUser == null || client.getProtocolMapperByName(OIDCLoginProtocol.LOGIN_PROTOCOL, ServiceAccountConstants.CLIENT_ID_PROTOCOL_MAPPER) == null) { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java index 06cc3cc91c..4340229231 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java @@ -42,7 +42,7 @@ public abstract class OIDCRedirectUriBuilder { // http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes - public static class QueryRedirectUriBuilder extends OIDCRedirectUriBuilder { + private static class QueryRedirectUriBuilder extends OIDCRedirectUriBuilder { protected QueryRedirectUriBuilder(KeycloakUriBuilder uriBuilder) { super(uriBuilder); @@ -64,7 +64,7 @@ public abstract class OIDCRedirectUriBuilder { // http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes - public static class FragmentRedirectUriBuilder extends OIDCRedirectUriBuilder { + private static class FragmentRedirectUriBuilder extends OIDCRedirectUriBuilder { private StringBuilder fragment; @@ -98,7 +98,7 @@ public abstract class OIDCRedirectUriBuilder { // http://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html - public static class FormPostRedirectUriBuilder extends OIDCRedirectUriBuilder { + private static class FormPostRedirectUriBuilder extends OIDCRedirectUriBuilder { private Map params = new HashMap<>(); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCResponseType.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCResponseType.java index 6377b22a47..9313aa60bc 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCResponseType.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCResponseType.java @@ -46,6 +46,16 @@ public class OIDCResponseType { return new OIDCResponseType(allowedTypes); } + public static OIDCResponseType parse(List responseTypes) { + OIDCResponseType result = new OIDCResponseType(new ArrayList()); + for (String respType : responseTypes) { + OIDCResponseType responseType = parse(respType); + result.responseTypes.addAll(responseType.responseTypes); + } + + return result; + } + private static void validateAllowedTypes(List responseTypes) { if (responseTypes.size() == 0) { throw new IllegalStateException("No responseType provided"); @@ -53,9 +63,6 @@ public class OIDCResponseType { if (responseTypes.contains(NONE) && responseTypes.size() > 1) { throw new IllegalArgumentException("None not allowed with some other response_type"); } - if (responseTypes.contains(ID_TOKEN) && responseTypes.size() == 1) { - throw new IllegalArgumentException("Not supported to use response_type=id_token alone"); - } if (responseTypes.contains(TOKEN) && responseTypes.size() == 1) { throw new IllegalArgumentException("Not supported to use response_type=token alone"); } @@ -72,7 +79,7 @@ public class OIDCResponseType { } public boolean isImplicitFlow() { - return hasResponseType(TOKEN) && hasResponseType(ID_TOKEN) && !hasResponseType(CODE); + return hasResponseType(ID_TOKEN) && !hasResponseType(CODE); } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationException.java b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationException.java new file mode 100644 index 0000000000..71c9c3d077 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientregistration/ClientRegistrationException.java @@ -0,0 +1,23 @@ +package org.keycloak.services.clientregistration; + +/** + * @author Marek Posolda + */ +public class ClientRegistrationException extends RuntimeException { + + public ClientRegistrationException() { + super(); + } + + public ClientRegistrationException(String message) { + super(message); + } + + public ClientRegistrationException(Throwable throwable) { + super(throwable); + } + + public ClientRegistrationException(String message, Throwable throwable) { + super(message, throwable); + } +} diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java index a7f9f2c82f..594a5f061f 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java @@ -1,21 +1,48 @@ package org.keycloak.services.clientregistration.oidc; +import org.keycloak.OAuth2Constants; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.utils.OIDCResponseType; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.oidc.OIDCClientRepresentation; +import org.keycloak.services.clientregistration.ClientRegistrationException; import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; /** * @author Stian Thorgersen */ public class DescriptionConverter { - public static ClientRepresentation toInternal(OIDCClientRepresentation clientOIDC) { + public static ClientRepresentation toInternal(OIDCClientRepresentation clientOIDC) throws ClientRegistrationException { ClientRepresentation client = new ClientRepresentation(); client.setClientId(clientOIDC.getClientId()); client.setName(clientOIDC.getClientName()); client.setRedirectUris(clientOIDC.getRedirectUris()); client.setBaseUrl(clientOIDC.getClientUri()); + + List oidcResponseTypes = clientOIDC.getResponseTypes(); + if (oidcResponseTypes == null || oidcResponseTypes.isEmpty()) { + oidcResponseTypes = Collections.singletonList(OIDCResponseType.CODE); + } + List oidcGrantTypes = clientOIDC.getGrantTypes(); + + try { + OIDCResponseType responseType = OIDCResponseType.parse(oidcResponseTypes); + client.setStandardFlowEnabled(responseType.hasResponseType(OIDCResponseType.CODE)); + client.setImplicitFlowEnabled(responseType.isImplicitOrHybridFlow()); + if (oidcGrantTypes != null) { + client.setDirectAccessGrantsEnabled(oidcGrantTypes.contains(OAuth2Constants.PASSWORD)); + client.setServiceAccountsEnabled(oidcGrantTypes.contains(OAuth2Constants.CLIENT_CREDENTIALS)); + } + } catch (IllegalArgumentException iae) { + throw new ClientRegistrationException(iae.getMessage(), iae); + } + return client; } @@ -28,7 +55,45 @@ public class DescriptionConverter { response.setRedirectUris(client.getRedirectUris()); response.setRegistrationAccessToken(client.getRegistrationAccessToken()); response.setRegistrationClientUri(uri.toString()); + response.setResponseTypes(getOIDCResponseTypes(client)); + response.setGrantTypes(getOIDCGrantTypes(client)); return response; } + private static List getOIDCResponseTypes(ClientRepresentation client) { + List responseTypes = new ArrayList<>(); + if (client.isStandardFlowEnabled()) { + responseTypes.add(OAuth2Constants.CODE); + responseTypes.add(OIDCResponseType.NONE); + } + if (client.isImplicitFlowEnabled()) { + responseTypes.add(OIDCResponseType.ID_TOKEN); + responseTypes.add("id_token token"); + } + if (client.isStandardFlowEnabled() && client.isImplicitFlowEnabled()) { + responseTypes.add("code id_token"); + responseTypes.add("code token"); + responseTypes.add("code id_token token"); + } + return responseTypes; + } + + private static List getOIDCGrantTypes(ClientRepresentation client) { + List grantTypes = new ArrayList<>(); + if (client.isStandardFlowEnabled()) { + grantTypes.add(OAuth2Constants.AUTHORIZATION_CODE); + } + if (client.isImplicitFlowEnabled()) { + grantTypes.add(OAuth2Constants.IMPLICIT); + } + if (client.isDirectAccessGrantsEnabled()) { + grantTypes.add(OAuth2Constants.PASSWORD); + } + if (client.isServiceAccountsEnabled()) { + grantTypes.add(OAuth2Constants.CLIENT_CREDENTIALS); + } + grantTypes.add(OAuth2Constants.REFRESH_TOKEN); + return grantTypes; + } + } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java index e60720bc87..82b8825eb9 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java @@ -1,5 +1,6 @@ package org.keycloak.services.clientregistration.oidc; +import org.jboss.logging.Logger; import org.keycloak.common.util.Time; import org.keycloak.events.EventBuilder; import org.keycloak.models.KeycloakSession; @@ -9,6 +10,7 @@ import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.services.ErrorResponseException; import org.keycloak.services.clientregistration.AbstractClientRegistrationProvider; import org.keycloak.services.clientregistration.ClientRegistrationAuth; +import org.keycloak.services.clientregistration.ClientRegistrationException; import org.keycloak.services.clientregistration.ErrorCodes; import javax.ws.rs.*; @@ -21,6 +23,8 @@ import java.net.URI; */ public class OIDCClientRegistrationProvider extends AbstractClientRegistrationProvider { + private static final Logger log = Logger.getLogger(OIDCClientRegistrationProvider.class); + public OIDCClientRegistrationProvider(KeycloakSession session) { super(session); } @@ -33,12 +37,17 @@ public class OIDCClientRegistrationProvider extends AbstractClientRegistrationPr throw new ErrorResponseException(ErrorCodes.INVALID_CLIENT_METADATA, "Client Identifier included", Response.Status.BAD_REQUEST); } - ClientRepresentation client = DescriptionConverter.toInternal(clientOIDC); - client = create(client); - URI uri = session.getContext().getUri().getAbsolutePathBuilder().path(client.getClientId()).build(); - clientOIDC = DescriptionConverter.toExternalResponse(client, uri); - clientOIDC.setClientIdIssuedAt(Time.currentTime()); - return Response.created(uri).entity(clientOIDC).build(); + try { + ClientRepresentation client = DescriptionConverter.toInternal(clientOIDC); + client = create(client); + URI uri = session.getContext().getUri().getAbsolutePathBuilder().path(client.getClientId()).build(); + clientOIDC = DescriptionConverter.toExternalResponse(client, uri); + clientOIDC.setClientIdIssuedAt(Time.currentTime()); + return Response.created(uri).entity(clientOIDC).build(); + } catch (ClientRegistrationException cre) { + log.error(cre.getMessage()); + throw new ErrorResponseException(ErrorCodes.INVALID_CLIENT_METADATA, "Client metadata invalid", Response.Status.BAD_REQUEST); + } } @GET @@ -54,11 +63,16 @@ public class OIDCClientRegistrationProvider extends AbstractClientRegistrationPr @Path("{clientId}") @Consumes(MediaType.APPLICATION_JSON) public Response updateOIDC(@PathParam("clientId") String clientId, OIDCClientRepresentation clientOIDC) { - ClientRepresentation client = DescriptionConverter.toInternal(clientOIDC); - client = update(clientId, client); - URI uri = session.getContext().getUri().getAbsolutePathBuilder().path(client.getClientId()).build(); - clientOIDC = DescriptionConverter.toExternalResponse(client, uri); - return Response.ok(clientOIDC).build(); + try { + ClientRepresentation client = DescriptionConverter.toInternal(clientOIDC); + client = update(clientId, client); + URI uri = session.getContext().getUri().getAbsolutePathBuilder().path(client.getClientId()).build(); + clientOIDC = DescriptionConverter.toExternalResponse(client, uri); + return Response.ok(clientOIDC).build(); + } catch (ClientRegistrationException cre) { + log.error(cre.getMessage()); + throw new ErrorResponseException(ErrorCodes.INVALID_CLIENT_METADATA, "Client metadata invalid", Response.Status.BAD_REQUEST); + } } @DELETE diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index 90eab4a54e..a4d5f27f77 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -34,6 +34,7 @@ import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.RealmEventsConfigRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.services.clientregistration.ClientRegistrationException; import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.LDAPConnectionTestManager; import org.keycloak.services.managers.RealmManager; diff --git a/services/src/test/java/org/keycloak/test/ResponseTypeTest.java b/services/src/test/java/org/keycloak/test/ResponseTypeTest.java index b3f77a7b7f..dd15aa90e9 100644 --- a/services/src/test/java/org/keycloak/test/ResponseTypeTest.java +++ b/services/src/test/java/org/keycloak/test/ResponseTypeTest.java @@ -1,5 +1,8 @@ package org.keycloak.test; +import java.util.Arrays; +import java.util.Collections; + import org.junit.Assert; import org.junit.Test; import org.keycloak.protocol.oidc.utils.OIDCResponseType; @@ -16,7 +19,7 @@ public class ResponseTypeTest { assertFail("foo"); assertSuccess("code"); assertSuccess("none"); - assertFail("id_token"); + assertSuccess("id_token"); assertFail("token"); assertFail("refresh_token"); assertSuccess("id_token token"); @@ -27,6 +30,38 @@ public class ResponseTypeTest { assertFail("code refresh_token"); } + @Test + public void testMultipleResponseTypes() { + try { + OIDCResponseType.parse(Arrays.asList("code", "token")); + Assert.fail("Not expected to parse with success"); + } catch (IllegalArgumentException iae) { + } + + OIDCResponseType responseType = OIDCResponseType.parse(Collections.singletonList("code")); + Assert.assertTrue(responseType.hasResponseType("code")); + Assert.assertFalse(responseType.hasResponseType("none")); + Assert.assertFalse(responseType.isImplicitOrHybridFlow()); + + responseType = OIDCResponseType.parse(Arrays.asList("code", "none")); + Assert.assertTrue(responseType.hasResponseType("code")); + Assert.assertTrue(responseType.hasResponseType("none")); + Assert.assertFalse(responseType.isImplicitOrHybridFlow()); + + responseType = OIDCResponseType.parse(Arrays.asList("code", "code token")); + Assert.assertTrue(responseType.hasResponseType("code")); + Assert.assertFalse(responseType.hasResponseType("none")); + Assert.assertTrue(responseType.hasResponseType("token")); + Assert.assertFalse(responseType.hasResponseType("id_token")); + Assert.assertTrue(responseType.isImplicitOrHybridFlow()); + Assert.assertFalse(responseType.isImplicitFlow()); + + responseType = OIDCResponseType.parse(Arrays.asList("id_token", "id_token token")); + Assert.assertFalse(responseType.hasResponseType("code")); + Assert.assertTrue(responseType.isImplicitOrHybridFlow()); + Assert.assertTrue(responseType.isImplicitFlow()); + } + private void assertSuccess(String responseType) { OIDCResponseType.parse(responseType); } 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 9696274241..f0fbe2bedc 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 @@ -2,12 +2,16 @@ package org.keycloak.testsuite.client; import org.junit.Before; import org.junit.Test; +import org.keycloak.OAuth2Constants; import org.keycloak.client.registration.Auth; import org.keycloak.client.registration.ClientRegistrationException; +import org.keycloak.common.util.CollectionUtil; +import org.keycloak.protocol.oidc.utils.OIDCResponseType; import org.keycloak.representations.idm.ClientInitialAccessCreatePresentation; import org.keycloak.representations.idm.ClientInitialAccessPresentation; import org.keycloak.representations.oidc.OIDCClientRepresentation; +import java.util.Arrays; import java.util.Collections; import static org.junit.Assert.*; @@ -49,6 +53,8 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { assertEquals("http://root", response.getClientUri()); assertEquals(1, response.getRedirectUris().size()); assertEquals("http://redirect", response.getRedirectUris().get(0)); + assertEquals(Arrays.asList("code", "none"), response.getResponseTypes()); + assertEquals(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.REFRESH_TOKEN), response.getGrantTypes()); } @Test @@ -59,6 +65,8 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { OIDCClientRepresentation rep = reg.oidc().get(response.getClientId()); assertNotNull(rep); assertNotEquals(response.getRegistrationAccessToken(), rep.getRegistrationAccessToken()); + assertTrue(CollectionUtil.collectionEquals(Arrays.asList("code", "none"), response.getResponseTypes())); + assertTrue(CollectionUtil.collectionEquals(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.REFRESH_TOKEN), response.getGrantTypes())); } @Test @@ -67,11 +75,26 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { reg.auth(Auth.token(response)); response.setRedirectUris(Collections.singletonList("http://newredirect")); + response.setResponseTypes(Arrays.asList("code", "id_token token", "code id_token token")); + response.setGrantTypes(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD)); OIDCClientRepresentation updated = reg.oidc().update(response); - assertEquals(1, updated.getRedirectUris().size()); - assertEquals("http://newredirect", updated.getRedirectUris().get(0)); + assertTrue(CollectionUtil.collectionEquals(Collections.singletonList("http://newredirect"), updated.getRedirectUris())); + assertTrue(CollectionUtil.collectionEquals(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.IMPLICIT, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD), updated.getGrantTypes())); + assertTrue(CollectionUtil.collectionEquals(Arrays.asList(OAuth2Constants.CODE, OIDCResponseType.NONE, OIDCResponseType.ID_TOKEN, "id_token token", "code id_token", "code token", "code id_token token"), updated.getResponseTypes())); + } + + @Test + public void updateClientError() throws ClientRegistrationException { + try { + OIDCClientRepresentation response = create(); + reg.auth(Auth.token(response)); + response.setResponseTypes(Arrays.asList("code", "token")); + reg.oidc().update(response); + fail("Not expected to end with success"); + } catch (ClientRegistrationException cre) { + } } @Test diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/AssertEvents.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/AssertEvents.java index 84e724b99e..517f6c3d96 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/AssertEvents.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/AssertEvents.java @@ -135,7 +135,7 @@ public class AssertEvents implements TestRule, EventListenerProviderFactory { return expect(EventType.CLIENT_LOGIN) .detail(Details.CODE_ID, isCodeId()) .detail(Details.CLIENT_AUTH_METHOD, ClientIdAndSecretAuthenticator.PROVIDER_ID) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.CLIENT_CREDENTIALS) + .detail(Details.GRANT_TYPE, OAuth2Constants.CLIENT_CREDENTIALS) .removeDetail(Details.CODE_ID) .session(isUUID()); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java index 469030821e..1a8362beae 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/CustomFlowTest.java @@ -218,7 +218,7 @@ public class CustomFlowTest { .client(clientId) .user(userId) .session(accessToken.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) .detail(Details.USERNAME, login) diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/GroupTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/GroupTest.java index a5f23883cb..1c057e716d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/GroupTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/GroupTest.java @@ -257,7 +257,7 @@ public class GroupTest { .client(clientId) .user(userId) .session(accessToken.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) .detail(Details.USERNAME, login) diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java index 898066aad4..d0e8bff62d 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java @@ -189,7 +189,7 @@ public class ClientAuthSignedJWTTest { events.expectLogin() .client("client2") .session(accessToken.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) .detail(Details.USERNAME, "test-user@localhost") diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java index a4b2855b0b..5ca6388a08 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java @@ -319,7 +319,7 @@ public class OfflineTokenTest { .client("offline-client") .user(userId) .session(token.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, token.getId()) .detail(Details.REFRESH_TOKEN_ID, offlineToken.getId()) .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_OFFLINE) @@ -361,7 +361,7 @@ public class OfflineTokenTest { .client("offline-client") .user(userId) .session(token.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, token.getId()) .detail(Details.REFRESH_TOKEN_ID, offlineToken.getId()) .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_OFFLINE) diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java index b64a683ed9..e834785192 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java @@ -94,7 +94,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest { .client(clientId) .user(userId) .session(accessToken.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) .detail(Details.USERNAME, login) @@ -130,7 +130,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest { events.expectLogin() .client("resource-owner") .session(accessToken.getSessionState()) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.TOKEN_ID, accessToken.getId()) .detail(Details.REFRESH_TOKEN_ID, refreshToken.getId()) .removeDetail(Details.CODE_ID) @@ -286,7 +286,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest { events.expectLogin() .client("resource-owner") .session((String) null) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .removeDetail(Details.CODE_ID) .removeDetail(Details.REDIRECT_URI) .removeDetail(Details.CONSENT) @@ -308,7 +308,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest { .client("resource-owner") .user((String) null) .session((String) null) - .detail(Details.RESPONSE_TYPE, OAuth2Constants.PASSWORD) + .detail(Details.GRANT_TYPE, OAuth2Constants.PASSWORD) .detail(Details.USERNAME, "invalid") .removeDetail(Details.CODE_ID) .removeDetail(Details.REDIRECT_URI)