Fix duplicate keystone auth_token config options

Currently, Mistral registers the configuration options under
keystoneclient middleware auth_token twice: by default when the
keystoneclient.middleware.auth_token module is loaded and in mistral.config
under a separate keystone group. This change removes the keystone group
in mistral.config, modifies API to use the default opts registered under
keystone_authtoken, and adds unit tests to ensure the keystone middleware
is called when auth is enabled.

Change-Id: I18948f7fa8b93b458335eb08176427a75c568873
Implements: blueprint mistral-config-cleanup

etc/mistral.conf.example

@@ -61,7 +61,7 @@ auth_enable = True
 #connection = mysql://root:password@localhost:3306/mistral
 connection = sqlite:///mistral.sqlite
 
-[keystone]
+[keystone_authtoken]
 auth_uri=http://localhost:5000/v3
 auth_host=localhost
 auth_port=5000

mistral/api/access_control.py

@@ -22,12 +22,10 @@ from oslo.config import cfg
 
 _ENFORCER = None
 
-auth_token.CONF = cfg.CONF
-
 
 def setup(app):
     if cfg.CONF.pecan.auth_enable:
-        return auth_token.AuthProtocol(app, conf=dict(cfg.CONF.keystone))
+        return auth_token.AuthProtocol(app, dict(cfg.CONF.keystone_authtoken))
     else:
         return app
 

mistral/config.py

@@ -19,7 +19,6 @@ Configuration options registration and useful routines.
 """
 
 from oslo.config import cfg
-from keystoneclient.middleware import auth_token
 
 from mistral.openstack.common import log
 from mistral import version
@@ -110,7 +109,6 @@ CONF = cfg.CONF
 CONF.register_opts(api_opts, group='api')
 CONF.register_opts(engine_opts, group='engine')
 CONF.register_opts(pecan_opts, group='pecan')
-CONF.register_opts(auth_token.opts, group='keystone')
 CONF.register_opts(db_opts, group='database')
 CONF.register_opts(rabbit_opts, group='rabbit')
 CONF.register_opts(executor_opts, group='executor')

mistral/tests/api/base.py

@@ -64,3 +64,11 @@ class FunctionalTest(base.DbTestCase):
             self.assertIn('Bad response: 404 Not Found', str(error))
             return
         self.fail('Expected 404 Not found but got OK')
+
+    def assertUnauthorized(self, url):
+        try:
+            self.app.get(url, headers={'Accept': 'application/json'})
+        except AppError as error:
+            self.assertIn('Bad response: 401 Unauthorized', str(error))
+            return
+        self.fail('Expected 401 Unauthorized but got OK')

mistral/tests/api/test_auth.py

@@ -0,0 +1,108 @@
+# -*- coding: utf-8 -*-
+#
+#    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 mock
+import uuid
+import datetime
+import pecan
+import pecan.testing
+
+from oslo.config import cfg
+from keystoneclient.middleware import auth_token
+
+from mistral.tests.api import base
+from mistral.db import api as db_api
+from mistral.openstack.common import timeutils
+
+
+WORKBOOKS = [
+    {
+        'name': "my_workbook",
+        'description': "My cool Mistral workbook",
+        'tags': ['deployment', 'demo']
+    }
+]
+
+
+PKI_TOKEN_VERIFIED = {
+    'token': {
+        'methods': ['password'],
+        'roles': [{'id': uuid.uuid4().hex,
+                   'name': 'admin'}],
+        'expires_at': timeutils.isotime(datetime.datetime.utcnow() +
+                                        datetime.timedelta(seconds=60)),
+        'project': {
+            'domain': {'id': 'default', 'name': 'Default'},
+            'id': uuid.uuid4().hex,
+            'name': 'Mistral'
+        },
+        'catalog': [],
+        'extras': {},
+        'user': {
+            'domain': {'id': 'default', 'name': 'Default'},
+            'id': uuid.uuid4().hex,
+            'name': 'admin'
+        },
+        'issued_at': timeutils.isotime()
+    }
+}
+
+
+class TestKeystoneMiddleware(base.FunctionalTest):
+    """Test that the keystone middleware AuthProtocol is executed
+    when enabled.
+    """
+
+    def setUp(self):
+        super(TestKeystoneMiddleware, self).setUp()
+        cfg.CONF.set_default('auth_enable', True, group='pecan')
+        self.app = pecan.testing.load_test_app({
+            'app': {
+                'root': cfg.CONF.pecan.root,
+                'modules': cfg.CONF.pecan.modules,
+                'debug': cfg.CONF.pecan.debug,
+                'auth_enable': cfg.CONF.pecan.auth_enable
+            }
+        })
+
+    def tearDown(self):
+        # By default, unit tests in Mistral has auth disabled and will fail
+        # if this option is not reset to False after this test is completed.
+        cfg.CONF.set_default('auth_enable', False, group='pecan')
+        super(TestKeystoneMiddleware, self).tearDown()
+
+    @mock.patch.object(
+        auth_token.AuthProtocol, '_get_user_token_from_header',
+        mock.MagicMock(return_value=''))
+    @mock.patch.object(
+        auth_token.AuthProtocol, '_validate_user_token',
+        mock.MagicMock(return_value=PKI_TOKEN_VERIFIED))
+    @mock.patch.object(
+        db_api, "workbook_get",
+        mock.MagicMock(return_value=WORKBOOKS[0]))
+    def test_auth_succeed(self):
+        resp = self.app.get('/v1/workbooks/my_workbook')
+        self.assertEqual(resp.status_int, 200)
+        self.assertDictEqual(WORKBOOKS[0], resp.json)
+
+    @mock.patch.object(
+        auth_token.AuthProtocol, '_get_user_token_from_header',
+        mock.MagicMock(return_value=''))
+    @mock.patch.object(
+        db_api, "workbook_get",
+        mock.MagicMock(return_value=WORKBOOKS[0]))
+    def test_auth_fail(self):
+        # 401 unauthorized response is expected because the method
+        # _validate_user_token is not mocked in this test.
+        self.assertUnauthorized('/v1/workbooks/my_workbook')