[Buildbot-devel] [PATCH v2 09/12] Add a possibility to password-protect the forms on the WebStatus.
Benoit Sigoure
tsuna at lrde.epita.fr
Sun Nov 18 23:59:41 UTC 2007
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 <tsuna at lrde.epita.fr>
---
NEWS | 8 +++++
buildbot/status/web/authentication.py | 51 +++++++++++++++++++++++++++++
buildbot/status/web/base.py | 57 +++++++++++++++++++++++++-------
buildbot/status/web/baseweb.py | 53 +++++++++++++++++++++++++++---
buildbot/status/web/build.py | 7 +++-
buildbot/status/web/builder.py | 20 +++++++++---
docs/buildbot.texinfo | 16 +++++++--
7 files changed, 184 insertions(+), 28 deletions(-)
create mode 100644 buildbot/status/web/authentication.py
diff --git a/NEWS b/NEWS
index fcbe502..4f3b499 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,14 @@ User visible changes in Buildbot. -*- outline -*-
*** builder names must not start with an underscore (`_').
+*** 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)
** Things You Need To Know
diff --git a/buildbot/status/web/authentication.py b/buildbot/status/web/authentication.py
new file mode 100644
index 0000000..f6e1b75
--- /dev/null
+++ b/buildbot/status/web/authentication.py
@@ -0,0 +1,51 @@
+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:
+ """Base class for authentication methods."""
+
+ err = ""
+ """Last error."""
+
+ def errmsg(self):
+ """Get the last error message (reason why authentication
+ failed)."""
+ 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 565a135..4577767 100644
--- a/buildbot/status/web/base.py
+++ b/buildbot/status/web/base.py
@@ -53,30 +53,50 @@ def make_row(label, field):
label = html.escape(label)
return ROW_TEMPLATE % {"label": label, "field": field}
-def make_stop_form(stopURL):
- data = """<form action="%s" class='command stopbuild'>
+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):
+ """Create a form whose submit button sends the request to C{stopURL}. If
+ C{useLoginPassword} is true, this form will have a password field."""
+
+ data = """<form action="%s" class="command stopbuild">
<p>To stop this build, fill out the following fields and
- click the 'Stop' button</p>\n""" % stopURL
- data += make_row("Your name:",
- "<input type='text' name='username' />")
+ click the `Stop' button</p>\n""" % stopURL
+ 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):
+def make_force_build_form(forceURL, useLoginPassword):
+ """Create a form whose submit button sends the request to C{forceURL}. If
+ C{useLoginPassword} is true, this form will have a password field."""
+
data = """<form action="%s" class="command forcebuild">
<p>To force a build, fill out the following fields and
- click the 'Force Build' button</p>""" % forceURL
+ click the `Force Build' button</p>""" % forceURL
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 = {
@@ -244,9 +264,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", ["<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
diff --git a/buildbot/status/web/baseweb.py b/buildbot/status/web/baseweb.py
index 1c5453a..ecfdda6 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,11 @@ class OneLinePerBuild(HtmlResource, OneLineMixin):
if building:
stopURL = "builders/_all/stop"
- data += make_stop_form(stopURL)
+ data += make_stop_form(stopURL, self.isUsingLoginPassword(req))
if online:
forceURL = "builders/_all/force"
- data += make_force_build_form(forceURL)
+ data += make_force_build_form(forceURL,
+ self.isUsingLoginPassword(req))
return data
@@ -232,10 +234,11 @@ class OneBoxPerBuilder(HtmlResource):
if building:
stopURL = "builders/_all/stop"
- data += make_stop_form(stopURL)
+ data += make_stop_form(stopURL, self.isUsingLoginPassword(req))
if online:
forceURL = "builders/_all/force"
- data += make_force_build_form(forceURL)
+ data += make_force_build_form(forceURL,
+ self.isUsingLoginPassword(req))
return data
@@ -350,7 +353,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 +389,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 +408,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 +493,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 +511,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 52b41ab..b2eed77 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 4ea47cf..e028c9d 100644
--- a/buildbot/status/web/builder.py
+++ b/buildbot/status/web/builder.py
@@ -103,7 +103,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 += """
<p>All buildslaves appear to be offline, so it's not possible
@@ -131,13 +132,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):
@@ -151,9 +158,12 @@ class StatusResourceBuilder(HtmlResource, OneLineMixin):
if not revision:
revision = None
- # TODO: if we can authenticate that a particular User clicked 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 c2779b5..8d8e0ef 100644
--- a/docs/buildbot.texinfo
+++ b/docs/buildbot.texinfo
@@ -6063,10 +6063,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
+ at 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.
+
+ at 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)))
+ at end example
@node Buildbot Web Resources, XMLRPC server, WebStatus Configuration Parameters, WebStatus
--
1.5.3.5.756.gc887a
More information about the devel
mailing list