From a14f1cec3751f18f56c28c7a5ea20b54ab319bc6 Mon Sep 17 00:00:00 2001
From: Eric Price <ecprice@mit.edu>
Date: Mon, 14 Apr 2008 00:44:00 -0400
Subject: [PATCH] Resolve #69, by checking that administrators are either
 users or groups in the athena cell.

svn path=/trunk/packages/sipb-xen-www/; revision=413
---
 code/cache_acls.py   |    5 ++++-
 code/getafsgroups.py |   25 ++++++++++---------------
 code/validation.py   |    9 ++++++---
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/code/cache_acls.py b/code/cache_acls.py
index 81827b0..f7575e1 100644
--- a/code/cache_acls.py
+++ b/code/cache_acls.py
@@ -28,7 +28,10 @@ def expandName(name):
         if isUser(name):
             return [name]
         name = 'system:'+name
-    return getafsgroups.getAfsGroupMembers(name, 'athena.mit.edu')
+    try:
+        return getafsgroups.getAfsGroupMembers(name, 'athena.mit.edu')
+    except getafsgroups.AfsProcessError:
+        return []
 
 def accessList(m):
     people = set()
diff --git a/code/getafsgroups.py b/code/getafsgroups.py
index e2fece0..13f8cf7 100644
--- a/code/getafsgroups.py
+++ b/code/getafsgroups.py
@@ -28,10 +28,11 @@ class AfsProcessError(Exception):
     pass
 
 def getAfsGroupMembers(group, cell):
-    p = subprocess.Popen(["pts", "membership", group, '-c', cell], 
+    p = subprocess.Popen(["pts", "membership", "-noauth", group, '-c', cell], 
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if p.wait():
-        return []
+    err = p.stderr.read()
+    if err: #Error code doesn't reveal missing groups, but stderr does
+        raise AfsProcessError(err)
     return [line.strip() for line in p.stdout.readlines()[1:]]
 
 def getLockerPath(locker):
@@ -39,12 +40,6 @@ def getLockerPath(locker):
         raise AfsProcessError("Locker '%s' is invalid." % locker)
     return '/mit/' + locker
 
-def checkAfsGroup(user, group, cell):
-    """
-    checkAfsGroup(user, group) returns True if and only if user is in AFS group group in cell cell
-    """
-    return user in getAfsGroupMembers(group, cell)
-
 def getCell(locker):
     p = subprocess.Popen(["fs", "whichcell", getLockerPath(locker)], 
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -81,18 +76,18 @@ def notLockerOwner(user, locker):
         return str(e)
 
     for entry in values:
-        if entry == user or (entry[0:6] == "system" and 
-                                checkAfsGroup(user, entry, cell)):
+        if entry == user or (entry[0:6] == "system" and
+                                user in getAfsGroupMembers(entry, cell)):
             return False
     return "You don't have admin bits on " + getLockerPath(locker)
 
 
 if __name__ == "__main__":
 #    print list(getldapgroups("tabbott"))
-    print checkAfsGroup("tabbott", "system:debathena", 'athena.mit.edu')
-    print checkAfsGroup("tabbott", "system:debathena", 'sipb.mit.edu')
-    print checkAfsGroup("tabbott", "system:debathena-root", 'athena.mit.edu')
-    print checkAfsGroup("tabbott", "system:hmmt-request", 'athena.mit.edu')
+    print "tabbott" in getAfsGroupMembers("system:debathena", 'athena.mit.edu')
+    print "tabbott" in getAfsGroupMembers("system:debathena", 'sipb.mit.edu')
+    print "tabbott" in getAfsGroupMembers("system:debathena-root", 'athena.mit.edu')
+    print "tabbott" in getAfsGroupMembers("system:hmmt-request", 'athena.mit.edu')
     print notLockerOwner("tabbott", "tabbott")
     print notLockerOwner("tabbott", "debathena")
     print notLockerOwner("tabbott", "sipb")
diff --git a/code/validation.py b/code/validation.py
index 9189764..4886638 100644
--- a/code/validation.py
+++ b/code/validation.py
@@ -158,9 +158,12 @@ def testAdmin(user, admin, machine):
         if cache_acls.isUser(admin):
             return admin
         admin = 'system:' + admin
-    if getafsgroups.checkAfsGroup(user, admin, 'athena.mit.edu'):
-        return admin
-    #XXX Should we require that user is in cache_acls.expandName(admin)?
+    try:
+        if user in getafsgroups.getAfsGroupMembers(admin, 'athena.mit.edu'):
+            return admin
+    except getafsgroups.AfsProcessError, e:
+        raise InvalidInput('administrator', admin, str(e))
+    #XXX Should we require that user is in the admin group?
     return admin
     
 def testOwner(user, owner, machine=None):
-- 
1.7.9.5