#133 - #134 with SAML federation

eosc-kc · Mar 1, 2022 · 87f5b74 · 87f5b74
1 parent 976449c
commit 87f5b74
Show file tree

Hide file tree

Showing 13 changed files with 39 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,8 @@ Full Keycloak upstream jira issue can be shown if filtered by Fix version.
 ### Changed
 - Allow omitting NameIDFormat [RCIAM-882](https://jira.argo.grnet.gr/browse/RCIAM-882)
 - Add signing page roles in account console[RCIAM-860](https://jira.argo.grnet.gr/browse/RCIAM-860)
+- EntityId in configuration of SAML IdP[EOSC-KC-133](https://github.com/eosc-kc/keycloak/issues/133)
+- Record SAML login events based on SAML IdP entityID [EOSC-KC-134](https://github.com/eosc-kc/keycloak/issues/134)
 ### Fixed
 - check for null samlResponse for federation
 

diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java
@@ -39,6 +39,7 @@ public interface Details {
     String AUTH_METHOD = "auth_method";
     String IDENTITY_PROVIDER = "identity_provider";
     String IDENTITY_PROVIDER_USERNAME = "identity_provider_identity";
+    String IDENTITY_PROVIDER_ENTITYID = "identity_provider_entityId";
     String REGISTER_METHOD = "register_method";
     String USERNAME = "username";
     String FIRST_NAME = "first_name";

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -397,7 +397,7 @@ private Consumer<UserSessionModel> processLogout(AtomicReference<LogoutRequestTy
         }
 
         private String getEntityId(UriInfo uriInfo, RealmModel realm) {
-            String configEntityId = config.getEntityId();
+            String configEntityId = config.getSpEntityId();
 
             if (configEntityId == null || configEntityId.isEmpty())
                 return UriBuilder.fromUri(uriInfo.getBaseUri()).path("realms").path(realm.getName()).build().toString();

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
@@ -205,7 +205,7 @@ public Response performLogin(AuthenticationRequest request) {
     }
 
     private String getEntityId(UriInfo uriInfo, RealmModel realm) {
-        String configEntityId = getConfig().getEntityId();
+        String configEntityId = getConfig().getSpEntityId();
 
         if (configEntityId == null || configEntityId.isEmpty())
             return UriBuilder.fromUri(uriInfo.getBaseUri()).path("realms").path(realm.getName()).build().toString();

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
@@ -41,6 +41,7 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
     public static final XmlKeyInfoKeyNameTransformer DEFAULT_XML_KEY_INFO_KEY_NAME_TRANSFORMER = XmlKeyInfoKeyNameTransformer.NONE;
 
     public static final String ENTITY_ID = "entityId";
+    public static final String SP_ENTITY_ID = "spEntityId";
     public static final String ADD_EXTENSIONS_ELEMENT_WITH_KEY_INFO = "addExtensionsElementWithKeyInfo";
     public static final String BACKCHANNEL_SUPPORTED = "backchannelSupported";
     public static final String ENCRYPTION_PUBLIC_KEY = "encryptionPublicKey";
@@ -85,6 +86,14 @@ public void setEntityId(String entityId) {
         getConfig().put(ENTITY_ID, entityId);
     }
 
+    public String getSpEntityId() {
+        return getConfig().get(SP_ENTITY_ID);
+    }
+
+    public void setSpEntityId(String spEntityId) {
+        getConfig().put(SP_ENTITY_ID, spEntityId);
+    }
+
     public String getSingleSignOnServiceUrl() {
         return getConfig().get(SINGLE_SIGN_ON_SERVICE_URL);
     }
@@ -422,6 +431,7 @@ public void setLastRefreshTime(long lastRefreshTime) {
     public void validate(RealmModel realm) {
         SslRequired sslRequired = realm.getSslRequired();
 
+        checkUrl(SslRequired.NONE, getEntityId(), ENTITY_ID);
         checkUrl(sslRequired, getSingleLogoutServiceUrl(), SINGLE_LOGOUT_SERVICE_URL);
         checkUrl(sslRequired, getSingleSignOnServiceUrl(), SINGLE_SIGN_ON_SERVICE_URL);
         checkUrl(sslRequired, getMetadataUrl(), METADATA_URL);

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
@@ -102,6 +102,7 @@ public IdentityProviderModel parseConfig(KeycloakSession session, InputStream in
                     String singleSignOnServiceUrl = null;
                     boolean postBindingResponse = false;
                     boolean postBindingLogout = false;
+                    samlIdentityProviderConfig.setEntityId(entityType.getEntityID());
                     for (EndpointType endpoint : idpDescriptor.getSingleSignOnService()) {
                         if (endpoint.getBinding().toString().equals(JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get())) {
                             singleSignOnServiceUrl = endpoint.getLocation().toString();

diff --git a/services/src/main/java/org/keycloak/broker/saml/federation/SAMLIdPFederationProvider.java b/services/src/main/java/org/keycloak/broker/saml/federation/SAMLIdPFederationProvider.java
@@ -313,6 +313,8 @@ private void parseIdP(IdentityProviderModel identityProviderModel, Date validUnt
             identityProviderModel.setEnabled(true);
         }
 
+		identityProviderModel.getConfig().put("entityId", entity.getEntityID());
+
         LocalizedNameType displayName = idpDescriptor.getExtensions() != null
             && idpDescriptor.getExtensions().getUIInfo() != null
                 ? idpDescriptor.getExtensions().getUIInfo().getDisplayName().stream()

diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
@@ -37,6 +37,7 @@
 import org.keycloak.broker.provider.util.IdentityBrokerState;
 import org.keycloak.broker.saml.SAMLEndpoint;
 import org.keycloak.broker.saml.federation.SAMLIdPFederationProvider;
+import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
 import org.keycloak.broker.social.SocialIdentityProvider;
 import org.keycloak.common.ClientConnection;
 import org.keycloak.common.util.Base64Url;
@@ -636,6 +637,9 @@ public Response authenticated(BrokeredIdentityContext context) {
                 .detail(Details.IDENTITY_PROVIDER, providerId)
                 .detail(Details.IDENTITY_PROVIDER_USERNAME, context.getUsername());
 
+        if ("saml".equals(identityProviderConfig.getProviderId()) && identityProviderConfig.getConfig().get(SAMLIdentityProviderConfig.ENTITY_ID) != null)
+            this.event.detail(Details.IDENTITY_PROVIDER_ENTITYID,identityProviderConfig.getConfig().get(SAMLIdentityProviderConfig.ENTITY_ID));
+
         UserModel federatedUser = this.session.users().getUserByFederatedIdentity(this.realmModel, federatedIdentityModel);
         boolean shouldMigrateId = false;
         // try to find the user using legacy ID

diff --git a/...rquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/...rquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
@@ -933,7 +933,8 @@ private void assertSamlConfig(Map<String, String> config) {
           "signingCertificate",
           "addExtensionsElementWithKeyInfo",
           "loginHint",
-          "hideOnLoginPage"
+          "hideOnLoginPage",
+          "entityId"
         ));
         assertThat(config, hasEntry("validateSignature", "true"));
         assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml"));
@@ -944,6 +945,7 @@ private void assertSamlConfig(Map<String, String> config) {
         assertThat(config, hasEntry("addExtensionsElementWithKeyInfo", "false"));
         assertThat(config, hasEntry("nameIDPolicyFormat", "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"));
         assertThat(config, hasEntry("hideOnLoginPage", "true"));
+        assertThat(config, hasEntry("entityId", "http://localhost:8080/auth/realms/master"));
         assertThat(config, hasEntry(is("signingCertificate"), notNullValue()));
     }
 

diff --git a/...ests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlCustomEntityIdBrokerTest.java b/...ests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlCustomEntityIdBrokerTest.java
@@ -66,7 +66,7 @@ public void testCustomEntityNotSet() throws Exception {
     public void testCustomEntityIdSet() throws Exception {
         // Comparison type set, no classrefs, no declrefs -> No RequestedAuthnContext
         try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
-            .setAttribute(SAMLIdentityProviderConfig.ENTITY_ID, "http://my.custom.entity.id")
+            .setAttribute(SAMLIdentityProviderConfig.SP_ENTITY_ID, "http://my.custom.entity.id")
             .update())
         {
             // Build the login request document

diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -802,8 +802,10 @@ import-from-url=Import from URL
 identity-provider.import-from-url.tooltip=Import metadata from a remote IDP discovery descriptor.
 import-from-file=Import from file
 identity-provider.import-from-file.tooltip=Import metadata from a downloaded IDP discovery descriptor.
-identity-provider.saml.entity-id=Service Provider Entity ID
-identity-provider.saml.entity-id.tooltip=The Entity ID that will be used to uniquely identify this SAML Service Provider
+identity-provider.saml.entity-id=Entity ID
+identity-provider.saml.entity-id.tooltip=SAML Identity Provider Entity ID(unique identifier)
+identity-provider.saml.sp-entity-id=Service Provider Entity ID
+identity-provider.saml.sp-entity-id.tooltip=The Entity ID that will be used to uniquely identify this SAML Service Provider
 identity-provider.saml.protocol-endpoints.saml=SAML 2.0 Service Provider Metadata
 identity-provider.saml.protocol-endpoints.saml.tooltip=Shows the configuration of the Service Provider endpoint
 identity-provider.saml.attribute-consuming-service-index=Attribute Consuming Service Index

diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js
@@ -1162,7 +1162,7 @@ module.controller('RealmIdentityProviderCtrl', function($scope, $filter, $upload
             $scope.identityProvider.config.xmlSigKeyInfoKeyNameTransformer = $scope.xmlKeyNameTranformers[1];			
             $scope.identityProvider.config.allowCreate = 'true';
         }
-        $scope.identityProvider.config.entityId = $scope.identityProvider.config.entityId || (authUrl + '/realms/' + realm.realm);
+        $scope.identityProvider.config.spEntityId = $scope.identityProvider.config.spEntityId || (authUrl + '/realms/' + realm.realm);
     }
 
     $scope.hidePassword = true;

diff --git a/.../src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html b/.../src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
@@ -183,12 +183,19 @@
                 <kc-tooltip>{{:: 'lastRefreshTime.tooltip' |translate}}</kc-tooltip>
             </div>
             <div class="form-group clearfix">
-                <label class="col-md-2 control-label" for="entityId"><span class="required">*</span> {{:: 'identity-provider.saml.entity-id' | translate}}</label>
+                <label class="col-md-2 control-label" for="entityId"> {{:: 'identity-provider.saml.entity-id' | translate}}</label>
                 <div class="col-md-6">
                     <input kc-no-reserved-chars class="form-control" id="entityId" type="text" ng-model="identityProvider.config.entityId" required>
                 </div>
                 <kc-tooltip>{{:: 'identity-provider.saml.entity-id.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group clearfix">
+                <label class="col-md-2 control-label" for="spEntityId"><span class="required">*</span> {{:: 'identity-provider.saml.sp-entity-id' | translate}}</label>
+                <div class="col-md-6">
+                    <input kc-no-reserved-chars class="form-control" id="spEntityId" type="text" ng-model="identityProvider.config.spEntityId" required>
+                </div>
+                <kc-tooltip>{{:: 'identity-provider.saml.sp-entity-id.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group clearfix">
                 <label class="col-md-2 control-label" for="singleSignOnServiceUrl"><span class="required">*</span> {{:: 'single-signon-service-url' | translate}}</label>
                 <div class="col-md-6">