From 9b26de39356f7d7f771d1bdbbfd017e517849b60 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Tue, 13 Nov 2007 18:55:52 +0100 Subject: [PATCH v4 09/12] Add a possibility to password-protect the forms on the WebStatus. It is now possible to instantiate the WebStatus with a list of login/password. Only users with a valid login/password can force/stop builds. * NEWS: Mention the new feature. * buildbot/status/web/authentication.py: New file. (IAuth): New interface. (AuthBase): New base class. (BasicAuth): Implement IAuth. * buildbot/status/web/base.py (make_name_login_password_form): New helper to factor some common code. (make_stop_form, make_force_build_form): Use it. Accept a 2nd mandatory boolean argument. (HtmlResource.isUsingLoginPassword, HtmlResource.authUser): New dispatch methods. * buildbot/status/web/baseweb.py (OneLinePerBuild.body), (OneBoxPerBuilder.body): Adjust. (WebStatus.__init__): Accept a new `auth' keyword argument. (WebStatus.isUsingLoginPassword, WebStatus.authUser): New. * buildbot/status/web/build.py (StatusResourceBuild.body): Adjust. (StatusResourceBuild.stop): Check the credentials of the user, if needed. * buildbot/status/web/builder.py (StatusResourceBuilder.body), (StatusResourceBuilder.force): Likewise. * docs/buildbot.texinfo (WebStatus Configuration Parameters): Document the new `userpass' argument of WebStatus. Signed-off-by: Benoit Sigoure --- NEWS | 6 +++ buildbot/status/web/authentication.py | 47 +++++++++++++++++++++ buildbot/status/web/base.py | 74 +++++++++++++++++++++++---------- buildbot/status/web/baseweb.py | 57 ++++++++++++++++++++++--- buildbot/status/web/build.py | 7 +++- buildbot/status/web/builder.py | 20 +++++++-- docs/buildbot.texinfo | 16 ++++++- 7 files changed, 190 insertions(+), 37 deletions(-) create mode 100644 buildbot/status/web/authentication.py diff --git a/NEWS b/NEWS index 26a01eb..7fdcd11 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,12 @@ These are now reserved for internal buildbot purposes, such as the magic "_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) diff --git a/buildbot/status/web/authentication.py b/buildbot/status/web/authentication.py new file mode 100644 index 0000000..c8c6875 --- /dev/null +++ b/buildbot/status/web/authentication.py @@ -0,0 +1,47 @@ + +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 diff --git a/buildbot/status/web/base.py b/buildbot/status/web/base.py index b5e8065..7b6e5e8 100644 --- a/buildbot/status/web/base.py +++ b/buildbot/status/web/base.py @@ -53,40 +53,59 @@ def make_row(label, field): 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, + '') + if useLoginPassword: + data += make_row("Your password:", + '') + 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 = """
-

To stop all builds, fill out the following fields and - push the 'Stop' button

\n""" % stopURL + what = "all builds" else: - data = """ -

To stop this build, fill out the following fields and - push the 'Stop' button

\n""" % stopURL - data += make_row("Your name:", - "") + what = "this build" + data = """ +

To stop %s, fill out the following fields and + click the `Stop' button

\n""" % (stopURL, what) + data += make_name_login_password_form(useLoginPassword) + data += make_row("Reason for stopping build:", - "") + '') data += '
\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 = """
-

To force a build on all Builders, fill out the following fields - and push the 'Force Build' button

""" % forceURL + where = " on all builders" else: - data = """ -

To force a build, fill out the following fields and - push the 'Force Build' button

""" % forceURL + where = "" + data = """ +

To force a build%s, fill out the following fields and + click the `Force Build' button

""" % (forceURL, where) return (data - + make_row("Your name:", - "") + + make_name_login_password_form(useLoginPassword) + make_row("Reason for build:", - "") + '') + make_row("Branch to build:", - "") + '') + make_row("Revision to build:", - "") + '') + '
\n') colormap = { @@ -254,9 +273,20 @@ class HtmlResource(resource.Resource): 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", [""])[0] + password = request.args.get("passwd", [""])[0] + if login == "" or password == "": + return False + return request.site.buildbot_service.authUser(login, password) + def getChangemaster(self, request): return request.site.buildbot_service.parent.change_svc diff --git a/buildbot/status/web/baseweb.py b/buildbot/status/web/baseweb.py index 8682735..66b1022 100644 --- a/buildbot/status/web/baseweb.py +++ b/buildbot/status/web/baseweb.py @@ -19,6 +19,7 @@ from buildbot.status.web.builder import BuildersResource 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,10 +124,13 @@ class OneLinePerBuild(HtmlResource, OneLineMixin): 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,10 +236,13 @@ class OneBoxPerBuilder(HtmlResource): 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,7 +357,8 @@ class WebStatus(service.MultiService): # 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,6 +393,12 @@ class WebStatus(service.MultiService): 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,6 +412,14 @@ class WebStatus(service.MultiService): 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,7 +497,7 @@ class WebStatus(service.MultiService): 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,11 +515,34 @@ class WebStatus(service.MultiService): 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] diff --git a/buildbot/status/web/build.py b/buildbot/status/web/build.py index 2522b92..432b282 100644 --- a/buildbot/status/web/build.py +++ b/buildbot/status/web/build.py @@ -48,7 +48,7 @@ class StatusResourceBuild(HtmlResource): 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,6 +166,11 @@ class StatusResourceBuild(HtmlResource): 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" % \ diff --git a/buildbot/status/web/builder.py b/buildbot/status/web/builder.py index 1851dd1..fbc8fb5 100644 --- a/buildbot/status/web/builder.py +++ b/buildbot/status/web/builder.py @@ -107,7 +107,8 @@ class StatusResourceBuilder(HtmlResource, OneLineMixin): 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 += """

All buildslaves appear to be offline, so it's not possible @@ -135,13 +136,19 @@ class StatusResourceBuilder(HtmlResource, OneLineMixin): 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,9 +162,12 @@ class StatusResourceBuilder(HtmlResource, OneLineMixin): 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: diff --git a/docs/buildbot.texinfo b/docs/buildbot.texinfo index 5c6ca17..278cf4a 100644 --- a/docs/buildbot.texinfo +++ b/docs/buildbot.texinfo @@ -6120,10 +6120,20 @@ True, then the web page will provide a ``Force Build'' button that 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 -- 1.5.3.5.737.gdee1b