Fixes to authz_ownership module, tests pass now.

2 files changed, 29 insertions, 20 deletions

diff --git a/cobbler/modules/authz_ownership.py b/cobbler/modules/authz_ownership.py
index 9b271f4..43cf523 100644
--- a/cobbler/modules/authz_ownership.py
+++ b/cobbler/modules/authz_ownership.py
@@ -69,7 +69,7 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
 
     # classify the type of operation
     save_or_remove = False
-    for criteria in ["save","remove","modify"]:
+    for criteria in ["save_","remove_","modify_"]:
         if resource.find(criteria) != -1:
            save_or_remove = True
 
@@ -78,15 +78,18 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
 
     found_user = False
     for g in user_groups:
-        if user in user_groups[g]:
-           found_user = True
-           # if user is in the admin group, always authorize
-           # regardless of the ownership of the object.
-           if g == "admin":
-               if debug:
-                  print "[OWNERSHIP] user % is an admin, PASS" % user
-               return 1
-           break
+        for x in user_groups[g]:
+           if debug:
+               print "[OWNERSHIP] noted user %s in group %s" % (x,g)
+           if x == user:
+               found_user = True
+               # if user is in the admin group, always authorize
+               # regardless of the ownership of the object.
+               if g == "admins" or g == "admin":
+                   if debug:
+                       print "[OWNERSHIP] user %s is an admin, PASS" % user
+                   return 1
+               break
 
     if not found_user:
         # if the user isn't anywhere in the file, reject regardless
@@ -107,6 +110,8 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
 
     obj = None
     if resource.find("remove") != -1:
+        if debug:
+           print "[OWNERSHIP] looking up object %s" % (arg1)
         if resource == "remove_distro":
            obj = api_handle.find_distro(arg1)
         elif resource == "remove_profile":
@@ -115,7 +120,9 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
            obj = api_handle.find_system(arg1)
         elif resource == "remove_repo":
            obj = api_handle.find_system(arg1)
-    else:
+    elif resource.find("save") != -1 or resource.find("modify") != -1:
+        if debug:
+           print "[OWNERSHIP] object being considered is: %s for %s" % (arg1, resource)
         obj = arg1
 
     # if the object has no ownership data, allow access regardless
@@ -132,9 +139,9 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
               print "[OWNERSHIP] user %s in match list, PASS" % user
            return 1
         for group in user_groups:
-           if user in user_groups[group]:
+           if group == allowed and user in user_groups[group]:
               if debug:
-                 print "[OWNERSHIP] user %s matched by group, PASS" % user
+                 print "[OWNERSHIP] user %s matched by group (%s), PASS" % (user, group)
               return 1
     
     # can't find user or group in ownership list and ownership is defined
@@ -145,11 +152,13 @@ def authorize(api_handle,user,resource,arg1=None,arg2=None,debug=False):
            
 
 if __name__ == "__main__":
+    # real tests are contained in tests/tests.py
     import api as cobbler_api
     api = cobbler_api.BootAPI()
     print __parse_config()
     print authorize(api, "admin1", "sync")
     d = api.find_distro("F9B-i386")
+    d.set_owners(["allowed"])
+    api.add_distro(d)
     print authorize(api, "admin1", "save_distro", d, debug=True)
-
-    # real tests are contained in tests/tests.py
+    print authorize(api, "basement2", "save_distro", d, debug=True)
diff --git a/tests/tests.py b/tests/tests.py
index 546c83f..426ddf2 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -111,9 +111,9 @@ class Ownership(BootTest):
         profile = self.api.find_profile(name="testprofile0")
         system = self.api.find_system(name="drwily.rdu.redhat.com")
         repo = self.api.find_repo(name="test_repo")
-        self.assertTrue(distro.set_owners("superlab,basement1"))
-        self.assertTrue(profile.set_owners("superlab,basement1"))
-        self.assertTrue(system.set_owners("superlab,basement1"))
+        self.assertTrue(distro.set_owners(["superlab","basement1"]))
+        self.assertTrue(profile.set_owners(["superlab","basement1"]))
+        self.assertTrue(system.set_owners(["superlab","basement1"]))
         self.assertTrue(repo.set_owners([]))
         self.api.add_distro(distro)
         self.api.add_profile(profile)
@@ -144,8 +144,8 @@ class Ownership(BootTest):
 
         xo = self.api.find_distro("testdistro0")
         xn = "testdistro0"
-        ro = self.api.find_repo("testrepo0")
-        rn = "testrepo0"
+        ro = self.api.find_repo("test_repo")
+        rn = "test_repo"
 
         # ensure admin1 can edit (he's an admin) and do other tasks
         # same applies to basement1 who is explicitly added as a user