From 00cb89215017707e96dd1cff51d2b323b92ca512 Mon Sep 17 00:00:00 2001
From: Sharad Agarwal <sharad@apache.org>
Date: Thu, 29 Apr 2010 09:31:30 +0000
Subject: [PATCH] HADOOP-6634. Fix AccessControlList to use short names to
 verify access control. Contributed by Vinod Kumar Vavilapalli.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@939242 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES.txt                                   |  3 +
 .../security/authorize/AccessControlList.java |  2 +-
 .../authorize/TestAccessControlList.java      | 78 +++++++++++++++----
 3 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 360f104fc6..eb582b0ddf 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -382,6 +382,9 @@ Trunk (unreleased changes)
     HADOOP-6722. NetUtils.connect should check that it hasn't connected a socket
     to itself. (Todd Lipcon via tomwhite)
 
+    HADOOP-6634. Fix AccessControlList to use short names to verify access 
+    control. (Vinod Kumar Vavilapalli via sharad)
+ 
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES
diff --git a/src/java/org/apache/hadoop/security/authorize/AccessControlList.java b/src/java/org/apache/hadoop/security/authorize/AccessControlList.java
index 5bed0388d5..c63f42fd9f 100644
--- a/src/java/org/apache/hadoop/security/authorize/AccessControlList.java
+++ b/src/java/org/apache/hadoop/security/authorize/AccessControlList.java
@@ -93,7 +93,7 @@ Set<String> getGroups() {
   }
 
   public boolean isUserAllowed(UserGroupInformation ugi) {
-    if (allAllowed || users.contains(ugi.getUserName())) {
+    if (allAllowed || users.contains(ugi.getShortUserName())) {
       return true;
     } else {
       for(String group: ugi.getGroupNames()) {
diff --git a/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java b/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java
index f1c018d966..581e434da8 100644
--- a/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java
+++ b/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java
@@ -20,6 +20,7 @@
 import java.util.Iterator;
 import java.util.Set;
 
+import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.security.authorize.AccessControlList;
 
 
@@ -77,19 +78,7 @@ public void testAccessControlList() throws Exception {
     assertEquals(groups.size(), 1);
     assertEquals(groups.iterator().next(), "tardis");
 
-    Iterator<String> iter;
-    acl = new AccessControlList("drwho,joe tardis,users");
-    users = acl.getUsers();
-    assertEquals(users.size(), 2);
-    iter = users.iterator();
-    assertEquals(iter.next(), "drwho");
-    assertEquals(iter.next(), "joe");
-    groups = acl.getGroups();
-    assertEquals(groups.size(), 2);
-    iter = groups.iterator();
-    assertEquals(iter.next(), "tardis");
-    assertEquals(iter.next(), "users");
-    
+    Iterator<String> iter;    
     acl = new AccessControlList("drwho,joe tardis, users");
     users = acl.getUsers();
     assertEquals(users.size(), 2);
@@ -102,4 +91,67 @@ public void testAccessControlList() throws Exception {
     assertEquals(iter.next(), "tardis");
     assertEquals(iter.next(), "users");
   }
+
+  /**
+   * Verify the method isUserAllowed()
+   */
+  public void testIsUserAllowed() {
+    AccessControlList acl;
+
+    UserGroupInformation drwho =
+        UserGroupInformation.createUserForTesting("drwho@APACHE.ORG",
+            new String[] { "aliens", "humanoids", "timelord" });
+    UserGroupInformation susan =
+        UserGroupInformation.createUserForTesting("susan@APACHE.ORG",
+            new String[] { "aliens", "humanoids", "timelord" });
+    UserGroupInformation barbara =
+        UserGroupInformation.createUserForTesting("barbara@APACHE.ORG",
+            new String[] { "humans", "teachers" });
+    UserGroupInformation ian =
+        UserGroupInformation.createUserForTesting("ian@APACHE.ORG",
+            new String[] { "humans", "teachers" });
+
+    acl = new AccessControlList("drwho humanoids");
+    assertUserAllowed(drwho, acl);
+    assertUserAllowed(susan, acl);
+    assertUserNotAllowed(barbara, acl);
+    assertUserNotAllowed(ian, acl);
+
+    acl = new AccessControlList("drwho");
+    assertUserAllowed(drwho, acl);
+    assertUserNotAllowed(susan, acl);
+    assertUserNotAllowed(barbara, acl);
+    assertUserNotAllowed(ian, acl);
+
+    acl = new AccessControlList("drwho ");
+    assertUserAllowed(drwho, acl);
+    assertUserNotAllowed(susan, acl);
+    assertUserNotAllowed(barbara, acl);
+    assertUserNotAllowed(ian, acl);
+
+    acl = new AccessControlList(" humanoids");
+    assertUserAllowed(drwho, acl);
+    assertUserAllowed(susan, acl);
+    assertUserNotAllowed(barbara, acl);
+    assertUserNotAllowed(ian, acl);
+
+    acl = new AccessControlList("drwho,ian aliens,teachers");
+    assertUserAllowed(drwho, acl);
+    assertUserAllowed(susan, acl);
+    assertUserAllowed(barbara, acl);
+    assertUserAllowed(ian, acl);
+  }
+
+  private void assertUserAllowed(UserGroupInformation ugi,
+      AccessControlList acl) {
+    assertTrue("User " + ugi + " is not granted the access-control!!",
+        acl.isUserAllowed(ugi));
+  }
+
+  private void assertUserNotAllowed(UserGroupInformation ugi,
+      AccessControlList acl) {
+    assertFalse("User " + ugi
+        + " is incorrectly granted the access-control!!",
+        acl.isUserAllowed(ugi));
+  }
 }