oscerd commented on code in PR #26430:
URL: https://github.com/apache/camel/pull/26430#discussion_r4006938725
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java:
##########
@@ -185,6 +192,22 @@ private static boolean includesAnything(Set<String>
filter) {
return filter == null || !filter.isEmpty();
}
+ /**
+ * A credential header is only sent when the configuration names it, never
through the wildcard.
+ */
+ private boolean isWithheldCredential(String name) {
+ return includedHeaders == null && CREDENTIAL_HEADERS.contains(name);
+ }
+
+ private static Set<String> credentialHeaders() {
+ Set<String> names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
+ names.add("Authorization");
+ names.add("Proxy-Authorization");
+ names.add("Cookie");
Review Comment:
Fixed in 21acb67. `static final` only pinned the reference, so the `TreeSet`
behind it was still open to `add`/`clear` — wrapped with
`Collections.unmodifiableSet`.
_Claude Code on behalf of @oscerd_
##########
components/camel-opa/src/main/docs/opa-component.adoc:
##########
@@ -120,7 +120,15 @@
https://github.com/apache/camel/blob/main/components/camel-opa/src/test/resource
which is a worked example of both a plain boolean rule and a decision object
with deny reasons; `OpaIT`
alongside it shows the matching routes end to end.
-`headers` carries every message header by default. Set `includeHeaders` to a
comma-separated list of names
+`headers` carries every message header by default, with one exception: the
headers that carry a caller credential
+verbatim — `Authorization`, `Proxy-Authorization`, `Cookie` and `Set-Cookie` —
are *withheld* from the wildcard.
+OPA's decision logging ships the whole `input` document, frequently to a
remote collector, so the wildcard should
+not quietly export credentials off the box. A policy that genuinely needs one
can still have it by naming the
+header: `includeHeaders=Authorization,user` sends it. Matching is
case-insensitive, so `authorization` is withheld
+too.
+
+Prefer `includeProperties` for identity: a token that an earlier step has
already *verified* belongs there, as
+<<authorizing-an-identity>> describes, rather than handing the raw credential
to the policy to re-check. Set `includeHeaders` to a comma-separated list of
names
(matched case-insensitively) when the policy only needs a few of them. `body`
is *not* sent unless `includeBody`
Review Comment:
Right, and thanks for checking the catalog copy too. Fixed in 21acb67.
My edit replaced the opening of that paragraph but left its tail, so the new
`includeProperties` sentence ran straight into "Set `includeHeaders` to a
comma-separated list of names (matched case-insensitively)" — which also
restated case-insensitivity two sentences after I had just made the same point.
Split into three paragraphs: headers-and-credentials (keeping the "narrow
the list" advice, minus the duplicate), `includeProperties`, then `body`. The
catalog copy regenerated from the source during the full build, so both are
clean now.
_Claude Code on behalf of @oscerd_
##########
components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaInputDocumentTest.java:
##########
@@ -90,6 +90,37 @@ void convertsANonJsonBodyToItsStringForm() throws Exception {
assertThat(input).containsEntry("body", "the payload");
}
+ @Test
+ void withholdsCredentialHeadersFromTheWildcard() throws Exception {
+ Map<String, Object> input = inputSentFor(ENDPOINT, e -> {
+ e.getMessage().setHeader("user", "alice");
+ e.getMessage().setHeader("Authorization", "Bearer s3cr3t");
+ e.getMessage().setHeader("Cookie", "session=s3cr3t");
+ e.getMessage().setHeader("Proxy-Authorization", "Basic s3cr3t");
+ });
+
Review Comment:
Good catch — that was the sharpest of the three. Fixed in 21acb67.
With `containsOnlyKeys("user")` the test only proves the headers it actually
sets are absent, so a `Set-Cookie` filter that silently stopped working would
have kept passing. The list and the test now agree on all four.
_Claude Code on behalf of @oscerd_
--
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]
