[ 
https://issues.apache.org/jira/browse/ARTEMIS-4420?focusedWorklogId=920378&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-920378
 ]

ASF GitHub Bot logged work on ARTEMIS-4420:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 22/May/24 09:10
            Start Date: 22/May/24 09:10
    Worklog Time Spent: 10m 
      Work Description: gtully commented on code in PR #4897:
URL: https://github.com/apache/activemq-artemis/pull/4897#discussion_r1609588605


##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java:
##########
@@ -219,6 +239,18 @@ public synchronized void start() throws Exception {
       cleanupTmp();
       server.start();
 
+      /*

Review Comment:
   I would revert this change. 
   the metrics call to jmx which is instrumented to audit, with out this filter 
the audit on metrics will be less informative.
   in addition, there are cases where access to metrics will need 
authentication, especially when there is RBAC on JMX mbeans.
    



##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java:
##########
@@ -166,6 +173,19 @@ public synchronized void start() throws Exception {
                handlers.addHandler(webContext);
                webContext.setInitParameter(DIR_ALLOWED, "false");
                
webContext.getSessionHandler().getSessionCookieConfig().setComment("__SAME_SITE_STRICT__");
+               webContext.addEventListener(new ServletContextListener() {
+                  @Override
+                  public void contextInitialized(ServletContextEvent sce) {
+                     sce.getServletContext().addListener(new 
ServletRequestListener() {
+                        @Override
+                        public void requestDestroyed(ServletRequestEvent sre) {
+                           ServletRequestListener.super.requestDestroyed(sre);
+                           AuditLogger.currentCaller.remove();

Review Comment:
   that looks right





Issue Time Tracking
-------------------

    Worklog Id:     (was: 920378)
    Time Spent: 50m  (was: 40m)

> User authentication leaks into non-Artemis servlets
> ---------------------------------------------------
>
>                 Key: ARTEMIS-4420
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4420
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>    Affects Versions: 2.30.0
>            Reporter: Dries Harnie
>            Priority: Minor
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> ActiveMQ Artemis supports audit logs, which log all administrative actions 
> that happen on the broker.
> These logs identify the "current user" for an administrative access [by one 
> of two 
> methods|https://github.com/apache/activemq-artemis/blob/main/artemis-commons/src/main/java/org/apache/activemq/artemis/logs/AuditLogger.java#L67-L73]:
>  # The {{Subject}} associated with the current security manager context, or
>  # A {{{}ThreadLocal<Subject>{}}}, which is set by JolokiaFilter as part of 
> interaction with the admin console.
> For a non-Artemis servlet such as [the metrics 
> plugin|https://github.com/rh-messaging/artemis-prometheus-metrics-plugin], 
> this {{ThreadLocal}} is set to whatever {{Subject}} made the previous request 
> on this thread. This leads to situations where metric accesses are logged as 
> being done by ghost users.
> To reproduce the issue:
>  # Set up Artemis with the default admin/admin user and [the metrics 
> plugin|https://github.com/rh-messaging/artemis-prometheus-metrics-plugin].
>  # Enable audit logging ({{{}logger.audit_base{}}} should be at {{INFO}} 
> level)
>  # Tail -f the audit log and start the server
>  # Log in to the admin console
>  # Observe that a lot of audit logs fly by for {*}admin(amq)@127.0.0.1{*}.
>  # Access the metrics with eg {{{}curl http://localhost:8161/metrics/{}}}.
>  # Observe that a lot of audit logs fly by for {*}admin(amq)@127.0.0.1{*}, 
> even though these requests are completely anonymous.
>  
> I think the solution involves a modification to 
> {{org.apache.activemq.artemis.component.JolokiaFilter}} but I do not 
> understand the purpose of the code after the {{doFilter}} invocation.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to