Handle certificates mentioned in HTTP connection during local entry deployment. #2243
Conversation
| @@ -0,0 +1,9 @@ | |||
| package org.apache.synapse.transport.dynamicconfigurations; | |||
There was a problem hiding this comment.
Shall we add the license headers?
There was a problem hiding this comment.
Please check other places as well.
| File certificateFile = new File(certificateFilePath); | ||
| String certificateAlias = certificateFile.getName().split("\\.")[0]; | ||
| try { | ||
| FileInputStream certificateFileInputStream = FileUtils.openInputStream(new File(certificateFilePath)); |
There was a problem hiding this comment.
Shall we use try block with resources to close the stream automatically? Relevant to other input/output streams.
| sslSenderTrustStoreHolder.setKeyStore(sslSenderTrustStore); | ||
| KeyStoreReloaderHolder.getInstance().reloadAllKeyStores(); | ||
| } catch (CertificateException | IOException | KeyStoreException | NoSuchAlgorithmException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Rather than throwing a generic runtime exception, shall we add a more descriptive message?
ex: ("Failed to load certificate file to trust store", e)
| FileInputStream fileInputStream = new FileInputStream(sslSenderTrustStoreHolder.getLocation()); | ||
| InputStream dest = IOUtils.toBufferedInputStream(fileInputStream); | ||
| fileInputStream.close(); | ||
| sslSenderTrustStore.load(dest, sslSenderTrustStoreHolder.getPassword().toCharArray()); |
There was a problem hiding this comment.
Do we need these redundant truststore loads as we do it in line 145?
There was a problem hiding this comment.
Yes. They represent 2 things. First load is to load the certificate to the JKS and the second load is used to load the new JKS and create a new connection.
| try { | ||
| keyStoreLoader.loadKeyStore(transportOutDescription); | ||
| } catch (AxisFault e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Why are we throwing a runtime exception here? instead throw an AxisFault with a proper message.
| public void testGetInstance() { | ||
| SslSenderTrustStoreHolder instance = SslSenderTrustStoreHolder.getInstance(); | ||
| SslSenderTrustStoreHolder instance2 = SslSenderTrustStoreHolder.getInstance(); | ||
| Assert.assertEquals(instance, instance2); |
There was a problem hiding this comment.
Let's add an assert message here.
| @@ -79,6 +102,66 @@ public String deploySynapseArtifact(OMElement artifactConfig, String fileName, | |||
| return null; | |||
| } | |||
|
|
|||
| private void handleHttpConnectorCertificates(OMElement element) throws DeploymentException { | |||
There was a problem hiding this comment.
This doesn;t have to be specific to http connector right? If so shall we change the method name?
...p/src/test/java/org/apache/synapse/transport/nhttp/config/SslSenderTrustStoreHolderTest.java
Show resolved
Hide resolved
Purpose
Certificates specified in the HTTP connector connection need to be loaded into the respective JKS during the local entry deployment. Following this, the PassThroughHttpSSLSender have to be reloaded with the updated SSLContext.