From 8d0ad2c93b57b00187e5e02f6fe440c6b3592018 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Fri, 9 Nov 2018 19:21:44 -0800
Subject: [PATCH] Correct HTTP OPTIONS method

When HTTP OPTIONS method was used, keystone was incorrectly classifying
the request to require enforcement. OPTIONS is handled automatically
by flask and needs no additional implementation. It is now explicitly
exempted from the "unenforced api" assertion.

Change-Id: Ifdb850c1fbc10c05108466ad68d808f3f5c20b37
closes-bug: #1801778
---
 keystone/server/flask/common.py               |  6 ++-
 .../tests/unit/server/test_keystone_flask.py  | 37 +++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/keystone/server/flask/common.py b/keystone/server/flask/common.py
index f48e90c403..9d12dea947 100644
--- a/keystone/server/flask/common.py
+++ b/keystone/server/flask/common.py
@@ -146,7 +146,11 @@ def _assert_rbac_enforcement_called(resp):
            'enforcer.RBACKEnforcer.enforce_call()`) has not been called; API '
            'is unenforced.')
     g = flask.g
-    assert getattr(g, enforcer._ENFORCEMENT_CHECK_ATTR, False), msg  # nosec
+    # NOTE(morgan): OPTIONS is a special case and is handled by flask
+    # internally. We should never be enforcing on OPTIONS calls.
+    if flask.request.method != 'OPTIONS':
+        assert getattr(  # nosec
+            g, enforcer._ENFORCEMENT_CHECK_ATTR, False), msg  # nosec
     return resp
 
 
diff --git a/keystone/tests/unit/server/test_keystone_flask.py b/keystone/tests/unit/server/test_keystone_flask.py
index 984d78f8d9..efa05226f2 100644
--- a/keystone/tests/unit/server/test_keystone_flask.py
+++ b/keystone/tests/unit/server/test_keystone_flask.py
@@ -484,6 +484,43 @@ class TestKeystoneFlaskCommon(rest.RestfulTestCase):
             resp = c.post('/v3/test_api', json=body)
             self.assertEqual(body, resp.json['post_body'])
 
+    def test_HTTP_OPTIONS_is_unenforced(self):
+        # Standup a test mapped resource and call OPTIONS on it. This will
+        # return a header "Allow" with the valid methods, in this case
+        # OPTIONS and POST. Ensure that the response otherwise conforms
+        # as expected.
+
+        class MappedResource(flask_restful.Resource):
+            def post(self):
+                # we don't actually use this or call it.
+                pass
+
+        resource_map = flask_common.construct_resource_map(
+            resource=MappedResource,
+            url='test_api',
+            alternate_urls=[],
+            resource_kwargs={},
+            rel='test',
+            status=json_home.Status.STABLE,
+            path_vars=None,
+            resource_relation_func=json_home.build_v3_resource_relation)
+
+        restful_api = _TestRestfulAPI(resource_mapping=[resource_map],
+                                      resources=[])
+        self.public_app.app.register_blueprint(restful_api.blueprint)
+        with self.test_client() as c:
+            r = c.options('/v3/test_api')
+            # make sure we split the data and left/right strip off whitespace
+            # The use of a SET here is to ensure the exact values are in place
+            # even if hash-seeds change order. `r.data` will be an empty
+            # byte-string. `Content-Length` will be 0.
+            self.assertEqual(
+                set(['OPTIONS', 'POST']),
+                set([v.lstrip().rstrip()
+                     for v in r.headers['Allow'].split(',')]))
+            self.assertEqual(r.headers['Content-Length'], '0')
+            self.assertEqual(r.data, b'')
+
     def test_mapped_resource_routes(self):
         # Test non-standard URL routes ("mapped") function as expected