Add unit tests, modify existing tests and bug fixes identified at man…

…ual tests

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/APIAuthenticationHandler.java

@@ -544,7 +544,6 @@ private void handleNoAuthentication(MessageContext messageContext){
         authContext.setStopOnQuotaReach(true);
         //Requests are throttled by the ApiKey that is set here. In an unauthenticated scenario, we will use the client's IP address for throttling.
         authContext.setApiKey(clientIP);
-        //TODO: verify the key type
         authContext.setKeyType(APIConstants.API_KEY_TYPE_PRODUCTION);
         //This name is hardcoded as anonymous because there is no associated user token
         authContext.setUsername(APIConstants.END_USER_ANONYMOUS);

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/MutualSSLAuthenticator.java

@@ -71,10 +71,10 @@ public class MutualSSLAuthenticator implements Authenticator {
     public MutualSSLAuthenticator(String apiLevelPolicy, boolean isMandatory, String certificateDetails) {
         this.apiLevelPolicy = apiLevelPolicy;
         certificates = new HashMap<>();
-        HashMap<String, String> certificateData = new HashMap<>();
         if (StringUtils.isNotEmpty(certificateDetails)) {
             String[] certificateParts = certificateDetails.substring(1, certificateDetails.length() - 1).split(",");
             for (String certificatePart : certificateParts) {
+                HashMap<String, String> certificateData = new HashMap<>();
                 int tierDivisionIndex = certificatePart.lastIndexOf("=");
                 if (tierDivisionIndex > 0) {
                     String uniqueIdentifier = certificatePart.substring(0, tierDivisionIndex).trim();
@@ -181,6 +181,12 @@ private void setAuthContext(MessageContext messageContext, Certificate certifica
         String subjectDN = x509Certificate.getSubjectDN().getName();
         String uniqueIdentifier = (x509Certificate.getSerialNumber() + "_" + x509Certificate.getSubjectDN()).replaceAll(",",
                         "#").replaceAll("\"", "'").trim();
+        /* Since there can be previously deleted certificates persisted in the trust store that matches with the
+        certificate object but not in the certificates list for the particular API.
+        */
+        if (certificates.get(uniqueIdentifier) == null ) {
+            handleCertificateNotAssociatedToAPIFailure(messageContext);
+        }
         String tier = certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_TIER);
         String keyType = certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_KEY_TYPE);
         if (StringUtils.isEmpty(tier)) {
@@ -236,13 +242,10 @@ private void setAuthContext(MessageContext messageContext, Certificate[] certifi
         }
         if (StringUtils.isEmpty(tier) || StringUtils.isEmpty(keyType)) {
             subjectDNIdentifiers = getUniqueIdentifierFromCompleteCertificateChain(x509Certificates, subjectDNIdentifiers);
-        }
-        if (StringUtils.isEmpty(tier)) {
             tier = getTierFromCompleteCertificateChain(subjectDNIdentifiers);
-        }
-        if (StringUtils.isEmpty(keyType)) {
             keyType = getKeyTypeFromCompleteCertificateChain(subjectDNIdentifiers);
         }
+
         if (StringUtils.isEmpty(tier) || StringUtils.isEmpty(keyType)) {
             handleCertificateNotAssociatedToAPIFailure(messageContext);
         }
@@ -292,7 +295,8 @@ private String getTierFromCompleteCertificateChain(List<String> uniqueIdentifier
 
         String tier = null;
         for (String uniqueIdentifier : uniqueIdentifiers) {
-            tier = certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_TIER);
+            tier = certificates.get(uniqueIdentifier) == null ? null :
+                    certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_TIER);
             if (StringUtils.isNotEmpty(tier)) {
                 break;
             }
@@ -311,7 +315,8 @@ private String getKeyTypeFromCompleteCertificateChain(List<String> uniqueIdentif
 
         String keyType = null;
         for (String uniqueIdentifier : uniqueIdentifiers) {
-            keyType = certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_KEY_TYPE);
+            keyType = certificates.get(uniqueIdentifier) == null ? null :
+                    certificates.get(uniqueIdentifier).get(APIConstants.CLIENT_CERTIFICATE_KEY_TYPE);
             if (StringUtils.isNotEmpty(keyType)) {
                 break;
             }

components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/certificatemgt/CertificateManagerImplTest.java

@@ -30,6 +30,7 @@
 import org.wso2.carbon.apimgt.api.dto.CertificateInformationDTO;
 import org.wso2.carbon.apimgt.api.dto.CertificateMetadataDTO;
 import org.wso2.carbon.apimgt.api.dto.ClientCertificateDTO;
+import org.wso2.carbon.apimgt.impl.APIConstants;
 import org.wso2.carbon.apimgt.impl.TestUtils;
 import org.wso2.carbon.apimgt.impl.certificatemgt.exceptions.CertificateAliasExistsException;
 import org.wso2.carbon.apimgt.impl.certificatemgt.exceptions.CertificateManagementException;
@@ -421,23 +422,26 @@ public void testAddClientCertificate() throws CertificateManagementException {
                 .stub(PowerMockito.method(CertificateMgtUtils.class, "validateCertificate"))
                 .toReturn(ResponseCode.ALIAS_EXISTS_IN_TRUST_STORE);
         ResponseCode responseCode = certificateManager
-                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null, MultitenantConstants.SUPER_TENANT_ID, "org1");
+                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null,
+                        APIConstants.API_KEY_TYPE_PRODUCTION, MultitenantConstants.SUPER_TENANT_ID, "org1");
         Assert.assertEquals("Response code was wrong while trying add a client certificate with an existing alias",
                 ResponseCode.ALIAS_EXISTS_IN_TRUST_STORE.getResponseCode(), responseCode.getResponseCode());
         PowerMockito
                 .stub(PowerMockito.method(CertificateMgtUtils.class, "validateCertificate"))
                 .toReturn(ResponseCode.SUCCESS);
         Mockito.when(certificateMgtDAO.checkWhetherAliasExist(ALIAS, TENANT_ID)).thenReturn(true);
         responseCode = certificateManager
-                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null, MultitenantConstants.SUPER_TENANT_ID, "org1");
+                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null,
+                        APIConstants.API_KEY_TYPE_PRODUCTION, MultitenantConstants.SUPER_TENANT_ID, "org1");
         Assert.assertEquals("Response code was wrong while trying add a client certificate with an existing alias",
                 ResponseCode.ALIAS_EXISTS_IN_TRUST_STORE.getResponseCode(), responseCode.getResponseCode());
         Mockito.when(certificateMgtDAO.checkWhetherAliasExist(ALIAS, TENANT_ID)).thenReturn(false);
         Mockito.when(certificateMgtDAO
                 .addClientCertificate(Mockito.anyString(), Mockito.any(), Mockito.anyString(), Mockito.anyString(),
-                        Mockito.anyInt(), Mockito.anyString())).thenReturn(true);
+                        Mockito.anyString(), Mockito.anyInt(), Mockito.anyString())).thenReturn(true);
         responseCode = certificateManager
-                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null, MultitenantConstants.SUPER_TENANT_ID, "org1");
+                .addClientCertificate(null, BASE64_ENCODED_CERT, ALIAS, null,
+                        APIConstants.API_KEY_TYPE_PRODUCTION, MultitenantConstants.SUPER_TENANT_ID, "org1");
         Assert.assertEquals("Response code was wrong while trying add a client certificate",
                 ResponseCode.SUCCESS.getResponseCode(), responseCode.getResponseCode());
     }
@@ -452,16 +456,17 @@ public void testUpdateClientCertificate() throws APIManagementException {
                 .stub(PowerMockito.method(CertificateMgtUtils.class, "validateCertificate"))
                 .toReturn(ResponseCode.CERTIFICATE_EXPIRED);
         ResponseCode responseCode = certificateManager
-                .updateClientCertificate(BASE64_ENCODED_CERT, ALIAS, null, MultitenantConstants.SUPER_TENANT_ID,
-                        organization);
+                .updateClientCertificate(BASE64_ENCODED_CERT, ALIAS, null,
+                        APIConstants.API_KEY_TYPE_PRODUCTION, MultitenantConstants.SUPER_TENANT_ID, organization);
         Assert.assertEquals("Response code was wrong while trying add a expired client certificate",
                 ResponseCode.CERTIFICATE_EXPIRED.getResponseCode(), responseCode.getResponseCode());
         PowerMockito
                 .stub(PowerMockito.method(CertificateMgtUtils.class, "validateCertificate"))
                 .toReturn(ResponseCode.SUCCESS);
         PowerMockito.stub(PowerMockito.method(CertificateMgtDAO.class, "updateClientCertificate")).toReturn(false);
         responseCode = certificateManager
-                .updateClientCertificate(BASE64_ENCODED_CERT, ALIAS, null, MultitenantConstants.SUPER_TENANT_ID,
+                .updateClientCertificate(BASE64_ENCODED_CERT, ALIAS, null, APIConstants.API_KEY_TYPE_PRODUCTION,
+                        MultitenantConstants.SUPER_TENANT_ID,
                         organization);
         Assert.assertEquals("Response code was wrong, for a failure in update",
                 ResponseCode.INTERNAL_SERVER_ERROR.getResponseCode(), responseCode.getResponseCode());

components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/dao/test/CertificateMgtDaoTest.java

@@ -31,6 +31,7 @@
 import org.wso2.carbon.apimgt.api.dto.CertificateMetadataDTO;
 import org.wso2.carbon.apimgt.api.dto.ClientCertificateDTO;
 import org.wso2.carbon.apimgt.api.model.APIIdentifier;
+import org.wso2.carbon.apimgt.impl.APIConstants;
 import org.wso2.carbon.apimgt.impl.APIManagerConfiguration;
 import org.wso2.carbon.apimgt.impl.APIManagerConfigurationServiceImpl;
 import org.wso2.carbon.apimgt.impl.certificatemgt.exceptions.CertificateAliasExistsException;
@@ -208,7 +209,21 @@ public void testAddClientCertificate() throws CertificateManagementException {
     @Test
     public void testUpdateClientCertificateOfNonExistingAlias() throws CertificateManagementException {
         Assert.assertFalse("Update of client certificate for a non existing alias succeeded",
-                certificateMgtDAO.updateClientCertificate(certificate, "test1", "test", TENANT_ID, "org1"));
+                certificateMgtDAO.updateClientCertificate(certificate, "test1", "test",
+                        APIConstants.API_KEY_TYPE_PRODUCTION, TENANT_ID, "org1"));
+    }
+
+    /**
+     * This method tests the behaviour of updateClientCertificate method when trying to update keyType of an
+     * existing client certificate entry.
+     *
+     * @throws CertificateManagementException Certificate Management Exception.
+     */
+    @Test
+    public void testUpdateKeyTypeOfExistingAlias() throws CertificateManagementException {
+        Assert.assertFalse("Update of key type for an existing client certificate entry succeeded",
+                certificateMgtDAO.updateClientCertificate(certificate, "test1", "test",
+                        APIConstants.API_KEY_TYPE_SANDBOX, TENANT_ID, "org1"));
     }
 
     /**
@@ -221,7 +236,8 @@ public void testUpdateClientCertificateOfExistingAlias() throws CertificateManag
         try {
             addClientCertificate();
             Assert.assertTrue("Update of client certificate for an existing alias failed",
-                    certificateMgtDAO.updateClientCertificate(null, "test", "test", TENANT_ID, "org1"));
+                    certificateMgtDAO.updateClientCertificate(null, "test", "test",
+                            APIConstants.API_KEY_TYPE_PRODUCTION, TENANT_ID, "org1"));
         } finally {
             deleteClientCertificate();
         }
@@ -293,6 +309,8 @@ public void testGetClientCertificates() throws CertificateManagementException {
         clientCertificateDTOS = certificateMgtDAO.getClientCertificates(TENANT_ID, "test", apiIdentifier, organization);
         Assert.assertEquals("The client certificate DTO list that matches the search criteria is not returned", 1,
                 clientCertificateDTOS.size());
+        Assert.assertNotNull("The key type of a client certificate cannot be null",
+                clientCertificateDTOS.get(0).getKeyType());
         clientCertificateDTOS = certificateMgtDAO.getClientCertificates(TENANT_ID, null, apiIdentifier, organization);
         Assert.assertEquals("The client certificate DTO list that matches the search criteria is not returned", 1,
                 clientCertificateDTOS.size());
@@ -324,7 +342,8 @@ public void testGetDeletedClientCertificates() throws CertificateManagementExcep
      * @throws CertificateManagementException Certificate Management Exception.
      */
     private boolean addClientCertificate() throws CertificateManagementException {
-        return certificateMgtDAO.addClientCertificate(certificate, apiIdentifier, "test", "Gold", TENANT_ID, "org1");
+        return certificateMgtDAO.addClientCertificate(certificate, apiIdentifier, "test", "Gold",
+                APIConstants.API_KEY_TYPE_PRODUCTION, TENANT_ID, "org1");
     }
 
     /**

components/apimgt/org.wso2.carbon.apimgt.keymgt/src/test/resources/dbscripts/h2.sql

@@ -1715,6 +1715,7 @@ CREATE TABLE IF NOT EXISTS `AM_API_CLIENT_CERTIFICATE` (
   `CERTIFICATE` BLOB NOT NULL,
   `REMOVED` BOOLEAN NOT NULL DEFAULT 0,
   `TIER_NAME` VARCHAR (512),
+  `KEY_TYPE` VARCHAR(20) NOT NULL DEFAULT 'PRODUCTION',
   FOREIGN KEY (API_ID) REFERENCES AM_API (API_ID) ON DELETE CASCADE ON UPDATE CASCADE,
   PRIMARY KEY (`ALIAS`,`TENANT_ID`, `REMOVED`)
 );

features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/h2.sql

@@ -2014,6 +2014,7 @@ CREATE TABLE IF NOT EXISTS AM_API_CLIENT_CERTIFICATE (
   CERTIFICATE BLOB NOT NULL,
   REMOVED BOOLEAN NOT NULL DEFAULT 0,
   TIER_NAME VARCHAR (512),
+  KEY_TYPE VARCHAR(20) NOT NULL DEFAULT 'PRODUCTION',
   REVISION_UUID VARCHAR(255) NOT NULL DEFAULT 'Current API',
   FOREIGN KEY (API_ID) REFERENCES AM_API (API_ID) ON DELETE CASCADE ON UPDATE CASCADE,
   PRIMARY KEY (ALIAS,TENANT_ID, REMOVED, REVISION_UUID)

features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql.sql

@@ -1961,6 +1961,7 @@ CREATE TABLE IF NOT EXISTS `AM_API_CLIENT_CERTIFICATE` (
   `CERTIFICATE` BLOB NOT NULL,
   `REMOVED` BOOLEAN NOT NULL DEFAULT 0,
   `TIER_NAME` VARCHAR (512),
+  `KEY_TYPE` VARCHAR(20) NOT NULL DEFAULT 'PRODUCTION',
   `REVISION_UUID` VARCHAR(255) NOT NULL DEFAULT 'Current API',
   FOREIGN KEY (API_ID) REFERENCES AM_API (API_ID) ON DELETE CASCADE ON UPDATE CASCADE,
   PRIMARY KEY (`ALIAS`, `TENANT_ID`, `REMOVED`, `REVISION_UUID`)

features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql_cluster.sql

@@ -2141,6 +2141,7 @@ CREATE TABLE IF NOT EXISTS `AM_API_CLIENT_CERTIFICATE` (
   `CERTIFICATE` BLOB NOT NULL,
   `REMOVED` BOOLEAN NOT NULL DEFAULT 0,
   `TIER_NAME` VARCHAR (512),
+  `KEY_TYPE` VARCHAR(20) NOT NULL DEFAULT 'PRODUCTION',
   `REVISION_UUID` VARCHAR(255) NOT NULL DEFAULT 'Current API',
   FOREIGN KEY (API_ID) REFERENCES AM_API (API_ID),
   PRIMARY KEY (`ALIAS`, `TENANT_ID`, `REMOVED`, `REVISION_UUID`)

features/apimgt/org.wso2.carbon.apimgt.keymanager.feature/src/main/resources/sql/h2.sql

@@ -1701,6 +1701,7 @@ CREATE TABLE IF NOT EXISTS `AM_API_CLIENT_CERTIFICATE` (
   `CERTIFICATE` BLOB NOT NULL,
   `REMOVED` BOOLEAN NOT NULL DEFAULT 0,
   `TIER_NAME` VARCHAR (512),
+  `KEY_TYPE` VARCHAR(20) NOT NULL DEFAULT 'PRODUCTION',
   FOREIGN KEY (API_ID) REFERENCES AM_API (API_ID) ON DELETE CASCADE ON UPDATE CASCADE,
   PRIMARY KEY (`ALIAS`,`TENANT_ID`, `REMOVED`)
 );