[
https://issues.apache.org/jira/browse/KNOX-3105?focusedWorklogId=959889&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-959889
]
ASF GitHub Bot logged work on KNOX-3105:
----------------------------------------
Author: ASF GitHub Bot
Created on: 04/Mar/25 06:22
Start Date: 04/Mar/25 06:22
Worklog Time Spent: 10m
Work Description: 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();`
Issue Time Tracking
-------------------
Worklog Id: (was: 959889)
Time Spent: 20m (was: 10m)
> Add Topology Level Config for Truststore to RemoteAuthProvider
> --------------------------------------------------------------
>
> Key: KNOX-3105
> URL: https://issues.apache.org/jira/browse/KNOX-3105
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Reporter: Larry McCay
> Assignee: Larry McCay
> Priority: Major
> Fix For: 2.2.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> I originally had this topology level config only for the truststore and
> password but decided that it should be configured at the gateway level.
> However, it is much easier to use specific truststores for dev and testing
> environments than adding a cert from one Knox to another's truststore which
> may have other certs, etc.
> This change will add the params for location and password with alias service
> support of the password.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
