Skip to content

Fix wrong using of certificates in ZAAS client #1514

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

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Fix wrong using of certificates in ZAAS client #1514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ceae38
fix
pj892031 Jun 3, 2021
c253636
fix by review
pj892031 Jun 4, 2021
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
23 changes: 23 additions & 0 deletions zaas-client/src/main/java/org/zowe/apiml/zaasclient/config/ConfigProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
*/
package org.zowe.apiml.zaasclient.config;

import lombok.Builder;
import lombok.Data;
import lombok.experimental.Tolerate;

@Data
@Builder
public class ConfigProperties {

private String apimlHost;
private String apimlPort;
private String apimlBaseUrl;
Expand All @@ -24,4 +28,23 @@ public class ConfigProperties {
private char[] trustStorePassword;
private boolean httpOnly;
private boolean nonStrictVerifySslCertificatesOfServices;

@Tolerate
public ConfigProperties() {
Copy link
Member

Choose a reason for hiding this comment

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

Did you consider using NoArgs and AllArgs annotations instead of this experimental one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was my first attempt, but it doesn't work together with @builder. Just this annotation solved it.

// no args constructor
}

public ConfigProperties withoutKeyStore() {
return ConfigProperties.builder()
.apimlHost(apimlHost)
.apimlPort(apimlPort)
.apimlBaseUrl(apimlBaseUrl)
.trustStoreType(trustStoreType)
.trustStorePath(trustStorePath)
.trustStorePassword(trustStorePassword)
.httpOnly(httpOnly)
.nonStrictVerifySslCertificatesOfServices(nonStrictVerifySslCertificatesOfServices)
.build();
}

}
19 changes: 17 additions & 2 deletions zaas-client/src/main/java/org/zowe/apiml/zaasclient/service/internal/ZaasClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.zowe.apiml.zaasclient.config.ConfigProperties;
import org.zowe.apiml.zaasclient.exception.ZaasClientErrorCodes;
import org.zowe.apiml.zaasclient.exception.ZaasClientException;
import org.zowe.apiml.zaasclient.exception.ZaasConfigurationErrorCodes;
import org.zowe.apiml.zaasclient.exception.ZaasConfigurationException;
import org.zowe.apiml.zaasclient.service.ZaasClient;
import org.zowe.apiml.zaasclient.service.ZaasToken;
Expand All @@ -26,12 +27,17 @@ public class ZaasClientImpl implements ZaasClient {
private final PassTicketService passTickets;

public ZaasClientImpl(ConfigProperties configProperties) throws ZaasConfigurationException {
if (!configProperties.isHttpOnly() && (configProperties.getKeyStorePath() == null)) {
throw new ZaasConfigurationException(ZaasConfigurationErrorCodes.KEY_STORE_NOT_PROVIDED);
}

CloseableClientProvider httpClientProvider = getTokenProvider(configProperties);
CloseableClientProvider httpClientProviderWithoutCert = getTokenProviderWithoutCert(configProperties, httpClientProvider);
Copy link
Member

Choose a reason for hiding this comment

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

In case of HTTP , the same provider object is used, in case of HTTPS, there are 2 objects being created, is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is, in the case of HTTP there is no needed to make two instances. But the purpose is the question because HTTP is not possible to use (except the theoretical configuration). It should be just for ATTLS, but then it is too difficult (maybe impossible) to establish to respect URL and using a client certificate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should focus on the possibility, how to use ZAAS client with AT-TLS in the next issue. Thank you for finding that.


String baseUrl = String.format("%s://%s:%s%s", getScheme(configProperties.isHttpOnly()), configProperties.getApimlHost(), configProperties.getApimlPort(),
configProperties.getApimlBaseUrl());
tokens = new ZaasJwtService(httpClientProvider, baseUrl);
tokens = new ZaasJwtService(httpClientProviderWithoutCert, baseUrl);
passTickets = new PassTicketServiceImpl(httpClientProvider, baseUrl);

}

private CloseableClientProvider getTokenProvider(ConfigProperties configProperties) throws ZaasConfigurationException {
Expand All @@ -40,7 +46,16 @@ private CloseableClientProvider getTokenProvider(ConfigProperties configProperti
} else {
return new ZaasHttpsClientProvider(configProperties);
}
}

private CloseableClientProvider getTokenProviderWithoutCert (
ConfigProperties configProperties,
CloseableClientProvider defaultCloseableClientProvider) throws ZaasConfigurationException
{
if (configProperties.isHttpOnly()) {
return defaultCloseableClientProvider;
}
return getTokenProvider(configProperties.withoutKeyStore());
}

private Object getScheme(boolean httpOnly) {
Expand Down
5 changes: 1 addition & 4 deletions ...ent/src/main/java/org/zowe/apiml/zaasclient/service/internal/ZaasHttpsClientProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,8 @@ public void clearCookieStore() {

@Override
public synchronized CloseableHttpClient getHttpClient() throws ZaasConfigurationException {
if (keyStorePath == null) {
throw new ZaasConfigurationException(ZaasConfigurationErrorCodes.KEY_STORE_NOT_PROVIDED);
}
if (httpsClientWithKeyStoreAndTrustStore == null) {
if (kmf == null) {
if ((kmf == null) && (keyStorePath != null)) {
initializeKeyStoreManagerFactory();
}
httpsClientWithKeyStoreAndTrustStore = sharedHttpClientConfiguration(getSSLContext())
Expand Down
1 change: 1 addition & 0 deletions ...ent/src/test/java/org/zowe/apiml/zaasclient/service/internal/ZaasClientImplHttpTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ void testHttpOnlyZaasClientCanBeCreated() throws ZaasConfigurationException {
configProperties.setApimlPort("10010");
configProperties.setApimlBaseUrl("/api/v1/gateway/auth");
configProperties.setNonStrictVerifySslCertificatesOfServices(false);
configProperties.setKeyStorePath("keystorePath");
ZaasClient client = new ZaasClientImpl(configProperties);
assertNotNull(client);
}
Expand Down
12 changes: 12 additions & 0 deletions zaas-client/src/test/java/org/zowe/apiml/zaasclient/service/internal/ZaasClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.zowe.apiml.zaasclient.config.ConfigProperties;
import org.zowe.apiml.zaasclient.exception.ZaasClientErrorCodes;
import org.zowe.apiml.zaasclient.exception.ZaasClientException;
import org.zowe.apiml.zaasclient.exception.ZaasConfigurationException;
Expand Down Expand Up @@ -146,4 +147,15 @@ void givenValidToken_whenLogoutIsCalled_thenSuccessLogout() {
assertDoesNotThrow(() -> underTest.logout("apimlAuthenticationToken=" + VALID_TOKEN));
}

@Test
void givenNullKeyStorePath_whenTheClientIsConstructed_thenExceptionIsThrown() {
ConfigProperties config = new ConfigProperties();
config.setTrustStorePassword(VALID_PASSWORD.toCharArray());
config.setTrustStorePath("src/test/resources/localhost.truststore.p12");
config.setTrustStoreType("PKCS12");
ZaasConfigurationException zaasException = assertThrows(ZaasConfigurationException.class, () -> new ZaasClientImpl(config));

assertThat(zaasException.getErrorCode().getId(), is("ZWEAS501E"));
}

}
12 changes: 0 additions & 12 deletions ...rc/test/java/org/zowe/apiml/zaasclient/service/internal/ZaasHttpsClientProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,6 @@ void giveInvalidTrustStorePath_whenTheClientIsConstructed_thenExceptionsIsThrown
assertThat(zaasException.getErrorCode().getId(), is("ZWEAS503E"));
}

@Test
void givenNullKeyStorePath_whenTheClientIsConstructed_thenExceptionIsThrown() throws ZaasConfigurationException {
ConfigProperties config = new ConfigProperties();
config.setTrustStorePassword(PASSWORD);
config.setTrustStorePath("src/test/resources/localhost.truststore.p12");
config.setTrustStoreType("PKCS12");
ZaasHttpsClientProvider provider = new ZaasHttpsClientProvider(config);
ZaasConfigurationException zaasException = assertThrows(ZaasConfigurationException.class, provider::getHttpClient);

assertThat(zaasException.getErrorCode().getId(), is("ZWEAS501E"));
}

@Test
void givenInvalidKeyStorePath_whenTheClientIsConstructed_thenExceptionIsThrown() throws ZaasConfigurationException {
ConfigProperties config = new ConfigProperties();
Expand Down