diff --git a/api/src/main/java/org/apache/unomi/api/security/SecretHashService.java b/api/src/main/java/org/apache/unomi/api/security/SecretHashService.java new file mode 100644 index 000000000..aaab1481b --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/security/SecretHashService.java @@ -0,0 +1,45 @@ +/* + * 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; + +/** + * One-way hashing for secrets that must never be stored in plaintext. This is distinct from + * {@link EncryptionService}, which handles reversible encryption keys. + *

+ * API keys are machine-generated with high entropy and hashed with SHA-256 (lowercase hex) for + * storage and online verification. + */ +public interface SecretHashService { + + /** + * Hashes a secret with SHA-256 for storage. + * + * @param plaintext the secret to hash; must not be {@code null} + * @return lowercase hex-encoded SHA-256 digest + * @throws IllegalArgumentException if {@code plaintext} is {@code null} + */ + String hash(String plaintext); + + /** + * Verifies a secret against a stored SHA-256 digest using a constant-time comparison. + * + * @param plaintext the plaintext secret to verify + * @param storedHash the stored digest, as produced by {@link #hash(String)} + * @return {@code true} if the secret matches, {@code false} otherwise + */ + boolean verify(String plaintext, String storedHash); +} diff --git a/api/src/main/java/org/apache/unomi/api/security/SecurityService.java b/api/src/main/java/org/apache/unomi/api/security/SecurityService.java index 3d7e8a8f9..6b5f92cc3 100644 --- a/api/src/main/java/org/apache/unomi/api/security/SecurityService.java +++ b/api/src/main/java/org/apache/unomi/api/security/SecurityService.java @@ -70,6 +70,27 @@ public interface SecurityService { */ void clearCurrentSubject(); + /** + * Returns exactly the subject bound by {@link #setCurrentSubject(Subject)}, ignoring any + * active JAAS context or privileged subject override. Unlike {@link #getCurrentSubject()}, + * this never resolves to a higher-priority subject, so callers that need to snapshot and + * later restore only the request-subject slot (e.g. {@code executeAsSystem}) don't + * accidentally capture a privileged subject and leak it into the request-subject slot on + * restore. + * + * @return the request subject currently bound via {@link #setCurrentSubject(Subject)}, or + * {@code null} if none is bound + */ + Subject getRequestSubject(); + + /** + * Clears only the request-subject slot set by {@link #setCurrentSubject(Subject)}, without + * affecting any active privileged subject (unlike {@link #clearCurrentSubject()}, which + * clears both). Used to restore that slot to "unset" after a save/restore scope such as + * {@code executeAsSystem}. + */ + void clearRequestSubject(); + /** * Checks if the current context has a specific role by examining subjects in the following order: * 1. JAAS context 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..93f57f2a3 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 @@ -18,6 +18,7 @@ import org.apache.unomi.api.Item; +import java.security.SecureRandom; import java.util.Date; /** @@ -31,6 +32,17 @@ public class ApiKey extends Item { */ public static final String ITEM_TYPE = "apiKey"; + /** Prefix prepended to generated API key values. */ + public static final String KEY_PREFIX = "unomi_v1_"; + + /** Number of random bytes used when generating a new API key. */ + public static final int KEY_RANDOM_BYTES = 32; + + /** Number of trailing characters shown after the mask marker. */ + private static final int VISIBLE_SUFFIX_LENGTH = 4; + + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + /** * Enum defining the types of API keys. */ @@ -47,9 +59,16 @@ public enum ApiKeyType { } /** - * The API key value. + * SHA-256 hex digest of the API key ({@link org.apache.unomi.api.security.SecretHashService#hash(String)}). + * The plaintext key is never persisted; it is only returned once at creation time. + */ + 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 key; + private String maskedKey; /** * The type of API key (public or private). @@ -90,19 +109,67 @@ public ApiKey() { } /** - * Gets the API key value. - * @return the API key value + * Generates a new plaintext API key. + * + * @return a newly generated plaintext API key with the {@link #KEY_PREFIX} prefix + */ + public static String generatePlainTextKey() { + 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; + } + + /** + * Produces a display-safe masked representation of a plaintext API key. + * + * @param plainTextKey the plaintext API key to mask; may be {@code null} + * @return the masked key (e.g. {@code unomi_v1_****ab12}), or {@code null} when {@code plainTextKey} is {@code null} + */ + public static String maskPlainTextKey(String plainTextKey) { + if (plainTextKey == null) { + return null; + } + boolean hasPrefix = plainTextKey.startsWith(KEY_PREFIX); + String body = hasPrefix ? plainTextKey.substring(KEY_PREFIX.length()) : plainTextKey; + int suffixLength = Math.min(VISIBLE_SUFFIX_LENGTH, body.length()); + String lastVisible = body.substring(body.length() - suffixLength); + return (hasPrefix ? KEY_PREFIX : "") + "****" + lastVisible; + } + + /** + * Gets the SHA-256 digest of the API key. + * @return the key hash as lowercase hex + */ + public String getKeyHash() { + return keyHash; + } + + /** + * Sets the SHA-256 digest 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 getKey() { - return key; + public String getMaskedKey() { + return maskedKey; } /** - * Sets the API key value. - * @param key the API key value to set + * Sets the display-safe masked representation of the key. + * @param maskedKey the masked key to set */ - public void setKey(String key) { - this.key = key; + public void setMaskedKey(String maskedKey) { + this.maskedKey = maskedKey; } /** 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..a4d1dfc5c 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 @@ -269,11 +269,12 @@ public void setAuthorizedIPs(Set authorizedIPs) { } /** - * Gets the currently active private API key for the tenant. - * 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 + * Gets a display-safe, masked representation of the tenant's currently active private API + * key (e.g. for showing in UIs or audit logs). It resolves the most recently created, + * non-revoked, non-expired private key, but since the plaintext key is never persisted (see + * UNOMI-938), this cannot be used to authenticate as the tenant; the real plaintext key is + * only available once, at creation time, via {@link ApiKeyCreationResult}. + * @return the masked private API key, or null if no valid private key exists */ @XmlTransient public String getPrivateApiKey() { @@ -286,16 +287,17 @@ public String getPrivateApiKey() { .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); } /** - * Gets the currently active public API key for the tenant. - * 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 + * Gets a display-safe, masked representation of the tenant's currently active public API + * key (e.g. for showing in UIs or audit logs). It resolves the most recently created, + * non-revoked, non-expired public key, but since the plaintext key is never persisted (see + * UNOMI-938), this cannot be used in client-side applications; the real plaintext key is + * only available once, at creation time, via {@link ApiKeyCreationResult}. + * @return the masked public API key, or null if no valid public key exists */ @XmlTransient public String getPublicApiKey() { @@ -308,7 +310,7 @@ public String getPublicApiKey() { .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/ApiKeyTest.java b/api/src/test/java/org/apache/unomi/api/tenants/ApiKeyTest.java new file mode 100644 index 000000000..fb3ff3983 --- /dev/null +++ b/api/src/test/java/org/apache/unomi/api/tenants/ApiKeyTest.java @@ -0,0 +1,50 @@ +/* + * 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; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class ApiKeyTest { + + @Test + public void generatePlainTextKeyUsesConfiguredPrefixAndLength() { + String key = ApiKey.generatePlainTextKey(); + assertTrue(key.startsWith(ApiKey.KEY_PREFIX)); + assertEquals(ApiKey.KEY_PREFIX.length() + ApiKey.KEY_RANDOM_BYTES * 2, key.length()); + assertTrue(key.substring(ApiKey.KEY_PREFIX.length()).matches("[0-9A-F]+")); + } + + @Test + public void maskPlainTextKeyShowsPrefixAndLastFourCharacters() { + String key = ApiKey.KEY_PREFIX + "ABCDEFGH"; + assertEquals(ApiKey.KEY_PREFIX + "****EFGH", ApiKey.maskPlainTextKey(key)); + } + + @Test + public void maskPlainTextKeyReturnsNullForNullInput() { + assertNull(ApiKey.maskPlainTextKey(null)); + } + + @Test + public void maskPlainTextKeyHandlesShortBody() { + assertEquals(ApiKey.KEY_PREFIX + "****X", ApiKey.maskPlainTextKey(ApiKey.KEY_PREFIX + "X")); + } +} 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 void testGetPrivateApiKeyWithOnlyPublicKeys() { List 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 void testGetPrivateApiKeyWithRevokedKeys() { List 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 void testGetPrivateApiKeyWithExpiredKeys() { List 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 void testGetPrivateApiKeyWithValidKey() { List 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 void testGetPrivateApiKeyWithMultipleKeysReturnsLatest() { 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 void testGetPrivateApiKeyAlwaysResolvesFromApiKeys() { List 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 void testGetPublicApiKeyWithOnlyPrivateKeys() { List 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 void testGetPublicApiKeyWithValidKey() { List 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 void testGetPublicApiKeyAlwaysResolvesFromApiKeys() { List 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 void testGetActivePrivateApiKeys() { // 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 void testGetActivePrivateApiKeys() { List 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 void testGetActivePublicApiKeys() { // 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 void testGetActivePublicApiKeys() { List 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 void testGetActiveApiKeys() { // 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 void testGetActiveApiKeys() { List 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 void testMixedApiKeyTypes() { List 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 void testApiKeyExpirationLogic() { // 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 void testApiKeyExpirationLogic() { // 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 void testComplexApiKeyScenarios() { // 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 void testComplexApiKeyScenarios() { 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 void testApiKeyCreationDateOrdering() { // 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 void testApiKeyCreationDateOrdering() { // 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.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 @@ protected ObjectMapper getObjectMapper() { 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 void waitForStartup() throws InterruptedException { 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 void waitForStartup() throws InterruptedException { 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 void shutdown() { 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 @@ protected CloseableHttpResponse executeHttpRequest(HttpUriRequest request, AuthT // 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 @@ protected CloseableHttpResponse executeHttpRequest(HttpUriRequest request, AuthT // 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 @@ protected CloseableHttpResponse executeHttpRequest(HttpUriRequest request, AuthT // 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 void testMultipleLoginOnSameBrowser() throws Exception { 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 void testMultipleLoginOnSameBrowser() throws Exception { 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.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 void testUpdateEventFromContextAuthorizedThirdParty_Success() throws Exce 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 void testUpdateEventFromContextAuthorizedThirdParty_Success() throws Exce } 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 void testCreateEventWithPropertiesValidation_Success() throws Exception { //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 void testCreateEventWithPropertyValueValidation_Failure() throws Exceptio //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 void testCreateEventWithPropertyNameValidation_Failure() throws Exception //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 void test_advanced_ControlGroup_test() throws Exception { 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 void testContextEndpointAuthentication() throws Exception { 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 void testContextEndpointAuthentication() throws Exception { // 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 void testContextEndpointAuthentication() throws Exception { 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 controlGroupConfig, List expectedVariants, @@ -1010,12 +1011,14 @@ public void testConcealedProperties() throws Exception { 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.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 void testRestEndpoint() throws Exception { 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 void testRestEndpoint() throws Exception { // 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 void testTenantEndpointAuthentication() throws Exception { 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 void testPublicEndpointAuthentication() throws Exception { // 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 void testPrivateEndpointAuthentication() throws Exception { // 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 void testPrivateEndpointAuthentication() throws Exception { // 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 void testPrivateEndpointAuthentication() throws Exception { // 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 void testApiKeyAuthentication() throws Exception { 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 void testApiKeyAuthentication() throws Exception { } // 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 void testApiKeyAuthentication() throws Exception { 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 void testPublicPrivateApiKeys() throws Exception { 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 void testPublicPrivateApiKeys() throws Exception { 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 void testTenantLookupByApiKey() throws Exception { 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 @@ private void testV3ModeBehavior() throws Exception { // 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 @@ private void testV2ModeBehavior() throws Exception { // 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 @@ private void testV2ModeBehavior() throws Exception { // 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 void testV2CompatibilityProtectedEventNegativeCases() throws Exception { } } - 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 @@ 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 static CloseableHttpResponse doRequest(HttpUriRequest request, int expect 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/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java index 3f62cb884..dc90146c2 100644 --- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java +++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java @@ -777,6 +777,10 @@ private String loadMappingFile(URL predefinedMappingURL) throws IOException { return queryCount(Query.of(q -> q.matchAll(m -> m)), itemType); } + @Override public long getAllItemsCount(String itemType, String tenantId) { + return queryCount(Query.of(q -> q.matchAll(m -> m)), itemType, tenantId); + } + @Override public PartialList getAllItems(final Class clazz, int offset, int size, String sortBy) { return getAllItems(clazz, offset, size, sortBy, null); } @@ -2090,12 +2094,16 @@ public PartialList rangeQuery(String fieldName, String from, } private long queryCount(final Query query, final String itemType) { + return queryCount(query, itemType, getTenantId()); + } + + private long queryCount(final Query query, final String itemType, final String tenantId) { return new InClassLoaderExecute(metricsService, this.getClass().getName() + ".queryCount", this.bundleContext, this.fatalIllegalStateErrors, throwExceptions) { @Override protected Long execute(Object... args) throws IOException { CountRequest countRequest = CountRequest.of( - builder -> builder.index(getIndexNameForQuery(itemType)).query(wrapWithTenantAndItemTypeQuery(itemType, query, getTenantId()))); + builder -> builder.index(getIndexNameForQuery(itemType)).query(wrapWithTenantAndItemTypeQuery(itemType, query, tenantId))); return esClient.count(countRequest).count(); } }.catchingExecuteInClassLoader(true); diff --git a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java index 48ca106d7..05c2c50c6 100644 --- a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java +++ b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java @@ -685,6 +685,11 @@ public long getAllItemsCount(String itemType) { return queryCount(Query.of(q -> q.matchAll(t -> t)), itemType); } + @Override + public long getAllItemsCount(String itemType, String tenantId) { + return queryCount(Query.of(q -> q.matchAll(t -> t)), itemType, tenantId); + } + @Override public PartialList getAllItems(final Class clazz, int offset, int size, String sortBy) { return getAllItems(clazz, offset, size, sortBy, null); @@ -1974,12 +1979,16 @@ public long queryCount(Condition query, String itemType) { } private long queryCount(final Query filter, final String itemType) { + return queryCount(filter, itemType, getTenantId()); + } + + private long queryCount(final Query filter, final String itemType, final String tenantId) { return new InClassLoaderExecute(metricsService, this.getClass().getName() + ".queryCount", this.bundleContext, this.fatalIllegalStateErrors, throwExceptions) { @Override protected Long execute(Object... args) throws IOException { CountResponse response = client.count(count -> count.index(getIndexNameForQuery(itemType)) - .query(wrapWithTenantAndItemTypeQuery(itemType, filter, getTenantId()))); + .query(wrapWithTenantAndItemTypeQuery(itemType, filter, tenantId))); return response.count(); } }.catchingExecuteInClassLoader(true); diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java index 8e8b22622..0f6ca0d3b 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java @@ -619,6 +619,16 @@ default CustomItem loadCustomItem(String itemId, String customItemType) { */ long getAllItemsCount(String itemType); + /** + * Retrieves the number of items with the specified type as defined by the Item subclass public field {@code ITEM_TYPE} for the given tenant. + * + * @param itemType the String representation of the item type we want to retrieve the count of, as defined by its class' {@code ITEM_TYPE} field + * @param tenantId the ID of the tenant whose items should be counted + * @return the number of items of the specified type for the given tenant + * @see Item Item for a discussion of {@code ITEM_TYPE} + */ + long getAllItemsCount(String itemType, String tenantId); + /** * Retrieves the number of items with the specified type as defined by the Item subclass public field {@code ITEM_TYPE} matching the optional specified condition and * aggregated according to the specified {@link BaseAggregate}. diff --git a/rest/src/main/java/org/apache/unomi/rest/security/RequiresTenant.java b/rest/src/main/java/org/apache/unomi/rest/security/RequiresTenant.java index 132399229..597d49058 100644 --- a/rest/src/main/java/org/apache/unomi/rest/security/RequiresTenant.java +++ b/rest/src/main/java/org/apache/unomi/rest/security/RequiresTenant.java @@ -21,6 +21,14 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +/** + * Marks a JAX-RS resource method as tenant-scoped. {@link SecurityFilter} enforces this by + * reading the {@code tenantId} {@code @PathParam} from the request URI (e.g. + * {@code /tenants/{tenantId}/...}) and checking it against the caller's subject via + * {@link org.apache.unomi.api.security.SecurityService#hasTenantAccess(String)}. The annotated + * method's resource path must declare a {@code {tenantId}} path segment, or every request will + * be rejected with a 400. + */ @Target({ElementType.METHOD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) public @interface RequiresTenant { diff --git a/rest/src/main/java/org/apache/unomi/rest/security/SecurityFilter.java b/rest/src/main/java/org/apache/unomi/rest/security/SecurityFilter.java index d7359d82f..c2b0445aa 100644 --- a/rest/src/main/java/org/apache/unomi/rest/security/SecurityFilter.java +++ b/rest/src/main/java/org/apache/unomi/rest/security/SecurityFilter.java @@ -29,6 +29,7 @@ import javax.ws.rs.container.ResourceInfo; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; +import javax.ws.rs.core.UriInfo; import javax.ws.rs.ext.Provider; import java.io.IOException; import java.lang.reflect.Method; @@ -40,12 +41,18 @@ public class SecurityFilter implements ContainerRequestFilter { private static final Logger logger = LoggerFactory.getLogger(SecurityFilter.class); + /** Name of the {@code @PathParam} that identifies the tenant a {@link RequiresTenant} endpoint operates on. */ + private static final String TENANT_PATH_PARAM = "tenantId"; + @Reference private SecurityService securityService; @Context private ResourceInfo resourceInfo; + @Context + private UriInfo uriInfo; + @Override public void filter(ContainerRequestContext requestContext) throws IOException { Method method = resourceInfo.getResourceMethod(); @@ -71,18 +78,20 @@ public void filter(ContainerRequestContext requestContext) throws IOException { } } - // Check tenants-based access + // Check tenants-based access: the tenant being accessed comes from the request path + // (e.g. /tenants/{tenantId}/...), never from the caller's own subject — otherwise the + // check would just compare the subject's tenant against itself and always pass. if (tenantAnnotation != null) { - String tenantId = requestContext.getHeaderString("X-Unomi-Tenant"); - if (tenantId == null) { + String requestedTenantId = uriInfo.getPathParameters().getFirst(TENANT_PATH_PARAM); + if (requestedTenantId == null) { requestContext.abortWith(Response.status(Response.Status.BAD_REQUEST) .entity("Tenant ID is required") .build()); return; } - if (!securityService.hasTenantAccess(tenantId)) { + if (!securityService.hasTenantAccess(requestedTenantId)) { requestContext.abortWith(Response.status(Response.Status.FORBIDDEN) - .entity("User does not have access to tenants") + .entity("User does not have access to tenant") .build()); return; } 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.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 @@ private synchronized void refreshServer() { // 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 @@ 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 Response deleteTenant(@PathParam("tenantId") String tenantId) { * @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/ExecutionContextManagerImpl.java b/services-common/src/main/java/org/apache/unomi/services/common/security/ExecutionContextManagerImpl.java index 6cf39713a..c56de5cdc 100644 --- a/services-common/src/main/java/org/apache/unomi/services/common/security/ExecutionContextManagerImpl.java +++ b/services-common/src/main/java/org/apache/unomi/services/common/security/ExecutionContextManagerImpl.java @@ -78,7 +78,12 @@ public void setCurrentContext(ExecutionContext context) { @Override public T executeAsSystem(Supplier operation) { ExecutionContext previousContext = currentContext.get(); - Subject previousSubject = securityService.getCurrentSubject(); + // Snapshot only the request-subject slot this method mutates below (via + // setCurrentSubject), not the JAAS/privileged-aware getCurrentSubject(): otherwise, if a + // privileged subject is active on this (possibly pooled) thread, it would be captured + // here and then copied into the request-subject slot on restore, leaking it there even + // after the privileged scope itself ends. + Subject previousSubject = securityService.getRequestSubject(); try { if (operation == null) { throw new IllegalArgumentException("System operation cannot be null"); @@ -116,7 +121,11 @@ public T executeAsSystem(Supplier operation) { } else { currentContext.remove(); } - securityService.setCurrentSubject(previousSubject); + if (previousSubject == null) { + securityService.clearRequestSubject(); + } else { + securityService.setCurrentSubject(previousSubject); + } } catch (Exception e) { LOGGER.error("Error restoring previous context: {}", e.getMessage(), e); // Do not rethrow — would suppress the original operation exception if both fail together diff --git a/services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java b/services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java index b084dff8a..f945d90f2 100644 --- a/services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java +++ b/services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; /** @@ -47,7 +48,7 @@ public class KarafSecurityService implements SecurityService { /** The system tenant identifier used for system-wide operations. */ public static final String SYSTEM_TENANT = "system"; - private final Subject SYSTEM_SUBJECT; + private final AtomicReference systemSubject = new AtomicReference<>(); private SecurityServiceConfiguration configuration; private EncryptionService encryptionService; @@ -60,7 +61,7 @@ public class KarafSecurityService implements SecurityService { * Creates the security service and initializes the system subject. */ public KarafSecurityService() { - SYSTEM_SUBJECT = createSystemSubject(); + systemSubject.set(createSystemSubject()); } private Subject createSystemSubject() { @@ -90,12 +91,14 @@ public void destroy() { } private void updateSystemSubject() { - SYSTEM_SUBJECT.getPrincipals().clear(); - SYSTEM_SUBJECT.getPrincipals().add(new TenantPrincipal(SYSTEM_TENANT)); - SYSTEM_SUBJECT.getPrincipals().add(new UserPrincipal("system")); + Subject subject = new Subject(); + subject.getPrincipals().add(new TenantPrincipal(SYSTEM_TENANT)); + subject.getPrincipals().add(new UserPrincipal("system")); for (String role : configuration.getSystemRoles()) { - SYSTEM_SUBJECT.getPrincipals().add(new RolePrincipal(role)); + subject.getPrincipals().add(new RolePrincipal(role)); } + subject.setReadOnly(); + systemSubject.set(subject); } /** @@ -169,6 +172,16 @@ public void clearCurrentSubject() { privilegedSubject.remove(); } + @Override + public Subject getRequestSubject() { + return currentSubject.get(); + } + + @Override + public void clearRequestSubject() { + currentSubject.remove(); + } + /** * Sets a temporary privileged subject for operations that require elevated permissions. * This subject will be used in addition to the current subject for permission checks. @@ -315,7 +328,7 @@ public byte[] getTenantEncryptionKey(String tenantId) { @Override public Subject getSystemSubject() { - return SYSTEM_SUBJECT; + return systemSubject.get(); } @Override diff --git a/services-common/src/test/java/org/apache/unomi/services/common/security/KarafSecurityServiceTest.java b/services-common/src/test/java/org/apache/unomi/services/common/security/KarafSecurityServiceTest.java index a69244f65..686b0f03e 100644 --- a/services-common/src/test/java/org/apache/unomi/services/common/security/KarafSecurityServiceTest.java +++ b/services-common/src/test/java/org/apache/unomi/services/common/security/KarafSecurityServiceTest.java @@ -78,6 +78,7 @@ public void tearDown() { public void testGetSystemSubject() { Subject systemSubject = securityService.getSystemSubject(); assertNotNull("System subject should not be null", systemSubject); + assertTrue("System subject should be read-only", systemSubject.isReadOnly()); Set principals = systemSubject.getPrincipals(); assertTrue("System subject should have UserPrincipal", @@ -94,6 +95,28 @@ public void testGetSystemSubject() { roles.contains(UnomiRoles.SYSTEM_MAINTENANCE)); } + @Test + public void testSystemSubjectImmutableAfterReconfiguration() { + Subject originalSystemSubject = securityService.getSystemSubject(); + Set originalRoles = extractRoles(originalSystemSubject.getPrincipals()); + + SecurityServiceConfiguration newConfig = new SecurityServiceConfiguration(); + newConfig.setSystemRoles(new HashSet<>(Collections.singletonList(UnomiRoles.ADMINISTRATOR))); + securityService.setConfiguration(newConfig); + securityService.init(); + + Subject updatedSystemSubject = securityService.getSystemSubject(); + assertNotSame("System subject should be replaced atomically", + originalSystemSubject, updatedSystemSubject); + assertTrue("Updated system subject should be read-only", updatedSystemSubject.isReadOnly()); + assertEquals("Original subject principals should be unchanged", originalRoles, + extractRoles(originalSystemSubject.getPrincipals())); + assertTrue("Updated system subject should reflect new configuration", + extractRoles(updatedSystemSubject.getPrincipals()).contains(UnomiRoles.ADMINISTRATOR)); + assertFalse("Updated system subject should not retain removed roles", + extractRoles(updatedSystemSubject.getPrincipals()).contains(UnomiRoles.SYSTEM_MAINTENANCE)); + } + @Test public void testCurrentSubjectManagement() { // Test initial state diff --git a/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantQuotaService.java b/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantQuotaService.java index 9b3dc4ac1..0b0e7c017 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantQuotaService.java +++ b/services/src/main/java/org/apache/unomi/services/impl/tenants/TenantQuotaService.java @@ -66,7 +66,16 @@ public void deactivate() { } private ResourceQuota getTenantQuota(String tenantId) { - Tenant tenant = persistenceService.load(tenantId, Tenant.class); + Tenant tenant; + try { + tenant = persistenceService.load(tenantId, Tenant.class); + } catch (Exception e) { + // Distinguish "failed to load" from "tenant has no quota configured" in the logs: + // both currently fail open (unlimited) below, but a load failure should be visible + // to operators instead of silently looking identical to an unconfigured quota. + logger.error("Failed to load tenant {} while checking quota; failing open for this check", tenantId, e); + return null; + } return tenant != null ? tenant.getResourceQuota() : null; } @@ -102,17 +111,19 @@ private void updateUsageStatistics() { return; // Skip if shutting down or persistence service is unavailable } - try { - for (String tenantId : usageCache.keySet()) { - if (shutdownNow) return; // Check shutdown flag during iteration - + for (String tenantId : usageCache.keySet()) { + if (shutdownNow) return; // Check shutdown flag during iteration + + try { TenantUsage usage = usageCache.get(tenantId); - usage.setProfileCount(persistenceService.getAllItemsCount("profile")); - usage.setEventCount(persistenceService.getAllItemsCount("event")); + usage.setProfileCount(persistenceService.getAllItemsCount("profile", tenantId)); + usage.setEventCount(persistenceService.getAllItemsCount("event", tenantId)); // Note: Storage size calculation would require additional implementation + } catch (Exception e) { + // Isolate failures per tenant so one tenant's error (e.g. a not-yet-ready index) + // doesn't leave every other tenant's usage counts stale for this refresh cycle. + logger.error("Error updating usage statistics for tenant {}", tenantId, e); } - } catch (Exception e) { - logger.error("Error updating tenant usage statistics", e); } } 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..363a9a70c 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.SecretHashService; 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.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 lifecycleListeners = new CopyOnWriteArrayList<>(); private PersistenceService persistenceService; private ExecutionContextManager executionContextManager; + private SecretHashService secretHashService; public void setPersistenceService(PersistenceService persistenceService) { this.persistenceService = persistenceService; @@ -49,6 +49,10 @@ public void setExecutionContextManager(ExecutionContextManager executionContextM this.executionContextManager = executionContextManager; } + public void setSecretHashService(SecretHashService secretHashService) { + this.secretHashService = secretHashService; + } + public void bindListener(TenantLifecycleListener listener) { lifecycleListeners.add(listener); LOGGER.debug("Added tenant lifecycle listener: {}", listener.getClass().getName()); @@ -96,29 +100,42 @@ public Tenant createTenant(String requestedId, Map properties) { // Save tenant first to ensure it exists persistenceService.save(tenant); - // Generate both public and private API keys - generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); - generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + try { + // Generate both public and private API keys + generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + } catch (RuntimeException e) { + // Roll back rather than leave a partially-initialized tenant (e.g. missing its + // private key) persisted after a failure partway through key generation. + persistenceService.remove(tenant.getItemId(), Tenant.class); + throw e; + } 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 = ApiKey.generatePlainTextKey(); + ApiKey apiKey = new ApiKey(); apiKey.setItemId(UUID.randomUUID().toString()); - String key = generateSecureKey(); - apiKey.setKey(key); + apiKey.setKeyHash(secretHashService.hash(plainTextKey)); + apiKey.setMaskedKey(ApiKey.maskPlainTextKey(plainTextKey)); apiKey.setKeyType(keyType); apiKey.setCreationDate(new Date()); if (validityPeriod != null) { @@ -136,7 +153,7 @@ public ApiKey generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, persistenceService.save(tenant); } - return apiKey; + return new ApiKeyCreationResult(apiKey, plainTextKey); }); } @@ -150,12 +167,6 @@ public Tenant getTenant(String tenantId) { 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 +176,18 @@ public void saveTenant(Tenant tenant) { 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 +206,17 @@ public boolean validateApiKeyWithType(String tenantId, String key, ApiKey.ApiKey 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 + && secretHashService.verify(plainTextKey, apiKey.getKeyHash()); + } + @Override public ApiKey getApiKey(String tenantId, ApiKey.ApiKeyType keyType) { return executionContextManager.executeAsSystem(() -> { @@ -216,14 +233,7 @@ public ApiKey getApiKey(String tenantId, ApiKey.ApiKeyType keyType) { @Override public Tenant getTenantByApiKey(String apiKey) { - return executionContextManager.executeAsSystem(() -> { - List tenants = persistenceService.getAllItems(Tenant.class); - return tenants.stream() - .filter(tenant -> tenant.getApiKeys().stream() - .anyMatch(key -> key.getKey().equals(apiKey))) - .findFirst() - .orElse(null); - }); + return getTenantByApiKey(apiKey, null); } @Override @@ -231,8 +241,8 @@ public Tenant getTenantByApiKey(String apiKey, ApiKey.ApiKeyType keyType) { return executionContextManager.executeAsSystem(() -> { List 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 -> (keyType == null || key.getKeyType() == keyType) && matchesKey(key, apiKey))) .findFirst() .orElse(null); }); diff --git a/services/src/main/java/org/apache/unomi/services/security/SecretHashServiceImpl.java b/services/src/main/java/org/apache/unomi/services/security/SecretHashServiceImpl.java new file mode 100644 index 000000000..6d44dfdb4 --- /dev/null +++ b/services/src/main/java/org/apache/unomi/services/security/SecretHashServiceImpl.java @@ -0,0 +1,64 @@ +/* + * 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.security; + +import org.apache.unomi.api.security.SecretHashService; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; + +/** + * Default {@link SecretHashService} implementation using SHA-256 for one-way secret storage. + */ +public class SecretHashServiceImpl implements SecretHashService { + + /** Digest algorithm for stored API keys. */ + public static final String HASH_ALGORITHM = "SHA-256"; + + @Override + public String hash(String plaintext) { + if (plaintext == null) { + throw new IllegalArgumentException("plaintext cannot be null"); + } + return sha256Hex(plaintext); + } + + @Override + public boolean verify(String plaintext, String storedHash) { + if (plaintext == null || storedHash == null) { + return false; + } + return MessageDigest.isEqual( + sha256Hex(plaintext).getBytes(StandardCharsets.UTF_8), + storedHash.getBytes(StandardCharsets.UTF_8)); + } + + private String sha256Hex(String plaintext) { + try { + byte[] digest = MessageDigest.getInstance(HASH_ALGORITHM) + .digest(plaintext.getBytes(StandardCharsets.UTF_8)); + StringBuilder hex = new StringBuilder(digest.length * 2); + for (byte b : digest) { + hex.append(String.format("%02x", b)); + } + return hex.toString(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("Unable to compute SHA-256 hash", e); + } + } +} diff --git a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 1bba07ee3..0f93bb1d1 100644 --- a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -456,9 +456,12 @@ + + + diff --git a/services/src/test/java/org/apache/unomi/services/impl/ExecutionContextManagerImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/ExecutionContextManagerImplTest.java index 4a732c83d..12e0cea35 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/ExecutionContextManagerImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/ExecutionContextManagerImplTest.java @@ -16,6 +16,7 @@ */ package org.apache.unomi.services.impl; +import org.apache.karaf.jaas.boot.principal.UserPrincipal; import org.apache.unomi.api.ExecutionContext; import org.apache.unomi.api.security.SecurityService; import org.apache.unomi.api.security.SecurityServiceConfiguration; @@ -34,6 +35,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -88,6 +90,7 @@ public void testExecuteAsSystem() { Set systemPermissions = new HashSet<>(Arrays.asList("READ", "WRITE", SecurityServiceConfiguration.PERMISSION_DELETE, "ADMIN")); // Mock security service behavior + when(securityService.getRequestSubject()).thenReturn(null); when(securityService.getSystemSubject()).thenReturn(systemSubject); when(securityService.extractRolesFromSubject(systemSubject)).thenReturn(systemRoles); when(securityService.getPermissionsForRole(UnomiRoles.ADMINISTRATOR)).thenReturn(systemPermissions); @@ -106,6 +109,55 @@ public void testExecuteAsSystem() { verify(securityService).getSystemSubject(); verify(securityService).extractRolesFromSubject(systemSubject); verify(securityService).getPermissionsForRole(UnomiRoles.ADMINISTRATOR); + verify(securityService).clearRequestSubject(); + verify(securityService, never()).setCurrentSubject(null); + } + + @Test + public void testExecuteAsSystemRestoresPreviousSubject() { + Subject previousSubject = new Subject(); + previousSubject.getPrincipals().add(new UserPrincipal("previous")); + Subject systemSubject = new Subject(); + systemSubject.getPrincipals().add(new UserPrincipal("system")); + Set systemRoles = new HashSet<>(Arrays.asList(UnomiRoles.ADMINISTRATOR)); + Set systemPermissions = new HashSet<>(Arrays.asList("ADMIN")); + + when(securityService.getRequestSubject()).thenReturn(previousSubject); + when(securityService.getSystemSubject()).thenReturn(systemSubject); + when(securityService.extractRolesFromSubject(systemSubject)).thenReturn(systemRoles); + when(securityService.getPermissionsForRole(UnomiRoles.ADMINISTRATOR)).thenReturn(systemPermissions); + + contextManager.executeAsSystem(() -> "done"); + + verify(securityService).setCurrentSubject(systemSubject); + verify(securityService).setCurrentSubject(previousSubject); + verify(securityService, never()).clearRequestSubject(); + } + + @Test + public void testExecuteAsSystemDoesNotLeakPrivilegedSubjectIntoRequestSubject() { + // Regression test: previously, executeAsSystem captured getCurrentSubject() (which + // resolves a privileged subject ahead of the request subject) and restored it via + // setCurrentSubject(), copying an active privileged subject into the request-subject + // slot even though it was never actually stored there. + Subject systemSubject = new Subject(); + systemSubject.getPrincipals().add(new UserPrincipal("system")); + Set systemRoles = new HashSet<>(Arrays.asList(UnomiRoles.ADMINISTRATOR)); + Set systemPermissions = new HashSet<>(Arrays.asList("ADMIN")); + + // The request-subject slot itself is empty; only a privileged subject is active. + when(securityService.getRequestSubject()).thenReturn(null); + when(securityService.getSystemSubject()).thenReturn(systemSubject); + when(securityService.extractRolesFromSubject(systemSubject)).thenReturn(systemRoles); + when(securityService.getPermissionsForRole(UnomiRoles.ADMINISTRATOR)).thenReturn(systemPermissions); + + contextManager.executeAsSystem(() -> "done"); + + // Restore must clear the request-subject slot, not copy anything into it, and must + // never touch the privileged subject at all. + verify(securityService).clearRequestSubject(); + verify(securityService, never()).setCurrentSubject(null); + verify(securityService, never()).clearCurrentSubject(); } @Test diff --git a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java index facd96002..1afbb3d32 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java +++ b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java @@ -1358,6 +1358,27 @@ public long getAllItemsCount(String itemType) { return filteredItems.size(); } + @Override + public long getAllItemsCount(String itemType, String tenantId) { + if (itemType == null || tenantId == null) { + return 0; + } + + LOGGER.debug("Counting all items of type {} for tenant {}", itemType, tenantId); + + Map filteredItems = new HashMap<>(); + for (Map.Entry entry : itemsById.entrySet()) { + Item item = entry.getValue(); + if (item.getItemType().equals(itemType) && tenantId.equals(item.getTenantId())) { + String itemKey = entry.getKey(); + if (isItemAvailableForQuery(itemKey, itemType)) { + filteredItems.put(itemKey, item); + } + } + } + return filteredItems.size(); + } + @Override public Map getSingleValuesMetrics(Condition condition, String[] metrics, String field, String itemType) { if (metrics == null || metrics.length == 0 || field == null) { diff --git a/services/src/test/java/org/apache/unomi/services/impl/TestSecretHashService.java b/services/src/test/java/org/apache/unomi/services/impl/TestSecretHashService.java new file mode 100644 index 000000000..c1fcda8ff --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/impl/TestSecretHashService.java @@ -0,0 +1,45 @@ +/* + * 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; + +import org.apache.unomi.api.security.SecretHashService; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +/** + * Fast {@link SecretHashService} for in-memory test doubles such as {@link TestTenantService}. + * Production-grade hashing is covered by {@link org.apache.unomi.services.security.SecretHashServiceImplTest}. + */ +public class TestSecretHashService implements SecretHashService { + + @Override + public String hash(String plaintext) { + if (plaintext == null) { + throw new IllegalArgumentException("plaintext cannot be null"); + } + return "test:" + Base64.getEncoder().encodeToString(plaintext.getBytes(StandardCharsets.UTF_8)); + } + + @Override + public boolean verify(String plaintext, String storedHash) { + if (plaintext == null || storedHash == null) { + return false; + } + return storedHash.equals(hash(plaintext)); + } +} 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..59723a95f 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,13 @@ */ package org.apache.unomi.services.impl; +import org.apache.unomi.api.security.SecretHashService; 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 javax.xml.bind.DatatypeConverter; -import java.security.SecureRandom; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -31,7 +31,15 @@ public class TestTenantService implements TenantService { private ThreadLocal currentTenantId = new ThreadLocal<>(); private Map tenants = new ConcurrentHashMap<>(); private ThreadLocal inSystemOperation = new ThreadLocal<>(); - private static final SecureRandom secureRandom = new SecureRandom(); + private final SecretHashService secretHashService; + + public TestTenantService() { + this(new TestSecretHashService()); + } + + public TestTenantService(SecretHashService secretHashService) { + this.secretHashService = secretHashService; + } public void setInSystemOperation(boolean inSystemOperation) { this.inSystemOperation.set(inSystemOperation); @@ -86,12 +94,17 @@ public boolean validateApiKeyWithType(String tenantId, String key, ApiKey.ApiKey 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 + && secretHashService.verify(plainTextKey, apiKey.getKeyHash()); + } + @Override public Tenant createTenant(String tenantId, Map properties) { Tenant tenant = new Tenant(); @@ -112,16 +125,18 @@ public Tenant createTenant(String tenantId, Map properties) { } @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 = ApiKey.generatePlainTextKey(); + ApiKey apiKey = new ApiKey(); apiKey.setItemId(UUID.randomUUID().toString()); - String key = generateSecureKey(); - apiKey.setKey(key); + apiKey.setKeyHash(secretHashService.hash(plainTextKey)); + apiKey.setMaskedKey(ApiKey.maskPlainTextKey(plainTextKey)); apiKey.setKeyType(keyType); apiKey.setCreationDate(new Date()); if (validityPeriod != null) { @@ -139,7 +154,7 @@ public ApiKey generateApiKeyWithType(String tenantId, ApiKey.ApiKeyType keyType, saveTenant(tenant); } - return apiKey; + return new ApiKeyCreationResult(apiKey, plainTextKey); } @Override @@ -147,7 +162,7 @@ public Tenant getTenantByApiKey(String apiKey) { 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 +172,7 @@ public Tenant getTenantByApiKey(String apiKey, ApiKey.ApiKeyType keyType) { 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 +188,4 @@ public ApiKey getApiKey(String tenantId, ApiKey.ApiKeyType keyType) { } 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..5885614ba 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,7 +17,9 @@ 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.apache.unomi.services.security.SecretHashServiceImpl; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -26,12 +28,18 @@ /** * Test for the TestTenantService to verify it works correctly with API key functionality. + * Uses the production {@link SecretHashServiceImpl} so hashing behaviour is validated here; + * other tests use the default fast {@link TestSecretHashService} via {@link TestTenantService#TestTenantService()}. */ public class TestTenantServiceTest { + private TestTenantService newTenantService() { + return new TestTenantService(new SecretHashServiceImpl()); + } + @Test public void testCreateTenantWithApiKeys() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); // Create a tenant Tenant tenant = tenantService.createTenant("test-tenant", Collections.emptyMap()); @@ -58,18 +66,20 @@ public void testCreateTenantWithApiKeys() { @Test public void testGenerateApiKeyWithType() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); // Create a tenant first 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"); @@ -79,12 +89,12 @@ public void testGenerateApiKeyWithType() { @Test public void testValidateApiKey() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); - // 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"); @@ -95,12 +105,12 @@ public void testValidateApiKey() { @Test public void testValidateApiKeyWithType() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); - // 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), @@ -115,11 +125,11 @@ public void testValidateApiKeyWithType() { @Test public void testGetTenantByApiKey() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); - // 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); @@ -137,7 +147,7 @@ public void testGetTenantByApiKey() { @Test public void testGetApiKey() { - TestTenantService tenantService = new TestTenantService(); + TestTenantService tenantService = newTenantService(); // Create a tenant tenantService.createTenant("test-tenant", Collections.emptyMap()); diff --git a/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantQuotaServiceTest.java b/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantQuotaServiceTest.java new file mode 100644 index 000000000..5125f1e38 --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantQuotaServiceTest.java @@ -0,0 +1,85 @@ +/* + * 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.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 java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class TenantQuotaServiceTest { + + @Mock + private PersistenceService persistenceService; + + private TenantQuotaService tenantQuotaService; + + @BeforeEach + public void setUp() throws Exception { + tenantQuotaService = new TenantQuotaService(); + tenantQuotaService.setPersistenceService(persistenceService); + + Field usageCacheField = TenantQuotaService.class.getDeclaredField("usageCache"); + usageCacheField.setAccessible(true); + @SuppressWarnings("unchecked") + Map usageCache = (Map) usageCacheField.get(tenantQuotaService); + usageCache.put("tenant-a", new TenantUsage()); + usageCache.put("tenant-b", new TenantUsage()); + } + + @Test + public void updateUsageStatisticsUsesPerTenantCounts() throws Exception { + when(persistenceService.getAllItemsCount(eq("profile"), eq("tenant-a"))).thenReturn(10L); + when(persistenceService.getAllItemsCount(eq("event"), eq("tenant-a"))).thenReturn(20L); + when(persistenceService.getAllItemsCount(eq("profile"), eq("tenant-b"))).thenReturn(100L); + when(persistenceService.getAllItemsCount(eq("event"), eq("tenant-b"))).thenReturn(200L); + + Method updateMethod = TenantQuotaService.class.getDeclaredMethod("updateUsageStatistics"); + updateMethod.setAccessible(true); + updateMethod.invoke(tenantQuotaService); + + Field usageCacheField = TenantQuotaService.class.getDeclaredField("usageCache"); + usageCacheField.setAccessible(true); + @SuppressWarnings("unchecked") + Map usageCache = (Map) usageCacheField.get(tenantQuotaService); + + assertEquals(10L, usageCache.get("tenant-a").getProfileCount()); + assertEquals(20L, usageCache.get("tenant-a").getEventCount()); + assertEquals(100L, usageCache.get("tenant-b").getProfileCount()); + assertEquals(200L, usageCache.get("tenant-b").getEventCount()); + + verify(persistenceService).getAllItemsCount("profile", "tenant-a"); + verify(persistenceService).getAllItemsCount("event", "tenant-a"); + verify(persistenceService).getAllItemsCount("profile", "tenant-b"); + verify(persistenceService).getAllItemsCount("event", "tenant-b"); + verify(persistenceService, never()).getAllItemsCount("profile"); + verify(persistenceService, never()).getAllItemsCount("event"); + } +} 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..aad74bebf --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/impl/tenants/TenantServiceImplTest.java @@ -0,0 +1,185 @@ +/* + * 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.SecretHashService; +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.never; +import static org.mockito.Mockito.verify; +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 SecretHashService secretHashService; + + private TenantServiceImpl tenantService; + + @BeforeEach + public void setUp() { + tenantService = new TenantServiceImpl(); + tenantService.setPersistenceService(persistenceService); + tenantService.setExecutionContextManager(executionContextManager); + tenantService.setSecretHashService(secretHashService); + + 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 hash implementation. + when(secretHashService.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 getTenantByApiKeyRejectsWhenVerifyFails() { + Tenant tenant = new Tenant(); + tenant.setItemId("tenant-with-keys"); + ApiKey apiKey = new ApiKey(); + apiKey.setKeyHash("stored-hash"); + apiKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); + tenant.setApiKeys(new ArrayList<>(List.of(apiKey))); + + when(persistenceService.getAllItems(Tenant.class)).thenReturn(List.of(tenant)); + when(secretHashService.verify("wrong-key", "stored-hash")).thenReturn(false); + + assertNull(tenantService.getTenantByApiKey("wrong-key"), + "A key that fails verification should be rejected"); + verify(secretHashService).verify("wrong-key", "stored-hash"); + } + + @Test + public void getTenantByApiKeyAcceptsWhenVerifySucceeds() { + Tenant tenant = new Tenant(); + tenant.setItemId("tenant-with-keys"); + ApiKey apiKey = new ApiKey(); + apiKey.setKeyHash("stored-hash"); + apiKey.setKeyType(ApiKey.ApiKeyType.PUBLIC); + tenant.setApiKeys(new ArrayList<>(List.of(apiKey))); + + when(persistenceService.getAllItems(Tenant.class)).thenReturn(List.of(tenant)); + when(secretHashService.verify("candidate-key", "stored-hash")).thenReturn(true); + + Tenant found = tenantService.getTenantByApiKey("candidate-key"); + assertEquals("tenant-with-keys", found.getItemId(), + "A verified key should resolve to its tenant"); + verify(secretHashService).verify("candidate-key", "stored-hash"); + } + + @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/services/src/test/java/org/apache/unomi/services/security/SecretHashServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/security/SecretHashServiceImplTest.java new file mode 100644 index 000000000..2d4a12b9f --- /dev/null +++ b/services/src/test/java/org/apache/unomi/services/security/SecretHashServiceImplTest.java @@ -0,0 +1,85 @@ +/* + * 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.security; + +import org.apache.unomi.api.tenants.ApiKey; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SecretHashServiceImplTest { + + private final SecretHashServiceImpl service = new SecretHashServiceImpl(); + + @Test + void hashProducesLowercaseHexSha256() { + String stored = service.hash("secret-value"); + assertEquals(64, stored.length()); + assertTrue(stored.matches("[0-9a-f]+")); + } + + @Test + void hashIsDeterministic() { + assertEquals(service.hash("same-secret"), service.hash("same-secret")); + } + + @Test + void hashDiffersForDifferentInputs() { + assertNotEquals(service.hash("secret-a"), service.hash("secret-b")); + } + + @Test + void verifyAcceptsCorrectPlaintext() { + String plaintext = "my-api-key-value"; + String stored = service.hash(plaintext); + assertTrue(service.verify(plaintext, stored)); + } + + @Test + void verifyRejectsWrongPlaintext() { + String stored = service.hash("correct"); + assertFalse(service.verify("wrong", stored)); + } + + @Test + void verifyRejectsNullPlaintext() { + assertFalse(service.verify(null, service.hash("x"))); + } + + @Test + void verifyRejectsNullStoredHash() { + assertFalse(service.verify("x", null)); + } + + @Test + void hashRejectsNullPlaintext() { + assertThrows(IllegalArgumentException.class, () -> service.hash(null)); + } + + @Test + void apiKeyHashAndVerifyRoundTrip() { + String key = ApiKey.generatePlainTextKey(); + String stored = service.hash(key); + assertTrue(service.verify(key, stored)); + assertFalse(service.verify(key + "x", stored)); + } +} 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..fc2468dcc 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 @@ -3,6 +3,8 @@ import org.apache.unomi.shell.migration.utils.MigrationUtils import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths +import java.nio.charset.StandardCharsets +import java.security.MessageDigest import java.security.SecureRandom import java.time.ZonedDateTime import java.time.format.DateTimeFormatter @@ -33,6 +35,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 SecretHashServiceImpl.hash does +// (SHA-256, lowercase hex) so it can be verified by TenantServiceImpl after migration without +// ever persisting the plaintext value (see UNOMI-938). +// +// IMPORTANT: this script cannot depend on the `services` bundle, so the algorithm must match +// SecretHashServiceImpl.HASH_ALGORITHM ("SHA-256") and UTF-8 encoding. +def hashApiKey = { String plainTextKey -> + byte[] digest = MessageDigest.getInstance("SHA-256") + .digest(plainTextKey.getBytes(StandardCharsets.UTF_8)) + StringBuilder hex = new StringBuilder(digest.length * 2) + for (byte b : digest) { + hex.append(String.format("%02x", b)) + } + return hex.toString() +} + +// Masks a plaintext API key the same way ApiKey.maskPlainTextKey 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 +82,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 +149,8 @@ ${sep} "lastModifiedBy": "system-migration-3.1.0", "creationDate" : "${isoDate}", "lastModificationDate" : "${isoDate}", - "key" : "${generatedPublicKey}", + "keyHash" : "${publicKeyHash}", + "maskedKey" : "${publicKeyMasked}", "keyType" : "PUBLIC", "revoked" : false }, @@ -128,7 +161,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-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.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.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 PROPERTY_NAMES = List.of( - "itemId", "name", "description", "keyType", "key", "tenantId", "validityPeriod" + "itemId", "name", "description", "keyType", "tenantId", "validityPeriod" ); @Reference @@ -59,7 +61,7 @@ protected String[] getHeadersWithoutTenant() { "Name", "Description", "Key Type", - "Key" + "Key (masked)" }; } @@ -91,7 +93,7 @@ protected String[] buildRow(Object item) { apiKey.getName(), apiKey.getDescription(), apiKey.getKeyType().toString(), - apiKey.getKey() + apiKey.getMaskedKey() }; } @@ -107,10 +109,16 @@ public String create(Map properties) { 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)) {