From 654f1f74ff7684fb216893473ceac0930ed09bcb Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Tue, 7 Dec 2021 18:06:53 +0100 Subject: [PATCH] Refactor StorageId Fixes #9031 --- .../java/org/keycloak/storage/StorageId.java | 106 +++++++++++++----- .../org/keycloak/storage/StorageIdTest.java | 88 +++++++++++++++ 2 files changed, 163 insertions(+), 31 deletions(-) create mode 100644 server-spi/src/test/java/org/keycloak/storage/StorageIdTest.java diff --git a/server-spi/src/main/java/org/keycloak/storage/StorageId.java b/server-spi/src/main/java/org/keycloak/storage/StorageId.java index 3a2a14111f..eab722ce81 100644 --- a/server-spi/src/main/java/org/keycloak/storage/StorageId.java +++ b/server-spi/src/main/java/org/keycloak/storage/StorageId.java @@ -21,35 +21,52 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.UserModel; import java.io.Serializable; +import java.util.Objects; /** * @author Bill Burke * @version $Revision: 1 $ */ public class StorageId implements Serializable { - private String id; - private String providerId; - private String externalId; + private final String providerId; + private final String externalId; public StorageId(String id) { - this.id = id; if (!id.startsWith("f:")) { + providerId = null; externalId = id; - return; + } else { + int providerIndex = id.indexOf(':', 2); + providerId = id.substring(2, providerIndex); + externalId = id.substring(providerIndex + 1); } - int providerIndex = id.indexOf(':', 2); - providerId = id.substring(2, providerIndex); - externalId = id.substring(providerIndex + 1); - } public StorageId(String providerId, String externalId) { - this.id = "f:" + providerId + ":" + externalId; + if (providerId != null && providerId.contains(":")) { + throw new IllegalArgumentException("Provider must not contain a colon (:) character"); + } this.providerId = providerId; this.externalId = externalId; } + public boolean isLocal() { + return getProviderId() == null; + } + + public String getId() { + return providerId == null ? externalId : ("f:" + providerId + ":" + externalId); + } + + public String getProviderId() { + return providerId; + } + + public String getExternalId() { + return externalId; + } + /** * generate the id string that should be returned by UserModel.getId() * @@ -68,40 +85,67 @@ public class StorageId implements Serializable { return new StorageId(keycloakId).getProviderId(); } - - - public static String resolveProviderId(UserModel user) { - return new StorageId(user.getId()).getProviderId(); - } - public static boolean isLocalStorage(UserModel user) { - return new StorageId(user.getId()).getProviderId() == null; - } public static boolean isLocalStorage(String id) { return new StorageId(id).getProviderId() == null; } + /** + * @deprecated Use {@link #providerId(String)} instead. + */ + public static String resolveProviderId(UserModel user) { + return providerId(user.getId()); + } + + /** + * @deprecated Use {@link #isLocalStorage(String)} instead. + */ + public static boolean isLocalStorage(UserModel user) { + return isLocalStorage(user.getId()); + } + + /** + * @deprecated Use {@link #providerId(String)} instead. + */ public static String resolveProviderId(ClientModel client) { - return new StorageId(client.getId()).getProviderId(); + return providerId(client.getId()); } + + /** + * @deprecated Use {@link #isLocalStorage(String)} instead. + */ public static boolean isLocalStorage(ClientModel client) { - return new StorageId(client.getId()).getProviderId() == null; - } - public boolean isLocal() { - return getProviderId() == null; - + return isLocalStorage(client.getId()); } - public String getId() { - return id; + @Override + public int hashCode() { + return Objects.hash(providerId, externalId); } - public String getProviderId() { - return providerId; + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + final StorageId other = (StorageId) obj; + if ( ! Objects.equals(this.providerId, other.providerId)) { + return false; + } + if ( ! Objects.equals(this.externalId, other.externalId)) { + return false; + } + return true; } - public String getExternalId() { - return externalId; + @Override + public String toString() { + return getId(); } - } diff --git a/server-spi/src/test/java/org/keycloak/storage/StorageIdTest.java b/server-spi/src/test/java/org/keycloak/storage/StorageIdTest.java new file mode 100644 index 0000000000..c7c84c31a4 --- /dev/null +++ b/server-spi/src/test/java/org/keycloak/storage/StorageIdTest.java @@ -0,0 +1,88 @@ +/* + * Copyright 2021 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.storage; + +import org.keycloak.component.ComponentModel; +import org.junit.Test; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * + * @author hmlnarik + */ +public class StorageIdTest { + + @Test + public void testStatic() { + final String localId = "123"; + assertThat(StorageId.externalId(localId), is("123")); + assertThat(StorageId.providerId(localId), nullValue()); + assertTrue(StorageId.isLocalStorage(localId)); + + final String remoteId = "f:abc:123"; + assertThat(StorageId.externalId(remoteId), is("123")); + assertThat(StorageId.providerId(remoteId), is("abc")); + assertFalse(StorageId.isLocalStorage(remoteId)); + + final ComponentModel cm = new ComponentModel(); + cm.setId("localId"); + assertThat(StorageId.keycloakId(cm, localId), is("f:localId:123")); + } + + @Test + public void testLocalId() { + StorageId id = new StorageId("123"); + assertThat(id, notNullValue()); + assertThat(id.getExternalId(), is("123")); + assertThat(id.getProviderId(), nullValue()); + assertThat(id.getId(), is("123")); + assertTrue(id.isLocal()); + } + + @Test + public void testExternalIdString() { + StorageId id = new StorageId("f:abc:123"); + assertThat(id, notNullValue()); + assertThat(id.getExternalId(), is("123")); + assertThat(id.getProviderId(), is("abc")); + assertThat(id.getId(), is("f:abc:123")); + assertFalse(id.isLocal()); + } + + @Test + public void testExternalIdTwoStrings() { + StorageId id = new StorageId("abc", "123"); + assertThat(id, notNullValue()); + assertThat(id.getExternalId(), is("123")); + assertThat(id.getProviderId(), is("abc")); + assertThat(id.getId(), is("f:abc:123")); + assertFalse(id.isLocal()); + } + + @Test + public void testEquals() { + assertThat(new StorageId("abc", "123"), equalTo(new StorageId("f:abc:123"))); + assertThat(new StorageId("abc", "123"), not(equalTo(new StorageId("f:abc:1234")))); + } +}