vyommani commented on code in PR #872:
URL: https://github.com/apache/ranger/pull/872#discussion_r3146658526
##########
plugin-nifi-registry/src/test/java/org/apache/ranger/services/nifi/registry/client/TestNiFiRegistryClient.java:
##########
@@ -150,177 +143,60 @@ public void testConnectionTestSuccess() {
}
@Test
- public void testConnectionTestFailure() throws NoSuchAlgorithmException,
KeyManagementException {
+ public void testConnectionTestFailure() {
final String errorMsg = "unknown error";
- registryClient = new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode(), false, null);
+
+ registryClient = new
NiFiRegistryClient("http://localhost:18080/nifi-registry-api/policiesresources";,
null) {
+ @Override
+ protected Client buildClient() {
+ return new MockNiFiRegistryClient(errorMsg,
Response.Status.BAD_REQUEST.getStatusCode()).buildClient();
+ }
+ };
HashMap<String, Object> ret = registryClient.connectionTest();
Assertions.assertNotNull(ret);
Assertions.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));
- Assertions.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));
- Assertions.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");
- Assertions.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");
- Assertions.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));
- Assertions.assertFalse(sslClient.lastVerifyResult);
- }
-
/**
* Extend NiFiRegistryClient to return mock responses.
*/
- private static final class MockNiFiRegistryClient extends
NiFiRegistryClient {
- private final int statusCode;
+ private static final class MockNiFiRegistryClient {
+ 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,
boolean useSSL, String hostname) throws NoSuchAlgorithmException,
KeyManagementException {
- super(useSSL ? ("https://"; + (hostname != null ? hostname :
"localhost") + ":443") : POLICIES_RESOURCES,
- useSSL ? createInitializedSSLContext() : null);
+
+ private MockNiFiRegistryClient(String responseEntity, int statusCode) {
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");
+ public Client buildClient() {
+ Client mockClient = Mockito.mock(Client.class);
+ WebTarget mockWebTarget = Mockito.mock(WebTarget.class);
+ Invocation.Builder mockBuilder =
Mockito.mock(Invocation.Builder.class);
+ Response mockResponse = Mockito.mock(Response.class);
+
+ when(mockClient.target(anyString())).thenReturn(mockWebTarget);
+
+
when(mockWebTarget.request(any(String.class))).thenReturn(mockBuilder);
+
+ OngoingStubbing<Response> ongoingStubbing =
when(mockBuilder.get());
+
+ try {
+ if (statusCode == 200) {
+ ongoingStubbing.thenReturn(mockResponse);
+ when(mockResponse.getStatus()).thenReturn(statusCode);
+
when(mockResponse.readEntity(any(Class.class))).thenReturn(new
ByteArrayInputStream(responseEntity.getBytes(StandardCharsets.UTF_8)));
+ } else {
+ ongoingStubbing.thenReturn(mockResponse);
+ when(mockResponse.getStatus()).thenReturn(statusCode);
+
when(mockResponse.readEntity(String.class)).thenReturn(responseEntity);
+
when(mockResponse.readEntity(InputStream.class)).thenReturn(new
ByteArrayInputStream(responseEntity.getBytes(StandardCharsets.UTF_8)));
+ }
+ } catch (Exception e) {
}
- 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<List<?>> altNames = Collections.singletonList(
- Arrays.asList(2, sanHostname.toLowerCase()));
- doReturn(altNames).when(mockCert).getSubjectAlternativeNames();
- doAnswer(invocation -> {
- Boolean result = (Boolean) invocation.callRealMethod();
- lastVerifyResult = result;
- return result;
- }).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<List<?>> intermediateAltNames =
Collections.singletonList(
- Arrays.asList(2, HOSTNAME.toLowerCase()));
-
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 -> {
- Boolean result = (Boolean) invocation.callRealMethod();
- lastVerifyResult = result;
- return result;
- }).when(hostnameVerifierSpy).verify(any(String.class),
any(SSLSession.class));
- }
-
- @Override
- protected WebResource getWebResource() {
- return Mockito.mock(WebResource.class);
- }
-
- @Override
- protected ClientResponse getResponse(WebResource resource, String
accept) {
- if (useSSL) {
- hostnameVerifierSpy.verify(hostname, mockSession);
- }
- ClientResponse response = Mockito.mock(ClientResponse.class);
- when(response.getStatus()).thenReturn(statusCode);
- when(response.getEntityInputStream()).thenReturn(new
ByteArrayInputStream(responseEntity.getBytes(StandardCharsets.UTF_8)));
- return response;
+ return mockClient;
Review Comment:
I noticed several tests related to SSL, certificates, and HostnameVerifier
were removed.
I'm not certain whether the NiFi client uses its own custom
HostnameVerifier, but if it does, removing these tests might leave gaps in
coverage for SSL/hostname validation behavior.
Could you confirm if these scenarios are still covered elsewhere, or if the
custom verifier is no longer used? Want to make sure we're not missing edge
cases here.
--
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]
