Merge branch '2186-change-hook-auth' of git://github.com/MichaelMayor…

…ov/buildbot

master/buildbot/status/web/baseweb.py

@@ -44,6 +44,9 @@
 from buildbot.status.web.root import RootPage
 from buildbot.status.web.users import UsersResource
 from buildbot.status.web.change_hook import ChangeHookResource
+from twisted.cred.portal import IRealm, Portal
+from twisted.web import resource, guard
+from twisted.cred.checkers import InMemoryUsernamePasswordDatabaseDontUse
 
 # this class contains the WebStatus class.  Basic utilities are in base.py,
 # and specific pages are each in their own module.
@@ -149,7 +152,8 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None,
                  order_console_by_time=False, changecommentlink=None,
                  revlink=None, projects=None, repositories=None,
                  authz=None, logRotateLength=None, maxRotatedFiles=None,
-                 change_hook_dialects = {}, provide_feeds=None, jinja_loaders=None):
+                 change_hook_dialects = {}, provide_feeds=None, jinja_loaders=None,
+                 change_hook_auth=None):
         """Run a web server that provides Buildbot status.
 
         @type  http_port: int or L{twisted.application.strports} string
@@ -314,6 +318,12 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None,
 
         self.authz = authz
 
+        # check for correctness of HTTP auth parameters
+        if change_hook_auth is not None:
+            if not isinstance(change_hook_auth, tuple) or len(change_hook_auth) != 2:
+                config.error("Invalid credentials for change_hook auth")
+        self.change_hook_auth = change_hook_auth
+
         self.orderConsoleByTime = order_console_by_time
 
         # If we were given a site object, go ahead and use it. (if not, we add one later)
@@ -345,7 +355,10 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None,
         self.change_hook_dialects = {}
         if change_hook_dialects:
             self.change_hook_dialects = change_hook_dialects
-            self.putChild("change_hook", ChangeHookResource(dialects = self.change_hook_dialects))
+            resource_obj = ChangeHookResource(dialects=self.change_hook_dialects)
+            if self.change_hook_auth is not None:
+                resource_obj = self.setupProtectedResource(resource_obj)
+            self.putChild("change_hook", resource_obj)
 
         # Set default feeds
         if provide_feeds is None:
@@ -355,6 +368,27 @@ def __init__(self, http_port=None, distrib_port=None, allowForce=None,
 
         self.jinja_loaders = jinja_loaders
 
+    def setupProtectedResource(self, resource_obj):
+        class SimpleRealm(object):
+            """
+            A realm which gives out L{ChangeHookResource} instances for authenticated
+            users.
+            """
+            implements(IRealm)
+
+            def requestAvatar(self, avatarId, mind, *interfaces):
+                if resource.IResource in interfaces:
+                    return (resource.IResource, resource_obj, lambda: None)
+                raise NotImplementedError()
+
+        login, password = self.change_hook_auth
+        checker = InMemoryUsernamePasswordDatabaseDontUse()
+        checker.addUser(login, password)
+        portal = Portal(SimpleRealm(), [checker])
+        credentialFactory = guard.BasicCredentialFactory('Protected area')
+        wrapper = guard.HTTPAuthSessionWrapper(portal, [credentialFactory])
+        return wrapper
+
     def setupUsualPages(self, numbuilds, num_events, num_events_max):
         #self.putChild("", IndexOrWaterfallRedirection())
         self.putChild("waterfall", WaterfallStatusResource(num_events=num_events,

master/docs/manual/cfg-statustargets.rst

@@ -726,10 +726,21 @@ useful in cases where you cannot expose the WebStatus for public consumption.
 
 .. warning::
 
-    The incoming HTTP requests for this hook are not authenticated in
-    any way.  Anyone who can access the web status can "fake" a request from
-    GitHub, potentially causing the buildmaster to run arbitrary code.  See
-    :bb:bug:`2186` for work to fix this problem.
+    The incoming HTTP requests for this hook are not authenticated by default.
+    Anyone who can access the web status can "fake" a request from
+    GitHub, potentially causing the buildmaster to run arbitrary code.
+
+To protect URL against unauthorized access you should use ``change_hook_auth``
+option. ::
+
+    c['status'].append(html.WebStatus(..
+                                      change_hook_auth=('user', 'password')))
+
+Then, create github service hook ``https://help.github.com/articles/post-receive-hooks``
+with WebHook URL looking as follow
+``http://user:password@builds.mycompany.com/bbot/change_hook/github``
+
+Note that not using ``change_hook_auth`` can expose you to security risks
 
 Google Code hook
 ################