KEYCLOAK-8519 OIDCScopeTest.testClientDisplayedOnConsentScreenWithEmptyConsentText failing on Oracle

model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java

@@ -21,20 +21,17 @@ import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientScopeModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
-import org.keycloak.models.ModelException;
 import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleContainerModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.jpa.entities.ClientAttributeEntity;
 import org.keycloak.models.jpa.entities.ClientEntity;
 import org.keycloak.models.jpa.entities.ClientScopeClientMappingEntity;
-import org.keycloak.models.jpa.entities.ClientScopeEntity;
-import org.keycloak.models.jpa.entities.ClientScopeRoleMappingEntity;
 import org.keycloak.models.jpa.entities.ProtocolMapperEntity;
 import org.keycloak.models.jpa.entities.RoleEntity;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
-import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
 
 import javax.persistence.EntityManager;
 import javax.persistence.TypedQuery;
@@ -44,6 +41,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -302,25 +300,45 @@ public class ClientAdapter implements ClientModel, JpaModel<ClientEntity> {
 
     @Override
     public void setAttribute(String name, String value) {
-        entity.getAttributes().put(name, value);
+        for (ClientAttributeEntity attr : entity.getAttributes()) {
+            if (attr.getName().equals(name)) {
+                attr.setValue(value);
+                return;
+            }
+        }
 
+        ClientAttributeEntity attr = new ClientAttributeEntity();
+        attr.setName(name);
+        attr.setValue(value);
+        attr.setClient(entity);
+        em.persist(attr);
+        entity.getAttributes().add(attr);
     }
 
     @Override
     public void removeAttribute(String name) {
-        entity.getAttributes().remove(name);
+        Iterator<ClientAttributeEntity> it = entity.getAttributes().iterator();
+        while (it.hasNext()) {
+            ClientAttributeEntity attr = it.next();
+            if (attr.getName().equals(name)) {
+                it.remove();
+                em.remove(attr);
+            }
+        }
     }
 
     @Override
     public String getAttribute(String name) {
-        return entity.getAttributes().get(name);
+        return getAttributes().get(name);
     }
 
     @Override
     public Map<String, String> getAttributes() {
-        Map<String, String> copy = new HashMap<>();
-        copy.putAll(entity.getAttributes());
-        return copy;
+        Map<String, String> attrs = new HashMap<>();
+        for (ClientAttributeEntity attr : entity.getAttributes()) {
+            attrs.put(attr.getName(), attr.getValue());
+        }
+        return attrs;
     }
 
     @Override

model/jpa/src/main/java/org/keycloak/models/jpa/ClientScopeAdapter.java

@@ -24,6 +24,7 @@ import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleContainerModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.jpa.entities.ClientScopeAttributeEntity;
 import org.keycloak.models.jpa.entities.ClientScopeEntity;
 import org.keycloak.models.jpa.entities.ClientScopeRoleMappingEntity;
 import org.keycloak.models.jpa.entities.ProtocolMapperEntity;
@@ -34,6 +35,7 @@ import javax.persistence.EntityManager;
 import javax.persistence.TypedQuery;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -282,18 +284,37 @@ public class ClientScopeAdapter implements ClientScopeModel, JpaModel<ClientScop
 
     @Override
     public void setAttribute(String name, String value) {
-        entity.getAttributes().put(name, value);
+        for (ClientScopeAttributeEntity attr : entity.getAttributes()) {
+            if (attr.getName().equals(name)) {
+                attr.setValue(value);
+                return;
+            }
+        }
+
+        ClientScopeAttributeEntity attr = new ClientScopeAttributeEntity();
+        attr.setName(name);
+        attr.setValue(value);
+        attr.setClientScope(entity);
+        em.persist(attr);
+        entity.getAttributes().add(attr);
 
     }
 
     @Override
     public void removeAttribute(String name) {
-        entity.getAttributes().remove(name);
+        Iterator<ClientScopeAttributeEntity> it = entity.getAttributes().iterator();
+        while (it.hasNext()) {
+            ClientScopeAttributeEntity attr = it.next();
+            if (attr.getName().equals(name)) {
+                it.remove();
+                em.remove(attr);
+            }
+        }
     }
 
     @Override
     public String getAttribute(String name) {
-        return entity.getAttributes().get(name);
+        return getAttributes().get(name);
     }
 
     public static ClientScopeEntity toClientScopeEntity(ClientScopeModel model, EntityManager em) {
@@ -305,9 +326,11 @@ public class ClientScopeAdapter implements ClientScopeModel, JpaModel<ClientScop
 
     @Override
     public Map<String, String> getAttributes() {
-        Map<String, String> copy = new HashMap<>();
-        copy.putAll(entity.getAttributes());
-        return copy;
+        Map<String, String> attrs = new HashMap<>();
+        for (ClientScopeAttributeEntity attr : entity.getAttributes()) {
+            attrs.put(attr.getName(), attr.getValue());
+        }
+        return attrs;
     }
 
 

model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientAttributeEntity.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.models.jpa.entities;
+
+import java.io.Serializable;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.Id;
+import javax.persistence.IdClass;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+@Table(name="CLIENT_ATTRIBUTES")
+@Entity
+@IdClass(ClientAttributeEntity.Key.class)
+public class ClientAttributeEntity {
+
+    @Id
+    @ManyToOne(fetch= FetchType.LAZY)
+    @JoinColumn(name = "CLIENT_ID")
+    protected ClientEntity client;
+
+    @Id
+    @Column(name="NAME")
+    protected String name;
+
+    @Column(name = "VALUE", length = 4000)
+    protected String value;
+
+    public ClientEntity getClient() {
+        return client;
+    }
+
+    public void setClient(ClientEntity client) {
+        this.client = client;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public String getValue() {
+        return value;
+    }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
+
+
+    public static class Key implements Serializable {
+
+        protected ClientEntity client;
+
+        protected String name;
+
+        public Key() {
+        }
+
+        public Key(ClientEntity client, String name) {
+            this.client = client;
+            this.name = name;
+        }
+
+        public ClientEntity getClient() {
+            return client;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            ClientAttributeEntity.Key key = (ClientAttributeEntity.Key) o;
+
+            if (client != null ? !client.getId().equals(key.client != null ? key.client.getId() : null) : key.client != null) return false;
+            if (name != null ? !name.equals(key.name != null ? key.name : null) : key.name != null) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = client != null ? client.getId().hashCode() : 0;
+            result = 31 * result + (name != null ? name.hashCode() : 0);
+            return result;
+        }
+    }
+
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!(o instanceof ClientAttributeEntity)) return false;
+
+        ClientAttributeEntity key = (ClientAttributeEntity) o;
+
+        if (client != null ? !client.getId().equals(key.client != null ? key.client.getId() : null) : key.client != null) return false;
+        if (name != null ? !name.equals(key.name != null ? key.name : null) : key.name != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = client != null ? client.getId().hashCode() : 0;
+        result = 31 * result + (name != null ? name.hashCode() : 0);
+        return result;
+    }
+}

model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java

@@ -104,11 +104,8 @@ public class ClientEntity {
     @CollectionTable(name = "REDIRECT_URIS", joinColumns={ @JoinColumn(name="CLIENT_ID") })
     protected Set<String> redirectUris = new HashSet<String>();
 
-    @ElementCollection
-    @MapKeyColumn(name="NAME")
-    @Column(name="VALUE", length = 4000)
-    @CollectionTable(name="CLIENT_ATTRIBUTES", joinColumns={ @JoinColumn(name="CLIENT_ID") })
-    protected Map<String, String> attributes = new HashMap<String, String>();
+    @OneToMany(cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "client")
+    protected Collection<ClientAttributeEntity> attributes = new ArrayList<>();
 
     @ElementCollection
     @MapKeyColumn(name="BINDING_NAME")
@@ -278,11 +275,11 @@ public class ClientEntity {
         this.fullScopeAllowed = fullScopeAllowed;
     }
 
-    public Map<String, String> getAttributes() {
+    public Collection<ClientAttributeEntity> getAttributes() {
         return attributes;
     }
 
-    public void setAttributes(Map<String, String> attributes) {
+    public void setAttributes(Collection<ClientAttributeEntity> attributes) {
         this.attributes = attributes;
     }
 

model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeAttributeEntity.java

@@ -0,0 +1,140 @@
+/*
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.models.jpa.entities;
+
+import java.io.Serializable;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.Id;
+import javax.persistence.IdClass;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+@Table(name="CLIENT_SCOPE_ATTRIBUTES")
+@Entity
+@IdClass(ClientScopeAttributeEntity.Key.class)
+public class ClientScopeAttributeEntity {
+
+    @Id
+    @ManyToOne(fetch= FetchType.LAZY)
+    @JoinColumn(name = "SCOPE_ID")
+    protected ClientScopeEntity clientScope;
+
+    @Id
+    @Column(name="NAME")
+    protected String name;
+
+    @Column(name = "VALUE", length = 2048)
+    protected String value;
+
+    public ClientScopeEntity getClientScope() {
+        return clientScope;
+    }
+
+    public void setClientScope(ClientScopeEntity clientScope) {
+        this.clientScope = clientScope;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public String getValue() {
+        return value;
+    }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
+
+
+    public static class Key implements Serializable {
+
+        protected ClientScopeEntity clientScope;
+
+        protected String name;
+
+        public Key() {
+        }
+
+        public Key(ClientScopeEntity clientScope, String name) {
+            this.clientScope = clientScope;
+            this.name = name;
+        }
+
+        public ClientScopeEntity getClientScope() {
+            return clientScope;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            ClientScopeAttributeEntity.Key key = (ClientScopeAttributeEntity.Key) o;
+
+            if (clientScope != null ? !clientScope.getId().equals(key.clientScope != null ? key.clientScope.getId() : null) : key.clientScope != null) return false;
+            if (name != null ? !name.equals(key.name != null ? key.name : null) : key.name != null) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = clientScope != null ? clientScope.getId().hashCode() : 0;
+            result = 31 * result + (name != null ? name.hashCode() : 0);
+            return result;
+        }
+    }
+
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!(o instanceof ClientScopeAttributeEntity)) return false;
+
+        ClientScopeAttributeEntity key = (ClientScopeAttributeEntity) o;
+
+        if (clientScope != null ? !clientScope.getId().equals(key.clientScope != null ? key.clientScope.getId() : null) : key.clientScope != null) return false;
+        if (name != null ? !name.equals(key.name != null ? key.name : null) : key.name != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = clientScope != null ? clientScope.getId().hashCode() : 0;
+        result = 31 * result + (name != null ? name.hashCode() : 0);
+        return result;
+    }
+}

model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientScopeEntity.java

@@ -22,22 +22,17 @@ import org.hibernate.annotations.Nationalized;
 import javax.persistence.Access;
 import javax.persistence.AccessType;
 import javax.persistence.CascadeType;
-import javax.persistence.CollectionTable;
 import javax.persistence.Column;
-import javax.persistence.ElementCollection;
 import javax.persistence.Entity;
 import javax.persistence.FetchType;
 import javax.persistence.Id;
 import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
-import javax.persistence.MapKeyColumn;
 import javax.persistence.OneToMany;
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -66,11 +61,8 @@ public class ClientScopeEntity {
     private String protocol;
 
 
-    @ElementCollection
-    @MapKeyColumn(name="NAME")
-    @Column(name="VALUE", length = 2048)
-    @CollectionTable(name="CLIENT_SCOPE_ATTRIBUTES", joinColumns={ @JoinColumn(name="SCOPE_ID") })
-    protected Map<String, String> attributes = new HashMap<String, String>();
+    @OneToMany(cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "clientScope")
+    protected Collection<ClientScopeAttributeEntity> attributes = new ArrayList<>();
 
     public RealmEntity getRealm() {
         return realm;
@@ -120,11 +112,11 @@ public class ClientScopeEntity {
         this.protocol = protocol;
     }
 
-    public Map<String, String> getAttributes() {
+    public Collection<ClientScopeAttributeEntity> getAttributes() {
         return attributes;
     }
 
-    public void setAttributes(Map<String, String> attributes) {
+    public void setAttributes(Collection<ClientScopeAttributeEntity> attributes) {
         this.attributes = attributes;
     }
 

model/jpa/src/main/resources/META-INF/persistence.xml

@@ -21,6 +21,7 @@
     version="1.0">
     <persistence-unit name="keycloak-default" transaction-type="RESOURCE_LOCAL">
         <class>org.keycloak.models.jpa.entities.ClientEntity</class>
+        <class>org.keycloak.models.jpa.entities.ClientAttributeEntity</class>
         <class>org.keycloak.models.jpa.entities.CredentialEntity</class>
         <class>org.keycloak.models.jpa.entities.CredentialAttributeEntity</class>
         <class>org.keycloak.models.jpa.entities.RealmEntity</class>
@@ -54,6 +55,7 @@
         <class>org.keycloak.models.jpa.entities.GroupRoleMappingEntity</class>
         <class>org.keycloak.models.jpa.entities.UserGroupMembershipEntity</class>
         <class>org.keycloak.models.jpa.entities.ClientScopeEntity</class>
+        <class>org.keycloak.models.jpa.entities.ClientScopeAttributeEntity</class>
         <class>org.keycloak.models.jpa.entities.ClientScopeRoleMappingEntity</class>
         <class>org.keycloak.models.jpa.entities.ClientScopeClientMappingEntity</class>
         <class>org.keycloak.models.jpa.entities.DefaultClientScopeRealmMappingEntity</class>

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java

@@ -22,6 +22,7 @@ import org.junit.Test;
 import org.keycloak.admin.client.resource.ClientScopesResource;
 import org.keycloak.admin.client.resource.ProtocolMappersResource;
 import org.keycloak.admin.client.resource.RoleMappingResource;
+import org.keycloak.common.util.ObjectUtil;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
 import org.keycloak.models.AccountRoles;
@@ -135,12 +136,19 @@ public class ClientScopeTest extends AbstractClientTest {
         scopeRep.setName("scope1");
         scopeRep.setDescription("scope1-desc");
         scopeRep.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL);
+
+        Map<String, String> attrs = new HashMap<>();
+        attrs.put("someAttr", "someAttrValue");
+        attrs.put("emptyAttr", "");
+        scopeRep.setAttributes(attrs);
         String scope1Id = createClientScope(scopeRep);
 
         // Assert created attributes
         scopeRep = clientScopes().get(scope1Id).toRepresentation();
         Assert.assertEquals("scope1", scopeRep.getName());
         Assert.assertEquals("scope1-desc", scopeRep.getDescription());
+        Assert.assertEquals("someAttrValue", scopeRep.getAttributes().get("someAttr"));
+        Assert.assertTrue(ObjectUtil.isBlank(scopeRep.getAttributes().get("emptyAttr")));
         Assert.assertEquals(OIDCLoginProtocol.LOGIN_PROTOCOL, scopeRep.getProtocol());
 
 
@@ -149,6 +157,9 @@ public class ClientScopeTest extends AbstractClientTest {
         scopeRep.setDescription("scope1-desc-updated");
         scopeRep.setProtocol(SamlProtocol.LOGIN_PROTOCOL);
 
+        // Test update attribute to some non-blank value
+        scopeRep.getAttributes().put("emptyAttr", "someValue");
+
         clientScopes().get(scope1Id).update(scopeRep);
 
         assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientScopeResourcePath(scope1Id), scopeRep, ResourceType.CLIENT_SCOPE);
@@ -158,6 +169,8 @@ public class ClientScopeTest extends AbstractClientTest {
         Assert.assertEquals("scope1-updated", scopeRep.getName());
         Assert.assertEquals("scope1-desc-updated", scopeRep.getDescription());
         Assert.assertEquals(SamlProtocol.LOGIN_PROTOCOL, scopeRep.getProtocol());
+        Assert.assertEquals("someAttrValue", scopeRep.getAttributes().get("someAttr"));
+        Assert.assertEquals("someValue", scopeRep.getAttributes().get("emptyAttr"));
 
         // Remove scope1
         clientScopes().get(scope1Id).remove();