From 8be6f95f99e776c5b46845cb42d1c2c03c25f802 Mon Sep 17 00:00:00 2001
From: Jerry He <jerryjch@apache.org>
Date: Fri, 2 Sep 2016 10:09:44 -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     | 25 ++++++++++++-------
 .../security/access/TestAccessController.java |  4 +--
 2 files changed, 18 insertions(+), 11 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 a147b1204a0..7d1345f5d1a 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
@@ -1313,17 +1313,21 @@ public class AccessController extends BaseMasterAndRegionObserver
   public void preSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot, final HTableDescriptor hTableDescriptor)
       throws IOException {
-    requirePermission("snapshot", hTableDescriptor.getTableName(), null, null,
+    requirePermission("snapshot " + snapshot.getName(), hTableDescriptor.getTableName(), null, null,
       Permission.Action.ADMIN);
   }
 
   @Override
   public void preListSnapshot(ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot) throws IOException {
-    if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, getActiveUser())) {
+    User user = getActiveUser();
+    if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, user)) {
       // list it, if user is the owner of snapshot
+      AuthResult result = AuthResult.allow("listSnapshot " + snapshot.getName(),
+          "Snapshot owner check allowed", user, null, null, null);
+      logResult(result);
     } else {
-      requirePermission("listSnapshot", Action.ADMIN);
+      requirePermission("listSnapshot " + snapshot.getName(), Action.ADMIN);
     }
   }
   
@@ -1331,7 +1335,7 @@ public class AccessController extends BaseMasterAndRegionObserver
   public void preCloneSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot, final HTableDescriptor hTableDescriptor)
       throws IOException {
-    requirePermission("clone", Action.ADMIN);
+    requirePermission("cloneSnapshot " + snapshot.getName(), Action.ADMIN);
   }
 
   @Override
@@ -1339,21 +1343,24 @@ public class AccessController extends BaseMasterAndRegionObserver
       final SnapshotDescription snapshot, final HTableDescriptor hTableDescriptor)
       throws IOException {
     if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, getActiveUser())) {
-      requirePermission("restoreSnapshot", hTableDescriptor.getTableName(), null, null,
+      requirePermission("restoreSnapshot " + snapshot.getName(), hTableDescriptor.getTableName(), null, null,
         Permission.Action.ADMIN);
     } else {
-      requirePermission("restore", Action.ADMIN);
+      requirePermission("restoreSnapshot " + snapshot.getName(), Action.ADMIN);
     }
   }
 
   @Override
   public void preDeleteSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final SnapshotDescription snapshot) throws IOException {
-    if (SnapshotDescriptionUtils.isSnapshotOwner(snapshot, getActiveUser())) {
+    User user = getActiveUser();
+    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("deleteSnapshot", Action.ADMIN);
+      requirePermission("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 2e77c785d11..221241e2722 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
@@ -2051,7 +2051,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;
       }
     };
@@ -2122,7 +2122,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;
       }
     };