address comments

wso2 · Dec 10, 2024 · e812eec · e812eec
1 parent dd985fb
commit e812eec
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 115 deletions.
diff --git a/components/identity-core/org.wso2.carbon.identity.core/pom.xml b/components/identity-core/org.wso2.carbon.identity.core/pom.xml
@@ -208,6 +208,7 @@
                             org.apache.commons.collections; version="${commons-collections.wso2.osgi.version.range}",
                             org.apache.commons.collections4; version = "${commons-collections4.wso2.osgi.version.range}",
                             ua_parser; version="${ua_parser.version.range}",
+                            org.wso2.carbon.utils.security;version="${carbon.kernel.package.import.version.range}",
                         </Import-Package>
                         <Export-Package>
                             !org.wso2.carbon.identity.core.internal,

diff --git a/...n.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java b/...n.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java
@@ -268,10 +268,10 @@ private Certificate getCertificate(String tenantDomain, String context) throws I
             getCertificate(tenantDomain);
         }
         int tenantId = IdentityTenantUtil.getTenantId(tenantDomain);
-        if (publicCerts.containsKey(String.valueOf(tenantId +
-                IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context))) {
-            return publicCerts.get(String.valueOf(tenantId +
-                    IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context));
+        if (publicCerts.containsKey(tenantId +
+                IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context)) {
+            return publicCerts.get(tenantId +
+                    IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context);
         }
 
         KeyStoreManager keyStoreManager = KeyStoreManager.getInstance(tenantId);
@@ -288,8 +288,8 @@ private Certificate getCertificate(String tenantDomain, String context) throws I
                             tenantDomain), e);
         }
 
-        publicCerts.put(String.valueOf(tenantId +
-                IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context), publicCert);
+        publicCerts.put(tenantId +
+                IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context, publicCert);
         return publicCert;
     }
 

diff --git a/...y.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityKeyStoreResolverUtil.java b/...y.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityKeyStoreResolverUtil.java
@@ -21,6 +21,7 @@
 import org.apache.commons.lang.StringUtils;
 import org.wso2.carbon.core.RegistryResources;
 import org.wso2.carbon.identity.core.util.IdentityKeyStoreResolverConstants.ErrorMessages;
+import org.wso2.carbon.utils.security.KeystoreUtils;
 
 import javax.xml.namespace.QName;
 
@@ -67,12 +68,12 @@ public static String buildTenantKeyStoreName(String tenantDomain, String context
         String ksName = tenantDomain.trim().replace(".", "-");
 
         // Append context if provided
-        if (!StringUtils.isEmpty(context)) {
+        if (StringUtils.isNotBlank(context)) {
             ksName = ksName + IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR + context.trim();
         }
 
         // Add the keystore extension
-        return ksName + IdentityKeyStoreResolverConstants.KEY_STORE_EXTENSION;
+        return ksName + KeystoreUtils.getKeyStoreFileExtension(tenantDomain);
     }
 
     /**

diff --git a/...2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityUtil.java b/...2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityUtil.java
@@ -2001,7 +2001,7 @@ public static boolean validateSignatureFromTenant(String data, byte[] signature,
             } else {
                 // Fetch certificate within the provided context
                 publicKey = IdentityKeyStoreResolver.getInstance()
-                        .getCertificate(tenantDomain, null, context)
+                        .getCertificate(tenantDomain,  null, context)
                         .getPublicKey();
             }
 

diff --git a/...entity.core/src/test/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolverTest.java b/...entity.core/src/test/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolverTest.java
@@ -30,6 +30,7 @@
 import org.wso2.carbon.identity.core.util.IdentityConfigParser;
 import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
 import org.wso2.carbon.utils.multitenancy.MultitenantConstants;
+import org.wso2.carbon.utils.security.KeystoreUtils;
 
 import java.io.FileInputStream;
 import java.lang.reflect.Field;
@@ -84,6 +85,7 @@ public class IdentityKeyStoreResolverTest extends TestCase {
 
     private MockedStatic<IdentityConfigParser> identityConfigParser;
     private MockedStatic<IdentityTenantUtil> identityTenantUtil;
+    private MockedStatic<KeystoreUtils> keystoreUtils;
 
     private IdentityKeyStoreResolver identityKeyStoreResolver;
 
@@ -143,13 +145,15 @@ public void setUp() throws Exception {
         when(keyStoreManager.getCertificate("CUSTOM/" + CUSTOM_KEY_STORE, null)).thenReturn(customCertificate);
 
         identityKeyStoreResolver = IdentityKeyStoreResolver.getInstance();
+        keystoreUtils = mockStatic(KeystoreUtils.class);
     }
 
     @AfterClass
     public void close() {
 
         identityConfigParser.close();
         identityTenantUtil.close();
+        keystoreUtils.close();
     }
 
     @Test
@@ -210,6 +214,7 @@ public Object[][] keyStoreDataProvider() {
     @Test(dataProvider = "KeyStoreDataProvider")
     public void testGetKeyStore(String tenantDomain, InboundProtocol inboundProtocol, KeyStore expectedKeyStore) throws Exception {
 
+        keystoreUtils.when(() -> KeystoreUtils.getKeyStoreFileExtension(tenantDomain)).thenReturn(".jks");
         assertEquals(expectedKeyStore, identityKeyStoreResolver.getKeyStore(tenantDomain, inboundProtocol));
     }
 
@@ -229,6 +234,7 @@ public Object[][] privateKeyDataProvider() {
     @Test(dataProvider = "PrivateKeyDataProvider")
     public void testGetPrivateKey(String tenantDomain, InboundProtocol inboundProtocol, PrivateKey expectedKey)  throws Exception {
 
+        keystoreUtils.when(() -> KeystoreUtils.getKeyStoreFileExtension(tenantDomain)).thenReturn(".jks");
         assertEquals(expectedKey, identityKeyStoreResolver.getPrivateKey(tenantDomain, inboundProtocol));
     }
 
@@ -248,6 +254,7 @@ public Object[][] publicCertificateDataProvider() {
     @Test(dataProvider = "PublicCertificateDataProvider")
     public void testGetCertificate(String tenantDomain, InboundProtocol inboundProtocol, X509Certificate expectedCert) throws Exception {
 
+        keystoreUtils.when(() -> KeystoreUtils.getKeyStoreFileExtension(tenantDomain)).thenReturn(".jks");
         assertEquals(expectedCert, identityKeyStoreResolver.getCertificate(tenantDomain, inboundProtocol));
     }
 

diff --git a/...re/src/test/java/org/wso2/carbon/identity/core/util/IdentityKeyStoreResolverUtilTest.java b/...re/src/test/java/org/wso2/carbon/identity/core/util/IdentityKeyStoreResolverUtilTest.java
@@ -18,9 +18,14 @@
 
 package org.wso2.carbon.identity.core.util;
 
+import org.mockito.MockedStatic;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
+import org.wso2.carbon.utils.security.KeystoreUtils;
 
+import static org.mockito.Mockito.mockStatic;
 import static org.testng.Assert.assertEquals;
 
 import static org.wso2.carbon.identity.core.util.IdentityKeyStoreResolverUtil.buildCustomKeyStoreName;
@@ -34,6 +39,14 @@
  */
 public class IdentityKeyStoreResolverUtilTest {
 
+    private MockedStatic<KeystoreUtils> keystoreUtils;
+
+    @BeforeClass
+    public void setUp() throws Exception {
+
+        keystoreUtils = mockStatic(KeystoreUtils.class);
+    }
+
     @DataProvider(name = "CorrectTenantKeyStoreNameDataProvider")
     public Object[][] correctTenantKeyStoreNameDataProvider() {
 
@@ -43,10 +56,17 @@ public Object[][] correctTenantKeyStoreNameDataProvider() {
         };
     }
 
+    @AfterClass
+    public void close() {
+
+        keystoreUtils.close();
+    }
+
     @Test(dataProvider = "CorrectTenantKeyStoreNameDataProvider")
     public void testCorrectBuildTenantKeyStoreName(String tenantDomain, String expectedResult) throws IdentityKeyStoreResolverException {
 
-        assertEquals(expectedResult, buildTenantKeyStoreName(tenantDomain));
+        keystoreUtils.when(() -> KeystoreUtils.getKeyStoreFileExtension(tenantDomain)).thenReturn(".jks");
+        assertEquals(buildTenantKeyStoreName(tenantDomain), expectedResult);
     }
 
     @DataProvider(name = "IncorrectTenantKeyStoreNameDataProvider")

diff --git a/...rbon.identity.core/src/test/java/org/wso2/carbon/identity/core/util/IdentityUtilTest.java b/...rbon.identity.core/src/test/java/org/wso2/carbon/identity/core/util/IdentityUtilTest.java
@@ -56,6 +56,7 @@
 import org.wso2.carbon.utils.CarbonUtils;
 import org.wso2.carbon.utils.ConfigurationContextService;
 import org.wso2.carbon.utils.NetworkUtils;
+import org.wso2.carbon.utils.security.KeystoreUtils;
 
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -150,6 +151,7 @@ public class IdentityUtilTest {
     MockedStatic<SignatureUtil> signatureUtil;
     MockedStatic<IdentityKeyStoreResolver> identityKeyStoreResolver;
     MockedStatic<KeyStoreManager> keyStoreManager;
+    private MockedStatic<KeystoreUtils> keystoreUtils;
 
 
     @BeforeMethod
@@ -164,6 +166,7 @@ public void setUp() throws Exception {
         signatureUtil = mockStatic(SignatureUtil.class);
         identityKeyStoreResolver = mockStatic(IdentityKeyStoreResolver.class);
         keyStoreManager = mockStatic(KeyStoreManager.class);
+        keystoreUtils = mockStatic(KeystoreUtils.class);
 
         serverConfiguration.when(ServerConfiguration::getInstance).thenReturn(mockServerConfiguration);
         identityCoreServiceComponent.when(
@@ -203,6 +206,7 @@ public void tearDown() throws Exception {
         signatureUtil.close();
         identityKeyStoreResolver.close();
         keyStoreManager.close();
+        keystoreUtils.close();
     }
 
     @Test(description = "Test converting a certificate to PEM format")
@@ -1116,6 +1120,7 @@ public void testSignWithTenantKey() throws Exception {
         String data = "testData";
         String superTenantDomain = "carbon.super";
         keyStoreManager.when(() -> KeyStoreManager.getInstance(anyInt())).thenReturn(mockKeyStoreManager);
+        keystoreUtils.when(() -> KeystoreUtils.getKeyStoreFileExtension(superTenantDomain)).thenReturn(".jks");
         when(mockKeyStoreManager.getDefaultPrivateKey()).thenReturn(mockPrivateKey);
         when(mockKeyStoreManager.getPrivateKey(anyString(), anyString())).thenReturn(mockPrivateKey);
 

diff --git a/...gt/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGenerator.java b/...gt/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGenerator.java
@@ -35,5 +35,5 @@ public interface IdentityKeyStoreGenerator {
      * @param context      the context for which the KeyStore is to be generated.
      * @throws KeyStoreManagementException if an error occurs during KeyStore creation or initialization.
      */
-    void generateContextKeyStore(String tenantDomain, String context) throws KeyStoreManagementException;
+    void generateKeyStoreIfNotExists(String tenantDomain, String context) throws KeyStoreManagementException;
 }