mozreview: test security of ldap assocations (Bug 1237702). r?gps draft
authorSteven MacLeod <smacleod@mozilla.com>
Thu, 07 Jan 2016 14:14:59 -0500
changeset 6647 a239367132003ef025aec13338b598ab8fa61a96
parent 6628 96657fb045a9a38fa63342d249c02315d50a0575
push id505
push usersmacleod@mozilla.com
push dateThu, 07 Jan 2016 20:06:18 +0000
reviewersgps
bugs1237702, 28445
mozreview: test security of ldap assocations (Bug 1237702). r?gps https://reviewboard.mozilla.org/r/28445 reminded me we don't actually have tests that make sure random users cannot change ldap associations. I've added tests for an unprivileged user changing their own ldap association, an unprivileged user changing another users association, and a test for changing ldap association without authenticating.
hgext/reviewboard/tests/test-ldap-association.t
testing/vcttesting/reviewboard/mach_commands.py
--- a/hgext/reviewboard/tests/test-ldap-association.t
+++ b/hgext/reviewboard/tests/test-ldap-association.t
@@ -55,12 +55,73 @@ Perform a push with the user
   (review requests lack reviewers; visit review url to assign reviewers)
   (visit review url to publish these review requests so others can see them)
 
 The user should now have an associated ldap_username
 
   $ rbmanage dump-user-ldap user1
   ldap username: user1@example.com
 
+Create another user
+
+  $ mozreview create-user user2@example.com tastypassword 'User Two [:user2]' --uid 2001 --scm-level 1
+  Created user 7
+  $ user1key=`mozreview create-api-key user2@example.com`
+  $ exportbzauth user2@example.com tastypassword
+
+Dump the user so it gets mirrored over to Review Board
+
+  $ rbmanage dump-user user2 > /dev/null
+
+The user should not have an ldap username associated with them
+
+  $ rbmanage dump-user-ldap user2
+  no ldap username associated with user2
+
+The user should not be able to change their own ldap email association
+
+  $ rbmanage associate-ldap-user user2 user2@example.com --request-username=user2 --request-password=tastypassword
+  API Error: 403: 101: You don't have permission for this
+  [1]
+
+The user should still have no ldap username associated with them
+
+  $ rbmanage dump-user-ldap user2
+  no ldap username associated with user2
+
+A user without the special permission should not be able to change the association of
+another user
+
+  $ rbmanage associate-ldap-user user2 user2@example.com --request-username=user1 --request-password=password1
+  API Error: 403: 101: You don't have permission for this
+  [1]
+
+The user should still have no ldap username associated with them
+
+  $ rbmanage dump-user-ldap user2
+  no ldap username associated with user2
+
+An unauthenticated request should not be able to change an association
+
+  $ rbmanage associate-ldap-user user2 user2@example.com --anonymous
+  API Error: 401: 103: You are not logged in
+  [1]
+
+The user should still have no ldap username associated with them
+
+  $ rbmanage dump-user-ldap user2
+  no ldap username associated with user2
+
+The special user should be able to associate the ldap account after all of
+the failed attempts
+
+  $ rbmanage associate-ldap-user user2 user2@example.com
+  user2@example.com associated with user2
+
+The user should now have an associated ldap_username
+
+  $ rbmanage dump-user-ldap user2
+  ldap username: user2@example.com
+
 Cleanup
 
   $ mozreview stop
   stopped 10 containers
--- a/testing/vcttesting/reviewboard/mach_commands.py
+++ b/testing/vcttesting/reviewboard/mach_commands.py
@@ -143,17 +143,17 @@ def short_review_request_dict(rr):
 class ReviewBoardCommands(object):
     def __init__(self, context):
         from vcttesting.mozreview import MozReview
         if 'MOZREVIEW_HOME' in os.environ:
             self.mr = MozReview(os.environ['MOZREVIEW_HOME'])
         else:
             self.mr = None
 
-    def _get_client(self, username=None, password=None):
+    def _get_client(self, username=None, password=None, anonymous=False):
         from rbtools.api.client import RBClient
         from rbtools.api.transport.sync import SyncTransport
 
         class NoCacheTransport(SyncTransport):
             """API transport with disabled caching."""
             def enable_cache(self):
                 pass
 
@@ -169,21 +169,26 @@ class ReviewBoardCommands(object):
         # in $HOME/.rbtools-cookies. We want to be able to easily switch
         # between users, so we clear that cookie between calls to the
         # server and reauthenticate every time.
         try:
             os.remove(os.path.join(os.environ.get('HOME'), '.rbtools-cookies'))
         except Exception:
             pass
 
+        if anonymous:
+            return RBClient(self.mr.reviewboard_url,
+                            transport_cls=NoCacheTransport)
+
         return RBClient(self.mr.reviewboard_url, username=username,
                         password=password, transport_cls=NoCacheTransport)
 
-    def _get_root(self, username=None, password=None):
-        return self._get_client(username=username, password=password).get_root()
+    def _get_root(self, username=None, password=None, anonymous=False):
+        return self._get_client(username=username, password=password,
+                                anonymous=anonymous).get_root()
 
     def _get_rb(self, path=None):
         from vcttesting.reviewboard import MozReviewBoard
 
         if self.mr:
             return self.mr.get_reviewboard()
         elif 'BUGZILLA_HOME' in os.environ and path:
             return MozReviewBoard(None, os.environ['BUGZILLA_URL'],
@@ -516,25 +521,45 @@ class ReviewBoardCommands(object):
             print('ldap username: %s' % user)
         else:
             print('no ldap username associated with %s' % username)
 
     @Command('associate-ldap-user', category='reviewboard',
         description='Associate an LDAP email address with a user.')
     @CommandArgument('username', help='Username to associate with ldap')
     @CommandArgument('email', help='LDAP email to associate')
-    def associate_ldap_user(self, username, email):
-        # We use the "mozreview" account which has the special permission
-        # to read / associate ldap email addresses.
-        root = self._get_root(username="mozreview", password="mrpassword")
+    @CommandArgument('--request-username',
+                     default='mozreview',
+                     help='Username to make request with')
+    @CommandArgument('--request-password',
+                     default='mrpassword',
+                     help='Password to make request with')
+    @CommandArgument('--anonymous',
+                     action='store_true',
+                     default=False,
+                     help='Make the request anonymously')
+    def associate_ldap_user(self, username, email, request_username,
+                            request_password, anonymous):
+        from rbtools.api.errors import APIError
+
+        # We use the "mozreview" account by default which has the special
+        # permission to read / associate ldap email addresses.
+        root = self._get_root(username=request_username,
+                              password=request_password,
+                              anonymous=anonymous)
         ext = root.get_extension(
             extension_name='mozreview.extension.MozReviewExtension')
 
-        association = ext.get_ldap_associations().get_item(username)
-        association.update(ldap_username=email)
+        try:
+            association = ext.get_ldap_associations().get_item(username)
+            association.update(ldap_username=email)
+        except APIError as e:
+            print('API Error: %s: %s: %s' % (e.http_status, e.error_code,
+                e.rsp['err']['msg']))
+            return 1
 
         print('%s associated with %s' % (email, username))
 
     @Command('dump-autoland-requests', category='reviewboard',
              description='Dump the table of autoland requests.')
     def dump_autoland_requests(self):
         root = self._get_root()
         ext = root.get_extension(