Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NPE thrown when <KeyStoreMapping> Tag Not Available. #5978

Merged
merged 5 commits into from
Oct 9, 2024
Merged

Fix NPE thrown when <KeyStoreMapping> Tag Not Available. #5978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d987279
Fix NPE issue
Binara-Sachin Oct 2, 2024
9f83caa
Add test cases for NPE issue
Binara-Sachin Oct 6, 2024
cb3f80d
Minor improvements
Binara-Sachin Oct 6, 2024
ef551e1
Improvements to unit tests
Binara-Sachin Oct 7, 2024
b3cd30c
Improvements to unit tests
Binara-Sachin Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions ...n.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,24 @@ private String getCustomKeyStoreConfig(String keyStoreName, String configName)

private void parseIdentityKeyStoreMappingConfigs() {

OMElement keyStoreMappingsElem = IdentityConfigParser.getInstance().getConfigElement(
IdentityKeyStoreResolverConstants.CONFIG_ELEM_SECURITY).getFirstChildWithName(
IdentityKeyStoreResolverUtil.getQNameWithIdentityNameSpace(
IdentityKeyStoreResolverConstants.CONFIG_ELEM_KEYSTORE_MAPPING));
OMElement keyStoreMappingsElem = null;
IdentityConfigParser configParser = IdentityConfigParser.getInstance();
if (configParser != null) {
OMElement securityElem = configParser.getConfigElement(
IdentityKeyStoreResolverConstants.CONFIG_ELEM_SECURITY);
if (securityElem != null) {
keyStoreMappingsElem = securityElem.getFirstChildWithName(
IdentityKeyStoreResolverUtil.getQNameWithIdentityNameSpace(
IdentityKeyStoreResolverConstants.CONFIG_ELEM_KEYSTORE_MAPPING));
}
}

if (keyStoreMappingsElem == null) {
Copy link
Contributor

@pamodaaw pamodaaw Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the above logic null-safe?

LOG.warn(String.format("%s.%s element not found in identity.xml file.",
IdentityKeyStoreResolverConstants.CONFIG_ELEM_SECURITY,
IdentityKeyStoreResolverConstants.CONFIG_ELEM_KEYSTORE_MAPPING));
return;
}

// Parse OAuth KeyStore Mapping.
OMElement oauthKeyStoreMapping = keyStoreMappingsElem.getFirstChildWithName(
Expand Down
133 changes: 69 additions & 64 deletions ...entity.core/src/test/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,27 @@
package org.wso2.carbon.identity.core;

import junit.framework.TestCase;
import org.apache.axiom.om.OMElement;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.testng.annotations.*;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.wso2.carbon.base.CarbonBaseConstants;
import org.wso2.carbon.core.util.KeyStoreManager;
import org.wso2.carbon.identity.core.model.IdentityKeyStoreMapping;
import org.wso2.carbon.identity.core.util.IdentityConfigParser;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.utils.ServerConstants;
import org.wso2.carbon.utils.multitenancy.MultitenantConstants;

import java.io.FileInputStream;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.Key;
import java.security.KeyStore;
import java.security.cert.Certificate;
import java.util.Map;
import java.security.PrivateKey;
import java.security.cert.X509Certificate;
import java.util.concurrent.ConcurrentHashMap;

import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -76,47 +71,39 @@ public class IdentityKeyStoreResolverTest extends TestCase {
private KeyStore tenantKeyStore;
private KeyStore customKeyStore;

private static Key primaryKey;
private static Key tenantKey;
private static Key customKey;
private static PrivateKey primaryKey;
private static PrivateKey tenantKey;
private static PrivateKey customKey;

private static Certificate primaryCertificate;
private static Certificate tenantCertificate;
private static Certificate customCertificate;
private static X509Certificate primaryCertificate;
private static X509Certificate tenantCertificate;
private static X509Certificate customCertificate;

@Mock
private IdentityConfigParser mockIdentityConfigParser;

@Mock
private OMElement mockConfig;

private IdentityKeyStoreResolver identityKeyStoreResolver;

// Test key store mappings.
Map<InboundProtocol, IdentityKeyStoreMapping> keyStoreMappings = new ConcurrentHashMap<>();

private MockedStatic<IdentityConfigParser> identityConfigParser;
private MockedStatic<IdentityTenantUtil> identityTenantUtil;

private IdentityKeyStoreResolver identityKeyStoreResolver;

@BeforeClass
public void setUp() throws Exception {

// Use identity.xml file from test resources.
// Set test resource path.
String identityXmlPath = Paths.get(System.getProperty("user.dir"), "src", "test", "resources",
"identity.xml").toString();
System.setProperty(ServerConstants.CARBON_HOME, ".");
mockIdentityConfigParser = IdentityConfigParser.getInstance(identityXmlPath);

// Mock IdentityConfigParser.
mockIdentityConfigParser = IdentityConfigParser.getInstance(identityXmlPath);
identityConfigParser = mockStatic(IdentityConfigParser.class);
identityConfigParser.when(IdentityConfigParser::getInstance).thenReturn(mockIdentityConfigParser);

// Mock IdentityTenantUtil.
identityTenantUtil = mockStatic(IdentityTenantUtil.class);
identityTenantUtil.when(() -> IdentityTenantUtil.getTenantId(SUPER_TENANT_DOMAIN)).thenReturn(Integer.valueOf(SUPER_TENANT_ID));
identityTenantUtil.when(() -> IdentityTenantUtil.getTenantId(TENANT_DOMAIN)).thenReturn(Integer.valueOf(TENANT_ID));

identityKeyStoreResolver = IdentityKeyStoreResolver.getInstance();

// Mock getKeyStore method of key store manager.
System.setProperty(CarbonBaseConstants.CARBON_HOME,
Paths.get(System.getProperty("user.dir"), "src", "test", "resources").toString());
Expand All @@ -128,6 +115,14 @@ public void setUp() throws Exception {
customKeyStore = getKeyStoreFromFile(CUSTOM_KEY_STORE, CUSTOM_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME));

primaryKey = (PrivateKey) primaryKeyStore.getKey(PRIMARY_KEY_STORE_ALIAS, PRIMARY_KEY_STORE_PASSWORD.toCharArray());
tenantKey = (PrivateKey) tenantKeyStore.getKey(TENANT_KEY_STORE_ALIAS, TENANT_KEY_STORE_PASSWORD.toCharArray());
customKey = (PrivateKey) customKeyStore.getKey(CUSTOM_KEY_STORE_ALIAS, CUSTOM_KEY_STORE_PASSWORD.toCharArray());

primaryCertificate = (X509Certificate) primaryKeyStore.getCertificate(PRIMARY_KEY_STORE_ALIAS);
tenantCertificate = (X509Certificate) tenantKeyStore.getCertificate(TENANT_KEY_STORE_ALIAS);
customCertificate = (X509Certificate) customKeyStore.getCertificate(CUSTOM_KEY_STORE_ALIAS);

KeyStoreManager keyStoreManager = mock(KeyStoreManager.class);
ConcurrentHashMap<String, KeyStoreManager> mtKeyStoreManagers = new ConcurrentHashMap();

Expand All @@ -139,37 +134,15 @@ public void setUp() throws Exception {
when(keyStoreManager.getKeyStore(TENANT_KEY_STORE)).thenReturn(tenantKeyStore);
when(keyStoreManager.getKeyStore("CUSTOM/" + CUSTOM_KEY_STORE)).thenReturn(customKeyStore);

// Mock set private keys.
primaryKey = getKeyStoreFromFile(PRIMARY_KEY_STORE, PRIMARY_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getKey(PRIMARY_KEY_STORE_ALIAS, PRIMARY_KEY_STORE_PASSWORD.toCharArray());
tenantKey = getKeyStoreFromFile(TENANT_KEY_STORE, TENANT_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getKey(TENANT_KEY_STORE_ALIAS, TENANT_KEY_STORE_PASSWORD.toCharArray());
customKey = getKeyStoreFromFile(CUSTOM_KEY_STORE, CUSTOM_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getKey(CUSTOM_KEY_STORE_ALIAS, CUSTOM_KEY_STORE_PASSWORD.toCharArray());

Map<String, Key> privateKeys = new ConcurrentHashMap<>();
privateKeys.put(SUPER_TENANT_ID, primaryKey);
privateKeys.put(TENANT_ID, tenantKey);
privateKeys.put(InboundProtocol.OAUTH.toString(), customKey);
privateKeys.put(InboundProtocol.WS_TRUST.toString(), customKey);

setPrivateStaticField(IdentityKeyStoreResolver.class, "privateKeys", privateKeys);

// Mock set certificates.
primaryCertificate = getKeyStoreFromFile(PRIMARY_KEY_STORE, PRIMARY_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getCertificate(PRIMARY_KEY_STORE_ALIAS);
tenantCertificate = getKeyStoreFromFile(TENANT_KEY_STORE, TENANT_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getCertificate(TENANT_KEY_STORE_ALIAS);
customCertificate = getKeyStoreFromFile(CUSTOM_KEY_STORE, CUSTOM_KEY_STORE_PASSWORD,
System.getProperty(CarbonBaseConstants.CARBON_HOME)).getCertificate(CUSTOM_KEY_STORE_ALIAS);

Map<String, Certificate> publicCerts = new ConcurrentHashMap<>();
publicCerts.put(SUPER_TENANT_ID, primaryCertificate);
publicCerts.put(TENANT_ID, tenantCertificate);
publicCerts.put(InboundProtocol.OAUTH.toString(), customCertificate);
publicCerts.put(InboundProtocol.WS_TRUST.toString(), customCertificate);

setPrivateStaticField(IdentityKeyStoreResolver.class, "publicCerts", publicCerts);
when(keyStoreManager.getDefaultPrivateKey()).thenReturn(primaryKey);
when(keyStoreManager.getPrivateKey(TENANT_KEY_STORE, TENANT_KEY_STORE_ALIAS)).thenReturn(tenantKey);
when(keyStoreManager.getPrivateKey("CUSTOM/" + CUSTOM_KEY_STORE, null)).thenReturn(customKey);

when(keyStoreManager.getDefaultPrimaryCertificate()).thenReturn(primaryCertificate);
when(keyStoreManager.getCertificate(TENANT_KEY_STORE, TENANT_KEY_STORE_ALIAS)).thenReturn(tenantCertificate);
when(keyStoreManager.getCertificate("CUSTOM/" + CUSTOM_KEY_STORE, null)).thenReturn(customCertificate);

identityKeyStoreResolver = IdentityKeyStoreResolver.getInstance();
}

@AfterClass
Expand All @@ -188,6 +161,39 @@ public void testGetInstance() {
assertEquals(identityKeyStoreResolver1, identityKeyStoreResolver2);
}

@DataProvider(name = "MissingConfigDataProvider")
public String[] missingConfigDataProvider() {

return new String[] {
"identity_err1.xml",
"identity_err2.xml"
};
}

@Test(dataProvider = "MissingConfigDataProvider")
public void testMissingConfigs(String fileName) {

try {
// Set current instance to null before creating a new instance
Field identityKeyStoreResolverInstance = IdentityKeyStoreResolver.class.getDeclaredField("instance");
identityKeyStoreResolverInstance.setAccessible(true);
identityKeyStoreResolverInstance.set(null, null);

// Use custom identity.xml file from test resources.
String identityXmlPath = Paths.get(System.getProperty("user.dir"), "src", "test", "resources",
fileName).toString();

// Mock IdentityConfigParser.
mockIdentityConfigParser = IdentityConfigParser.getInstance(identityXmlPath);
identityConfigParser.when(IdentityConfigParser::getInstance).thenReturn(mockIdentityConfigParser);

// Test instance creation --> Config read.
IdentityKeyStoreResolver.getInstance();
} catch (Exception e) {
fail("Test failed due to exception: " + e);
}
}

@DataProvider(name = "KeyStoreDataProvider")
public Object[][] keyStoreDataProvider() {

Expand Down Expand Up @@ -221,7 +227,7 @@ public Object[][] privateKeyDataProvider() {
}

@Test(dataProvider = "PrivateKeyDataProvider")
public void testGetPrivateKey(String tenantDomain, InboundProtocol inboundProtocol, Key expectedKey) throws Exception {
public void testGetPrivateKey(String tenantDomain, InboundProtocol inboundProtocol, PrivateKey expectedKey) throws Exception {

assertEquals(expectedKey, identityKeyStoreResolver.getPrivateKey(tenantDomain, inboundProtocol));
}
Expand All @@ -240,7 +246,7 @@ public Object[][] publicCertificateDataProvider() {
}

@Test(dataProvider = "PublicCertificateDataProvider")
public void testGetCertificate(String tenantDomain, InboundProtocol inboundProtocol, Certificate expectedCert) throws Exception {
public void testGetCertificate(String tenantDomain, InboundProtocol inboundProtocol, X509Certificate expectedCert) throws Exception {

assertEquals(expectedCert, identityKeyStoreResolver.getCertificate(tenantDomain, inboundProtocol));
}
Expand All @@ -261,5 +267,4 @@ private void setPrivateStaticField(Class<?> clazz, String fieldName, Object newV
field.setAccessible(true);
field.set(null, newValue);
}

}
Loading
Loading