This is an automated email from the ASF dual-hosted git repository. sergehuber pushed a commit to branch UNOMI-875-pr4-security-tenant in repository https://gitbox.apache.org/repos/asf/unomi.git
commit ecb26a0176fa2929d6ad248110b177a663ae0622 Author: Serge Huber <[email protected]> AuthorDate: Sun Jul 5 14:54:52 2026 +0200 UNOMI-938: Hash API keys at rest and harden TenantService Store PBKDF2 key hashes and masked keys instead of plaintext in persistence. Return plaintext only once via ApiKeyCreationResult; suppress keyHash in REST via ApiKeyRestMixIn. Add legacy key read support and migrate-3.1.0-20 rehash script. TenantService null guards, create/delete error contracts, and IT updates. --- .../unomi/api/security/ApiKeyHashService.java | 61 +++++++++ .../java/org/apache/unomi/api/tenants/ApiKey.java | 63 +++++++-- .../unomi/api/tenants/ApiKeyCreationResult.java | 71 ++++++++++ .../java/org/apache/unomi/api/tenants/Tenant.java | 12 +- .../apache/unomi/api/tenants/TenantService.java | 12 +- .../org/apache/unomi/api/tenants/TenantTest.java | 76 +++++------ .../test/java/org/apache/unomi/itests/BaseIT.java | 34 +++-- .../test/java/org/apache/unomi/itests/BasicIT.java | 4 +- .../org/apache/unomi/itests/ContextServletIT.java | 29 ++-- .../java/org/apache/unomi/itests/TenantIT.java | 82 +++++++----- .../apache/unomi/itests/V2CompatibilityModeIT.java | 12 +- .../httpclient/HttpClientThatWaitsForUnomi.java | 15 ++- .../apache/unomi/rest/server/ApiKeyRestMixIn.java | 31 +++++ .../org/apache/unomi/rest/server/RestServer.java | 2 + .../apache/unomi/rest/tenants/TenantEndpoint.java | 5 +- .../common/security/ApiKeyHashServiceImpl.java | 104 +++++++++++++++ .../resources/OSGI-INF/blueprint/blueprint.xml | 9 ++ .../common/security/ApiKeyHashServiceImplTest.java | 96 +++++++++++++ .../services/impl/tenants/TenantServiceImpl.java | 78 +++++++---- .../resources/OSGI-INF/blueprint/blueprint.xml | 2 + .../unomi/services/impl/TestTenantService.java | 36 ++--- .../unomi/services/impl/TestTenantServiceTest.java | 33 ++--- .../impl/tenants/TenantServiceImplTest.java | 148 +++++++++++++++++++++ .../migrate-3.1.0-10-tenantInitialization.groovy | 43 +++++- .../migration/migrate-3.1.0-20-hashApiKeys.groovy | 126 ++++++++++++++++++ .../dev/commands/apikeys/ApiKeyCrudCommand.java | 18 ++- 26 files changed, 1004 insertions(+), 198 deletions(-) diff --git a/api/src/main/java/org/apache/unomi/api/security/ApiKeyHashService.java b/api/src/main/java/org/apache/unomi/api/security/ApiKeyHashService.java new file mode 100644 index 000000000..b257de52e --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/security/ApiKeyHashService.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.api.security; + +/** + * Service for hashing and verifying API keys so that only salted hashes are persisted, + * never the plaintext key value (see UNOMI-938). + */ +public interface ApiKeyHashService { + + /** + * Generates a new plaintext API key value. + * The returned value is only ever available in memory; callers are responsible for + * hashing it via {@link #hash(String)} before persisting anything and for returning + * the plaintext value to the caller exactly once. + * + * @return a newly generated plaintext API key + */ + String generateKey(); + + /** + * Hashes a plaintext API key for storage. + * + * @param plainTextKey the plaintext API key to hash + * @return the salted hash, in the format "iterations:base64(salt):base64(hash)" + */ + String hash(String plainTextKey); + + /** + * Verifies a plaintext API key against a previously computed hash, using a + * constant-time comparison to avoid timing attacks. + * + * @param plainTextKey the plaintext API key to verify + * @param storedHash the stored hash to verify against, as produced by {@link #hash(String)} + * @return {@code true} if the key matches the hash, {@code false} otherwise + */ + boolean verify(String plainTextKey, String storedHash); + + /** + * Produces a display-safe masked representation of a plaintext API key, suitable for + * showing in UIs and logs without exposing the secret (e.g. "unomi_v1_****ab12"). + * + * @param plainTextKey the plaintext API key to mask + * @return the masked key + */ + String mask(String plainTextKey); +} diff --git a/api/src/main/java/org/apache/unomi/api/tenants/ApiKey.java b/api/src/main/java/org/apache/unomi/api/tenants/ApiKey.java index 0d05dc5cc..674c4ad98 100644 --- a/api/src/main/java/org/apache/unomi/api/tenants/ApiKey.java +++ b/api/src/main/java/org/apache/unomi/api/tenants/ApiKey.java @@ -16,6 +16,7 @@ */ package org.apache.unomi.api.tenants; +import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.unomi.api.Item; import java.util.Date; @@ -47,9 +48,25 @@ public class ApiKey extends Item { } /** - * The API key value. + * The salted hash of the API key, in the format "iterations:base64(salt):base64(hash)". + * The plaintext key is never persisted; it is only returned once at creation time. */ - private String key; + private String keyHash; + + /** + * A display-safe, masked representation of the key (e.g. "unomi_v1_****ab12"), + * suitable for showing in UIs and logs without exposing the secret. + */ + private String maskedKey; + + /** + * Legacy plaintext key, populated only when deserializing documents created before + * API keys were hashed at rest (see UNOMI-938). It is read from the legacy "key" JSON + * property so that existing keys keep validating until the hashing migration runs, + * but it is never written back out. + */ + @JsonProperty(value = "key", access = JsonProperty.Access.READ_ONLY) + String legacyKey; /** * The type of API key (public or private). @@ -90,19 +107,45 @@ public class ApiKey extends Item { } /** - * Gets the API key value. - * @return the API key value + * Gets the salted hash of the API key. + * @return the key hash, in the format "iterations:base64(salt):base64(hash)" + */ + public String getKeyHash() { + return keyHash; + } + + /** + * Sets the salted hash of the API key. + * @param keyHash the key hash to set + */ + public void setKeyHash(String keyHash) { + this.keyHash = keyHash; + } + + /** + * Gets the display-safe masked representation of the key. + * @return the masked key (e.g. "unomi_v1_****ab12") + */ + public String getMaskedKey() { + return maskedKey; + } + + /** + * Sets the display-safe masked representation of the key. + * @param maskedKey the masked key to set */ - public String getKey() { - return key; + public void setMaskedKey(String maskedKey) { + this.maskedKey = maskedKey; } /** - * Sets the API key value. - * @param key the API key value to set + * Gets the legacy plaintext key, if this key was created before hashing at rest was + * introduced (UNOMI-938) and has not yet been migrated. Returns {@code null} once + * {@link #getKeyHash()} is populated. + * @return the legacy plaintext key, or {@code null} if not present */ - public void setKey(String key) { - this.key = key; + public String getLegacyKey() { + return legacyKey; } /** diff --git a/api/src/main/java/org/apache/unomi/api/tenants/ApiKeyCreationResult.java b/api/src/main/java/org/apache/unomi/api/tenants/ApiKeyCreationResult.java new file mode 100644 index 000000000..69d48e0af --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/tenants/ApiKeyCreationResult.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.api.tenants; + +/** + * Result of an API key creation operation. + * Carries the persisted {@link ApiKey} metadata (which only stores a hash and a masked + * representation of the key) together with the one-time plaintext key value. The plaintext + * key is only ever available at creation time; it cannot be recovered afterwards since it + * is not persisted (see UNOMI-938). + */ +public class ApiKeyCreationResult { + + private ApiKey apiKey; + private String plainTextKey; + + public ApiKeyCreationResult() { + } + + public ApiKeyCreationResult(ApiKey apiKey, String plainTextKey) { + this.apiKey = apiKey; + this.plainTextKey = plainTextKey; + } + + /** + * Gets the persisted API key metadata (type, masked key, dates, etc.), without the secret. + * @return the API key metadata + */ + public ApiKey getApiKey() { + return apiKey; + } + + /** + * Sets the persisted API key metadata. + * @param apiKey the API key metadata to set + */ + public void setApiKey(ApiKey apiKey) { + this.apiKey = apiKey; + } + + /** + * Gets the one-time plaintext key value. This is only available right after creation; + * it is never persisted and cannot be retrieved again afterwards. + * @return the plaintext API key + */ + public String getPlainTextKey() { + return plainTextKey; + } + + /** + * Sets the one-time plaintext key value. + * @param plainTextKey the plaintext API key to set + */ + public void setPlainTextKey(String plainTextKey) { + this.plainTextKey = plainTextKey; + } +} diff --git a/api/src/main/java/org/apache/unomi/api/tenants/Tenant.java b/api/src/main/java/org/apache/unomi/api/tenants/Tenant.java index f38b9b8d7..608449a72 100644 --- a/api/src/main/java/org/apache/unomi/api/tenants/Tenant.java +++ b/api/src/main/java/org/apache/unomi/api/tenants/Tenant.java @@ -273,7 +273,9 @@ public class Tenant extends Item { * This method resolves the active private API key from the API keys list. * It returns the most recently created, non-revoked, non-expired private key. * This key should be used for secure operations and administrative tasks. - * @return the active private API key, or null if no valid private key exists + * Since the plaintext key is never persisted (see UNOMI-938), this returns the + * display-safe masked key rather than the secret itself. + * @return the active private API key (masked), or null if no valid private key exists */ @XmlTransient public String getPrivateApiKey() { @@ -286,7 +288,7 @@ public class Tenant extends Item { .filter(key -> !key.isRevoked()) .filter(key -> key.getExpirationDate() == null || key.getExpirationDate().after(new Date())) .max(Comparator.comparing(ApiKey::getCreationDate)) - .map(ApiKey::getKey) + .map(ApiKey::getMaskedKey) .orElse(null); } @@ -295,7 +297,9 @@ public class Tenant extends Item { * This method resolves the active public API key from the API keys list. * It returns the most recently created, non-revoked, non-expired public key. * This key can be safely used in client-side applications. - * @return the active public API key, or null if no valid public key exists + * Since the plaintext key is never persisted (see UNOMI-938), this returns the + * display-safe masked key rather than the secret itself. + * @return the active public API key (masked), or null if no valid public key exists */ @XmlTransient public String getPublicApiKey() { @@ -308,7 +312,7 @@ public class Tenant extends Item { .filter(key -> !key.isRevoked()) .filter(key -> key.getExpirationDate() == null || key.getExpirationDate().after(new Date())) .max(Comparator.comparing(ApiKey::getCreationDate)) - .map(ApiKey::getKey) + .map(ApiKey::getMaskedKey) .orElse(null); } diff --git a/api/src/main/java/org/apache/unomi/api/tenants/TenantService.java b/api/src/main/java/org/apache/unomi/api/tenants/TenantService.java index a730b99b0..a5fdb0ab5 100644 --- a/api/src/main/java/org/apache/unomi/api/tenants/TenantService.java +++ b/api/src/main/java/org/apache/unomi/api/tenants/TenantService.java @@ -45,13 +45,15 @@ public interface TenantService { /** * Generates a new API key for the specified tenant with an optional validity period. + * The plaintext key is only ever available on the returned result; it is not persisted + * and cannot be retrieved again afterwards (see UNOMI-938). * * @param tenantId the ID of the tenant for which to generate the API key * @param validityPeriod the period (in milliseconds) for which the API key should be valid, null for no expiration - * @return the generated ApiKey object containing the key and associated metadata + * @return the generated key metadata together with its one-time plaintext value * @throws IllegalArgumentException if tenantId is null or does not exist */ - ApiKey generateApiKey(String tenantId, Long validityPeriod); + ApiKeyCreationResult generateApiKey(String tenantId, Long validityPeriod); /** * Retrieves a tenant by its ID. @@ -97,14 +99,16 @@ public interface TenantService { /** * Generates a new API key of the specified type for the tenant. + * The plaintext key is only ever available on the returned result; it is not persisted + * and cannot be retrieved again afterwards (see UNOMI-938). * * @param tenantId the ID of the tenant for which to generate the API key * @param keyType the type of API key to generate (PUBLIC or PRIVATE) * @param validityPeriod the period (in milliseconds) for which the API key should be valid, null for no expiration - * @return the generated ApiKey object containing the key and associated metadata + * @return the generated key metadata together with its one-time plaintext value * @throws IllegalArgumentException if tenantId is null or does not exist */ - ApiKey generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod); + ApiKeyCreationResult generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod); /** * Validates an API key for a given tenant and checks if it has the required type. diff --git a/api/src/test/java/org/apache/unomi/api/tenants/TenantTest.java b/api/src/test/java/org/apache/unomi/api/tenants/TenantTest.java index d5cd9cc23..7d77921b4 100644 --- a/api/src/test/java/org/apache/unomi/api/tenants/TenantTest.java +++ b/api/src/test/java/org/apache/unomi/api/tenants/TenantTest.java @@ -48,7 +48,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey publicKey = new ApiKey(); - publicKey.setKey("public-key-1"); + publicKey.setMaskedKey("public-key-1"); publicKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); publicKey.setRevoked(false); publicKey.setCreationDate(new Date()); @@ -65,7 +65,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey revokedKey = new ApiKey(); - revokedKey.setKey("private-key-1"); + revokedKey.setMaskedKey("private-key-1"); revokedKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); revokedKey.setRevoked(true); revokedKey.setCreationDate(new Date()); @@ -82,7 +82,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey expiredKey = new ApiKey(); - expiredKey.setKey("private-key-1"); + expiredKey.setMaskedKey("private-key-1"); expiredKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); expiredKey.setRevoked(false); expiredKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); // Expired @@ -100,7 +100,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey validKey = new ApiKey(); - validKey.setKey("private-key-1"); + validKey.setMaskedKey("private-key-1"); validKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); validKey.setRevoked(false); validKey.setCreationDate(new Date()); @@ -120,14 +120,14 @@ public class TenantTest { Date newDate = new Date(); ApiKey oldKey = new ApiKey(); - oldKey.setKey("private-key-old"); + oldKey.setMaskedKey("private-key-old"); oldKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); oldKey.setRevoked(false); oldKey.setCreationDate(oldDate); apiKeys.add(oldKey); ApiKey newKey = new ApiKey(); - newKey.setKey("private-key-new"); + newKey.setMaskedKey("private-key-new"); newKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); newKey.setRevoked(false); newKey.setCreationDate(newDate); @@ -144,7 +144,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey validKey = new ApiKey(); - validKey.setKey("private-key-1"); + validKey.setMaskedKey("private-key-1"); validKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); validKey.setRevoked(false); validKey.setCreationDate(new Date()); @@ -193,7 +193,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey privateKey = new ApiKey(); - privateKey.setKey("private-key-1"); + privateKey.setMaskedKey("private-key-1"); privateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); privateKey.setRevoked(false); privateKey.setCreationDate(new Date()); @@ -210,7 +210,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey validKey = new ApiKey(); - validKey.setKey("public-key-1"); + validKey.setMaskedKey("public-key-1"); validKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); validKey.setRevoked(false); validKey.setCreationDate(new Date()); @@ -227,7 +227,7 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey validKey = new ApiKey(); - validKey.setKey("public-key-1"); + validKey.setMaskedKey("public-key-1"); validKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); validKey.setRevoked(false); validKey.setCreationDate(new Date()); @@ -261,26 +261,26 @@ public class TenantTest { // Add various private keys ApiKey revokedKey = new ApiKey(); - revokedKey.setKey("revoked-private"); + revokedKey.setMaskedKey("revoked-private"); revokedKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); revokedKey.setRevoked(true); apiKeys.add(revokedKey); ApiKey expiredKey = new ApiKey(); - expiredKey.setKey("expired-private"); + expiredKey.setMaskedKey("expired-private"); expiredKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); expiredKey.setRevoked(false); expiredKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); apiKeys.add(expiredKey); ApiKey validKey1 = new ApiKey(); - validKey1.setKey("valid-private-1"); + validKey1.setMaskedKey("valid-private-1"); validKey1.setKeyType(ApiKey.ApiKeyType.PRIVATE); validKey1.setRevoked(false); apiKeys.add(validKey1); ApiKey validKey2 = new ApiKey(); - validKey2.setKey("valid-private-2"); + validKey2.setMaskedKey("valid-private-2"); validKey2.setKeyType(ApiKey.ApiKeyType.PRIVATE); validKey2.setRevoked(false); apiKeys.add(validKey2); @@ -289,8 +289,8 @@ public class TenantTest { List<ApiKey> activeKeys = tenant.getActivePrivateApiKeys(); assertEquals("Should return 2 active private keys", 2, activeKeys.size()); - assertTrue("Should contain valid-private-1", activeKeys.stream().anyMatch(key -> "valid-private-1".equals(key.getKey()))); - assertTrue("Should contain valid-private-2", activeKeys.stream().anyMatch(key -> "valid-private-2".equals(key.getKey()))); + assertTrue("Should contain valid-private-1", activeKeys.stream().anyMatch(key -> "valid-private-1".equals(key.getMaskedKey()))); + assertTrue("Should contain valid-private-2", activeKeys.stream().anyMatch(key -> "valid-private-2".equals(key.getMaskedKey()))); } @Test @@ -300,26 +300,26 @@ public class TenantTest { // Add various public keys ApiKey revokedKey = new ApiKey(); - revokedKey.setKey("revoked-public"); + revokedKey.setMaskedKey("revoked-public"); revokedKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); revokedKey.setRevoked(true); apiKeys.add(revokedKey); ApiKey expiredKey = new ApiKey(); - expiredKey.setKey("expired-public"); + expiredKey.setMaskedKey("expired-public"); expiredKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); expiredKey.setRevoked(false); expiredKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); apiKeys.add(expiredKey); ApiKey validKey1 = new ApiKey(); - validKey1.setKey("valid-public-1"); + validKey1.setMaskedKey("valid-public-1"); validKey1.setKeyType(ApiKey.ApiKeyType.PUBLIC); validKey1.setRevoked(false); apiKeys.add(validKey1); ApiKey validKey2 = new ApiKey(); - validKey2.setKey("valid-public-2"); + validKey2.setMaskedKey("valid-public-2"); validKey2.setKeyType(ApiKey.ApiKeyType.PUBLIC); validKey2.setRevoked(false); apiKeys.add(validKey2); @@ -328,8 +328,8 @@ public class TenantTest { List<ApiKey> activeKeys = tenant.getActivePublicApiKeys(); assertEquals("Should return 2 active public keys", 2, activeKeys.size()); - assertTrue("Should contain valid-public-1", activeKeys.stream().anyMatch(key -> "valid-public-1".equals(key.getKey()))); - assertTrue("Should contain valid-public-2", activeKeys.stream().anyMatch(key -> "valid-public-2".equals(key.getKey()))); + assertTrue("Should contain valid-public-1", activeKeys.stream().anyMatch(key -> "valid-public-1".equals(key.getMaskedKey()))); + assertTrue("Should contain valid-public-2", activeKeys.stream().anyMatch(key -> "valid-public-2".equals(key.getMaskedKey()))); } @Test @@ -339,26 +339,26 @@ public class TenantTest { // Add various keys ApiKey revokedPrivateKey = new ApiKey(); - revokedPrivateKey.setKey("revoked-private"); + revokedPrivateKey.setMaskedKey("revoked-private"); revokedPrivateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); revokedPrivateKey.setRevoked(true); apiKeys.add(revokedPrivateKey); ApiKey expiredPublicKey = new ApiKey(); - expiredPublicKey.setKey("expired-public"); + expiredPublicKey.setMaskedKey("expired-public"); expiredPublicKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); expiredPublicKey.setRevoked(false); expiredPublicKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); apiKeys.add(expiredPublicKey); ApiKey validPrivateKey = new ApiKey(); - validPrivateKey.setKey("valid-private"); + validPrivateKey.setMaskedKey("valid-private"); validPrivateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); validPrivateKey.setRevoked(false); apiKeys.add(validPrivateKey); ApiKey validPublicKey = new ApiKey(); - validPublicKey.setKey("valid-public"); + validPublicKey.setMaskedKey("valid-public"); validPublicKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); validPublicKey.setRevoked(false); apiKeys.add(validPublicKey); @@ -367,8 +367,8 @@ public class TenantTest { List<ApiKey> activeKeys = tenant.getActiveApiKeys(); assertEquals("Should return 2 active keys", 2, activeKeys.size()); - assertTrue("Should contain valid-private", activeKeys.stream().anyMatch(key -> "valid-private".equals(key.getKey()))); - assertTrue("Should contain valid-public", activeKeys.stream().anyMatch(key -> "valid-public".equals(key.getKey()))); + assertTrue("Should contain valid-private", activeKeys.stream().anyMatch(key -> "valid-private".equals(key.getMaskedKey()))); + assertTrue("Should contain valid-public", activeKeys.stream().anyMatch(key -> "valid-public".equals(key.getMaskedKey()))); } @Test @@ -407,14 +407,14 @@ public class TenantTest { List<ApiKey> apiKeys = new ArrayList<>(); ApiKey privateKey = new ApiKey(); - privateKey.setKey("private-key"); + privateKey.setMaskedKey("private-key"); privateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); privateKey.setRevoked(false); privateKey.setCreationDate(new Date()); apiKeys.add(privateKey); ApiKey publicKey = new ApiKey(); - publicKey.setKey("public-key"); + publicKey.setMaskedKey("public-key"); publicKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); publicKey.setRevoked(false); publicKey.setCreationDate(new Date()); @@ -433,7 +433,7 @@ public class TenantTest { // Create a key that expires in the future ApiKey futureExpiringKey = new ApiKey(); - futureExpiringKey.setKey("future-expiring-key"); + futureExpiringKey.setMaskedKey("future-expiring-key"); futureExpiringKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); futureExpiringKey.setRevoked(false); futureExpiringKey.setExpirationDate(new Date(System.currentTimeMillis() + 10000)); // 10 seconds in future @@ -442,7 +442,7 @@ public class TenantTest { // Create a key that has already expired ApiKey expiredKey = new ApiKey(); - expiredKey.setKey("expired-key"); + expiredKey.setMaskedKey("expired-key"); expiredKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); expiredKey.setRevoked(false); expiredKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); // 1 second ago @@ -461,14 +461,14 @@ public class TenantTest { // Add various types of keys ApiKey revokedPrivateKey = new ApiKey(); - revokedPrivateKey.setKey("revoked-private"); + revokedPrivateKey.setMaskedKey("revoked-private"); revokedPrivateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); revokedPrivateKey.setRevoked(true); revokedPrivateKey.setCreationDate(new Date(System.currentTimeMillis() - 5000)); apiKeys.add(revokedPrivateKey); ApiKey expiredPrivateKey = new ApiKey(); - expiredPrivateKey.setKey("expired-private"); + expiredPrivateKey.setMaskedKey("expired-private"); expiredPrivateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); expiredPrivateKey.setRevoked(false); expiredPrivateKey.setExpirationDate(new Date(System.currentTimeMillis() - 1000)); @@ -476,14 +476,14 @@ public class TenantTest { apiKeys.add(expiredPrivateKey); ApiKey validPrivateKey = new ApiKey(); - validPrivateKey.setKey("valid-private"); + validPrivateKey.setMaskedKey("valid-private"); validPrivateKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); validPrivateKey.setRevoked(false); validPrivateKey.setCreationDate(new Date()); apiKeys.add(validPrivateKey); ApiKey validPublicKey = new ApiKey(); - validPublicKey.setKey("valid-public"); + validPublicKey.setMaskedKey("valid-public"); validPublicKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); validPublicKey.setRevoked(false); validPublicKey.setCreationDate(new Date()); @@ -505,7 +505,7 @@ public class TenantTest { // Create an older valid key ApiKey oldKey = new ApiKey(); - oldKey.setKey("old-key"); + oldKey.setMaskedKey("old-key"); oldKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); oldKey.setRevoked(false); oldKey.setCreationDate(oldDate); @@ -513,7 +513,7 @@ public class TenantTest { // Create a newer valid key ApiKey newKey = new ApiKey(); - newKey.setKey("new-key"); + newKey.setMaskedKey("new-key"); newKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); newKey.setRevoked(false); newKey.setCreationDate(newDate); diff --git a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java index e5d9e76a4..4fa624da5 100644 --- a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java @@ -48,6 +48,7 @@ import org.apache.unomi.api.rules.Rule; import org.apache.unomi.api.security.SecurityService; import org.apache.unomi.api.services.*; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.api.utils.ConditionBuilder; @@ -176,6 +177,9 @@ public abstract class BaseIT extends KarafTestSupport { protected Tenant testTenant; protected ApiKey testPublicKey; protected ApiKey testPrivateKey; + // One-time plaintext values of testPublicKey/testPrivateKey, captured at generation time (UNOMI-938). + protected String testPublicKeyValue; + protected String testPrivateKeyValue; protected SchedulerService schedulerService; protected static final String TEST_TENANT_ID = "itTestTenant"; @@ -292,9 +296,13 @@ public abstract class BaseIT extends KarafTestSupport { if (testTenant == null) { testTenant = tenantService.createTenant(TEST_TENANT_ID, Collections.emptyMap()); } - // Get the API keys - testPublicKey = tenantService.getApiKey(testTenant.getItemId(), ApiKey.ApiKeyType.PUBLIC); - testPrivateKey = tenantService.getApiKey(testTenant.getItemId(), ApiKey.ApiKeyType.PRIVATE); + // Generate fresh API keys and capture their one-time plaintext values (UNOMI-938). + ApiKeyCreationResult publicKeyResult = tenantService.generateApiKeyWithType(testTenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKeyCreationResult privateKeyResult = tenantService.generateApiKeyWithType(testTenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + testPublicKey = publicKeyResult.getApiKey(); + testPrivateKey = privateKeyResult.getApiKey(); + testPublicKeyValue = publicKeyResult.getPlainTextKey(); + testPrivateKeyValue = privateKeyResult.getPlainTextKey(); // Make sure the tenant is available for querying. persistenceService.refresh(); @@ -308,7 +316,7 @@ public abstract class BaseIT extends KarafTestSupport { enableCamelDebugIfRequested(); // Set up test tenant for HttpClientThatWaitsForUnomi - HttpClientThatWaitsForUnomi.setTestTenant(testTenant, testPublicKey, testPrivateKey); + HttpClientThatWaitsForUnomi.setTestTenant(testTenant, testPublicKeyValue, testPrivateKeyValue); // init httpClient without credentials provider - all auth handled via headers httpClient = initHttpClient(null); @@ -376,6 +384,8 @@ public abstract class BaseIT extends KarafTestSupport { testTenant = null; testPublicKey = null; testPrivateKey = null; + testPublicKeyValue = null; + testPrivateKeyValue = null; } catch (Exception e) { LOGGER.error("Error cleaning up test tenant", e); } @@ -1401,16 +1411,16 @@ public abstract class BaseIT extends KarafTestSupport { // Remove any existing auth headers first request.removeHeaders("Authorization"); // Only set X-Unomi-Api-Key header if it's not already set - if (request.getFirstHeader("X-Unomi-Api-Key") == null && testPublicKey != null) { - request.setHeader("X-Unomi-Api-Key", testPublicKey.getKey()); + if (request.getFirstHeader("X-Unomi-Api-Key") == null && testPublicKeyValue != null) { + request.setHeader("X-Unomi-Api-Key", testPublicKeyValue); } break; case PRIVATE_KEY: // Remove any existing auth headers first request.removeHeaders("X-Unomi-Api-Key"); // Only set Authorization header if it's not already set - if (request.getFirstHeader("Authorization") == null && testPrivateKey != null) { - String credentials = TEST_TENANT_ID + ":" + testPrivateKey.getKey(); + if (request.getFirstHeader("Authorization") == null && testPrivateKeyValue != null) { + String credentials = TEST_TENANT_ID + ":" + testPrivateKeyValue; request.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString(credentials.getBytes())); } break; @@ -1454,8 +1464,8 @@ public abstract class BaseIT extends KarafTestSupport { // Public endpoint - use public key request.removeHeaders("Authorization"); // Only set X-Unomi-Api-Key header if it's not already set - if (request.getFirstHeader("X-Unomi-Api-Key") == null && testPublicKey != null) { - request.setHeader("X-Unomi-Api-Key", testPublicKey.getKey()); + if (request.getFirstHeader("X-Unomi-Api-Key") == null && testPublicKeyValue != null) { + request.setHeader("X-Unomi-Api-Key", testPublicKeyValue); } } else if (normalizedPath.startsWith("/tenants")) { // Admin endpoint - use JAAS admin @@ -1469,8 +1479,8 @@ public abstract class BaseIT extends KarafTestSupport { // Private endpoint - use private key request.removeHeaders("X-Unomi-Api-Key"); // Only set Authorization header if it's not already set - if (request.getFirstHeader("Authorization") == null && testPrivateKey != null) { - String privateCredentials = TEST_TENANT_ID + ":" + testPrivateKey.getKey(); + if (request.getFirstHeader("Authorization") == null && testPrivateKeyValue != null) { + String privateCredentials = TEST_TENANT_ID + ":" + testPrivateKeyValue; request.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString(privateCredentials.getBytes())); } } diff --git a/itests/src/test/java/org/apache/unomi/itests/BasicIT.java b/itests/src/test/java/org/apache/unomi/itests/BasicIT.java index 9d0408352..bd650359b 100644 --- a/itests/src/test/java/org/apache/unomi/itests/BasicIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/BasicIT.java @@ -199,7 +199,7 @@ public class BasicIT extends BaseIT { EMAIL_VISITOR_1, SESSION_ID_3); HttpPost requestLoginVisitor1 = new HttpPost(getFullUrl("/cxs/context.json")); requestLoginVisitor1.addHeader("Cookie", requestResponsePageView1.getCookieHeaderValue()); - requestLoginVisitor1.addHeader("X-Unomi-Api-Key", testPublicKey.getKey()); + requestLoginVisitor1.addHeader("X-Unomi-Api-Key", testPublicKeyValue); requestLoginVisitor1.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequestLoginVisitor1), ContentType.create("application/json"))); TestUtils.RequestResponse requestResponseLoginVisitor1 = executeContextJSONRequest(requestLoginVisitor1, SESSION_ID_3); @@ -253,7 +253,7 @@ public class BasicIT extends BaseIT { EMAIL_VISITOR_2, SESSION_ID_4); HttpPost requestLoginVisitor2 = new HttpPost(getFullUrl("/cxs/context.json")); requestLoginVisitor2.addHeader("Cookie", requestResponsePageView1.getCookieHeaderValue()); - requestLoginVisitor2.addHeader("X-Unomi-Api-Key", testPublicKey.getKey()); + requestLoginVisitor2.addHeader("X-Unomi-Api-Key", testPublicKeyValue); requestLoginVisitor2.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequestLoginVisitor2), ContentType.create("application/json"))); TestUtils.RequestResponse requestResponseLoginVisitor2 = executeContextJSONRequest(requestLoginVisitor2, SESSION_ID_4); diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java index 0c8beb2eb..e714ac218 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java @@ -37,6 +37,7 @@ import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.segments.Scoring; import org.apache.unomi.api.segments.Segment; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.itests.TestUtils.RequestResponse; import org.junit.After; @@ -170,7 +171,7 @@ public class ContextServletIT extends BaseIT { contextRequest.setSessionId(session.getItemId()); contextRequest.setEvents(Arrays.asList(event)); HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); executeContextJSONRequest(request, sessionId, -1, false); @@ -181,7 +182,7 @@ public class ContextServletIT extends BaseIT { } private void addPublicTenantAuth(HttpPost request) { - request.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKey.getKey()); + request.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); } @Test @@ -460,7 +461,7 @@ public class ContextServletIT extends BaseIT { //Act HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); executeContextJSONRequest(request, null, -1, false); @@ -490,7 +491,7 @@ public class ContextServletIT extends BaseIT { //Act HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); executeContextJSONRequest(request); @@ -517,7 +518,7 @@ public class ContextServletIT extends BaseIT { //Act HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); executeContextJSONRequest(request); @@ -859,8 +860,8 @@ public class ContextServletIT extends BaseIT { public void testContextEndpointAuthentication() throws Exception { // Create a tenant for testing Tenant tenant = tenantService.createTenant("TestTenant", Collections.emptyMap()); - ApiKey publicKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); - ApiKey privateKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + ApiKeyCreationResult publicKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKeyCreationResult privateKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); // Test without any authentication ContextRequest contextRequest = new ContextRequest(); @@ -893,7 +894,7 @@ public class ContextServletIT extends BaseIT { Assert.assertEquals("JAAS authenticated request should succeed", 200, jaasResponse.getStatusLine().getStatusCode()); // Test with public API key (should succeed) - contextRequest.setPublicApiKey(publicKey.getKey()); + contextRequest.setPublicApiKey(publicKeyResult.getPlainTextKey()); request = new HttpPost(getFullUrl(CONTEXT_URL)); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); @@ -901,7 +902,7 @@ public class ContextServletIT extends BaseIT { // Test with private API key (should fail for public endpoint) request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, tenant, privateKey); + addPrivateTenantAuth(request, tenant, privateKeyResult.getPlainTextKey()); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); Assert.assertEquals("Private API key should be accepted for public endpoint to be able to update events and send restricted events", 200, response.getStatusCode()); @@ -910,9 +911,9 @@ public class ContextServletIT extends BaseIT { tenantService.deleteTenant(tenant.getItemId()); } - private static void addPrivateTenantAuth(HttpPost request, Tenant tenant, ApiKey privateKey) { + private static void addPrivateTenantAuth(HttpPost request, Tenant tenant, String privateKeyValue) { request.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( - (tenant.getItemId() + ":" + privateKey.getKey()).getBytes())); + (tenant.getItemId() + ":" + privateKeyValue).getBytes())); } private void performPersonalizationWithControlGroup(Map<String, String> controlGroupConfig, List<String> expectedVariants, @@ -1010,12 +1011,14 @@ public class ContextServletIT extends BaseIT { public void testContextRequestWithPublicApiKey() throws Exception { // Create tenant with API keys Tenant tenant = tenantService.createTenant("ContextApiKeyTest", Collections.emptyMap()); - ApiKey publicKey = tenantService.getApiKey(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC); + // Generate a fresh key to capture its one-time plaintext value (UNOMI-938: getApiKey() only returns + // metadata — a hash and a masked key — never the plaintext value). + ApiKeyCreationResult publicKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); // Create context request with public API key ContextRequest contextRequest = new ContextRequest(); contextRequest.setSessionId(TEST_SESSION_ID); - contextRequest.setPublicApiKey(publicKey.getKey()); + contextRequest.setPublicApiKey(publicKeyResult.getPlainTextKey()); // Send request HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); diff --git a/itests/src/test/java/org/apache/unomi/itests/TenantIT.java b/itests/src/test/java/org/apache/unomi/itests/TenantIT.java index 88df8b429..9c6c6803c 100644 --- a/itests/src/test/java/org/apache/unomi/itests/TenantIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/TenantIT.java @@ -31,6 +31,7 @@ import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.unomi.api.Profile; import org.apache.unomi.api.query.Query; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.ResourceQuota; import org.apache.unomi.api.tenants.Tenant; import org.junit.Assert; @@ -126,18 +127,20 @@ public class TenantIT extends BaseIT { HttpPost generateKeyRequest = new HttpPost(generateKeyUrl); String generateKeyResponse; - ApiKey newApiKey; + ApiKeyCreationResult newApiKeyResult; try (CloseableHttpResponse response = executeHttpRequest(generateKeyRequest, AuthType.JAAS_ADMIN)) { generateKeyResponse = EntityUtils.toString(response.getEntity()); - newApiKey = getObjectMapper().readValue(generateKeyResponse, ApiKey.class); + newApiKeyResult = getObjectMapper().readValue(generateKeyResponse, ApiKeyCreationResult.class); } - Assert.assertNotNull("New API key should not be null", newApiKey); - Assert.assertEquals("API key type should match requested type", ApiKey.ApiKeyType.PUBLIC, newApiKey.getKeyType()); + Assert.assertNotNull("New API key result should not be null", newApiKeyResult); + Assert.assertNotNull("New API key should not be null", newApiKeyResult.getApiKey()); + Assert.assertEquals("API key type should match requested type", ApiKey.ApiKeyType.PUBLIC, newApiKeyResult.getApiKey().getKeyType()); + String newApiKeyValue = newApiKeyResult.getPlainTextKey(); // Test validate API key String validateKeyUrl = String.format("%s/%s/apikeys/validate?key=%s&type=%s", - getFullUrl(REST_ENDPOINT), updatedTenant.getItemId(), newApiKey.getKey(), ApiKey.ApiKeyType.PUBLIC.name()); + getFullUrl(REST_ENDPOINT), updatedTenant.getItemId(), newApiKeyValue, ApiKey.ApiKeyType.PUBLIC.name()); int validateResponse; try (CloseableHttpResponse response = executeHttpRequest(new HttpGet(validateKeyUrl), AuthType.JAAS_ADMIN)) { validateResponse = response.getStatusLine().getStatusCode(); @@ -146,7 +149,7 @@ public class TenantIT extends BaseIT { // Test validate with wrong type String validateWrongTypeUrl = String.format("%s/%s/apikeys/validate?key=%s&type=%s", - getFullUrl(REST_ENDPOINT), updatedTenant.getItemId(), newApiKey.getKey(), ApiKey.ApiKeyType.PRIVATE.name()); + getFullUrl(REST_ENDPOINT), updatedTenant.getItemId(), newApiKeyValue, ApiKey.ApiKeyType.PRIVATE.name()); int validateWrongTypeResponse; try (CloseableHttpResponse response = executeHttpRequest(new HttpGet(validateWrongTypeUrl), AuthType.JAAS_ADMIN)) { validateWrongTypeResponse = response.getStatusLine().getStatusCode(); @@ -235,7 +238,12 @@ public class TenantIT extends BaseIT { public void testPublicEndpointAuthentication() throws Exception { // Create test tenant Tenant tenant = tenantService.createTenant("public-test-tenant", Collections.emptyMap()); - + + // Generate fresh keys to capture their one-time plaintext values (UNOMI-938: tenant.getPublicApiKey()/ + // getPrivateApiKey() only return masked keys, which cannot be used for authentication). + String privateKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null).getPlainTextKey(); + String publicKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); + // Refresh persistence to ensure tenant is immediately available for API key lookup persistenceService.refresh(); @@ -249,14 +257,14 @@ public class TenantIT extends BaseIT { // Test with private API key (should succeed - private keys have higher privileges) HttpGet publicRequest = new HttpGet(getFullUrl("/context.json?sessionId=" + sessionId)); publicRequest.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( - (tenant.getItemId() + ":" + tenant.getPrivateApiKey()).getBytes())); + (tenant.getItemId() + ":" + privateKeyValue).getBytes())); try (CloseableHttpResponse response = executeHttpRequest(publicRequest, AuthType.PRIVATE_KEY)) { Assert.assertEquals("Private API key should grant access to public endpoints (higher privileges)", 200, response.getStatusLine().getStatusCode()); } // Test with valid public API key (should succeed) publicRequest = new HttpGet(getFullUrl("/context.json?sessionId=" + sessionId)); - publicRequest.setHeader("X-Unomi-Api-Key", tenant.getPublicApiKey()); + publicRequest.setHeader("X-Unomi-Api-Key", publicKeyValue); try (CloseableHttpResponse response = executeHttpRequest(publicRequest, AuthType.PUBLIC_KEY)) { Assert.assertEquals("Valid public API key should grant access to public endpoints", 200, response.getStatusLine().getStatusCode()); } @@ -278,6 +286,12 @@ public class TenantIT extends BaseIT { // Create test tenant Tenant tenant = tenantService.createTenant("private-test-tenant", Collections.emptyMap()); + // Generate fresh keys to capture their one-time plaintext values (UNOMI-938: tenant.getPublicApiKey() + // only returns a masked key, which cannot be used for authentication). + String publicKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); + String privateKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null).getPlainTextKey(); + persistenceService.refresh(); + try { // Test without any authentication try (CloseableHttpResponse response = executeHttpRequest(new HttpGet(getFullUrl("/cxs/profiles/count")), AuthType.NONE)) { @@ -286,7 +300,7 @@ public class TenantIT extends BaseIT { // Test with public API key (should fail) HttpGet privateRequest = new HttpGet(getFullUrl("/cxs/profiles/count")); - privateRequest.setHeader("X-Unomi-Api-Key", tenant.getPublicApiKey()); + privateRequest.setHeader("X-Unomi-Api-Key", publicKeyValue); try (CloseableHttpResponse response = executeHttpRequest(privateRequest, AuthType.PUBLIC_KEY)) { Assert.assertEquals("Public API key should not grant access to private endpoints", 401, response.getStatusLine().getStatusCode()); } @@ -302,7 +316,7 @@ public class TenantIT extends BaseIT { // Test with valid private API key (should succeed) privateRequest = new HttpGet(getFullUrl("/cxs/profiles/count")); privateRequest.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( - (tenant.getItemId() + ":" + tenant.getPrivateApiKey()).getBytes())); + (tenant.getItemId() + ":" + privateKeyValue).getBytes())); try (CloseableHttpResponse response = executeHttpRequest(privateRequest, AuthType.PRIVATE_KEY)) { Assert.assertEquals("Valid private API key should grant access to private endpoints", 200, response.getStatusLine().getStatusCode()); } @@ -352,10 +366,10 @@ public class TenantIT extends BaseIT { try { // Test with private API key (should succeed) - ApiKey privateKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + ApiKeyCreationResult privateKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); HttpGet getRequest = new HttpGet(getFullUrl("/cxs/profiles/count")); getRequest.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( - (tenant.getItemId() + ":" + privateKey.getKey()).getBytes())); + (tenant.getItemId() + ":" + privateKeyResult.getPlainTextKey()).getBytes())); try (CloseableHttpResponse response = executeHttpRequest(getRequest, AuthType.PRIVATE_KEY)) { Assert.assertEquals("Private API key should grant access to private endpoints", 200, response.getStatusLine().getStatusCode()); } @@ -368,9 +382,9 @@ public class TenantIT extends BaseIT { } // Test with public API key (should fail) - ApiKey publicKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKeyCreationResult publicKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); getRequest = new HttpGet(getFullUrl("/cxs/profiles/count")); - getRequest.setHeader("X-Unomi-Api-Key", publicKey.getKey()); + getRequest.setHeader("X-Unomi-Api-Key", publicKeyResult.getPlainTextKey()); try (CloseableHttpResponse response = executeHttpRequest(getRequest, AuthType.PUBLIC_KEY)) { Assert.assertEquals("Public API key should not grant access to private endpoints", 401, response.getStatusLine().getStatusCode()); } @@ -390,9 +404,9 @@ public class TenantIT extends BaseIT { public void testExpiredApiKey() throws Exception { Tenant tenant = tenantService.createTenant("expired-tenant", Collections.emptyMap()); try { - ApiKey apiKey = tenantService.generateApiKey(tenant.getItemId(), 1L); // 1ms validity + ApiKeyCreationResult apiKeyResult = tenantService.generateApiKey(tenant.getItemId(), 1L); // 1ms validity Thread.sleep(2); // Wait for key to expire - Assert.assertFalse(tenantService.validateApiKey(tenant.getItemId(), apiKey.getKey())); + Assert.assertFalse(tenantService.validateApiKey(tenant.getItemId(), apiKeyResult.getPlainTextKey())); } finally { tenantService.deleteTenant(tenant.getItemId()); } @@ -454,8 +468,12 @@ public class TenantIT extends BaseIT { Tenant tenant = tenantService.createTenant("dual-key-tenant", Collections.emptyMap()); try { - ApiKey publicKey = tenantService.getApiKey(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC); - ApiKey privateKey = tenantService.getApiKey(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE); + // Generate fresh keys to capture their one-time plaintext values (UNOMI-938: getApiKey() only + // returns metadata — a hash and a masked key — never the plaintext value). + ApiKeyCreationResult publicKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKeyCreationResult privateKeyResult = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + ApiKey publicKey = publicKeyResult.getApiKey(); + ApiKey privateKey = privateKeyResult.getApiKey(); Assert.assertNotNull("Public key should exist", publicKey); Assert.assertNotNull("Private key should exist", privateKey); @@ -463,13 +481,13 @@ public class TenantIT extends BaseIT { Assert.assertEquals("Private key should have correct type", ApiKey.ApiKeyType.PRIVATE, privateKey.getKeyType()); Assert.assertTrue("Public key should validate as public", - tenantService.validateApiKeyWithType(tenant.getItemId(), publicKey.getKey(), ApiKey.ApiKeyType.PUBLIC)); + tenantService.validateApiKeyWithType(tenant.getItemId(), publicKeyResult.getPlainTextKey(), ApiKey.ApiKeyType.PUBLIC)); Assert.assertFalse("Public key should not validate as private", - tenantService.validateApiKeyWithType(tenant.getItemId(), publicKey.getKey(), ApiKey.ApiKeyType.PRIVATE)); + tenantService.validateApiKeyWithType(tenant.getItemId(), publicKeyResult.getPlainTextKey(), ApiKey.ApiKeyType.PRIVATE)); Assert.assertTrue("Private key should validate as private", - tenantService.validateApiKeyWithType(tenant.getItemId(), privateKey.getKey(), ApiKey.ApiKeyType.PRIVATE)); + tenantService.validateApiKeyWithType(tenant.getItemId(), privateKeyResult.getPlainTextKey(), ApiKey.ApiKeyType.PRIVATE)); Assert.assertFalse("Private key should not validate as public", - tenantService.validateApiKeyWithType(tenant.getItemId(), privateKey.getKey(), ApiKey.ApiKeyType.PUBLIC)); + tenantService.validateApiKeyWithType(tenant.getItemId(), privateKeyResult.getPlainTextKey(), ApiKey.ApiKeyType.PUBLIC)); } finally { tenantService.deleteTenant(tenant.getItemId()); } @@ -480,21 +498,23 @@ public class TenantIT extends BaseIT { Tenant tenant = tenantService.createTenant("lookup-tenant", Collections.emptyMap()); try { - ApiKey publicKey = tenantService.getApiKey(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC); - ApiKey privateKey = tenantService.getApiKey(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE); + // Generate fresh keys to capture their one-time plaintext values (UNOMI-938: getApiKey() only + // returns metadata — a hash and a masked key — never the plaintext value). + String publicKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); + String privateKeyValue = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null).getPlainTextKey(); persistenceService.refresh(); - Tenant foundByPublic = tenantService.getTenantByApiKey(publicKey.getKey()); - Tenant foundByPrivate = tenantService.getTenantByApiKey(privateKey.getKey()); + Tenant foundByPublic = tenantService.getTenantByApiKey(publicKeyValue); + Tenant foundByPrivate = tenantService.getTenantByApiKey(privateKeyValue); Assert.assertEquals("Should find correct tenant by public key", tenant.getItemId(), foundByPublic.getItemId()); Assert.assertEquals("Should find correct tenant by private key", tenant.getItemId(), foundByPrivate.getItemId()); - Tenant foundByPublicAsPublic = tenantService.getTenantByApiKey(publicKey.getKey(), ApiKey.ApiKeyType.PUBLIC); - Tenant foundByPublicAsPrivate = tenantService.getTenantByApiKey(publicKey.getKey(), ApiKey.ApiKeyType.PRIVATE); - Tenant foundByPrivateAsPrivate = tenantService.getTenantByApiKey(privateKey.getKey(), ApiKey.ApiKeyType.PRIVATE); - Tenant foundByPrivateAsPublic = tenantService.getTenantByApiKey(privateKey.getKey(), ApiKey.ApiKeyType.PUBLIC); + Tenant foundByPublicAsPublic = tenantService.getTenantByApiKey(publicKeyValue, ApiKey.ApiKeyType.PUBLIC); + Tenant foundByPublicAsPrivate = tenantService.getTenantByApiKey(publicKeyValue, ApiKey.ApiKeyType.PRIVATE); + Tenant foundByPrivateAsPrivate = tenantService.getTenantByApiKey(privateKeyValue, ApiKey.ApiKeyType.PRIVATE); + Tenant foundByPrivateAsPublic = tenantService.getTenantByApiKey(privateKeyValue, ApiKey.ApiKeyType.PUBLIC); Assert.assertNotNull("Should find tenant by public key when type matches", foundByPublicAsPublic); Assert.assertNull("Should not find tenant by public key when type is private", foundByPublicAsPrivate); diff --git a/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java b/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java index 04b41414d..e8318fb5b 100644 --- a/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java @@ -207,14 +207,14 @@ public class V2CompatibilityModeIT extends BaseIT { // Test V3-style request with public API key - should work request = new HttpPost(getFullUrl(CONTEXT_URL)); - request.addHeader(UNOMI_API_KEY_HEADER, testPublicKey.getKey()); + request.addHeader(UNOMI_API_KEY_HEADER, testPublicKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); assertEquals("V3-style request with public API key should work in V3 mode", 200, response.getStatusCode()); // Test V3-style request with private API key - should work request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); assertEquals("V3-style request with private API key should work in V3 mode", 200, response.getStatusCode()); @@ -265,7 +265,7 @@ public class V2CompatibilityModeIT extends BaseIT { // Test V3-style request with public API key - in V2 mode, V3 API keys are ignored (request succeeds but no events processed) request = new HttpPost(getFullUrl(CONTEXT_URL)); - request.addHeader(UNOMI_API_KEY_HEADER, testPublicKey.getKey()); + request.addHeader(UNOMI_API_KEY_HEADER, testPublicKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); assertEquals("V3-style request with public API key should return 200 in V2 compatibility mode", 200, response.getStatusCode()); @@ -273,7 +273,7 @@ public class V2CompatibilityModeIT extends BaseIT { // Test V3-style request with private API key - in V2 mode, V3 API keys are ignored (request succeeds but no events processed) request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPrivateTenantAuth(request, testTenant, testPrivateKey); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); response = executeContextJSONRequest(request, TEST_SESSION_ID); assertEquals("V3-style request with private API key should return 200 in V2 compatibility mode", 200, response.getStatusCode()); @@ -495,9 +495,9 @@ public class V2CompatibilityModeIT extends BaseIT { } } - private static void addPrivateTenantAuth(HttpPost request, Tenant tenant, ApiKey privateKey) { + private static void addPrivateTenantAuth(HttpPost request, Tenant tenant, String privateKeyValue) { request.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( - (tenant.getItemId() + ":" + privateKey.getKey()).getBytes())); + (tenant.getItemId() + ":" + privateKeyValue).getBytes())); } @Override diff --git a/itests/src/test/java/org/apache/unomi/itests/tools/httpclient/HttpClientThatWaitsForUnomi.java b/itests/src/test/java/org/apache/unomi/itests/tools/httpclient/HttpClientThatWaitsForUnomi.java index c39cd7ef8..95e5b6244 100644 --- a/itests/src/test/java/org/apache/unomi/itests/tools/httpclient/HttpClientThatWaitsForUnomi.java +++ b/itests/src/test/java/org/apache/unomi/itests/tools/httpclient/HttpClientThatWaitsForUnomi.java @@ -20,7 +20,6 @@ package org.apache.unomi.itests.tools.httpclient; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.unomi.api.tenants.ApiKey; import org.apache.unomi.api.tenants.Tenant; import org.eclipse.jetty.http.HttpStatus; @@ -33,10 +32,14 @@ public class HttpClientThatWaitsForUnomi { private static final int MAX_TRIES = 10; private static Tenant testTenant; - private static ApiKey testPublicKey; - private static ApiKey testPrivateKey; + private static String testPublicKey; + private static String testPrivateKey; - public static void setTestTenant(Tenant tenant, ApiKey publicKey, ApiKey privateKey) { + /** + * @param publicKey the one-time plaintext value of the tenant's public API key (see UNOMI-938) + * @param privateKey the one-time plaintext value of the tenant's private API key (see UNOMI-938) + */ + public static void setTestTenant(Tenant tenant, String publicKey, String privateKey) { testTenant = tenant; testPublicKey = publicKey; testPrivateKey = privateKey; @@ -57,13 +60,13 @@ public class HttpClientThatWaitsForUnomi { if (isPrivateEndpoint(path) || forcePrivate) { // For private endpoints, use Basic auth with tenant ID and private key if (testTenant != null && testPrivateKey != null && request.getFirstHeader("Authorization") == null) { - String credentials = testTenant.getItemId() + ":" + testPrivateKey.getKey(); + String credentials = testTenant.getItemId() + ":" + testPrivateKey; request.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString(credentials.getBytes())); } } else { // For public endpoints, use X-Unomi-Api-Key header if (testPublicKey != null && request.getFirstHeader("X-Unomi-Api-Key") == null) { - request.setHeader("X-Unomi-Api-Key", testPublicKey.getKey()); + request.setHeader("X-Unomi-Api-Key", testPublicKey); } } } diff --git a/rest/src/main/java/org/apache/unomi/rest/server/ApiKeyRestMixIn.java b/rest/src/main/java/org/apache/unomi/rest/server/ApiKeyRestMixIn.java new file mode 100644 index 000000000..25fa10f13 --- /dev/null +++ b/rest/src/main/java/org/apache/unomi/rest/server/ApiKeyRestMixIn.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.rest.server; + +import com.fasterxml.jackson.annotation.JsonIgnore; + +/** + * This mixin is used in {@link RestServer} to prevent the API key hash from ever being + * exposed through REST responses (see UNOMI-938). Only the masked key and metadata are + * serialized; the plaintext key is only available once, on {@code ApiKeyCreationResult}. + */ +public abstract class ApiKeyRestMixIn { + + public ApiKeyRestMixIn() { } + + @JsonIgnore abstract String getKeyHash(); +} diff --git a/rest/src/main/java/org/apache/unomi/rest/server/RestServer.java b/rest/src/main/java/org/apache/unomi/rest/server/RestServer.java index b9b74ce2a..b1ad21e97 100644 --- a/rest/src/main/java/org/apache/unomi/rest/server/RestServer.java +++ b/rest/src/main/java/org/apache/unomi/rest/server/RestServer.java @@ -34,6 +34,7 @@ import org.apache.unomi.api.EventsCollectorRequest; import org.apache.unomi.api.security.SecurityService; import org.apache.unomi.api.services.ConfigSharingService; import org.apache.unomi.api.services.ExecutionContextManager; +import org.apache.unomi.api.tenants.ApiKey; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.persistence.spi.CustomObjectMapper; import org.apache.unomi.rest.authentication.AuthenticationFilter; @@ -328,6 +329,7 @@ public class RestServer { // Build the server ObjectMapper objectMapper = new CustomObjectMapper(desers); + objectMapper.addMixIn(ApiKey.class, ApiKeyRestMixIn.class); JAXRSServerFactoryBean jaxrsServerFactoryBean = new JAXRSServerFactoryBean(); jaxrsServerFactoryBean.setAddress("/"); jaxrsServerFactoryBean.setBus(serverBus); diff --git a/rest/src/main/java/org/apache/unomi/rest/tenants/TenantEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/tenants/TenantEndpoint.java index 0372ed6b1..adb1d5693 100644 --- a/rest/src/main/java/org/apache/unomi/rest/tenants/TenantEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/tenants/TenantEndpoint.java @@ -19,6 +19,7 @@ package org.apache.unomi.rest.tenants; import org.apache.cxf.rs.security.cors.CrossOriginResourceSharing; import org.apache.unomi.api.security.UnomiRoles; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.rest.security.RequiresRole; @@ -145,13 +146,13 @@ public class TenantEndpoint { * @param tenantId the ID of the tenant * @param type the type of API key to generate (PUBLIC or PRIVATE) * @param validityDays the validity period in days (0 or null for no expiration) - * @return the generated API key + * @return the generated key metadata together with its one-time plaintext value * @throws WebApplicationException with 404 status if tenant is not found */ @POST @Path("/{tenantId}/apikeys") @Produces(MediaType.APPLICATION_JSON) - public ApiKey generateApiKey(@PathParam("tenantId") String tenantId, + public ApiKeyCreationResult generateApiKey(@PathParam("tenantId") String tenantId, @QueryParam("type") ApiKey.ApiKeyType type, @QueryParam("validityDays") Integer validityDays) { Tenant tenant = tenantService.getTenant(tenantId); diff --git a/services-common/src/main/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImpl.java b/services-common/src/main/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImpl.java new file mode 100644 index 000000000..95000621a --- /dev/null +++ b/services-common/src/main/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImpl.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.services.common.security; + +import org.apache.unomi.api.security.ApiKeyHashService; + +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.spec.InvalidKeySpecException; +import java.util.Base64; + +/** + * Default implementation of {@link ApiKeyHashService}, using PBKDF2WithHmacSHA512 to hash + * API keys before they are persisted (see UNOMI-938). Plaintext keys are never stored: + * only a salted hash and a masked, display-safe representation are kept. + */ +public class ApiKeyHashServiceImpl implements ApiKeyHashService { + private static final String KEY_PREFIX = "unomi_v1_"; + private static final int KEY_RANDOM_BYTES = 32; + private static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA512"; + private static final int ITERATIONS = 600_000; + private static final int SALT_LENGTH_BYTES = 16; + private static final int HASH_LENGTH_BITS = 256; + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + @Override + public String generateKey() { + byte[] randomBytes = new byte[KEY_RANDOM_BYTES]; + SECURE_RANDOM.nextBytes(randomBytes); + StringBuilder hex = new StringBuilder(randomBytes.length * 2); + for (byte b : randomBytes) { + hex.append(String.format("%02X", b)); + } + return KEY_PREFIX + hex; + } + + @Override + public String hash(String plainTextKey) { + if (plainTextKey == null) { + throw new IllegalArgumentException("plainTextKey cannot be null"); + } + byte[] salt = new byte[SALT_LENGTH_BYTES]; + SECURE_RANDOM.nextBytes(salt); + byte[] hash = pbkdf2(plainTextKey.toCharArray(), salt, ITERATIONS); + return ITERATIONS + ":" + Base64.getEncoder().encodeToString(salt) + ":" + Base64.getEncoder().encodeToString(hash); + } + + @Override + public boolean verify(String plainTextKey, String storedHash) { + if (plainTextKey == null || storedHash == null) { + return false; + } + String[] parts = storedHash.split(":"); + if (parts.length != 3) { + return false; + } + try { + int iterations = Integer.parseInt(parts[0]); + byte[] salt = Base64.getDecoder().decode(parts[1]); + byte[] expectedHash = Base64.getDecoder().decode(parts[2]); + byte[] actualHash = pbkdf2(plainTextKey.toCharArray(), salt, iterations); + return MessageDigest.isEqual(actualHash, expectedHash); + } catch (IllegalArgumentException e) { + return false; + } + } + + @Override + public String mask(String plainTextKey) { + if (plainTextKey == null) { + return null; + } + String withoutPrefix = plainTextKey.startsWith(KEY_PREFIX) ? plainTextKey.substring(KEY_PREFIX.length()) : plainTextKey; + String lastFour = withoutPrefix.length() >= 4 ? withoutPrefix.substring(withoutPrefix.length() - 4) : withoutPrefix; + return KEY_PREFIX + "****" + lastFour; + } + + private byte[] pbkdf2(char[] password, byte[] salt, int iterations) { + try { + PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, HASH_LENGTH_BITS); + SecretKeyFactory factory = SecretKeyFactory.getInstance(PBKDF2_ALGORITHM); + return factory.generateSecret(spec).getEncoded(); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new IllegalStateException("Unable to compute PBKDF2 hash", e); + } + } +} diff --git a/services-common/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/services-common/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 2e3d94a26..0adba6b35 100644 --- a/services-common/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/services-common/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -58,6 +58,9 @@ <bean id="auditServiceImpl" class="org.apache.unomi.services.common.security.AuditServiceImpl"> </bean> + <bean id="apiKeyHashServiceImpl" class="org.apache.unomi.services.common.security.ApiKeyHashServiceImpl"> + </bean> + <bean id="securityServiceImpl" class="org.apache.unomi.services.common.security.KarafSecurityService" init-method="init" destroy-method="destroy"> <property name="configuration" ref="securityServiceConfiguration"/> @@ -82,4 +85,10 @@ </service-properties> </service> + <service id="apiKeyHashService" ref="apiKeyHashServiceImpl" interface="org.apache.unomi.api.security.ApiKeyHashService"> + <service-properties> + <entry key="service.exported.interfaces" value="org.apache.unomi.api.security.ApiKeyHashService"/> + </service-properties> + </service> + </blueprint> diff --git a/services-common/src/test/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImplTest.java b/services-common/src/test/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImplTest.java new file mode 100644 index 000000000..5ba470d5d --- /dev/null +++ b/services-common/src/test/java/org/apache/unomi/services/common/security/ApiKeyHashServiceImplTest.java @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.services.common.security; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class ApiKeyHashServiceImplTest { + + private ApiKeyHashServiceImpl hashService; + + @Before + public void setUp() { + hashService = new ApiKeyHashServiceImpl(); + } + + @Test + public void generateKeyUsesUnomiPrefix() { + String key = hashService.generateKey(); + assertNotNull(key); + assertTrue("Key should use unomi_v1_ prefix", key.startsWith("unomi_v1_")); + } + + @Test + public void hashAndVerifyRoundTrip() { + String plainTextKey = hashService.generateKey(); + String storedHash = hashService.hash(plainTextKey); + + assertTrue("Stored hash should use iterations:salt:hash format", storedHash.contains(":")); + assertTrue(hashService.verify(plainTextKey, storedHash)); + assertFalse(hashService.verify("wrong-key", storedHash)); + } + + @Test + public void verifyRejectsNullInputs() { + String storedHash = hashService.hash(hashService.generateKey()); + assertFalse(hashService.verify(null, storedHash)); + assertFalse(hashService.verify("some-key", null)); + } + + @Test + public void verifyRejectsMalformedHash() { + assertFalse(hashService.verify("unomi_v1_ABCD", "not-a-valid-hash")); + assertFalse(hashService.verify("unomi_v1_ABCD", "1:bad-base64:also-bad")); + } + + @Test(expected = IllegalArgumentException.class) + public void hashRejectsNullPlaintext() { + hashService.hash(null); + } + + @Test + public void maskShowsPrefixAndLastFourChars() { + String plainTextKey = "unomi_v1_C606D77D1D219509637A82C062BCD17F13D6DF1501702DC396D4A12D63D4E5F2"; + String masked = hashService.mask(plainTextKey); + assertTrue(masked.startsWith("unomi_v1_****")); + assertTrue(masked.endsWith("E5F2")); + assertNotEquals(plainTextKey, masked); + } + + @Test + public void verifyUsesConstantTimeComparison() { + String plainTextKey = hashService.generateKey(); + String storedHash = hashService.hash(plainTextKey); + + long validStart = System.nanoTime(); + hashService.verify(plainTextKey, storedHash); + long validElapsed = System.nanoTime() - validStart; + + long invalidStart = System.nanoTime(); + hashService.verify(hashService.generateKey(), storedHash); + long invalidElapsed = System.nanoTime() - invalidStart; + + assertTrue("Verify timing should not differ by more than 50ms between valid and invalid keys", + Math.abs(validElapsed - invalidElapsed) < 50_000_000L); + } +} diff --git a/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantServiceImpl.java index c78f928da..d2fca52b6 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantServiceImpl.java @@ -16,9 +16,11 @@ */ package org.apache.unomi.services.impl.tenants; +import org.apache.unomi.api.security.ApiKeyHashService; import org.apache.unomi.api.services.ExecutionContextManager; import org.apache.unomi.api.services.TenantLifecycleListener; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.api.tenants.TenantStatus; @@ -26,20 +28,18 @@ import org.apache.unomi.persistence.spi.PersistenceService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.xml.bind.DatatypeConverter; -import java.security.SecureRandom; import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; public class TenantServiceImpl implements TenantService { private static final Logger LOGGER = LoggerFactory.getLogger(TenantServiceImpl.class); - private static final SecureRandom secureRandom = new SecureRandom(); private static final int MAX_TENANT_ID_LENGTH = 32; private static final String TENANT_ID_PATTERN = "^[a-zA-Z0-9][a-zA-Z0-9-_]*[a-zA-Z0-9]$"; private final List<TenantLifecycleListener> lifecycleListeners = new CopyOnWriteArrayList<>(); private PersistenceService persistenceService; private ExecutionContextManager executionContextManager; + private ApiKeyHashService apiKeyHashService; public void setPersistenceService(PersistenceService persistenceService) { this.persistenceService = persistenceService; @@ -49,6 +49,10 @@ public class TenantServiceImpl implements TenantService { this.executionContextManager = executionContextManager; } + public void setApiKeyHashService(ApiKeyHashService apiKeyHashService) { + this.apiKeyHashService = apiKeyHashService; + } + public void bindListener(TenantLifecycleListener listener) { lifecycleListeners.add(listener); LOGGER.debug("Added tenant lifecycle listener: {}", listener.getClass().getName()); @@ -103,22 +107,28 @@ public class TenantServiceImpl implements TenantService { persistenceService.refreshIndex(Tenant.class); // Reload tenant to get the updated version with API keys - return getTenant(tenant.getItemId()); + Tenant reloadedTenant = getTenant(tenant.getItemId()); + if (reloadedTenant == null) { + throw new IllegalStateException("Failed to reload tenant after creation: " + tenant.getItemId()); + } + return reloadedTenant; }); } @Override - public ApiKey generateApiKey(String tenantId, Long validityPeriod) { + public ApiKeyCreationResult generateApiKey(String tenantId, Long validityPeriod) { return generateApiKeyWithType(tenantId, ApiKey.ApiKeyType.PUBLIC, validityPeriod); } @Override - public ApiKey generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod) { + public ApiKeyCreationResult generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod) { return executionContextManager.executeAsSystem(() -> { + String plainTextKey = apiKeyHashService.generateKey(); + ApiKey apiKey = new ApiKey(); apiKey.setItemId(UUID.randomUUID().toString()); - String key = generateSecureKey(); - apiKey.setKey(key); + apiKey.setKeyHash(apiKeyHashService.hash(plainTextKey)); + apiKey.setMaskedKey(apiKeyHashService.mask(plainTextKey)); apiKey.setKeyType(keyType); apiKey.setCreationDate(new Date()); if (validityPeriod != null) { @@ -136,7 +146,7 @@ public class TenantServiceImpl implements TenantService { persistenceService.save(tenant); } - return apiKey; + return new ApiKeyCreationResult(apiKey, plainTextKey); }); } @@ -150,12 +160,6 @@ public class TenantServiceImpl implements TenantService { return executionContextManager.executeAsSystem(() -> persistenceService.load(tenantId, Tenant.class)); } - private String generateSecureKey() { - byte[] randomBytes = new byte[32]; - secureRandom.nextBytes(randomBytes); - return DatatypeConverter.printHexBinary(randomBytes); - } - @Override public void saveTenant(Tenant tenant) { executionContextManager.executeAsSystem(() -> persistenceService.save(tenant)); @@ -165,17 +169,18 @@ public class TenantServiceImpl implements TenantService { public void deleteTenant(String tenantId) { executionContextManager.executeAsSystem(() -> { Tenant tenant = persistenceService.load(tenantId, Tenant.class); - if (tenant != null) { - // Notify listeners before deletion - for (TenantLifecycleListener listener : lifecycleListeners) { - try { - listener.onTenantRemoved(tenantId); - } catch (Exception e) { - LOGGER.error("Error notifying listener {} of tenant removal: {}", listener.getClass().getName(), tenantId, e); - } + if (tenant == null) { + throw new IllegalArgumentException("Tenant not found: " + tenantId); + } + // Notify listeners before deletion + for (TenantLifecycleListener listener : lifecycleListeners) { + try { + listener.onTenantRemoved(tenantId); + } catch (Exception e) { + LOGGER.error("Error notifying listener {} of tenant removal: {}", listener.getClass().getName(), tenantId, e); } - persistenceService.remove(tenantId, Tenant.class); } + persistenceService.remove(tenantId, Tenant.class); }); } @@ -194,12 +199,27 @@ public class TenantServiceImpl implements TenantService { return false; } return tenant.getApiKeys().stream() - .anyMatch(apiKey -> apiKey.getKey().equals(key) && + .anyMatch(apiKey -> matchesKey(apiKey, key) && !apiKey.isRevoked() && (requiredType == null || apiKey.getKeyType() == requiredType) && (apiKey.getExpirationDate() == null || apiKey.getExpirationDate().after(new Date()))); } + /** + * Checks whether the given plaintext key matches the stored key. + * Supports both hashed keys (see UNOMI-938) and, transitionally, keys that have not + * yet been migrated and still carry their legacy plaintext value. + */ + private boolean matchesKey(ApiKey apiKey, String plainTextKey) { + if (plainTextKey == null) { + return false; + } + if (apiKey.getKeyHash() != null) { + return apiKeyHashService.verify(plainTextKey, apiKey.getKeyHash()); + } + return apiKey.getLegacyKey() != null && apiKey.getLegacyKey().equals(plainTextKey); + } + @Override public ApiKey getApiKey(String tenantId, ApiKey.ApiKeyType keyType) { return executionContextManager.executeAsSystem(() -> { @@ -219,8 +239,8 @@ public class TenantServiceImpl implements TenantService { return executionContextManager.executeAsSystem(() -> { List<Tenant> tenants = persistenceService.getAllItems(Tenant.class); return tenants.stream() - .filter(tenant -> tenant.getApiKeys().stream() - .anyMatch(key -> key.getKey().equals(apiKey))) + .filter(tenant -> tenant.getApiKeys() != null && tenant.getApiKeys().stream() + .anyMatch(key -> matchesKey(key, apiKey))) .findFirst() .orElse(null); }); @@ -231,8 +251,8 @@ public class TenantServiceImpl implements TenantService { return executionContextManager.executeAsSystem(() -> { List<Tenant> tenants = persistenceService.getAllItems(Tenant.class); return tenants.stream() - .filter(tenant -> tenant.getApiKeys().stream() - .anyMatch(key -> key.getKey().equals(apiKey) && key.getKeyType() == keyType)) + .filter(tenant -> tenant.getApiKeys() != null && tenant.getApiKeys().stream() + .anyMatch(key -> matchesKey(key, apiKey) && key.getKeyType() == keyType)) .findFirst() .orElse(null); }); diff --git a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 1bba07ee3..a82c18ad1 100644 --- a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -34,6 +34,7 @@ <reference id="bundleWatcher" interface="org.apache.unomi.lifecycle.BundleWatcher"/> <reference id="executionContextManager" interface="org.apache.unomi.api.services.ExecutionContextManager" /> <reference id="auditService" interface="org.apache.unomi.api.tenants.AuditService" availability="optional"/> + <reference id="apiKeyHashService" interface="org.apache.unomi.api.security.ApiKeyHashService"/> <reference id="eventAdmin" interface="org.osgi.service.event.EventAdmin"/> <!-- Reference Lists --> @@ -459,6 +460,7 @@ <bean id="tenantServiceImpl" class="org.apache.unomi.services.impl.tenants.TenantServiceImpl"> <property name="persistenceService" ref="persistenceService"/> <property name="executionContextManager" ref="executionContextManager"/> + <property name="apiKeyHashService" ref="apiKeyHashService"/> </bean> <bean id="tenantMigrationServiceImpl" class="org.apache.unomi.services.impl.tenants.TenantMigrationService"> diff --git a/services/src/test/java/org/apache/unomi/services/impl/TestTenantService.java b/services/src/test/java/org/apache/unomi/services/impl/TestTenantService.java index 82af4f434..d2edb4062 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/TestTenantService.java +++ b/services/src/test/java/org/apache/unomi/services/impl/TestTenantService.java @@ -16,13 +16,14 @@ */ package org.apache.unomi.services.impl; +import org.apache.unomi.api.security.ApiKeyHashService; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.api.tenants.TenantStatus; +import org.apache.unomi.services.common.security.ApiKeyHashServiceImpl; -import javax.xml.bind.DatatypeConverter; -import java.security.SecureRandom; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -31,7 +32,7 @@ public class TestTenantService implements TenantService { private ThreadLocal<String> currentTenantId = new ThreadLocal<>(); private Map<String, Tenant> tenants = new ConcurrentHashMap<>(); private ThreadLocal<Boolean> inSystemOperation = new ThreadLocal<>(); - private static final SecureRandom secureRandom = new SecureRandom(); + private final ApiKeyHashService apiKeyHashService = new ApiKeyHashServiceImpl(); public void setInSystemOperation(boolean inSystemOperation) { this.inSystemOperation.set(inSystemOperation); @@ -86,12 +87,17 @@ public class TestTenantService implements TenantService { return false; } return tenant.getApiKeys().stream() - .anyMatch(apiKey -> apiKey.getKey().equals(key) && + .anyMatch(apiKey -> matchesKey(apiKey, key) && !apiKey.isRevoked() && (requiredType == null || apiKey.getKeyType() == requiredType) && (apiKey.getExpirationDate() == null || apiKey.getExpirationDate().after(new Date()))); } + private boolean matchesKey(ApiKey apiKey, String plainTextKey) { + return plainTextKey != null && apiKey.getKeyHash() != null + && apiKeyHashService.verify(plainTextKey, apiKey.getKeyHash()); + } + @Override public Tenant createTenant(String tenantId, Map<String, Object> properties) { Tenant tenant = new Tenant(); @@ -112,16 +118,18 @@ public class TestTenantService implements TenantService { } @Override - public ApiKey generateApiKey(String tenantId, Long validityPeriod) { + public ApiKeyCreationResult generateApiKey(String tenantId, Long validityPeriod) { return generateApiKeyWithType(tenantId, ApiKey.ApiKeyType.PUBLIC, validityPeriod); } @Override - public ApiKey generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod) { + public ApiKeyCreationResult generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, Long validityPeriod) { + String plainTextKey = apiKeyHashService.generateKey(); + ApiKey apiKey = new ApiKey(); apiKey.setItemId(UUID.randomUUID().toString()); - String key = generateSecureKey(); - apiKey.setKey(key); + apiKey.setKeyHash(apiKeyHashService.hash(plainTextKey)); + apiKey.setMaskedKey(apiKeyHashService.mask(plainTextKey)); apiKey.setKeyType(keyType); apiKey.setCreationDate(new Date()); if (validityPeriod != null) { @@ -139,7 +147,7 @@ public class TestTenantService implements TenantService { saveTenant(tenant); } - return apiKey; + return new ApiKeyCreationResult(apiKey, plainTextKey); } @Override @@ -147,7 +155,7 @@ public class TestTenantService implements TenantService { return tenants.values().stream() .filter(tenant -> tenant.getApiKeys() != null && tenant.getApiKeys().stream() - .anyMatch(key -> key.getKey().equals(apiKey))) + .anyMatch(key -> matchesKey(key, apiKey))) .findFirst() .orElse(null); } @@ -157,7 +165,7 @@ public class TestTenantService implements TenantService { return tenants.values().stream() .filter(tenant -> tenant.getApiKeys() != null && tenant.getApiKeys().stream() - .anyMatch(key -> key.getKey().equals(apiKey) && key.getKeyType() == keyType)) + .anyMatch(key -> matchesKey(key, apiKey) && key.getKeyType() == keyType)) .findFirst() .orElse(null); } @@ -173,10 +181,4 @@ public class TestTenantService implements TenantService { } return null; } - - private String generateSecureKey() { - byte[] randomBytes = new byte[32]; - secureRandom.nextBytes(randomBytes); - return DatatypeConverter.printHexBinary(randomBytes); - } } diff --git a/services/src/test/java/org/apache/unomi/services/impl/TestTenantServiceTest.java b/services/src/test/java/org/apache/unomi/services/impl/TestTenantServiceTest.java index 8f10752b7..476ef254f 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/TestTenantServiceTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/TestTenantServiceTest.java @@ -17,6 +17,7 @@ package org.apache.unomi.services.impl; import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -64,12 +65,14 @@ public class TestTenantServiceTest { Tenant tenant = tenantService.createTenant("test-tenant", Collections.emptyMap()); // Generate a new public API key - ApiKey newPublicKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PUBLIC, null); + ApiKeyCreationResult newPublicKeyResult = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PUBLIC, null); // Verify the new key was generated - assertNotNull(newPublicKey, "New API key should not be null"); - assertEquals(ApiKey.ApiKeyType.PUBLIC, newPublicKey.getKeyType(), "Key type should be PUBLIC"); - assertNotNull(newPublicKey.getKey(), "Key value should not be null"); + assertNotNull(newPublicKeyResult, "New API key result should not be null"); + assertNotNull(newPublicKeyResult.getApiKey(), "New API key should not be null"); + assertEquals(ApiKey.ApiKeyType.PUBLIC, newPublicKeyResult.getApiKey().getKeyType(), "Key type should be PUBLIC"); + assertNotNull(newPublicKeyResult.getPlainTextKey(), "Plaintext key value should not be null"); + assertNotNull(newPublicKeyResult.getApiKey().getMaskedKey(), "Masked key should not be null"); // Reload tenant and verify the new key is there Tenant reloadedTenant = tenantService.getTenant("test-tenant"); @@ -81,10 +84,10 @@ public class TestTenantServiceTest { public void testValidateApiKey() { TestTenantService tenantService = new TestTenantService(); - // Create a tenant - Tenant tenant = tenantService.createTenant("test-tenant", Collections.emptyMap()); - String publicKey = tenant.getPublicApiKey(); - String privateKey = tenant.getPrivateApiKey(); + // Create a tenant, then generate fresh keys to capture their one-time plaintext values + tenantService.createTenant("test-tenant", Collections.emptyMap()); + String publicKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); + String privateKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PRIVATE, null).getPlainTextKey(); // Verify API key validation works assertTrue(tenantService.validateApiKey("test-tenant", publicKey), "Public API key should be valid"); @@ -97,10 +100,10 @@ public class TestTenantServiceTest { public void testValidateApiKeyWithType() { TestTenantService tenantService = new TestTenantService(); - // Create a tenant - Tenant tenant = tenantService.createTenant("test-tenant", Collections.emptyMap()); - String publicKey = tenant.getPublicApiKey(); - String privateKey = tenant.getPrivateApiKey(); + // Create a tenant, then generate fresh keys to capture their one-time plaintext values + tenantService.createTenant("test-tenant", Collections.emptyMap()); + String publicKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); + String privateKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PRIVATE, null).getPlainTextKey(); // Verify type-specific validation works assertTrue(tenantService.validateApiKeyWithType("test-tenant", publicKey, ApiKey.ApiKeyType.PUBLIC), @@ -117,9 +120,9 @@ public class TestTenantServiceTest { public void testGetTenantByApiKey() { TestTenantService tenantService = new TestTenantService(); - // Create a tenant - Tenant tenant = tenantService.createTenant("test-tenant", Collections.emptyMap()); - String publicKey = tenant.getPublicApiKey(); + // Create a tenant, then generate a fresh key to capture its one-time plaintext value + tenantService.createTenant("test-tenant", Collections.emptyMap()); + String publicKey = tenantService.generateApiKeyWithType("test-tenant", ApiKey.ApiKeyType.PUBLIC, null).getPlainTextKey(); // Verify tenant lookup by API key works Tenant foundTenant = tenantService.getTenantByApiKey(publicKey); diff --git a/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantServiceImplTest.java new file mode 100644 index 000000000..86c08b8a8 --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantServiceImplTest.java @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.unomi.services.impl.tenants; + +import org.apache.unomi.api.security.ApiKeyHashService; +import org.apache.unomi.api.services.ExecutionContextManager; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.persistence.spi.PersistenceService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.function.Supplier; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class TenantServiceImplTest { + + @Mock + private PersistenceService persistenceService; + + @Mock + private ExecutionContextManager executionContextManager; + + @Mock + private ApiKeyHashService apiKeyHashService; + + private TenantServiceImpl tenantService; + + @BeforeEach + public void setUp() { + tenantService = new TenantServiceImpl(); + tenantService.setPersistenceService(persistenceService); + tenantService.setExecutionContextManager(executionContextManager); + tenantService.setApiKeyHashService(apiKeyHashService); + + when(executionContextManager.executeAsSystem(any(Supplier.class))).thenAnswer(invocation -> { + Supplier<?> supplier = invocation.getArgument(0); + return supplier.get(); + }); + doAnswer(invocation -> { + Runnable runnable = invocation.getArgument(0); + runnable.run(); + return null; + }).when(executionContextManager).executeAsSystem(any(Runnable.class)); + + // Treat the "hash" as the plaintext key itself, so tests can assert on plain values + // without depending on the real PBKDF2 implementation. + when(apiKeyHashService.verify(anyString(), anyString())) + .thenAnswer(invocation -> Objects.equals(invocation.getArgument(0), invocation.getArgument(1))); + } + + @Test + public void getTenantByApiKeySkipsTenantsWithNullApiKeys() { + Tenant tenantWithoutKeys = new Tenant(); + tenantWithoutKeys.setItemId("tenant-no-keys"); + tenantWithoutKeys.setApiKeys(null); + + Tenant tenantWithKeys = new Tenant(); + tenantWithKeys.setItemId("tenant-with-keys"); + ApiKey apiKey = new ApiKey(); + apiKey.setKeyHash("valid-key"); + apiKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); + tenantWithKeys.setApiKeys(new ArrayList<>(List.of(apiKey))); + + when(persistenceService.getAllItems(Tenant.class)).thenReturn(List.of(tenantWithoutKeys, tenantWithKeys)); + + Tenant found = tenantService.getTenantByApiKey("valid-key"); + assertEquals("tenant-with-keys", found.getItemId(), "Should find tenant with matching API key"); + assertNull(tenantService.getTenantByApiKey("missing-key"), "Non-existent key should return null"); + } + + @Test + public void getTenantByApiKeyWithTypeSkipsTenantsWithNullApiKeys() { + Tenant tenantWithoutKeys = new Tenant(); + tenantWithoutKeys.setItemId("tenant-no-keys"); + tenantWithoutKeys.setApiKeys(null); + + Tenant tenantWithKeys = new Tenant(); + tenantWithKeys.setItemId("tenant-with-keys"); + ApiKey apiKey = new ApiKey(); + apiKey.setKeyHash("valid-key"); + apiKey.setKeyType(ApiKey.ApiKeyType.PRIVATE); + tenantWithKeys.setApiKeys(new ArrayList<>(List.of(apiKey))); + + when(persistenceService.getAllItems(Tenant.class)).thenReturn(List.of(tenantWithoutKeys, tenantWithKeys)); + + Tenant found = tenantService.getTenantByApiKey("valid-key", ApiKey.ApiKeyType.PRIVATE); + assertEquals("tenant-with-keys", found.getItemId(), "Should find tenant with matching typed API key"); + assertNull(tenantService.getTenantByApiKey("valid-key", ApiKey.ApiKeyType.PUBLIC), + "Key type mismatch should return null"); + } + + @Test + public void createTenantThrowsWhenReloadReturnsNull() { + Tenant savedTenant = new Tenant(); + savedTenant.setItemId("test-tenant"); + savedTenant.setApiKeys(new ArrayList<>()); + + when(persistenceService.load(eq("test-tenant"), eq(Tenant.class))) + .thenReturn(null, savedTenant, savedTenant, null); + + IllegalStateException exception = assertThrows(IllegalStateException.class, + () -> tenantService.createTenant("test-tenant", Collections.emptyMap())); + assertEquals("Failed to reload tenant after creation: test-tenant", exception.getMessage()); + } + + @Test + public void deleteTenantThrowsWhenTenantMissing() { + when(persistenceService.load("missing-tenant", Tenant.class)).thenReturn(null); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> tenantService.deleteTenant("missing-tenant")); + assertEquals("Tenant not found: missing-tenant", exception.getMessage()); + } +} diff --git a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-10-tenantInitialization.groovy b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-10-tenantInitialization.groovy index 3c5667702..ab0913080 100644 --- a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-10-tenantInitialization.groovy +++ b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-10-tenantInitialization.groovy @@ -1,11 +1,14 @@ import org.apache.unomi.shell.migration.service.MigrationContext import org.apache.unomi.shell.migration.utils.MigrationUtils +import javax.crypto.SecretKeyFactory +import javax.crypto.spec.PBEKeySpec import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths import java.security.SecureRandom import java.time.ZonedDateTime import java.time.format.DateTimeFormatter +import java.util.Base64 import java.util.UUID import static org.apache.unomi.shell.migration.service.MigrationConfig.* @@ -33,6 +36,29 @@ String tenantId = context.getConfigString(TENANT_ID) ZonedDateTime unifiedDate = ZonedDateTime.now() String isoDate = unifiedDate.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME) +// Hashes a plaintext API key the same way ApiKeyHashServiceImpl does (PBKDF2WithHmacSHA512, +// 600000 iterations, format "iterations:base64(salt):base64(hash)") so it can be verified by +// TenantServiceImpl after migration without ever persisting the plaintext value (see UNOMI-938). +def hashApiKey = { String plainTextKey -> + int iterations = 600_000 + int saltLengthBytes = 16 + int hashLengthBits = 256 + SecureRandom rng = new SecureRandom() + byte[] salt = new byte[saltLengthBytes] + rng.nextBytes(salt) + PBEKeySpec spec = new PBEKeySpec(plainTextKey.toCharArray(), salt, iterations, hashLengthBits) + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512") + byte[] hash = factory.generateSecret(spec).getEncoded() + return "${iterations}:${Base64.encoder.encodeToString(salt)}:${Base64.encoder.encodeToString(hash)}" +} + +// Masks a plaintext API key the same way ApiKeyHashServiceImpl does: "unomi_v1_****LAST4". +def maskApiKey = { String plainTextKey -> + String withoutPrefix = plainTextKey.startsWith("unomi_v1_") ? plainTextKey.substring(9) : plainTextKey + String lastFour = withoutPrefix.length() >= 4 ? withoutPrefix.substring(withoutPrefix.length() - 4) : withoutPrefix + return "unomi_v1_****${lastFour}" +} + // Delete API key files older than 24 hours left by previous migration runs Path secretsDir = Paths.get(System.getProperty("karaf.data", "data"), "migration", "secrets") if (Files.exists(secretsDir)) { @@ -57,11 +83,18 @@ context.performMigrationStep("3.1.0-create-tenant-index", () -> { SecureRandom rng = new SecureRandom() byte[] pubBytes = new byte[32]; rng.nextBytes(pubBytes) byte[] privBytes = new byte[32]; rng.nextBytes(privBytes) - String generatedPublicKey = pubBytes.collect { String.format('%02X', it) }.join() - String generatedPrivateKey = privBytes.collect { String.format('%02X', it) }.join() + String generatedPublicKey = "unomi_v1_" + pubBytes.collect { String.format('%02X', it) }.join() + String generatedPrivateKey = "unomi_v1_" + privBytes.collect { String.format('%02X', it) }.join() String publicKeyId = UUID.randomUUID().toString() String privateKeyId = UUID.randomUUID().toString() + // Only the salted hash and a masked, display-safe representation are ever persisted (see UNOMI-938). + // The plaintext values below are only available now, at generation time. + String publicKeyHash = hashApiKey(generatedPublicKey) + String privateKeyHash = hashApiKey(generatedPrivateKey) + String publicKeyMasked = maskApiKey(generatedPublicKey) + String privateKeyMasked = maskApiKey(generatedPrivateKey) + // Write keys to a time-limited file AND print to console — the only opportunity to record them Files.createDirectories(secretsDir) String safeDate = isoDate.replaceAll('[^a-zA-Z0-9-]', '-') @@ -117,7 +150,8 @@ ${sep} "lastModifiedBy": "system-migration-3.1.0", "creationDate" : "${isoDate}", "lastModificationDate" : "${isoDate}", - "key" : "${generatedPublicKey}", + "keyHash" : "${publicKeyHash}", + "maskedKey" : "${publicKeyMasked}", "keyType" : "PUBLIC", "revoked" : false }, @@ -128,7 +162,8 @@ ${sep} "lastModifiedBy": "system-migration-3.1.0", "creationDate" : "${isoDate}", "lastModificationDate" : "${isoDate}", - "key" : "${generatedPrivateKey}", + "keyHash" : "${privateKeyHash}", + "maskedKey" : "${privateKeyMasked}", "keyType" : "PRIVATE", "revoked" : false } diff --git a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-20-hashApiKeys.groovy b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-20-hashApiKeys.groovy new file mode 100644 index 000000000..339a1fe44 --- /dev/null +++ b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-20-hashApiKeys.groovy @@ -0,0 +1,126 @@ +import groovy.json.JsonOutput +import groovy.json.JsonSlurper +import org.apache.unomi.shell.migration.service.MigrationContext +import org.apache.unomi.shell.migration.utils.HttpUtils +import org.apache.unomi.shell.migration.utils.MigrationUtils +import javax.crypto.SecretKeyFactory +import javax.crypto.spec.PBEKeySpec +import java.security.SecureRandom +import java.util.Base64 + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +// UNOMI-938: earlier versions stored API keys in plaintext, in the "key" field of each entry +// of a tenant's "apiKeys" array. As of 3.1.0, only a PBKDF2 hash ("keyHash") and a display-safe +// masked value ("maskedKey") are persisted; the plaintext is never stored. A tenant document +// with a still-plaintext "key" field keeps working via TenantServiceImpl's legacy-key fallback, +// so this migration is not mandatory for correctness, but it closes the plaintext-at-rest exposure +// window by rehashing every legacy key still found in Elasticsearch and removing the plaintext value. + +MigrationContext context = migrationContext +String esAddress = context.getConfigString("esAddress") +String indexPrefix = context.getConfigString("indexPrefix") +def jsonSlurper = new JsonSlurper() + +// Hashes a plaintext API key the same way ApiKeyHashServiceImpl does (PBKDF2WithHmacSHA512, +// 600000 iterations, format "iterations:base64(salt):base64(hash)"). +def hashApiKey = { String plainTextKey -> + int iterations = 600_000 + int saltLengthBytes = 16 + int hashLengthBits = 256 + SecureRandom rng = new SecureRandom() + byte[] salt = new byte[saltLengthBytes] + rng.nextBytes(salt) + PBEKeySpec spec = new PBEKeySpec(plainTextKey.toCharArray(), salt, iterations, hashLengthBits) + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA512") + byte[] hash = factory.generateSecret(spec).getEncoded() + return "${iterations}:${Base64.encoder.encodeToString(salt)}:${Base64.encoder.encodeToString(hash)}" +} + +// Masks a plaintext API key the same way ApiKeyHashServiceImpl does: "unomi_v1_****LAST4". +def maskApiKey = { String plainTextKey -> + String withoutPrefix = plainTextKey.startsWith("unomi_v1_") ? plainTextKey.substring(9) : plainTextKey + String lastFour = withoutPrefix.length() >= 4 ? withoutPrefix.substring(withoutPrefix.length() - 4) : withoutPrefix + return "unomi_v1_****${lastFour}" +} + +context.performMigrationStep("3.1.0-hash-legacy-api-keys", () -> { + String tenantIndex = "${indexPrefix}-tenant" + + if (!MigrationUtils.indexExists(context.getHttpClient(), esAddress, tenantIndex)) { + context.printMessage("Tenant index does not exist, skipping API key hashing") + return + } + + context.printMessage("Scanning tenant index for legacy plaintext API keys to rehash") + + String scrollQuery = JsonOutput.toJson([query: [match_all: [:]], size: 100]) + int tenantsProcessed = 0 + int tenantsUpdated = 0 + int keysRehashed = 0 + + MigrationUtils.scrollQuery(context.getHttpClient(), esAddress, "/${tenantIndex}/_search", scrollQuery, "5m", (hits) -> { + def hitsArray = jsonSlurper.parseText(hits) + StringBuilder bulkUpdate = new StringBuilder() + + hitsArray.each { hit -> + tenantsProcessed++ + List apiKeys = hit._source?.apiKeys + if (apiKeys == null || apiKeys.isEmpty()) { + return + } + + boolean tenantChanged = false + List newApiKeys = apiKeys.collect { apiKey -> + String legacyKey = apiKey.key + if (legacyKey == null || apiKey.keyHash != null) { + // Already migrated (has a hash) or nothing to migrate; leave untouched. + return apiKey + } + Map rehashedKey = new LinkedHashMap(apiKey) + rehashedKey.remove("key") + rehashedKey.keyHash = hashApiKey(legacyKey) + rehashedKey.maskedKey = maskApiKey(legacyKey) + tenantChanged = true + keysRehashed++ + return rehashedKey + } + + if (tenantChanged) { + String tenantId = hit._id + bulkUpdate.append(JsonOutput.toJson([update: [_id: tenantId, _index: hit._index]])).append("\n") + bulkUpdate.append(JsonOutput.toJson([doc: [apiKeys: newApiKeys]])).append("\n") + tenantsUpdated++ + } + } + + if (bulkUpdate.length() > 0) { + try { + MigrationUtils.bulkUpdate(context.getHttpClient(), esAddress + "/_bulk", bulkUpdate.toString()) + } catch (Exception e) { + context.printMessage("Error rehashing API keys for a batch of tenants: ${e.message}") + } + } + }) + + if (tenantsUpdated > 0) { + HttpUtils.executePostRequest(context.getHttpClient(), esAddress + "/${tenantIndex}/_refresh", null, null) + } + + context.printMessage("Processed ${tenantsProcessed} tenant(s): rehashed ${keysRehashed} legacy API key(s) across ${tenantsUpdated} tenant(s)") +}) diff --git a/tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java b/tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java index 99f37041a..7afbb2989 100644 --- a/tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java +++ b/tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java @@ -22,6 +22,7 @@ import org.apache.unomi.api.PartialList; import org.apache.unomi.api.query.Query; import org.apache.unomi.api.tenants.ApiKey; import org.apache.unomi.api.tenants.ApiKey.ApiKeyType; +import org.apache.unomi.api.tenants.ApiKeyCreationResult; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.persistence.spi.CustomObjectMapper; @@ -30,6 +31,7 @@ import org.apache.unomi.shell.dev.services.CrudCommand; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; +import java.io.PrintStream; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -41,7 +43,7 @@ public class ApiKeyCrudCommand extends BaseCrudCommand { private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); private static final List<String> PROPERTY_NAMES = List.of( - "itemId", "name", "description", "keyType", "key", "tenantId", "validityPeriod" + "itemId", "name", "description", "keyType", "tenantId", "validityPeriod" ); @Reference @@ -59,7 +61,7 @@ public class ApiKeyCrudCommand extends BaseCrudCommand { "Name", "Description", "Key Type", - "Key" + "Key (masked)" }; } @@ -91,7 +93,7 @@ public class ApiKeyCrudCommand extends BaseCrudCommand { apiKey.getName(), apiKey.getDescription(), apiKey.getKeyType().toString(), - apiKey.getKey() + apiKey.getMaskedKey() }; } @@ -107,10 +109,16 @@ public class ApiKeyCrudCommand extends BaseCrudCommand { Long validityPeriod = vpRaw == null ? null : (vpRaw instanceof Number ? ((Number) vpRaw).longValue() : Long.parseLong(vpRaw.toString())); - ApiKey apiKey = tenantService.generateApiKeyWithType(tenantId, keyType, validityPeriod); - if (apiKey == null) { + ApiKeyCreationResult creationResult = tenantService.generateApiKeyWithType(tenantId, keyType, validityPeriod); + if (creationResult == null || creationResult.getApiKey() == null) { throw new IllegalStateException("Failed to generate API key for tenant: " + tenantId); } + ApiKey apiKey = creationResult.getApiKey(); + + PrintStream console = getConsole(); + console.println("API key created. This is the only time the plaintext key will be shown:"); + console.println(" " + creationResult.getPlainTextKey()); + String name = (String) properties.get("name"); String description = (String) properties.get("description"); if (StringUtils.isNotBlank(name) || StringUtils.isNotBlank(description)) {
