Merge "Fix copy requests to service accounts in Keystone"

3 files changed, 77 insertions, 33 deletions

diff --git a/swift/common/middleware/keystoneauth.py b/swift/common/middleware/keystoneauth.py
index d57f132ff..b50f77cf1 100644
--- a/swift/common/middleware/keystoneauth.py
+++ b/swift/common/middleware/keystoneauth.py
@@ -129,12 +129,12 @@ class KeystoneAuth(object):
         SERVICE_operator_roles = admin, swiftoperator
         SERVICE_service_roles = service
 
-    The keystoneauth middleware supports cross-tenant access control using
-    the syntax ``<tenant>:<user>`` to specify a grantee in container Access
-    Control Lists (ACLs). For a request to be granted by an ACL, the grantee
+    The keystoneauth middleware supports cross-tenant access control using the
+    syntax ``<tenant>:<user>`` to specify a grantee in container Access Control
+    Lists (ACLs). For a request to be granted by an ACL, the grantee
     ``<tenant>`` must match the UUID of the tenant to which the request
-    token is scoped and the grantee ``<user>`` must match the UUID of the
-    user authenticated by the request token.
+    X-Auth-Token is scoped and the grantee ``<user>`` must match the UUID of
+    the user authenticated by that token.
 
     Note that names must no longer be used in cross-tenant ACLs because with
     the introduction of domains in keystone names are no longer globally
@@ -143,7 +143,7 @@ class KeystoneAuth(object):
     For backwards compatibility, ACLs using names will be granted by
     keystoneauth when it can be established that the grantee tenant,
     the grantee user and the tenant being accessed are either not yet in a
-    domain (e.g. the request token has been obtained via the keystone v2
+    domain (e.g. the X-Auth-Token has been obtained via the keystone v2
     API) or are all in the default domain to which legacy accounts would
     have been migrated. The default domain is identified by its UUID,
     which by default has the value ``default``. This can be changed by
@@ -406,6 +406,10 @@ class KeystoneAuth(object):
         return None
 
     def authorize(self, env_identity, req):
+        # Cleanup - make sure that a previously set swift_owner setting is
+        # cleared now. This might happen for example with COPY requests.
+        req.environ.pop('swift_owner', None)
+
         tenant_id, tenant_name = env_identity['tenant']
         user_id, user_name = env_identity['user']
         referrers, roles = swift_acl.parse_acl(getattr(req, 'acl', None))
@@ -473,6 +477,7 @@ class KeystoneAuth(object):
         # in operator_roles? service_roles? in service_roles?     swift_owner?
         # ------------------ -------------- --------------------  ------------
         # yes                yes            yes                   yes
+        # yes                yes            no                    no
         # yes                no             don't care            yes
         # no                 don't care     don't care            no
         # ------------------ -------------- --------------------  ------------
@@ -483,14 +488,16 @@ class KeystoneAuth(object):
         service_roles = self.account_rules[account_prefix]['service_roles']
         have_service_role = set(service_roles).intersection(
             set(user_service_roles))
+        allowed = False
         if have_operator_role and (service_roles and have_service_role):
-            req.environ['swift_owner'] = True
+            allowed = True
         elif have_operator_role and not service_roles:
-            req.environ['swift_owner'] = True
-        if req.environ.get('swift_owner'):
+            allowed = True
+        if allowed:
             log_msg = 'allow user with role(s) %s as account admin'
             self.logger.debug(log_msg, ','.join(have_operator_role.union(
                                                 have_service_role)))
+            req.environ['swift_owner'] = True
             return
 
         # If user is of the same name of the tenant then make owner of it.
diff --git a/swift/common/middleware/tempauth.py b/swift/common/middleware/tempauth.py
index 5e3f980e6..2b517ac9a 100644
--- a/swift/common/middleware/tempauth.py
+++ b/swift/common/middleware/tempauth.py
@@ -90,29 +90,34 @@ class TempAuth(object):
     to access an account. If you have several reseller prefix items, prefix
     the ``require_group`` parameter with the appropriate prefix.
 
-     X-Service-Token:
-
-     If an X-Service-Token is presented in the request headers, the groups
-     derived from the token are appended to the roles derived form
-     X-Auth-Token. If X-Auth-Token is missing or invalid, X-Service-Token
-     is not processed.
-
-     The X-Service-Token is useful when combined with multiple reseller prefix
-     items. In the following configuration, accounts prefixed SERVICE_
-     are only accessible if X-Auth-Token is form the end-user and
-     X-Service-Token is from the ``glance`` user::
-
-        [filter:tempauth]
-        use = egg:swift#tempauth
-        reseller_prefix = AUTH, SERVICE
-        SERVICE_require_group = .service
-        user_admin_admin = admin .admin .reseller_admin
-        user_joeacct_joe = joepw .admin
-        user_maryacct_mary = marypw .admin
-        user_glance_glance = glancepw .service
-
-     The name .service is an example. Unlike .admin and .reseller_admin
-     it is not a reserved name.
+    X-Service-Token:
+
+    If an X-Service-Token is presented in the request headers, the groups
+    derived from the token are appended to the roles derived from
+    X-Auth-Token. If X-Auth-Token is missing or invalid, X-Service-Token
+    is not processed.
+
+    The X-Service-Token is useful when combined with multiple reseller prefix
+    items. In the following configuration, accounts prefixed SERVICE_
+    are only accessible if X-Auth-Token is from the end-user and
+    X-Service-Token is from the ``glance`` user::
+
+       [filter:tempauth]
+       use = egg:swift#tempauth
+       reseller_prefix = AUTH, SERVICE
+       SERVICE_require_group = .service
+       user_admin_admin = admin .admin .reseller_admin
+       user_joeacct_joe = joepw .admin
+       user_maryacct_mary = marypw .admin
+       user_glance_glance = glancepw .service
+
+    The name .service is an example. Unlike .admin and .reseller_admin
+    it is not a reserved name.
+
+    Please note that ACLs can be set on service accounts and are matched
+    against the identity validated by X-Auth-Token. As such ACLs can grant
+    access to a service account's container without needing to provide a
+    service token, just like any other cross-reseller request using ACLs.
 
     Account ACLs:
         If a swift_owner issues a POST or PUT to the account, with the
diff --git a/test/unit/common/middleware/test_keystoneauth.py b/test/unit/common/middleware/test_keystoneauth.py
index 44195d30f..da9530a06 100644
--- a/test/unit/common/middleware/test_keystoneauth.py
+++ b/test/unit/common/middleware/test_keystoneauth.py
@@ -358,7 +358,8 @@ class SwiftAuthMultiple(SwiftAuth):
 class ServiceTokenFunctionality(unittest.TestCase):
 
     def _make_authed_request(self, conf, project_id, path, method='GET',
-                             user_role='admin', service_role=None):
+                             user_role='admin', service_role=None,
+                             environ=None):
         """Make a request with keystoneauth as auth
 
         By default, acts as though the user had presented a token
@@ -371,6 +372,8 @@ class ServiceTokenFunctionality(unittest.TestCase):
         :param method: the method (defaults to GET)
         :param user_role: the role of X-Auth-Token (defaults to 'admin')
         :param service_role: the role in X-Service-Token (defaults to none)
+        :param environ: a dict of items to be added to the request environ
+                       (defaults to none)
 
         :returns: response object
         """
@@ -381,6 +384,8 @@ class ServiceTokenFunctionality(unittest.TestCase):
         _, info_key = _get_cache_key(account, None)
         env = {info_key: {'status': 0, 'sysmeta': {}},
                'keystone.token_info': _fake_token_info(version='2')}
+        if environ:
+            env.update(environ)
         req = Request.blank(path, environ=env, headers=headers)
         req.method = method
         fake_app = FakeApp(iter([('200 OK', {}, '')]))
@@ -388,6 +393,33 @@ class ServiceTokenFunctionality(unittest.TestCase):
         resp = req.get_response(test_auth)
         return resp
 
+    def test_existing_swift_owner_ignored(self):
+        # a request without admin role is denied
+        resp = self._make_authed_request(
+            {'reseller_prefix': 'AUTH'}, '12345678', '/v1/AUTH_12345678',
+            environ={'swift_owner': False},
+            user_role='something_else')
+        self.assertEqual(resp.status_int, 403)
+
+        # ... even when swift_owner has previously been set True in request env
+        resp = self._make_authed_request(
+            {'reseller_prefix': 'AUTH'}, '12345678', '/v1/AUTH_12345678',
+            environ={'swift_owner': True},
+            user_role='something_else')
+        self.assertEqual(resp.status_int, 403)
+
+        # a request with admin role but to different account prefix is denied
+        resp = self._make_authed_request(
+            {'reseller_prefix': 'AUTH'}, '12345678', '/v1/SERVICE_12345678',
+            environ={'swift_owner': False})
+        self.assertEqual(resp.status_int, 403)
+
+        # ... even when swift_owner has previously been set True in request env
+        resp = self._make_authed_request(
+            {'reseller_prefix': 'AUTH'}, '12345678', '/v1/SERVICE_12345678',
+            environ={'swift_owner': True})
+        self.assertEqual(resp.status_int, 403)
+
     def test_unknown_prefix(self):
         resp = self._make_authed_request({}, '12345678', '/v1/BLAH_12345678')
         self.assertEqual(resp.status_int, 403)