Ticket #138: 0009-Add-a-possibility-to-password-protect-the-forms-on-t.patch

NEWS

@@ -10 +10 @@
 "_all" pseudo-builder that the web pages use to allow force-build buttons
 that start builds on all Builders at once.
 
+*** Password protected WebStatus
+The WebStatus constructor can take an instance of IAuth (from
+status.web.authentication).  The class BasicAuth accepts a `userpass' keyword
+argument in pretty much the same way as Try_Userpass does.
+Only users with a valid login/password can then force/stop builds from the
+WebStatus.
 
 * Release 0.7.6 (30 Sep 2007)
 

new file uildbot/status/web/authentication.py

@@ +1 @@
+
+from zope.interface import Interface, implements
+
+class IAuth(Interface):
+    """Represent an authentication method."""
+
+    def authenticate(self, login, password):
+        """Check whether C{login} / C{password} are valid"""
+
+    def errmsg(self):
+        """Get the last error message (reason why authentication
+           failed)."""
+
+class AuthBase:
+    err = ""
+
+    def errmsg(self):
+        return self.err
+
+class BasicAuth(AuthBase):
+    implements(IAuth)
+    """Implement a basic authentication mechanism against of list of
+       user/password."""
+
+    userpass = []
+    """List of couples (user, password)"""
+
+    def __init__(self, userpass):
+        """C{userpass} is a list of (user, password)."""
+        for user_pass_pair in userpass:
+            assert isinstance(user_pass_pair, tuple)
+            login, password = user_pass_pair
+            assert isinstance(login, str)
+            assert isinstance(password, str)
+        self.userpass = userpass
+
+    def authenticate(self, login, password):
+        """Check that C{login}/C{password} is a valid user/password pair."""
+        if not self.userpass:
+            self.err = "Invalid value for self.userpass"
+            return False
+        for l, p in self.userpass:
+            if login == l and password == p:
+                self.err = ""
+                return True
+        self.err = "Invalid login or password"
+        return False

buildbot/status/web/base.py

@@ -53 +53 @@
     label = html.escape(label)
     return ROW_TEMPLATE % {"label": label, "field": field}
 
-def make_stop_form(stopURL, on_all=False):
+def make_name_login_password_form(useLoginPassword):
+    """helper function that produces either one row of a form with a `name'
+       text input (if C{useLoginPassword} is C{False}) or two rows with a
+       `login' / `password' text input."""
+
+    if useLoginPassword:
+        user_label = "Your login:"
+    else:
+        user_label = "Your name:"
+    data = make_row(user_label,
+                    '<input type="text" name="username" />')
+    if useLoginPassword:
+        data += make_row("Your password:",
+                         '<input type="password" name="passwd" />')
+    return data
+
+def make_stop_form(stopURL, useLoginPassword, on_all=False):
+    """Create a form whose submit button sends the request to C{stopURL}.  If
+    C{useLoginPassword} is true, this form will have a password field."""
+
     if on_all:
-        data = """<form action="%s" class='command stopbuild'>
-          <p>To stop all builds, fill out the following fields and
-          push the 'Stop' button</p>\n""" % stopURL
+        what = "all builds"
     else:
-        data = """<form action="%s" class='command stopbuild'>
-          <p>To stop this build, fill out the following fields and
-          push the 'Stop' button</p>\n""" % stopURL
-    data += make_row("Your name:",
-                     "<input type='text' name='username' />")
+        what = "this build"
+    data = """<form action="%s" class="command stopbuild">
+      <p>To stop %s, fill out the following fields and
+      click the `Stop' button</p>\n""" % (stopURL, what)
+    data += make_name_login_password_form(useLoginPassword)
+
     data += make_row("Reason for stopping build:",
-                     "<input type='text' name='comments' />")
+                     '<input type="text" name="comments" />')
     data += '<input type="submit" value="Stop Builder" /></form>\n'
     return data
 
-def make_force_build_form(forceURL, on_all=False):
+def make_force_build_form(forceURL, useLoginPassword, on_all=False):
+    """Create a form whose submit button sends the request to C{forceURL}.  If
+    C{useLoginPassword} is true, this form will have a password field."""
+
     if on_all:
-        data = """<form action="%s" class="command forcebuild">
-          <p>To force a build on all Builders, fill out the following fields
-          and push the 'Force Build' button</p>""" % forceURL
+        where = " on all builders"
     else:
-        data = """<form action="%s" class="command forcebuild">
-          <p>To force a build, fill out the following fields and
-          push the 'Force Build' button</p>""" % forceURL
+        where = ""
+    data = """<form action="%s" class="command forcebuild">
+      <p>To force a build%s, fill out the following fields and
+      click the `Force Build' button</p>""" % (forceURL, where)
     return (data
-      + make_row("Your name:",
-                 "<input type='text' name='username' />")
+      + make_name_login_password_form(useLoginPassword)
       + make_row("Reason for build:",
-                 "<input type='text' name='comments' />")
+                 '<input type="text" name="comments" />')
       + make_row("Branch to build:",
-                 "<input type='text' name='branch' />")
+                 '<input type="text" name="branch" />')
       + make_row("Revision to build:",
-                 "<input type='text' name='revision' />")
+                 '<input type="text" name="revision" />')
       + '<input type="submit" value="Force Build" /></form>\n')
 
 colormap = {
@@ -254 +273 @@
 
     def getStatus(self, request):
         return request.site.buildbot_service.getStatus()
+
     def getControl(self, request):
         return request.site.buildbot_service.getControl()
 
+    def isUsingLoginPassword(self, request):
+        return request.site.buildbot_service.isUsingLoginPassword()
+
+    def authUser(self, request):
+        login = request.args.get("username", ["<unknown>"])[0]
+        password = request.args.get("passwd", ["<no-password>"])[0]
+        if login == "<unknown>" or password == "<no-password>":
+            return False
+        return request.site.buildbot_service.authUser(login, password)
+
     def getChangemaster(self, request):
         return request.site.buildbot_service.parent.change_svc
 

buildbot/status/web/baseweb.py

@@ -19 +19 @@
 from buildbot.status.web.slaves import BuildSlavesResource
 from buildbot.status.web.xmlrpc import XMLRPCServer
 from buildbot.status.web.about import AboutBuildbot
+from buildbot.status.web.authentication import IAuth
 
 # this class contains the status services (WebStatus and the older Waterfall)
 # which can be put in c['status']. It also contains some of the resources
@@ -123 +124 @@
 
         if building:
             stopURL = "builders/_all/stop"
-            data += make_stop_form(stopURL, True)
+            data += make_stop_form(stopURL, self.isUsingLoginPassword(req),
+                                   True)
         if online:
             forceURL = "builders/_all/force"
-            data += make_force_build_form(forceURL, True)
+            data += make_force_build_form(forceURL,
+                                          self.isUsingLoginPassword(req),
+                                          True)
 
         return data
 
@@ -232 +236 @@
 
         if building:
             stopURL = "builders/_all/stop"
-            data += make_stop_form(stopURL, True)
+            data += make_stop_form(stopURL, self.isUsingLoginPassword(req),
+                                   True)
         if online:
             forceURL = "builders/_all/force"
-            data += make_force_build_form(forceURL, True)
+            data += make_force_build_form(forceURL,
+                                          self.isUsingLoginPassword(req),
+                                          True)
 
         return data
 
@@ -350 +357 @@
     # not (we'd have to do a recursive traversal of all children to discover
     # all the changes).
 
-    def __init__(self, http_port=None, distrib_port=None, allowForce=False):
+    def __init__(self, http_port=None, distrib_port=None, allowForce=False,
+                 auth=None):
         """Run a web server that provides Buildbot status.
 
         @type  http_port: int or L{twisted.application.strports} string
@@ -385 +393 @@
                              the strports parser.
         @param allowForce: boolean, if True then the webserver will allow
                            visitors to trigger and cancel builds
+        @type  auth: a L{status.web.authentication.IAuth} or C{None}
+        @param auth: an object that performs authentication to restrain
+                     access to the C{allowForce} features.  Ignored if
+                     C{allowForce} is not True.  If C{auth} is C{None}, the
+                     legacy behavior is used: people can force/stop builds
+                     without auth.
         """
 
         service.MultiService.__init__(self)
@@ -398 +412 @@
                 distrib_port = "unix:%s" % distrib_port
         self.distrib_port = distrib_port
         self.allowForce = allowForce
+        if allowForce and auth:
+            assert IAuth.providedBy(auth)
+            self.auth = auth
+        else:
+            if auth:
+                log.msg("warning: discarding your authentication method: you"
+                        "must also set allowForce to True to use one.")
+            self.auth = None
 
         # this will be replaced once we've been attached to a parent (and
         # thus have a basedir and can reference BASEDIR/public_html/)
@@ -475 +497 @@
         self.site.resource = root
 
     def putChild(self, name, child_resource):
-        """This behaves a lot like root.putChild() . """
+        """This behaves a lot like root.putChild() ."""
         self.childrenToBeAdded[name] = child_resource
 
     def registerChannel(self, channel):
@@ -493 +515 @@
 
     def getStatus(self):
         return self.parent.getStatus()
+
     def getControl(self):
         if self.allowForce:
             return IControl(self.parent)
         return None
 
+    def isUsingLoginPassword(self):
+        """Return a boolean to indicate whether or not this WebStatus uses a
+           list of login/passwords for privileged actions."""
+        if self.auth:
+            return True
+        return False
+
+    def authUser(self, login, password):
+        """Check that login/password is a valid user/password pair and can be
+           allowed to perform a privileged action.  If this WebStatus is not
+           password protected, this function returns False (conservative
+           approach)."""
+        # Do not mess up this function, it's critical for the security of the
+        # WebStatus
+        if not self.isUsingLoginPassword():
+            return False
+        if self.auth.authenticate(login, password):
+            return True
+        log.msg("Authentication failed for `%s': %s" % (login,
+                                                        self.auth.errmsg()))
+        return False
+
     def getPortnum(self):
         # this is for the benefit of unit tests
         s = list(self)[0]

buildbot/status/web/build.py

@@ -48 +48 @@
 
             if self.build_control is not None:
                 stopURL = urllib.quote(req.childLink("stop"))
-                data += make_stop_form(stopURL)
+                data += make_stop_form(stopURL, self.isUsingLoginPassword(req))
 
         if b.isFinished():
             results = b.getResults()
@@ -166 +166 @@
         return data
 
     def stop(self, req):
+        if self.isUsingLoginPassword(req):
+            if not self.authUser(req):
+                # TODO: tell the web user that their request was denied
+                return Redirect("..")
+
         b = self.build_status
         c = self.build_control
         log.msg("web stopBuild of build %s:%s" % \

buildbot/status/web/builder.py

@@ -107 +107 @@
 
         if control is not None and connected_slaves:
             forceURL = urllib.quote(req.childLink("force"))
-            data += make_force_build_form(forceURL)
+            data += make_force_build_form(forceURL,
+                                          self.isUsingLoginPassword(req))
         elif control is not None:
             data += """
             <p>All buildslaves appear to be offline, so it's not possible
@@ -135 +136 @@
         r = "The web-page 'force build' button was pressed by '%s': %s\n" \
             % (name, reason)
         log.msg("web forcebuild of builder '%s', branch='%s', revision='%s'"
-                % (self.builder_status.getName(), branch, revision))
+                "by user '%s'" % (self.builder_status.getName(), branch,
+                                  revision, name))
 
         if not self.builder_control:
             # TODO: tell the web user that their request was denied
             log.msg("but builder control is disabled")
             return Redirect("..")
 
+        if self.isUsingLoginPassword(req):
+            if not self.authUser(req):
+                # TODO: tell the web user that their request was denied
+                return Redirect("..")
+
         # keep weird stuff out of the branch and revision strings. TODO:
         # centralize this somewhere.
         if not re.match(r'^[\w\.\-\/]*$', branch):
@@ -155 +162 @@
         if not revision:
             revision = None
 
-        # TODO: if we can authenticate that a particular User pushed the
-        # button, use their name instead of None, so they'll be informed of
-        # the results.
+        # TODO: we can authenticate that a particular User clicked the
+        # button, so we could use their name instead of None, so they'll be
+        # informed of the results.  The problem is that we must create a
+        # buildbot.changes.changes.Change instance which doesn't really fit
+        # this use case (it requires a list of changed files which is tedious
+        # to compute at this stage)
         s = SourceStamp(branch=branch, revision=revision)
         req = BuildRequest(r, s, self.builder_status.getName())
         try:

docs/buildbot.texinfo

@@ -6120 +6120 @@
 allows visitors to manually trigger builds. This is useful for
 developers to re-run builds that have failed because of intermittent
 problems in the test suite, or because of libraries that were not
-installed at the time of the previous build. You may not wish to allow
-strangers to cause a build to run: in that case, set this to False to
-remove these buttons. The default value is False.
+installed at the time of the previous build.  The default value is False.
 
+You may not wish to allow strangers to cause a build to run or to stop
+current builds, in that case you can pass an instance of
+@code{status.web.authentication.IAuth} as a @code{auth} keyword argument.
+The class @code{BasicAuth} implements a basic authentication mechanism
+using a list of login/password pairs provided from the configuration file.
+
+@example
+from buildbot.status.html import WebStatus
+from buildbot.status.web.authentication import BasicAuth
+users = [('login', 'password'), ('bob', 'secret-pass')]
+c['status'].append(WebStatus(http_port=8080, auth=BasicAuth(users)))
+@end example
 
 
 @node Buildbot Web Resources, XMLRPC server, WebStatus Configuration Parameters, WebStatus