more unit tests and integration tests

Signed-off-by: Pierre Tardy <tardyp@gmail.com>

master/buildbot/data/forceschedulers.py

@@ -39,6 +39,7 @@ class ForceSchedulerEndpoint(base.Endpoint):
     """
 
     def findForceScheduler(self, schedulername):
+        # eventually this may be db backed. This is why the API is async
         for sched in self.master.allSchedulers():
             if sched.name == schedulername and isinstance(sched, forcesched.ForceScheduler):
                 return defer.succeed(sched)
@@ -52,7 +53,7 @@ def get(self, resultSpec, kwargs):
     @defer.inlineCallbacks
     def control(self, action, args, kwargs):
         if action == "force":
-            sched = self.findForceScheduler(kwargs['schedulername'])
+            sched = yield self.findForceScheduler(kwargs['schedulername'])
             try:
                 res = yield sched.force("user", **args)
                 defer.returnValue(res)

master/buildbot/test/unit/test_www_authz.py

@@ -16,6 +16,7 @@
 from buildbot.test.util import www
 from buildbot.www import authz
 from twisted.trial import unittest
+from twisted.internet import defer
 from buildbot.test.fake import fakedb
 from buildbot.www.authz.roles import RolesFromGroups, RolesFromEmails, RolesFromOwner
 from buildbot.www.authz.endpointmatchers import AnyEndpointMatcher
@@ -28,7 +29,7 @@
 class Authz(www.WwwTestMixin, unittest.TestCase):
     def setUp(self):
         authzcfg = authz.Authz(
-            stringsMatcher=authz.Authz.fnmatchStrMatcher,  # simple matcher with '*' glob character
+            stringsMatcher=authz.fnmatchStrMatcher,  # simple matcher with '*' glob character
             # stringsMatcher = authz.Authz.reStrMatcher,  # if you prefer regular expressions
             allowRules=[
                 # admins can do anything,
@@ -87,24 +88,28 @@ def setUp(self):
                          buildrequestid=82, number=5),
         ])
 
-    def assertUserAllowed(self, ep, action, user):
-        ret = self.authz.isUserAllowed(ep.split("/"), action, self.users[user])
-        self.assertEqual(ret, True)
+    def assertUserAllowed(self, ep, action, options, user):
+        return self.authz.assertUserAllowed(tuple(ep.split("/")), action, options, self.users[user])
 
-    def assertUserForbidden(self, ep, action, user):
-        ret = self.authz.isUserAllowed(ep.split("/"), action, self.users[user])
-        self.assertEqual(ret, False)
+    @defer.inlineCallbacks
+    def assertUserForbidden(self, ep, action, options, user):
+        try:
+            yield self.authz.assertUserAllowed(tuple(ep.split("/")), action, options, self.users[user])
+        except authz.Forbidden, e:
+            self.assertIn("need to have role", repr(e))
 
+    @defer.inlineCallbacks
     def test_anyEndpoint(self):
-        self.assertUserAllowed("foo/bar", "get", "homer")
-        self.assertUserForbidden("foo/bar", "get", "bond")
+        yield self.assertUserAllowed("foo/bar", "get", {}, "homer")
+        yield self.assertUserForbidden("foo/bar", "get", {}, "bond")
 
+    @defer.inlineCallbacks
     def test_stopBuild(self):
         # admin can always stop
-        self.assertUserAllowed("builds/13", "stop", "homer")
+        yield self.assertUserAllowed("builds/13", "stop", {}, "homer")
         # owner can always stop
-        self.assertUserAllowed("buildrequests/82", "stop", "nineuser")
-        self.assertUserAllowed("buildrequests/82", "stop", "nineuser")
+        yield self.assertUserAllowed("builds/13", "stop", {}, "nineuser")
+        yield self.assertUserAllowed("buildrequests/82", "stop", {}, "nineuser")
         # not owner cannot stop
-        self.assertUserForbidden("buildrequests/82", "stop", "eightuser")
-        self.assertUserForbidden("buildrequests/82", "stop", "eightuser")
+        yield self.assertUserForbidden("builds/13", "stop", {}, "eightuser")
+        yield self.assertUserForbidden("buildrequests/82", "stop", {}, "eightuser")

master/buildbot/test/unit/test_www_roles.py

@@ -8,20 +8,19 @@ def setUp(self):
 
     def test_noGroups(self):
         ret = self.roles.getRolesFromUser(dict(
-            user="homer"), False)
+            user="homer"))
         self.assertEqual(ret, [])
 
     def test_noBuildbotGroups(self):
         ret = self.roles.getRolesFromUser(dict(
             user="homer",
-            groups=["employee"]), None)
+            groups=["employee"]))
         self.assertEqual(ret, [])
 
     def test_someBuildbotGroups(self):
         ret = self.roles.getRolesFromUser(dict(
             user="homer",
-            groups=["employee", "buildbot-maintainer", "buildbot-admin"]),
-            None)
+            groups=["employee", "buildbot-maintainer", "buildbot-admin"]))
         self.assertEqual(ret, ["maintainer", "admin"])
 
 
@@ -31,17 +30,17 @@ def setUp(self):
 
     def test_noUser(self):
         ret = self.roles.getRolesFromUser(dict(
-            user="lisa", email="lisa@school.com"), None)
+            user="lisa", email="lisa@school.com"))
         self.assertEqual(ret, [])
 
     def test_User1(self):
         ret = self.roles.getRolesFromUser(dict(
-            user="homer", email="homer@plant.com"), None)
+            user="homer", email="homer@plant.com"))
         self.assertEqual(ret, ["employee"])
 
     def test_User2(self):
         ret = self.roles.getRolesFromUser(dict(
-            user="burns", email="burns@plant.com"), None)
+            user="burns", email="burns@plant.com"))
         self.assertEqual(sorted(ret), ["boss", "employee"])
 
 

master/buildbot/test/util/www.py

@@ -126,7 +126,7 @@ def make_master(self, url=None, **kwargs):
             master.config.buildbotURL = url
         self.master.session = FakeSession()
         self.master.authz = cfg["authz"]
-        self.master.authz.master = self.master
+        self.master.authz.setMaster(self.master)
         return master
 
     def make_request(self, path=None, method='GET'):

master/buildbot/www/authz/__init__.py

@@ -1,2 +1,5 @@
 from buildbot.www.authz.authz import Authz
-__all__ = ["Authz"]
+from buildbot.www.authz.authz import fnmatchStrMatcher
+from buildbot.www.authz.authz import reStrMatcher
+from buildbot.www.authz.authz import Forbidden
+__all__ = ["Authz", "fnmatchStrMatcher", "reStrMatcher", "Forbidden"]

master/buildbot/www/authz/authz.py

@@ -1,6 +1,17 @@
 import fnmatch
 import re
 
+from twisted.internet import defer
+from twisted.web.error import Error
+from roles import RolesFromOwner
+from buildbot.interfaces import IConfigured
+from zope.interface import implements
+
+
+class Forbidden(Error):
+    def __init__(self, msg):
+        Error.__init__(self, 403, msg)
+
 
 # fnmatch and re.match are reversed API, we cannot just rename them
 def fnmatchStrMatcher(value, match):
@@ -12,6 +23,10 @@ def reStrMatcher(value, match):
 
 
 class Authz(object):
+    implements(IConfigured)
+
+    def getConfigDict(self):
+        return {}
 
     def __init__(self, allowRules=None, roleMatchers=None, stringsMatcher=fnmatchStrMatcher):
         self.match = stringsMatcher
@@ -20,7 +35,36 @@ def __init__(self, allowRules=None, roleMatchers=None, stringsMatcher=fnmatchStr
         if roleMatchers is None:
             roleMatchers = []
         self.allowRules = allowRules
-        self.roleMatchers = roleMatchers
+        self.roleMatchers = [r for r in roleMatchers if not isinstance(r, RolesFromOwner)]
+        self.ownerRoleMatchers = [r for r in roleMatchers if isinstance(r, RolesFromOwner)]
+
+    def setMaster(self, master):
+        self.master = master
+        for r in self.roleMatchers + self.ownerRoleMatchers + self.allowRules:
+            r.setAuthz(self)
+
+    def getRolesFromUser(self, userDetails):
+        roles = set()
+        for roleMatcher in self.roleMatchers:
+            roles.update(set(roleMatcher.getRolesFromUser(userDetails)))
+        return roles
+
+    def getOwnerRolesFromUser(self, userDetails, owner):
+        roles = set()
+        for roleMatcher in self.ownerRoleMatchers:
+            roles.update(set(roleMatcher.getRolesFromUser(userDetails, owner)))
+        return roles
 
-    def isUserAllowed(self, ep, action, userDetails):
-        return True
+    @defer.inlineCallbacks
+    def assertUserAllowed(self, ep, action, options, userDetails):
+        roles = self.getRolesFromUser(userDetails)
+        for rule in self.allowRules:
+            match = yield rule.match(ep, action, options)
+            if match:
+                roles.update(self.getOwnerRolesFromUser(userDetails, rule.owner))
+                if rule.role not in roles:
+                    if rule.defaultDeny:
+                        raise Forbidden("you need to have role '%s'" % (rule.role,))
+                elif not rule.defaultDeny:
+                    defer.returnValue(None)
+        defer.returnValue(None)

master/buildbot/www/authz/endpointmatchers.py

@@ -22,6 +22,7 @@ class EndpointMatcherBase(object):
     def __init__(self, role, defaultDeny=True):
         self.role = role
         self.defaultDeny = defaultDeny
+        self.owner = None
 
     def setAuthz(self, authz):
         self.authz = authz
@@ -43,23 +44,20 @@ def match(self, ep, action="get", options=None):
             return defer.succeed(False)
         return defer.succeed(False)
 
+    def __repr__(self):
+        # a repr for debugging. displays the class, and string attributes
+        args = []
+        for k, v in self.__dict__.items():
+            if isinstance(v, basestring):
+                args.append("%s='%s'" % (k, v))
+        return "%s(%s)" % (self.__class__.__name__, ", ".join(args))
+
 
 class AnyEndpointMatcher(EndpointMatcherBase):
     def __init__(self, **kwargs):
         EndpointMatcherBase.__init__(self, **kwargs)
 
-    def match(self, ep, action="get"):
-        return defer.succeed(True)
-
-
-class ViewBuildsEndpointMatcher(EndpointMatcherBase):
-    def __init__(self, branch=None, project=None, builder=None, **kwargs):
-        EndpointMatcherBase.__init__(self, **kwargs)
-        self.branch = branch
-        self.project = project
-        self.builder = builder
-
-    def match_BuildEndpoint_get(self, epobject, epdict, options):
+    def match(self, ep, action="get", options=None):
         return defer.succeed(True)
 
 
@@ -69,25 +67,46 @@ def __init__(self, builder=None, **kwargs):
         EndpointMatcherBase.__init__(self, **kwargs)
 
     @defer.inlineCallbacks
-    def match_BuildEndpoint_stop(self, epobject, epdict, options):
-        if self.builder is None:
-            # no filtering needed: we match!
-            defer.returnValue(True)
-        # if filtering needed, we need to get some more info
-        build = yield epobject.get({}, epdict)
-        if build['builderid'] is not None:
-            builder = yield self.master.data.get(('builders', build['builderid']))
+    def matchFromBuilderId(self, builderid):
+        if builderid is not None:
+            builder = yield self.master.data.get(('builders', builderid))
             buildername = builder['name']
             defer.returnValue(self.authz.match(buildername, self.builder))
         defer.returnValue(False)
 
-    def match_BuildRequestEndpoint_stop(self, epobject, epdict):
-        return defer.succeed(True)
+    @defer.inlineCallbacks
+    def getOwnerFromBuild(self, build):
+        br = yield self.master.data.get(("buildrequests", build['buildrequestid']))
+        owner = yield self.getOwnerFromBuildRequest(br)
+        defer.returnValue(owner)
 
+    @defer.inlineCallbacks
+    def getOwnerFromBuildRequest(self, buildrequest):
+        props = yield self.master.data.get(("buildsets", buildrequest['buildsetid'], "properties"))
+        if 'owner' in props:
+            defer.returnValue(props['owner'][0])
+        defer.returnValue(None)
 
-class BranchEndpointMatcher(EndpointMatcherBase):
-    def __init__(self, **kwargs):
-        EndpointMatcherBase.__init__(self, **kwargs)
+    @defer.inlineCallbacks
+    def match_BuildEndpoint_stop(self, epobject, epdict, options):
+        build = yield epobject.get({}, epdict)
+        self.owner = yield self.getOwnerFromBuild(build)
+        if self.builder is None:
+            # no filtering needed: we match!
+            defer.returnValue(True)
+        # if filtering needed, we need to get some more info
+        ret = yield self.matchFromBuilderId(build['builderid'])
+        defer.returnValue(ret)
+
+    def match_BuildRequestEndpoint_stop(self, epobject, epdict, options):
+        buildrequest = yield epobject.get({}, epdict)
+        self.owner = yield self.getOwnerFromBuildRequest(build)
+        if self.builder is None:
+            # no filtering needed: we match!
+            defer.returnValue(True)
+        # if filtering needed, we need to get some more info
+        ret = yield self.matchFromBuilderId(buildrequest['builderid'])
+        defer.returnValue(ret)
 
 
 class ForceBuildEndpointMatcher(EndpointMatcherBase):
@@ -98,7 +117,7 @@ def __init__(self, builder=None, **kwargs):
     @defer.inlineCallbacks
     def match_ForceSchedulerEndpoint_force(self, epobject, epdict, options):
         if self.builder is None:
-            # no filtering needed: we match!
+            # no filtering needed: we match without querying!
             defer.returnValue(True)
         sched = yield epobject.findForceScheduler(epdict['schedulername'])
         if sched is not None:
@@ -109,3 +128,26 @@ def match_ForceSchedulerEndpoint_force(self, epobject, epdict, options):
                 if self.authz.match(buildername, self.builder):
                     defer.returnValue(True)
         defer.returnValue(False)
+
+#####
+# not yet implemented
+
+
+class ViewBuildsEndpointMatcher(EndpointMatcherBase):
+    def __init__(self, branch=None, project=None, builder=None, **kwargs):
+        EndpointMatcherBase.__init__(self, **kwargs)
+        self.branch = branch
+        self.project = project
+        self.builder = builder
+
+    def match_BuildEndpoint_get(self, epobject, epdict, options):
+        return defer.succeed(True)
+
+
+class BranchEndpointMatcher(EndpointMatcherBase):
+    def __init__(self, branch, **kwargs):
+        self.branch = branch
+        EndpointMatcherBase.__init__(self, **kwargs)
+
+    def match_BuildEndpoint_get(self, epobject, epdict, options):
+        return defer.succeed(True)

master/buildbot/www/authz/roles.py

@@ -18,16 +18,20 @@ class RolesFromBase(object):
     def __init__(self):
         pass
 
-    def getRolesFromUser(self, userDetails, owner):
+    def getRolesFromUser(self, userDetails):
         return []
 
+    def setAuthz(self, authz):
+        self.authz = authz
+        self.master = authz.master
+
 
 class RolesFromGroups(RolesFromBase):
     def __init__(self, groupPrefix=""):
         RolesFromBase.__init__(self)
         self.groupPrefix = groupPrefix
 
-    def getRolesFromUser(self, userDetails, owner):
+    def getRolesFromUser(self, userDetails):
         roles = []
         if 'groups' in userDetails:
             for group in userDetails['groups']:
@@ -44,7 +48,7 @@ def __init__(self, **kwargs):
             for email in emails:
                 self.roles.setdefault(email, []).append(role)
 
-    def getRolesFromUser(self, userDetails, owner):
+    def getRolesFromUser(self, userDetails):
         if 'email' in userDetails:
             return self.roles.get(userDetails['email'], [])
         return []