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 how auth is configured. 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.
- 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]
