From 41774fafd083c6a37a2677695fdaefd0a0289944 Mon Sep 17 00:00:00 2001 From: Stefan W Date: Fri, 24 Mar 2023 14:18:13 +0100 Subject: [PATCH] Odoo provides basic handling of CORS preflight requests: if an endpoint is marked as `cors=` then it'll automatically reply allowing the request. *However* this is performed in `HttpRequest.dispatch` (likely in order to correctly handle the nodb case), which means it's executed after the auth handler has run... which means custom auth handlers will be called on preflight requests. This is a problem because they are missing relevant information (e.g. which endpoint they're invoked for), plus having to deal with preflight requests in every custom auth handler is annoying, and simply allowing preflights could cause issues if the decision diverges between the auth handler and the automatic handling. To fix this issue, extract the preflight *decision* into a separate method so we get the same decision-making process everywhere, and in case of CORS preflight set the auth to none to limit the eventual capacity for nuisance in the span between the bypassed auth and the automated preflight handling. Also change the signature of IrHttp._authenticate so it's clearer if a callsite was forgotten somehow (and this makes for less changes and duplication at the callsites). --- addons/http_routing/models/ir_http.py | 2 +- odoo/addons/base/models/ir_http.py | 7 ++- odoo/addons/test_auth_custom/__init__.py | 22 +++++++++ odoo/addons/test_auth_custom/__manifest__.py | 7 +++ .../addons/test_auth_custom/tests/__init__.py | 1 + .../test_auth_custom/tests/test_endpoints.py | 46 +++++++++++++++++++ odoo/http.py | 17 +++---- 7 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 odoo/addons/test_auth_custom/__init__.py create mode 100644 odoo/addons/test_auth_custom/__manifest__.py create mode 100644 odoo/addons/test_auth_custom/tests/__init__.py create mode 100644 odoo/addons/test_auth_custom/tests/test_endpoints.py diff --git a/addons/http_routing/models/ir_http.py b/addons/http_routing/models/ir_http.py index 820966c5abded..e3aece4849606 100644 --- a/addons/http_routing/models/ir_http.py +++ b/addons/http_routing/models/ir_http.py @@ -457,7 +457,7 @@ def _dispatch(cls): # check authentication level try: if func: - cls._authenticate(func.routing['auth']) + cls._authenticate(func) elif request.uid is None and request.is_frontend: cls._auth_method_public() except Exception as e: diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 0651ddba574fb..b6d8adde9611d 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/odoo/addons/base/models/ir_http.py @@ -108,7 +108,10 @@ def _auth_method_public(cls): request.uid = request.session.uid @classmethod - def _authenticate(cls, auth_method='user'): + def _authenticate(cls, endpoint): + auth_method = endpoint.routing["auth"] + if request._is_cors_preflight(endpoint): + auth_method = 'none' try: if request.session.uid: try: @@ -220,7 +223,7 @@ def _dispatch(cls): # check authentication level try: - auth_method = cls._authenticate(func.routing["auth"]) + auth_method = cls._authenticate(func) except Exception as e: return cls._handle_exception(e) diff --git a/odoo/addons/test_auth_custom/__init__.py b/odoo/addons/test_auth_custom/__init__.py new file mode 100644 index 0000000000000..f53c0fde34df7 --- /dev/null +++ b/odoo/addons/test_auth_custom/__init__.py @@ -0,0 +1,22 @@ +from odoo import models +from odoo.exceptions import AccessDenied +from odoo.http import Controller, route + + +class IrHttp(models.AbstractModel): + _inherit = 'ir.http' + + @classmethod + def _auth_method_thing(cls): + raise AccessDenied() + +class TestController(Controller): + # for HTTP endpoints, must allow OPTIONS or werkzeug won't match the route + # when dispatching the CORS preflight + @route('/test_auth_custom/http', type="http", auth="thing", cors="*", methods=['GET', 'OPTIONS']) + def _http(self): + raise NotImplementedError + + @route('/test_auth_custom/json', type="json", auth="thing", cors="*") + def _json(self): + raise NotImplementedError \ No newline at end of file diff --git a/odoo/addons/test_auth_custom/__manifest__.py b/odoo/addons/test_auth_custom/__manifest__.py new file mode 100644 index 0000000000000..de4bee296812c --- /dev/null +++ b/odoo/addons/test_auth_custom/__manifest__.py @@ -0,0 +1,7 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +{ + 'name': 'Tests that custom auth works & is not impaired by CORS', + 'category': 'Hidden', + 'data': [], +} \ No newline at end of file diff --git a/odoo/addons/test_auth_custom/tests/__init__.py b/odoo/addons/test_auth_custom/tests/__init__.py new file mode 100644 index 0000000000000..15b04031b5063 --- /dev/null +++ b/odoo/addons/test_auth_custom/tests/__init__.py @@ -0,0 +1 @@ +from . import test_endpoints \ No newline at end of file diff --git a/odoo/addons/test_auth_custom/tests/test_endpoints.py b/odoo/addons/test_auth_custom/tests/test_endpoints.py new file mode 100644 index 0000000000000..6401d82f65140 --- /dev/null +++ b/odoo/addons/test_auth_custom/tests/test_endpoints.py @@ -0,0 +1,46 @@ +from http import HTTPStatus + +import odoo.tools +from odoo.tests import HttpCase, HOST + + +class TestCustomAuth(HttpCase): + # suppress "WARNING: Access Error" when auth fails on json endpoints + @odoo.tools.mute_logger('odoo.http') + def test_json(self): + # straight request should fail + r = self.url_open('/test_auth_custom/json', headers={'Content-Type': 'application/json'}, data="{}") + e = r.json()['error'] + self.assertEqual(e['data']['name'], 'odoo.exceptions.AccessDenied') + + # but preflight should work + self.env['base'].flush() + url = "http://%s:%s/test_auth_custom/json" % (HOST, odoo.tools.config['http_port']) + r = self.opener.options(url, headers={ + 'Origin': 'localhost', + 'Access-Control-Request-Method': 'QUX', + 'Access-Control-Request-Headers': 'XYZ', + }) + self.assertTrue(r.ok) + self.assertEqual(r.headers['Access-Control-Allow-Origin'], '*') + self.assertEqual(r.headers['Access-Control-Allow-Methods'], 'POST', "json is always POST") + self.assertNotIn('XYZ', r.headers['Access-Control-Allow-Headers'], "headers are ignored") + + def test_http(self): + # straight request should fail + r = self.url_open('/test_auth_custom/http') + self.assertEqual(r.status_code, HTTPStatus.FORBIDDEN) + + # but preflight should work + self.env['base'].flush() + url = "http://%s:%s/test_auth_custom/http" % (HOST, odoo.tools.config['http_port']) + r = self.opener.options(url, headers={ + 'Origin': 'localhost', + 'Access-Control-Request-Method': 'QUX', + 'Access-Control-Request-Headers': 'XYZ', + }) + self.assertTrue(r.ok, r.text) + self.assertEqual(r.headers['Access-Control-Allow-Origin'], '*') + self.assertEqual(r.headers['Access-Control-Allow-Methods'], 'GET, OPTIONS', + "http is whatever's on the endpoint") + self.assertNotIn('XYZ', r.headers['Access-Control-Allow-Headers'], "headers are ignored") \ No newline at end of file diff --git a/odoo/http.py b/odoo/http.py index 67c7a883a8e0c..c3a5e6393788e 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -309,6 +309,9 @@ def _handle_exception(self, exception): # otherwise "no active exception to reraise" raise pycompat.reraise(type(exception), exception, sys.exc_info()[2]) + def _is_cors_preflight(self, endpoint): + return request.httprequest.method == 'OPTIONS' and endpoint and endpoint.routing.get('cors') + def _call_function(self, *args, **kwargs): request = self if self.endpoint.routing['type'] != self._request_type: @@ -765,11 +768,14 @@ def _handle_exception(self, exception): return e def dispatch(self): - if request.httprequest.method == 'OPTIONS' and request.endpoint and request.endpoint.routing.get('cors'): + if self._is_cors_preflight(request.endpoint): headers = { 'Access-Control-Max-Age': 60 * 60 * 24, - 'Access-Control-Allow-Headers': 'Origin, X-Requested-With, Content-Type, Accept' + 'Access-Control-Allow-Headers': 'Origin, X-Requested-With, Content-Type, Accept, Authorization' } + cors = request.endpoint.routing.get('cors', False) + if cors: + headers['Access-Control-Allow-Origin'] = cors return Response(status=200, headers=headers) if request.httprequest.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE') \ @@ -1233,12 +1239,7 @@ def set_default(self, template=None, qcontext=None, uid=None): # Support for Cross-Origin Resource Sharing if request.endpoint and 'cors' in request.endpoint.routing: self.headers.set('Access-Control-Allow-Origin', request.endpoint.routing['cors']) - methods = 'GET, POST' - if request.endpoint.routing['type'] == 'json': - methods = 'POST' - elif request.endpoint.routing.get('methods'): - methods = ', '.join(request.endpoint.routing['methods']) - self.headers.set('Access-Control-Allow-Methods', methods) + self.headers.set('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE, OPTIONS') @property def is_qweb(self):