From 0b6eccf4c3273dbe4355e179e94814dc6131d87b Mon Sep 17 00:00:00 2001
From: Jerry He <jerryjch@apache.org>
Date: Fri, 2 Sep 2016 09:47:28 -0700
Subject: [PATCH] HBASE-16311 Audit log for delete snapshot operation is
 missing in case of snapshot owner deleting the same (Yi Liang)

---
 .../security/access/AccessController.java     | 20 +++++++++++--------
 .../security/access/TestAccessController.java |  4 ++--
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 0e69060b015..ff27b4161de 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -1314,7 +1314,7 @@ public class AccessController extends BaseMasterAndRegionObserver
   public void preSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot, final HTableDescriptor hTableDescriptor)
       throws IOException {
-    requirePermission(getActiveUser(ctx), "snapshot", hTableDescriptor.getTableName(), null, null,
+    requirePermission(getActiveUser(ctx), "snapshot " + snapshot.getName(), hTableDescriptor.getTableName(), null, null,
       Permission.Action.ADMIN);
   }
 
@@ -1324,9 +1324,11 @@ public class AccessController extends BaseMasterAndRegionObserver
     User user = getActiveUser(ctx);
     if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, user)) {
       // list it, if user is the owner of snapshot
-      // TODO: We are not logging this for audit
+      AuthResult result = AuthResult.allow("listSnapshot " + snapshot.getName(),
+          "Snapshot owner check allowed", user, null, null, null);
+      logResult(result);
     } else {
-      requirePermission(user, "listSnapshot", Action.ADMIN);
+      requirePermission(user, "listSnapshot " + snapshot.getName(), Action.ADMIN);
     }
   }
 
@@ -1334,7 +1336,7 @@ public class AccessController extends BaseMasterAndRegionObserver
   public void preCloneSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot, final HTableDescriptor hTableDescriptor)
       throws IOException {
-    requirePermission(getActiveUser(ctx), "clone", Action.ADMIN);
+    requirePermission(getActiveUser(ctx), "cloneSnapshot " + snapshot.getName(), Action.ADMIN);
   }
 
   @Override
@@ -1343,10 +1345,10 @@ public class AccessController extends BaseMasterAndRegionObserver
       throws IOException {
     User user = getActiveUser(ctx);
     if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, user)) {
-      requirePermission(user, "restoreSnapshot", hTableDescriptor.getTableName(), null, null,
+      requirePermission(user, "restoreSnapshot " + snapshot.getName(), hTableDescriptor.getTableName(), null, null,
         Permission.Action.ADMIN);
     } else {
-      requirePermission(user, "restoreSnapshot", Action.ADMIN);
+      requirePermission(user, "restoreSnapshot " + snapshot.getName(), Action.ADMIN);
     }
   }
 
@@ -1356,9 +1358,11 @@ public class AccessController extends BaseMasterAndRegionObserver
     User user = getActiveUser(ctx);
     if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, user)) {
       // Snapshot owner is allowed to delete the snapshot
-      // TODO: We are not logging this for audit
+      AuthResult result = AuthResult.allow("deleteSnapshot " + snapshot.getName(),
+          "Snapshot owner check allowed", user, null, null, null);
+      logResult(result);
     } else {
-      requirePermission(user, "deleteSnapshot", Action.ADMIN);
+      requirePermission(user, "deleteSnapshot " + snapshot.getName(), Action.ADMIN);
     }
   }
 
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 20ff85ff704..0bc27797f8b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -2053,7 +2053,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preCloneSnapshot(ObserverContext.createAndPrepare(CP_ENV, null),
-          null, null);
+          snapshot, null);
         return null;
       }
     };
@@ -2124,7 +2124,7 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preCloneSnapshot(ObserverContext.createAndPrepare(CP_ENV, null),
-          null, null);
+          snapshot, null);
         return null;
       }
     };