NIFI-4925:

- Addressing memory leak from lingering authorization results that did not represent actual access attempts. This closes #2511.

Signed-off-by: Mark Payne <markap14@hotmail.com>

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationAuditorInvocationHandler.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.authorization;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class AuthorizationAuditorInvocationHandler implements InvocationHandler {
+
+    private final Authorizer authorizer;
+    private final AuthorizationAuditor auditor;
+
+    public AuthorizationAuditorInvocationHandler(final Authorizer authorizer, final AuthorizationAuditor auditor) {
+        this.authorizer = authorizer;
+        this.auditor = auditor;
+    }
+
+    @Override
+    public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+        try {
+            if (AuthorizationAuditor.class.getMethod("auditAccessAttempt", AuthorizationRequest.class, AuthorizationResult.class).equals(method)) {
+                return method.invoke(auditor, args);
+            } else {
+                return method.invoke(authorizer, args);
+            }
+        } catch (final InvocationTargetException e) {
+            // If the proxied instance throws an Exception, it'll be wrapped in an InvocationTargetException. We want
+            // to instead re-throw what the proxied instance threw, so we pull it out of the InvocationTargetException.
+            throw e.getCause();
+        }
+    }
+}

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java

@@ -101,9 +101,11 @@ public final class AuthorizerFactory {
     }
 
     public static Authorizer installIntegrityChecks(final Authorizer baseAuthorizer) {
+        Authorizer authorizer;
+
         if (baseAuthorizer instanceof ManagedAuthorizer) {
             final ManagedAuthorizer baseManagedAuthorizer = (ManagedAuthorizer) baseAuthorizer;
-            return new ManagedAuthorizer() {
+            authorizer = new ManagedAuthorizer() {
                 @Override
                 public String getFingerprint() throws AuthorizationAccessException {
                     return baseManagedAuthorizer.getFingerprint();
@@ -395,7 +397,7 @@ public final class AuthorizerFactory {
                 }
             };
         } else {
-            return new Authorizer() {
+            authorizer = new Authorizer() {
                 @Override
                 public AuthorizationResult authorize(AuthorizationRequest request) throws AuthorizationAccessException {
                     final AuthorizationResult result = baseAuthorizer.authorize(request);
@@ -422,6 +424,19 @@ public final class AuthorizerFactory {
                 }
             };
         }
+
+        // conditionally add support for the audit methods
+        if (baseAuthorizer instanceof AuthorizationAuditor) {
+            final AuthorizationAuditorInvocationHandler invocationHandler = new AuthorizationAuditorInvocationHandler(authorizer, (AuthorizationAuditor) baseAuthorizer);
+
+            final List<Class<?>> interfaceList = ClassUtils.getAllInterfaces(authorizer.getClass());
+            interfaceList.add(AuthorizationAuditor.class);
+            final Class<?>[] interfaces = interfaceList.toArray(new Class<?>[interfaceList.size()]);
+
+            authorizer = (Authorizer) Proxy.newProxyInstance(authorizer.getClass().getClassLoader(), interfaces, invocationHandler);
+        }
+
+        return authorizer;
     }
 
     /**
@@ -433,7 +448,6 @@ public final class AuthorizerFactory {
     public static Authorizer withNarLoader(final Authorizer baseAuthorizer, final ClassLoader classLoader) {
         final AuthorizerInvocationHandler invocationHandler = new AuthorizerInvocationHandler(baseAuthorizer, classLoader);
 
-        // extract all interfaces... baseAuthorizer is non null so getAllInterfaces is non null
         final List<Class<?>> interfaceList = ClassUtils.getAllInterfaces(baseAuthorizer.getClass());
         final Class<?>[] interfaces = interfaceList.toArray(new Class<?>[interfaceList.size()]);
 

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java

@@ -172,6 +172,8 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG
                     // ensure it was found
                     if (authorizer == null) {
                         throw new Exception(String.format("The specified authorizer '%s' could not be found.", authorizerIdentifier));
+                    } else {
+                        authorizer = AuthorizerFactory.installIntegrityChecks(authorizer);
                     }
                 }
             }
@@ -350,7 +352,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG
             authorizerClassLoader = new URLClassLoader(urls, authorizerClassLoader);
         }
 
-        return AuthorizerFactory.installIntegrityChecks(AuthorizerFactory.withNarLoader(instance, authorizerClassLoader));
+        return AuthorizerFactory.withNarLoader(instance, authorizerClassLoader);
     }
 
     private AuthorizerConfigurationContext loadAuthorizerConfiguration(final String identifier, final List<Property> properties) {

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java

@@ -289,6 +289,8 @@ public class AuthorizerFactoryTest {
         Authorizer authorizer = AuthorizerFactory.installIntegrityChecks(mockAuthorizer);
         authorizer.onConfigured(context);
 
+        assertTrue(authorizer instanceof AuthorizationAuditor);
+
         final AuthorizationRequest accessAttempt = new AuthorizationRequest.Builder()
                 .resource(new MockResource("resource1", "Resource 1"))
                 .identity("user-1")

nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/main/java/org/apache/nifi/ranger/authorization/RangerNiFiAuthorizer.java

@@ -46,9 +46,9 @@ import org.slf4j.LoggerFactory;
 import java.io.File;
 import java.net.MalformedURLException;
 import java.util.Date;
+import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.WeakHashMap;
-import java.util.concurrent.ConcurrentMap;
 
 /**
  * Authorizer implementation that uses Apache Ranger to make authorization decisions.
@@ -71,7 +71,7 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor {
     static final String HADOOP_SECURITY_AUTHENTICATION = "hadoop.security.authentication";
     static final String KERBEROS_AUTHENTICATION = "kerberos";
 
-    private final ConcurrentMap<AuthorizationRequest, RangerAccessResult> resultLookup = new ConcurrentHashMap<>();
+    private final Map<AuthorizationRequest, RangerAccessResult> resultLookup = new WeakHashMap<>();
 
     private volatile RangerBasePluginWithPolicies nifiPlugin = null;
     private volatile RangerDefaultAuditHandler defaultAuditHandler = null;
@@ -175,10 +175,14 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor {
 
         final RangerAccessResult result = nifiPlugin.isAccessAllowed(rangerRequest);
 
-        if (result != null && result.getIsAllowed()) {
+        // store the result for auditing purposes later if appropriate
-            // store the result for auditing purposes later
+        if (request.isAccessAttempt()) {
-            resultLookup.put(request, result);
+            synchronized (resultLookup) {
+                resultLookup.put(request, result);
+            }
+        }
 
+        if (result != null && result.getIsAllowed()) {
             // return approved
             return AuthorizationResult.approved();
         } else {
@@ -192,9 +196,6 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor {
                     logger.debug(String.format("Unable to authorize %s due to %s", identity, reason));
                 }
 
-                // store the result for auditing purposes later
-                resultLookup.put(request, result);
-
                 // a policy does exist for the resource so we were really denied access here
                 return AuthorizationResult.denied(request.getExplanationSupplier().get());
             } else {
@@ -206,7 +207,10 @@ public class RangerNiFiAuthorizer implements Authorizer, AuthorizationAuditor {
 
     @Override
     public void auditAccessAttempt(final AuthorizationRequest request, final AuthorizationResult result) {
-        final RangerAccessResult rangerResult = resultLookup.remove(request);
+        final RangerAccessResult rangerResult;
+        synchronized (resultLookup) {
+            rangerResult = resultLookup.remove(request);
+        }
 
         if (rangerResult != null && rangerResult.getIsAudited()) {
             AuthzAuditEvent event = defaultAuditHandler.getAuthzEvents(rangerResult);