From 1b735e63434a080e234c1f99e627406a2512b210 Mon Sep 17 00:00:00 2001 From: gholt Date: Wed, 5 Jan 2011 08:14:31 -0800 Subject: [PATCH] Fix to limit account DELETEs to just reseller admins --- swift/common/middleware/auth.py | 5 ++-- swift/common/middleware/swauth.py | 5 ++-- test/unit/common/middleware/test_auth.py | 34 +++++++++++++++++++++ test/unit/common/middleware/test_swauth.py | 35 ++++++++++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/swift/common/middleware/auth.py b/swift/common/middleware/auth.py index 71fc32dafa..1ce7bb20ca 100644 --- a/swift/common/middleware/auth.py +++ b/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): diff --git a/swift/common/middleware/swauth.py b/swift/common/middleware/swauth.py index ee81bce3b6..afb4ea89d3 100644 --- a/swift/common/middleware/swauth.py +++ b/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): diff --git a/test/unit/common/middleware/test_auth.py b/test/unit/common/middleware/test_auth.py index 737686f3d9..cabc7a9523 100644 --- a/test/unit/common/middleware/test_auth.py +++ b/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() diff --git a/test/unit/common/middleware/test_swauth.py b/test/unit/common/middleware/test_swauth.py index f52a0d8d5c..43e0ed55f6 100644 --- a/test/unit/common/middleware/test_swauth.py +++ b/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)