Skip to content

Commit 24557f7

Browse files
committed
[CDAP-21206] Fix task worker encryption for connection and oauth macro evaluator in rbac instances
1 parent c45f4fc commit 24557f7

File tree

4 files changed

+67
-49
lines changed

4 files changed

+67
-49
lines changed

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/services/AppFabricServer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ protected void startUp() throws Exception {
170170

171171
// Create encryption exemption handler hooks
172172
List<EncryptionExemptionHook> encryptionHandlerHooks = handlerHookNames.stream()
173-
.map(name -> new EncryptionExemptionHook(cConf, name))
173+
.map(name -> new EncryptionExemptionHook())
174174
.collect(Collectors.toList());
175175

176176
handlerHooks.addAll(auditHandlerHooks);

cdap-common/src/main/java/io/cdap/cdap/common/encryption/EncryptionExemptionHook.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package io.cdap.cdap.common.encryption;
1818

1919
import com.google.common.collect.ImmutableList;
20-
import io.cdap.cdap.common.conf.CConfiguration;
2120
import io.cdap.cdap.common.http.HttpHeaderNames;
2221
import io.cdap.cdap.security.spi.authentication.SecurityRequestContext;
2322
import io.cdap.http.AbstractHandlerHook;
@@ -34,28 +33,30 @@
3433
* Sets encryption exception metadata to {@link SecurityRequestContext}.
3534
*/
3635
public class EncryptionExemptionHook extends AbstractHandlerHook {
37-
private static final Logger LOG = LoggerFactory.getLogger(EncryptionExemptionHook.class);
38-
private final String serviceName;
3936

37+
private static final Logger LOG = LoggerFactory.getLogger(EncryptionExemptionHook.class);
4038
private static final List<Pattern> EXEMPTED_URIS = ImmutableList.of(
4139
Pattern.compile("/v3Internal/namespaces/([^/]+)/artifacts/([^/]+)/versions/([^/]+)(/.*)?$"),
4240
Pattern.compile("/v3/namespaces/([^/]+)/artifacts/([^/]+)/versions/([^/]+)(/.*)?$"),
43-
Pattern.compile("/v3Internal/namespaces/([^/]+)/credentials/workloadIdentity/provision(\\?scopes=(.*))?$"),
41+
Pattern.compile(
42+
"/v3Internal/namespaces/([^/]+)/credentials/workloadIdentity/provision(\\?scopes=(.*))?$"),
4443
Pattern.compile("/v3Internal/namespaces/([^/]+)/preferences([^/]+)"),
45-
Pattern.compile("/v3/namespaces/([^/]+)/securekeys/([^/]+)(/.*)?$")
46-
);
44+
Pattern.compile("/v3/namespaces/([^/]+)/securekeys/([^/]+)(/.*)?$"),
45+
Pattern.compile("/v3/namespaces/([^/]+)/apps/([^/]+)/services/([^/]+)/methods"
46+
+ "/v1/contexts/([^/]+)/connections(?:/.*)?$"),
47+
Pattern.compile("/v3/namespaces/([^/]+)/apps/([^/]+)/services/([^/]+)/methods"
48+
+ "/v1/oauth/provider/([^/]+)(?:/authurl|/credential/([^/]+)(?:/valid)?)?$")
4749

48-
public EncryptionExemptionHook(CConfiguration cConf, String serviceName) {
49-
this.serviceName = serviceName;
50-
}
50+
);
5151

5252
@Override
5353
public boolean preCall(HttpRequest request, HttpResponder responder, HandlerInfo handlerInfo) {
5454
try {
5555
for (Pattern uriPattern : EXEMPTED_URIS) {
5656
Matcher matcher = uriPattern.matcher(request.uri());
5757
if (matcher.matches()) {
58-
// For any pattern match, set the header to false to prevent Unauthenticated exception after decryption
58+
// For any pattern match, set the header to false, in order to prevent Unauthenticated exception
59+
// after decryption.
5960
request.headers().set(HttpHeaderNames.TASK_WORKER_DECRYPTION_HDR, "false");
6061
return true;
6162
}

cdap-common/src/main/java/io/cdap/cdap/common/http/CommonNettyHttpServiceBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void modify(ChannelPipeline pipeline) {
8383
this.setHandlerHooks(Collections.unmodifiableList(
8484
Arrays.asList(new MetricsReporterHook(cConf, metricsCollectionService, serviceName),
8585
new AuditLogSetterHook(cConf, serviceName),
86-
new EncryptionExemptionHook(cConf, serviceName))));
86+
new EncryptionExemptionHook())));
8787
}
8888

8989
/**

cdap-common/src/test/java/io/cdap/cdap/common/encryption/EncryptionExemptionHookTest.java

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,65 +18,82 @@
1818

1919
import static io.cdap.cdap.common.http.HttpHeaderNames.TASK_WORKER_DECRYPTION_HDR;
2020

21-
import io.cdap.cdap.common.conf.CConfiguration;
2221
import io.cdap.http.internal.HandlerInfo;
2322
import io.netty.handler.codec.http.DefaultHttpRequest;
2423
import io.netty.handler.codec.http.HttpMethod;
2524
import io.netty.handler.codec.http.HttpRequest;
2625
import io.netty.handler.codec.http.HttpVersion;
26+
import java.util.Arrays;
27+
import java.util.Collection;
2728
import org.junit.Assert;
29+
import org.junit.Before;
2830
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.junit.runners.Parameterized;
33+
import org.junit.runners.Parameterized.Parameters;
2934

35+
/**
36+
* Tests for {@link EncryptionExemptionHook}.
37+
*/
38+
@RunWith(Parameterized.class)
3039
public class EncryptionExemptionHookTest {
31-
private static final String TESTSERVICENAME = "test.Service";
32-
private static final String TESTHANDLERNAME = "test.handler";
33-
private static final String TESTMETHODNAME = "testMethod";
3440

35-
@Test
36-
public void testPatternMatchingSuccessful() {
37-
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
38-
"/v3/namespaces/default/securekeys/personal-token");
39-
HandlerInfo handlerInfo = new HandlerInfo(TESTHANDLERNAME, TESTMETHODNAME);
40-
EncryptionExemptionHook hook = new EncryptionExemptionHook(CConfiguration.create(), TESTSERVICENAME);
41+
private HandlerInfo handlerInfo;
42+
private EncryptionExemptionHook hook;
4143

42-
hook.preCall(request, null, handlerInfo);
44+
// Parameters for the test
45+
private final String uri;
46+
private final boolean expectedDecryptionHeader;
47+
private final String testName;
4348

44-
Assert.assertFalse(Boolean.parseBoolean(request.headers().get(TASK_WORKER_DECRYPTION_HDR)));
49+
public EncryptionExemptionHookTest(String uri, boolean expectedDecryptionHeader,
50+
String testName) {
51+
this.uri = uri;
52+
this.expectedDecryptionHeader = expectedDecryptionHeader;
53+
this.testName = testName;
4554
}
4655

47-
@Test
48-
public void testPatternMatchingCredentialsUriWithQueryParamsSuccessful() {
49-
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
50-
"/v3Internal/namespaces/default/credentials/workloadIdentity/"
51-
+ "provision?scopes=https://www.test.com/auth/test-platform");
52-
HandlerInfo handlerInfo = new HandlerInfo(TESTHANDLERNAME, TESTMETHODNAME);
53-
EncryptionExemptionHook hook = new EncryptionExemptionHook(CConfiguration.create(), TESTSERVICENAME);
54-
55-
hook.preCall(request, null, handlerInfo);
56+
// Define the data set for the tests
57+
@Parameters(name = "{2}")
58+
public static Collection<Object[]> data() {
59+
return Arrays.asList(new Object[][]{
60+
// Successful (Exempt) Tests - Header should be FALSE.
61+
{"/v3/namespaces/default/securekeys/personal-token", false, "SecureKeysExempt"},
62+
{"/v3Internal/namespaces/default/credentials/workloadIdentity/provision"
63+
+ "?scopes=https://www.test.com/auth/test-platform",
64+
false, "CredentialsWithQueryParamsExempt"},
65+
{"/v3Internal/namespaces/default/credentials/workloadIdentity/provision", false,
66+
"CredentialsWithoutQueryParamsExempt"},
67+
{"/v3/namespaces/system/apps/pipeline/services/studio/methods"
68+
+ "/v1/contexts/default/connections/testing",
69+
false, "ConnectionValidationExempt"},
70+
{"/v3/namespaces/system/apps/pipeline/services/studio/methods"
71+
+ "/v1/oauth/provider/provider/credential/REUSE_PROV_FALSE",
72+
false, "OAuthMacroEvaluatorExempt"},
73+
// Unsuccessful (Non-Exempt) Test - Header should be TRUE.
74+
{"testing", true, "NonExempt"}});
75+
}
5676

57-
Assert.assertFalse(Boolean.parseBoolean(request.headers().get(TASK_WORKER_DECRYPTION_HDR)));
77+
@Before
78+
public void setup() {
79+
handlerInfo = new HandlerInfo("test.handler", "testMethod");
80+
hook = new EncryptionExemptionHook();
5881
}
5982

6083
@Test
61-
public void testPatternMatchingCredentialsUriWithoutQueryParamsSuccessful() {
62-
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
63-
"/v3Internal/namespaces/default/credentials/workloadIdentity/provision");
64-
HandlerInfo handlerInfo = new HandlerInfo(TESTHANDLERNAME, TESTMETHODNAME);
65-
EncryptionExemptionHook hook = new EncryptionExemptionHook(CConfiguration.create(), TESTSERVICENAME);
84+
public void testPreCall() {
85+
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, this.uri);
6686

6787
hook.preCall(request, null, handlerInfo);
6888

69-
Assert.assertFalse(Boolean.parseBoolean(request.headers().get(TASK_WORKER_DECRYPTION_HDR)));
70-
}
71-
72-
@Test
73-
public void testPatternMatchingUnsuccessful() {
74-
HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "testingUri");
75-
HandlerInfo handlerInfo = new HandlerInfo(TESTHANDLERNAME, TESTMETHODNAME);
76-
EncryptionExemptionHook hook = new EncryptionExemptionHook(CConfiguration.create(), TESTSERVICENAME);
89+
String message = String.format("Test case '%s' failed for URI: %s. Expected header value: %s.",
90+
this.testName, this.uri, this.expectedDecryptionHeader);
7791

78-
hook.preCall(request, null, handlerInfo);
92+
Assert.assertEquals(message, this.expectedDecryptionHeader, isDecryptionHeaderSet(request));
93+
}
7994

80-
Assert.assertTrue(Boolean.parseBoolean(request.headers().get(TASK_WORKER_DECRYPTION_HDR)));
95+
private boolean isDecryptionHeaderSet(HttpRequest request) {
96+
String headerValue = request.headers().get(TASK_WORKER_DECRYPTION_HDR);
97+
return Boolean.parseBoolean(headerValue);
8198
}
8299
}

0 commit comments

Comments
 (0)