Now ignore negative rights, rather than treat them as positive.

Also, rearrange and clean up code.

svn path=/trunk/web/; revision=234

diff --git a/templates/getafsgroups.py b/templates/getafsgroups.py
index 2086ccb..2de20bc 100644 (file)
--- a/templates/getafsgroups.py
+++ b/templates/getafsgroups.py
@@ -24,52 +24,62 @@ import subprocess
 #             return True
 #     return False
 
-def checkAfsGroup(user, group, cell):
-    """
-    checkAfsGroup(user, group) returns True if and only if user is in AFS group group in cell cell
-    """
+class MyException(Exception):
+    pass
+
+def getAfsGroupMembers(group, cell):
     p = subprocess.Popen(["pts", "membership", group, '-c', cell], 
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     if p.wait():
-        return False
-    for line in p.stdout.readlines()[1:]:
-        if line.strip() == user:
-            return True
-    return False
+        return []
+    return [line.strip() for line in p.stdout.readlines()[1:]]
 
-def checkLockerOwner(user, locker, verbose=False):
+def checkAfsGroup(user, group, cell):
     """
-    checkLockerOwner(user, locker) returns True if and only if user administers locker.
-
-    If verbose is true, instead return the reason for failure, or None
-    if there is no failure.
+    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", "/mit/" + locker], 
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     if p.wait():
-        if verbose:
-            return p.stderr.read()
-        return False
-    cell = p.stdout.read().split()[-1][1:-1]
+        raise MyException(p.stderr.read())
+    return p.stdout.read().split()[-1][1:-1]
+
+def getLockerAcl(locker):
     p = subprocess.Popen(["fs", "listacl", "/mit/" + locker], 
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     if p.wait():
-        if verbose:
-            return p.stderr.read()
-        return False
-    for line in p.stdout.readlines()[1:]:
-        entry = line.split()
-        if not entry or entry[0] == "Negative":
+        raise MyException(p.stderr.read())
+    lines = p.stdout.readlines()
+    values = []
+    for line in lines[1:]:
+        fields = line.split()
+        if fields[0] == 'Negative':
             break
-        if entry[1] == "rlidwka":
-            if entry[0] == user or (entry[0][0:6] == "system" and 
-                                    checkAfsGroup(user, entry[0], cell)):
-                if verbose:
-                    return None
-                return True
-    if verbose:
-        return "You don't have admin bits on /mit/" + locker
-    return False
+        if 'rlidwka' in fields[1]:
+            values.append(fields[0])
+    return values
+
+def notLockerOwner(user, locker):
+    """
+    notLockerOwner(user, locker) returns false if and only if user administers locker.
+
+    If the user does not own the locker, returns the string reason for
+    the failure.
+    """
+    try:
+        cell = getCell(locker)
+        values = getLockerAcl(locker)
+    except MyException, e:
+        return str(e)
+
+    for entry in values:
+        if entry[0] == user or (entry[0][0:6] == "system" and 
+                                checkAfsGroup(user, entry[0], cell)):
+            return False
+    return "You don't have admin bits on /mit/" + locker
 
 
 if __name__ == "__main__":
@@ -78,9 +88,9 @@ if __name__ == "__main__":
     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 checkLockerOwner("tabbott", "tabbott")
-    print checkLockerOwner("tabbott", "debathena")
-    print checkLockerOwner("tabbott", "sipb")
-    print checkLockerOwner("tabbott", "lsc")
-    print checkLockerOwner("tabbott", "scripts")
-    print checkLockerOwner("ecprice", "hmmt")
+    print notLockerOwner("tabbott", "tabbott")
+    print notLockerOwner("tabbott", "debathena")
+    print notLockerOwner("tabbott", "sipb")
+    print notLockerOwner("tabbott", "lsc")
+    print notLockerOwner("tabbott", "scripts")
+    print notLockerOwner("ecprice", "hmmt")

diff --git a/templates/list.tmpl b/templates/list.tmpl
index d6c0e00..19b4c6e 100644 (file)
--- a/templates/list.tmpl
+++ b/templates/list.tmpl
@@ -67,6 +67,7 @@ $errorRow('cdrom', $err)
        <td><a href="info?machine_id=$machine.machine_id">$machine.name</a></td>
        <td>${machine.memory}M</td>
        <td>$machine.owner</td>
+       <td>$machine.administrator</td>
 #if $machine.nics
 #set $nic = $machine.nics[0]
        <td>$nic.ip</td>
@@ -106,6 +107,7 @@ $has_vnc[$machine]
        <td>Name</td>
        <td>Memory</td>
        <td>Owner</td>
+        <td>Administrator</td>
        <td>IP</td>
        <td>Uptime</td>
        <td>VNC</td>

diff --git a/templates/validation.py b/templates/validation.py
index 4165aa9..ccb4fef 100644 (file)
--- a/templates/validation.py
+++ b/templates/validation.py
@@ -75,7 +75,7 @@ def haveAccess(user, machine):
     if getafsgroups.checkAfsGroup(user, machine.administrator, 
                                   'athena.mit.edu'): #XXX Cell?
         return True
-    if getafsgroups.checkLockerOwner(user, machine.owner):
+    if not getafsgroups.notLockerOwner(user, machine.owner):
         return True
     return owns(user, machine)
 
@@ -83,7 +83,7 @@ def owns(user, machine):
     """Return whether a user owns a machine"""
     if user == 'moo':
         return True
-    return getafsgroups.checkLockerOwner(user, machine.owner)
+    return not getafsgroups.notLockerOwner(user, machine.owner)
 
 def validMachineName(name):
     """Check that name is valid for a machine name"""
@@ -168,7 +168,7 @@ def testOwner(user, owner, machine=None):
         return owner
     if owner is None:
         raise InvalidInput('owner', owner, "Owner must be specified")
-    value = getafsgroups.checkLockerOwner(user, owner, verbose=True)
+    value = getafsgroups.notLockerOwner(user, owner)
     if not value:
         return owner
     raise InvalidInput('owner', owner, value)