S3 tokens cleanups.

- Cleanups.
- Remove reference about config admin_username/password/token.
- Return proper http error on errors.
- Add unittests (skip them for now when swift is not installed).
- Fixes bug 956983.

Change-Id: I392fc274f3b01a5a0b5779dd13f9cd3b819ee65a

3 files changed, 213 insertions, 47 deletions

diff --git a/doc/source/configuringservices.rst b/doc/source/configuringservices.rst
index 8cd5eb80..bc89b2ca 100644
--- a/doc/source/configuringservices.rst
+++ b/doc/source/configuringservices.rst
@@ -220,12 +220,9 @@ S3 api.
 
     [filter:s3token]
     paste.filter_factory = keystone.middleware.s3_token:filter_factory
-    service_port = 5000
-    service_host = 127.0.0.1
     auth_port = 35357
     auth_host = 127.0.0.1
-    auth_token = ADMIN
-    admin_token = ADMIN
+    auth_protocol = http
 
     [filter:authtoken]
     paste.filter_factory = keystone.middleware.auth_token:filter_factory
@@ -233,6 +230,7 @@ S3 api.
     service_host = 127.0.0.1
     auth_port = 35357
     auth_host = 127.0.0.1
+    auth_protocol = http
     auth_token = ADMIN
     admin_token = ADMIN
 
diff --git a/keystone/middleware/s3_token.py b/keystone/middleware/s3_token.py
index 603b760c..4ef9d814 100644
--- a/keystone/middleware/s3_token.py
+++ b/keystone/middleware/s3_token.py
@@ -21,7 +21,17 @@
 # This source code is based ./auth_token.py and ./ec2_token.py.
 # See them for their copyright.
 
-"""Starting point for routing S3 requests."""
+"""
+S3 TOKEN MIDDLEWARE
+
+This WSGI component:
+
+* Get a request from the swift3 middleware with an S3 Authorization
+  access key.
+* Validate s3 token in Keystone.
+* Transform the account name to AUTH_%(tenant_name).
+
+"""
 
 import httplib
 import json
@@ -34,6 +44,10 @@ from swift.common import utils as swift_utils
 PROTOCOL_NAME = "S3 Token Authentication"
 
 
+class ServiceError(Exception):
+    pass
+
+
 class S3Token(object):
     """Auth Middleware that handles S3 authenticating client calls."""
 
@@ -43,29 +57,74 @@ class S3Token(object):
         self.logger = swift_utils.get_logger(conf, log_route='s3_token')
         self.logger.debug('Starting the %s component' % PROTOCOL_NAME)
 
+        # NOTE(chmou): We probably want to make sure that there is a _
+        # at the end of our reseller_prefix.
+        self.reseller_prefix = conf.get('reseller_prefix', 'AUTH_')
         # where to find the auth service (we use this to validate tokens)
         self.auth_host = conf.get('auth_host')
         self.auth_port = int(conf.get('auth_port', 35357))
-        self.auth_protocol = conf.get('auth_protocol', 'https')
+        auth_protocol = conf.get('auth_protocol', 'https')
+        if auth_protocol == 'http':
+            self.http_client_class = httplib.HTTPConnection
+        else:
+            self.http_client_class = httplib.HTTPSConnection
 
-        # Credentials used to verify this component with the Auth service since
-        # validating tokens is a privileged call
-        self.admin_token = conf.get('admin_token')
+    def _json_request(self, creds_json):
+        headers = {'Content-Type': 'application/json'}
+
+        try:
+            conn = self.http_client_class(self.auth_host, self.auth_port)
+            conn.request('POST', '/v2.0/s3tokens',
+                         body=creds_json,
+                         headers=headers)
+            response = conn.getresponse()
+            output = response.read()
+        except Exception, e:
+            self.logger.info('HTTP connection exception: %s' % e)
+            raise ServiceError('Unable to communicate with keystone')
+        finally:
+            conn.close()
+
+        if response.status < 200 or response.status >= 300:
+            raise ServiceError('Keystone reply error: status=%s reason=%s' % (
+                    response.status,
+                    response.reason))
+
+        return (response, output)
 
     def __call__(self, environ, start_response):
         """Handle incoming request. authenticate and send downstream."""
         req = webob.Request(environ)
-        parts = swift_utils.split_path(req.path, 1, 4, True)
-        version, account, container, obj = parts
+
+        try:
+            parts = swift_utils.split_path(req.path, 1, 4, True)
+            version, account, container, obj = parts
+        except ValueError:
+            msg = 'Not a path query, skipping.'
+            self.logger.debug(msg)
+            return self.app(environ, start_response)
 
         # Read request signature and access id.
         if not 'Authorization' in req.headers:
+            msg = 'No Authorization header. skipping.'
+            self.logger.debug(msg)
             return self.app(environ, start_response)
+
         token = req.headers.get('X-Auth-Token',
                                 req.headers.get('X-Storage-Token'))
+        if not token:
+            msg = 'You did not specify a auth or a storage token. skipping.'
+            self.logger.debug(msg)
+            return self.app(environ, start_response)
 
         auth_header = req.headers['Authorization']
-        access, signature = auth_header.split(' ')[-1].rsplit(':', 1)
+        try:
+            access, signature = auth_header.split(' ')[-1].rsplit(':', 1)
+        except(ValueError):
+            msg = 'You have an invalid Authorization header: %s'
+            self.logger.debug(msg % (auth_header))
+            return webob.exc.HTTPBadRequest()(environ, start_response)
+
         # NOTE(chmou): This is to handle the special case with nova
         # when we have the option s3_affix_tenant. We will force it to
         # connect to another account than the one
@@ -84,28 +143,8 @@ class S3Token(object):
         # Authenticate request.
         creds = {'credentials': {'access': access,
                                  'token': token,
-                                 'signature': signature,
-                                 'host': req.host,
-                                 'verb': req.method,
-                                 'path': req.path,
-                                 'expire': req.headers['Date'],
-                                 }}
-
+                                 'signature': signature}}
         creds_json = json.dumps(creds)
-        headers = {'Content-Type': 'application/json'}
-        if self.auth_protocol == 'http':
-            conn = httplib.HTTPConnection(self.auth_host, self.auth_port)
-        else:
-            conn = httplib.HTTPSConnection(self.auth_host, self.auth_port)
-
-        conn.request('POST', '/v2.0/s3tokens',
-                     body=creds_json,
-                     headers=headers)
-        resp = conn.getresponse()
-        if resp.status < 200 or resp.status >= 300:
-            raise Exception('Keystone reply error: status=%s reason=%s' % (
-                    resp.status,
-                    resp.reason))
 
         # NOTE(vish): We could save a call to keystone by having
         #             keystone return token, tenant, user, and roles
@@ -115,24 +154,23 @@ class S3Token(object):
         #              change token_auth to detect if we already
         #              identified and not doing a second query and just
         #              pass it thru to swiftauth in this case.
-        output = resp.read()
-        conn.close()
-        identity_info = json.loads(output)
+        (resp, output) = self._json_request(creds_json)
+
         try:
+            identity_info = json.loads(output)
             token_id = str(identity_info['access']['token']['id'])
-            tenant = (identity_info['access']['token']['tenant']['id'],
-                      identity_info['access']['token']['tenant']['name'])
-        except (KeyError, IndexError):
-            self.logger.debug('Error getting keystone reply: %s' %
-                              (str(output)))
-            raise
+            tenant = identity_info['access']['token']['tenant']
+        except (ValueError, KeyError):
+            error = 'Error on keystone reply: %d %s'
+            self.logger.debug(error % (resp.status, str(output)))
+            return webob.exc.HTTPBadRequest()(environ, start_response)
 
         req.headers['X-Auth-Token'] = token_id
-        tenant_to_connect = force_tenant or tenant[0]
-        self.logger.debug('Connecting with tenant: %s' %
-                              (tenant_to_connect))
-        environ['PATH_INFO'] = environ['PATH_INFO'].replace(
-                account, 'AUTH_%s' % tenant_to_connect)
+        tenant_to_connect = force_tenant or tenant['id']
+        self.logger.debug('Connecting with tenant: %s' % (tenant_to_connect))
+        new_tenant_name = '%s%s' % (self.reseller_prefix, tenant_to_connect)
+        environ['PATH_INFO'] = environ['PATH_INFO'].replace(account,
+                                                            new_tenant_name)
         return self.app(environ, start_response)
 
 
diff --git a/tests/test_s3_token_middleware.py b/tests/test_s3_token_middleware.py
new file mode 100644
index 00000000..b5f299b9
--- /dev/null
+++ b/tests/test_s3_token_middleware.py
@@ -0,0 +1,130 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 OpenStack LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import json
+
+import nose
+import webob
+
+from keystone import test
+
+try:
+    # NOTE(chmou): We don't want to force to have swift installed for
+    # unit test so we skip it we have an ImportError.
+    from keystone.middleware import s3_token
+    skip = False
+except ImportError:
+    skip = True
+
+
+class FakeHTTPResponse(object):
+    def __init__(self, status, body):
+        self.status = status
+        self.body = body
+
+    def read(self):
+        return self.body
+
+
+class FakeHTTPConnection(object):
+    def __init__(self, *args):
+        pass
+
+    def request(self, method, path, **kwargs):
+        ret = {'access': {'token': {'id': 'TOKEN_ID',
+                                    'tenant': {'id':  'TENANT_ID'}}}}
+        body = json.dumps(ret)
+        status = 201
+        self.resp = FakeHTTPResponse(status, body)
+
+    def getresponse(self):
+        return self.resp
+
+    def close(self):
+        pass
+
+
+class FakeApp(object):
+    """This represents a WSGI app protected by the auth_token middleware."""
+    def __call__(self, env, start_response):
+        resp = webob.Response()
+        resp.environ = env
+        return resp(env, start_response)
+
+
+class S3TokenMiddlewareTest(test.TestCase):
+    def setUp(self, expected_env=None):
+        # We probably going to end-up with the same strategy than
+        # test_swift_auth when this is decided.
+        if skip:
+            raise nose.SkipTest('no swift detected')
+        self.middleware = s3_token.S3Token(FakeApp(), {})
+        self.middleware.http_client_class = FakeHTTPConnection
+
+        self.response_status = None
+        self.response_headers = None
+        super(S3TokenMiddlewareTest, self).setUp()
+
+    def _start_fake_response(self, status, headers):
+        self.response_status = int(status.split(' ', 1)[0])
+        self.response_headers = dict(headers)
+
+    # Ignore the request and pass to the next middleware in the
+    # pipeline if no path has been specified.
+    def test_no_path_request(self):
+        req = webob.Request.blank('/')
+        self.middleware(req.environ, self._start_fake_response)
+        self.assertEqual(self.response_status, 200)
+
+    # Ignore the request and pass to the next middleware in the
+    # pipeline if no Authorization header has been specified
+    def test_without_authorization(self):
+        req = webob.Request.blank('/v1/AUTH_cfa/c/o')
+        self.middleware(req.environ, self._start_fake_response)
+        self.assertEqual(self.response_status, 200)
+
+    def test_without_auth_storage_token(self):
+        req = webob.Request.blank('/v1/AUTH_cfa/c/o')
+        req.headers['Authorization'] = 'badboy'
+        self.middleware(req.environ, self._start_fake_response)
+        self.assertEqual(self.response_status, 200)
+
+    def test_with_bogus_authorization(self):
+        req = webob.Request.blank('/v1/AUTH_cfa/c/o')
+        req.headers['Authorization'] = 'badboy'
+        req.headers['X-Storage-Token'] = 'token'
+        self.middleware(req.environ, self._start_fake_response)
+        self.assertEqual(self.response_status, 400)
+
+    def test_authorized(self):
+        req = webob.Request.blank('/v1/AUTH_cfa/c/o')
+        req.headers['Authorization'] = 'access:signature'
+        req.headers['X-Storage-Token'] = 'token'
+        resp = webob.Request(req.get_response(self.middleware).environ)
+        self.assertTrue(resp.path.startswith('/v1/AUTH_TENANT_ID'))
+        self.assertEqual(resp.headers['X-Auth-Token'], 'TOKEN_ID')
+
+    def test_authorization_nova_toconnect(self):
+        req = webob.Request.blank('/v1/AUTH_swiftint/c/o')
+        req.headers['Authorization'] = 'access:FORCED_TENANT_ID:signature'
+        req.headers['X-Storage-Token'] = 'token'
+        req = req.get_response(self.middleware)
+        path = req.environ['PATH_INFO']
+        self.assertTrue(path.startswith('/v1/AUTH_FORCED_TENANT_ID'))
+
+if __name__ == '__main__':
+    import unittest
+    unittest.main()