smolnar82 commented on code in PR #1001:
URL: https://github.com/apache/knox/pull/1001#discussion_r1978691497
##########
gateway-provider-security-authc-remote/src/main/java/org/apache/knox/gateway/filter/RemoteAuthFilter.java:
##########
@@ -72,11 +72,14 @@ public class RemoteAuthFilter implements Filter {
private static final String CONFIG_GROUP_HEADER = "remote.auth.group.header";
private static final String DEFAULT_CONFIG_USER_HEADER = "X-Knox-Actor-ID";
private static final String DEFAULT_CONFIG_GROUP_HEADER =
"X-Knox-Actor-Groups-*";
+ private static final String CONFIG_TRUSTSTORE_PATH =
"remote.auth.truststore.path";
Review Comment:
nit: the `remote.auth.` prefix could be a separate final constant (so that
all properties could start like `REMOTE_AUTH_PREFIX + `)
##########
gateway-provider-security-authc-remote/src/main/java/org/apache/knox/gateway/filter/RemoteAuthFilter.java:
##########
@@ -203,12 +217,20 @@ private HttpURLConnection
getHttpURLConnection(ServletContext servletContext) th
KeystoreService keystoreService =
services.getService(ServiceType.KEYSTORE_SERVICE);
if (keystoreService != null) {
try {
- truststore = keystoreService.getTruststoreForHttpClient();
+ // Try topology-specific truststore first if configured
Review Comment:
Given the updated logic, `getTrustStore` should be a separate private method.
##########
gateway-provider-security-authc-remote/src/test/java/org/apache/knox/gateway/filter/RemoteAuthFilterTest.java:
##########
@@ -65,22 +72,52 @@ public class RemoteAuthFilterTest {
private HttpServletRequest requestMock;
private HttpServletResponse responseMock;
private TestFilterChain chainMock;
+ private GatewayServices gatewayServicesMock;
+ private KeystoreService keystoreServiceMock;
+ private ServletContext servletContextMock;
+
@Before
- public void setUp() {
- FilterConfig filterConfigMock =
EasyMock.createNiceMock(FilterConfig.class);
+ public void createMocks() {
requestMock = EasyMock.createMock(HttpServletRequest.class);
responseMock = EasyMock.createMock(HttpServletResponse.class);
+ }
+
+ private void setUp(String trustStorePath, String trustStorePass, String
trustStoreType) {
+ // Reset existing mocks
+ EasyMock.reset(requestMock, responseMock);
+
+ FilterConfig filterConfigMock =
EasyMock.createNiceMock(FilterConfig.class);
chainMock = new TestFilterChain();
-
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.url")).andReturn("http://example.com/auth";).anyTimes();
+ // Create and configure Gateway Services mocks
+ gatewayServicesMock = EasyMock.createNiceMock(GatewayServices.class);
+ keystoreServiceMock = EasyMock.createNiceMock(KeystoreService.class);
+ servletContextMock = EasyMock.createNiceMock(ServletContext.class);
+
+ // Set up Gateway Services expectations
+
EasyMock.expect(gatewayServicesMock.getService(ServiceType.KEYSTORE_SERVICE))
+ .andReturn(keystoreServiceMock)
+ .anyTimes();
+
EasyMock.expect(servletContextMock.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE))
+ .andReturn(gatewayServicesMock)
+ .anyTimes();
+
+ // Basic config
+
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.url")).andReturn("https://example.com/auth";).anyTimes();
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.include.headers")).andReturn("Authorization").anyTimes();
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.cache.key")).andReturn("Authorization").anyTimes();
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.expire.after")).andReturn("5").anyTimes();
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.user.header")).andReturn(X_AUTHENTICATED_USER).anyTimes();
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.group.header"))
.andReturn(X_AUTHENTICATED_GROUP + "," +
X_AUTHENTICATED_GROUP_2 + ",X-Custom-Group-*").anyTimes();
- EasyMock.replay(filterConfigMock);
+ // Trust store config
+
EasyMock.expect(filterConfigMock.getInitParameter("remote.auth.truststore.path")).andReturn(trustStorePath).anyTimes();
Review Comment:
If I were you, I'd set the property names' visibility to package level ->
they can be used in the test class. E.g.:
`EasyMock.expect(filterConfigMock.getInitParameter(CONFIG_TRUSTSTORE_PATH)).andReturn(trustStorePath).anyTimes();`
--
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: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
