Copilot commented on code in PR #665:
URL: https://github.com/apache/ranger/pull/665#discussion_r2350355373
##########
plugin-nifi-registry/src/test/java/org/apache/ranger/services/nifi/registry/client/TestNiFiRegistryClient.java:
##########
@@ -126,26 +148,228 @@ public void testConnectionTestSuccess() {
}
@Test
- public void testConnectionTestFailure() {
+ public void testConnectionTestFailure() throws NoSuchAlgorithmException,
KeyManagementException {
final String errorMsg = "unknown error";
- registryClient = new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode());
+ registryClient = new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode(), false, null);
HashMap<String, Object> ret = registryClient.connectionTest();
Assert.assertNotNull(ret);
Assert.assertEquals(NiFiRegistryClient.FAILURE_MSG,
ret.get("message"));
}
+ @Test
+ public void testHostnameVerifierMatch() throws NoSuchAlgorithmException,
KeyManagementException, CertificateParsingException, SSLPeerUnverifiedException
{
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMock(HOSTNAME);
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertTrue(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierNoMatch() throws NoSuchAlgorithmException,
KeyManagementException, CertificateParsingException, SSLPeerUnverifiedException
{
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMock("other.com");
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierNoCerts() throws NoSuchAlgorithmException,
KeyManagementException, SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithNoCerts();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierEmptyCerts() throws
NoSuchAlgorithmException, KeyManagementException, SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithEmptyCerts();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierSanInIntermediateCertsFails() throws
NoSuchAlgorithmException, KeyManagementException, CertificateParsingException,
SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithSanInIntermediate();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
/**
* Extend NiFiRegistryClient to return mock responses.
*/
private static final class MockNiFiRegistryClient extends
NiFiRegistryClient {
- private int statusCode;
- private String responseEntity;
+ private final int statusCode;
+ private final String responseEntity;
+ private final boolean useSSL;
+ private final String hostname;
+ private HostnameVerifier hostnameVerifierSpy;
+ private boolean lastVerifyResult;
+ private SSLSession mockSession;
- private MockNiFiRegistryClient(String responseEntity, int statusCode) {
-
super("http://localhost:18080/nifi-registry-api/policiesresources";, null);
- this.statusCode = statusCode;
+ private MockNiFiRegistryClient(String responseEntity, int statusCode,
boolean useSSL, String hostname) throws NoSuchAlgorithmException,
KeyManagementException {
+ super(useSSL ? ("https://"; + (hostname != null ? hostname :
"localhost") + ":443") : POLICIES_RESOURCES,
+ useSSL ? createInitializedSSLContext() : null);
+ this.statusCode = statusCode;
this.responseEntity = responseEntity;
+ this.useSSL = useSSL;
+ this.hostname = hostname;
+ }
+
+ private static SSLContext createInitializedSSLContext() throws
NoSuchAlgorithmException, KeyManagementException {
+ SSLContext sslContext = SSLContext.getInstance("TLS");
+ sslContext.init(null, null, null);
+ return sslContext;
+ }
+
+ void setupSSLMock(String sanHostname) throws
CertificateParsingException, SSLPeerUnverifiedException {
+ if (!useSSL) {
+ throw new IllegalStateException("SSL setup not supported for
non-SSL mock");
+ }
+ hostnameVerifierSpy = spy(getHostnameVerifier());
+ mockSession = Mockito.mock(SSLSession.class);
+ SSLSessionContext mockContext =
Mockito.mock(SSLSessionContext.class);
+ doReturn(mockContext).when(mockSession).getSessionContext();
+
+ X509Certificate mockCert = Mockito.mock(X509Certificate.class);
+ Certificate[] certs = {mockCert};
+ doReturn(certs).when(mockSession).getPeerCertificates();
+
+ Collection<?> altNames = Mockito.mock(Collection.class);
+ List<?> generalName = Mockito.mock(List.class);
+ doReturn(2).when(generalName).size();
+ doReturn(2).when(generalName).get(0);
+ doReturn(sanHostname.toLowerCase()).when(generalName).get(1);
+
doReturn(java.util.Collections.singletonList(generalName).iterator()).when(altNames).iterator();
+ doReturn(altNames).when(mockCert).getSubjectAlternativeNames();
+
+ doAnswer(invocation -> {
+ String calledHostname = invocation.getArgument(0);
+ SSLSession calledSession = invocation.getArgument(1);
+ try {
+ Certificate[] certificates =
calledSession.getPeerCertificates();
+ if (certificates == null || certificates.length == 0) {
+ lastVerifyResult = false;
+ return false;
+ }
+ final X509Certificate x509Cert = (X509Certificate)
certificates[0];
+ final Collection<List<?>> sanCollection =
x509Cert.getSubjectAlternativeNames();
+ if (sanCollection == null) {
+ lastVerifyResult = false;
+ return false;
+ }
+ boolean match = false;
+ for (final List<?> generalNameEntry : sanCollection) {
+ if (generalNameEntry.size() > 1) {
+ final Object value = generalNameEntry.get(1);
+ if (value instanceof String) {
+ if (calledHostname.equalsIgnoreCase(((String)
value))) {
+ match = true;
+ break;
+ }
+ }
+ }
+ }
+ lastVerifyResult = match;
+ return match;
+ } catch (final SSLPeerUnverifiedException |
CertificateParsingException ex) {
+ lastVerifyResult = false;
+ return false;
+ }
+ }).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
Review Comment:
This duplicated hostname verification logic should be extracted into a
shared method or reuse the actual implementation from
NiFiRegistryHostnameVerifier to ensure consistency and avoid code duplication.
##########
plugin-nifi-registry/src/test/java/org/apache/ranger/services/nifi/registry/client/TestNiFiRegistryClient.java:
##########
@@ -126,26 +148,228 @@ public void testConnectionTestSuccess() {
}
@Test
- public void testConnectionTestFailure() {
+ public void testConnectionTestFailure() throws NoSuchAlgorithmException,
KeyManagementException {
final String errorMsg = "unknown error";
- registryClient = new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode());
+ registryClient = new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode(), false, null);
HashMap<String, Object> ret = registryClient.connectionTest();
Assert.assertNotNull(ret);
Assert.assertEquals(NiFiRegistryClient.FAILURE_MSG,
ret.get("message"));
}
+ @Test
+ public void testHostnameVerifierMatch() throws NoSuchAlgorithmException,
KeyManagementException, CertificateParsingException, SSLPeerUnverifiedException
{
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMock(HOSTNAME);
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertTrue(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierNoMatch() throws NoSuchAlgorithmException,
KeyManagementException, CertificateParsingException, SSLPeerUnverifiedException
{
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMock("other.com");
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierNoCerts() throws NoSuchAlgorithmException,
KeyManagementException, SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithNoCerts();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierEmptyCerts() throws
NoSuchAlgorithmException, KeyManagementException, SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithEmptyCerts();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
+ @Test
+ public void testHostnameVerifierSanInIntermediateCertsFails() throws
NoSuchAlgorithmException, KeyManagementException, CertificateParsingException,
SSLPeerUnverifiedException {
+ MockNiFiRegistryClient sslClient = new MockNiFiRegistryClient("", 200,
true, HOSTNAME);
+ sslClient.setupSSLMockWithSanInIntermediate();
+ sslClient.getResponse(sslClient.getWebResource(), "application/json");
+ verify(sslClient.hostnameVerifierSpy).verify(eq(HOSTNAME),
any(SSLSession.class));
+ Assert.assertFalse(sslClient.lastVerifyResult);
+ }
+
/**
* Extend NiFiRegistryClient to return mock responses.
*/
private static final class MockNiFiRegistryClient extends
NiFiRegistryClient {
- private int statusCode;
- private String responseEntity;
+ private final int statusCode;
+ private final String responseEntity;
+ private final boolean useSSL;
+ private final String hostname;
+ private HostnameVerifier hostnameVerifierSpy;
+ private boolean lastVerifyResult;
+ private SSLSession mockSession;
- private MockNiFiRegistryClient(String responseEntity, int statusCode) {
-
super("http://localhost:18080/nifi-registry-api/policiesresources";, null);
- this.statusCode = statusCode;
+ private MockNiFiRegistryClient(String responseEntity, int statusCode,
boolean useSSL, String hostname) throws NoSuchAlgorithmException,
KeyManagementException {
+ super(useSSL ? ("https://"; + (hostname != null ? hostname :
"localhost") + ":443") : POLICIES_RESOURCES,
+ useSSL ? createInitializedSSLContext() : null);
+ this.statusCode = statusCode;
this.responseEntity = responseEntity;
+ this.useSSL = useSSL;
+ this.hostname = hostname;
+ }
+
+ private static SSLContext createInitializedSSLContext() throws
NoSuchAlgorithmException, KeyManagementException {
+ SSLContext sslContext = SSLContext.getInstance("TLS");
+ sslContext.init(null, null, null);
+ return sslContext;
+ }
+
+ void setupSSLMock(String sanHostname) throws
CertificateParsingException, SSLPeerUnverifiedException {
+ if (!useSSL) {
+ throw new IllegalStateException("SSL setup not supported for
non-SSL mock");
+ }
+ hostnameVerifierSpy = spy(getHostnameVerifier());
+ mockSession = Mockito.mock(SSLSession.class);
+ SSLSessionContext mockContext =
Mockito.mock(SSLSessionContext.class);
+ doReturn(mockContext).when(mockSession).getSessionContext();
+
+ X509Certificate mockCert = Mockito.mock(X509Certificate.class);
+ Certificate[] certs = {mockCert};
+ doReturn(certs).when(mockSession).getPeerCertificates();
+
+ Collection<?> altNames = Mockito.mock(Collection.class);
+ List<?> generalName = Mockito.mock(List.class);
+ doReturn(2).when(generalName).size();
+ doReturn(2).when(generalName).get(0);
+ doReturn(sanHostname.toLowerCase()).when(generalName).get(1);
+
doReturn(java.util.Collections.singletonList(generalName).iterator()).when(altNames).iterator();
+ doReturn(altNames).when(mockCert).getSubjectAlternativeNames();
+
+ doAnswer(invocation -> {
+ String calledHostname = invocation.getArgument(0);
+ SSLSession calledSession = invocation.getArgument(1);
+ try {
+ Certificate[] certificates =
calledSession.getPeerCertificates();
+ if (certificates == null || certificates.length == 0) {
+ lastVerifyResult = false;
+ return false;
+ }
+ final X509Certificate x509Cert = (X509Certificate)
certificates[0];
+ final Collection<List<?>> sanCollection =
x509Cert.getSubjectAlternativeNames();
+ if (sanCollection == null) {
+ lastVerifyResult = false;
+ return false;
+ }
+ boolean match = false;
+ for (final List<?> generalNameEntry : sanCollection) {
+ if (generalNameEntry.size() > 1) {
+ final Object value = generalNameEntry.get(1);
+ if (value instanceof String) {
+ if (calledHostname.equalsIgnoreCase(((String)
value))) {
+ match = true;
+ break;
+ }
+ }
+ }
+ }
+ lastVerifyResult = match;
+ return match;
+ } catch (final SSLPeerUnverifiedException |
CertificateParsingException ex) {
+ lastVerifyResult = false;
+ return false;
+ }
+ }).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
+ }
+
+ void setupSSLMockWithNoCerts() throws SSLPeerUnverifiedException {
+ if (!useSSL) {
+ throw new IllegalStateException("SSL setup not supported for
non-SSL mock");
+ }
+ hostnameVerifierSpy = spy(getHostnameVerifier());
+ mockSession = Mockito.mock(SSLSession.class);
+ doReturn(null).when(mockSession).getPeerCertificates();
+
doReturn(false).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
+ lastVerifyResult = false;
+ }
+
+ void setupSSLMockWithEmptyCerts() throws SSLPeerUnverifiedException {
+ if (!useSSL) {
+ throw new IllegalStateException("SSL setup not supported for
non-SSL mock");
+ }
+ hostnameVerifierSpy = spy(getHostnameVerifier());
+ mockSession = Mockito.mock(SSLSession.class);
+ doReturn(new
Certificate[0]).when(mockSession).getPeerCertificates();
+
doReturn(false).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
+ lastVerifyResult = false;
+ }
+
+ void setupSSLMockWithSanInIntermediate() throws
CertificateParsingException, SSLPeerUnverifiedException {
+ if (!useSSL) {
+ throw new IllegalStateException("SSL setup not supported for
non-SSL mock");
+ }
+ hostnameVerifierSpy = spy(getHostnameVerifier());
+ mockSession = Mockito.mock(SSLSession.class);
+ SSLSessionContext mockContext =
Mockito.mock(SSLSessionContext.class);
+ doReturn(mockContext).when(mockSession).getSessionContext();
+
+ // Server cert (index 0): No SANs
+ X509Certificate serverCert = Mockito.mock(X509Certificate.class);
+ doReturn(null).when(serverCert).getSubjectAlternativeNames();
+
+ // Intermediate cert (index 1): Has SAN with hostname
+ X509Certificate intermediateCert =
Mockito.mock(X509Certificate.class);
+ Collection<?> intermediateAltNames =
Mockito.mock(Collection.class);
+ List<?> intermediateGeneralName = Mockito.mock(List.class);
+ doReturn(2).when(intermediateGeneralName).size();
+ doReturn(2).when(intermediateGeneralName).get(0);
+
doReturn(HOSTNAME.toLowerCase()).when(intermediateGeneralName).get(1);
+
doReturn(java.util.Collections.singletonList(intermediateGeneralName).iterator()).when(intermediateAltNames).iterator();
+
doReturn(intermediateAltNames).when(intermediateCert).getSubjectAlternativeNames();
+
+ // Root cert (index 2): No SANs
+ X509Certificate rootCert = Mockito.mock(X509Certificate.class);
+ doReturn(null).when(rootCert).getSubjectAlternativeNames();
+
+ Certificate[] certs = {serverCert, intermediateCert, rootCert};
+ doReturn(certs).when(mockSession).getPeerCertificates();
+
+ doAnswer(invocation -> {
+ String calledHostname = invocation.getArgument(0);
+ SSLSession calledSession = invocation.getArgument(1);
+ try {
+ Certificate[] certificates =
calledSession.getPeerCertificates();
+ if (certificates == null || certificates.length == 0) {
+ lastVerifyResult = false;
+ return false;
+ }
+ final X509Certificate x509Cert = (X509Certificate)
certificates[0];
+ final Collection<List<?>> sanCollection =
x509Cert.getSubjectAlternativeNames();
+ if (sanCollection == null) {
+ lastVerifyResult = false;
+ return false;
+ }
+ boolean match = false;
+ for (final List<?> generalNameEntry : sanCollection) {
+ if (generalNameEntry.size() > 1) {
+ final Object value = generalNameEntry.get(1);
+ if (value instanceof String) {
+ if (calledHostname.equalsIgnoreCase(((String)
value))) {
+ match = true;
+ break;
+ }
+ }
+ }
+ }
+ lastVerifyResult = match;
+ return match;
+ } catch (final SSLPeerUnverifiedException |
CertificateParsingException ex) {
+ lastVerifyResult = false;
+ return false;
+ }
+ }).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
Review Comment:
This is a third instance of duplicated hostname verification logic. All
these test methods should use a common implementation to ensure consistency and
maintainability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
