kfaraz commented on code in PR #15480:
URL: https://github.com/apache/druid/pull/15480#discussion_r1429018122
##########
server/src/main/java/org/apache/druid/rpc/indexing/OverlordClientImpl.java:
##########
@@ -96,7 +97,8 @@ public ListenableFuture<Void> runTask(final String taskId,
final Object taskObje
return FutureUtils.transform(
client.asyncRequest(
new RequestBuilder(HttpMethod.POST, "/druid/indexer/v1/task")
- .jsonContent(jsonMapper, taskObject),
+ .jsonContent(jsonMapper, taskObject)
+ .header(AuditManager.X_DRUID_AUTHOR,
AuditManager.AUTHOR_DRUID_SYSTEM),
Review Comment:
This seemed much easier to implement.
I encountered some issues with the other approach:
- The authenticated identity would depend on both auth impl and
configuration. I couldn't find a definite way to determine the internal service
identity. Basic auth creates the `druid_system` user but I don't see it being
used in internal communication. (On second thought, let me see if I can pull
the identity from the escalated client that is being injected)
- When there is no auth, all requests get the identity `allowAll`.
- When auth is enabled, a user could still use the internal druid service
username/password to invoke an API and bypass the audit (This is technically
still possible by setting the author header to this specific value but that is
less likely to happen. This is one of the reasons I chose to include a config
for auditing system requests too).
Please let me know your thoughts on this.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
