Fix to limit account DELETEs to just reseller admins

swift/common/middleware/auth.py

@@ -159,9 +159,10 @@ class DevAuth(object):
         user_groups = (req.remote_user or '').split(',')
         if '.reseller_admin' in user_groups:
             return None
-        if account in user_groups and (req.method != 'PUT' or container):
+        if account in user_groups and \
+                (req.method not in ('DELETE', 'PUT') or container):
             # If the user is admin for the account and is not trying to do an
-            # account PUT...
+            # account DELETE or PUT...
             return None
         referrers, groups = parse_acl(getattr(req, 'acl', None))
         if referrer_allowed(req.referer, referrers):

swift/common/middleware/swauth.py

@@ -208,9 +208,10 @@ class Swauth(object):
         if '.reseller_admin' in user_groups and \
                 account[len(self.reseller_prefix)].isalnum():
             return None
-        if account in user_groups and (req.method != 'PUT' or container):
+        if account in user_groups and \
+                (req.method not in ('DELETE', 'PUT') or container):
             # If the user is admin for the account and is not trying to do an
-            # account PUT...
+            # account DELETE or PUT...
             return None
         referrers, groups = parse_acl(getattr(req, 'acl', None))
         if referrer_allowed(req.referer, referrers):

test/unit/common/middleware/test_auth.py

@@ -432,6 +432,40 @@ class TestAuth(unittest.TestCase):
         resp = self.test_auth.authorize(req)
         self.assertEquals(resp and resp.status_int, 403)
 
+    def test_account_delete_permissions(self):
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp and resp.status_int, 403)
+
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,AUTH_other'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp and resp.status_int, 403)
+
+        # Even DELETEs to your own account as account admin should fail
+        req = Request.blank('/v1/AUTH_old',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,AUTH_old'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp and resp.status_int, 403)
+
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,.reseller_admin'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp, None)
+
+        # .super_admin is not something the middleware should ever see or care
+        # about
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,.super_admin'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp and resp.status_int, 403)
+
 
 if __name__ == '__main__':
     unittest.main()

test/unit/common/middleware/test_swauth.py

@@ -458,6 +458,41 @@ class TestAuth(unittest.TestCase):
         resp = self.test_auth.authorize(req)
         self.assertEquals(resp.status_int, 403)
 
+    def test_account_delete_permissions(self):
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp.status_int, 403)
+
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,AUTH_other'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp.status_int, 403)
+
+        # Even DELETEs to your own account as account admin should fail
+        req = Request.blank('/v1/AUTH_old',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,AUTH_old'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp.status_int, 403)
+
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,.reseller_admin'
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp, None)
+
+        # .super_admin is not something the middleware should ever see or care
+        # about
+        req = Request.blank('/v1/AUTH_new',
+                            environ={'REQUEST_METHOD': 'DELETE'})
+        req.remote_user = 'act:usr,act,.super_admin'
+        resp = self.test_auth.authorize(req)
+        resp = self.test_auth.authorize(req)
+        self.assertEquals(resp.status_int, 403)
+
     def test_get_token_fail(self):
         resp = Request.blank('/auth/v1.0').get_response(self.test_auth)
         self.assertEquals(resp.status_int, 401)