From 59ccf2a3bf894b75cf0b6981776a1f88b474ec1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20S=C3=B8dring?= Date: Tue, 18 Jun 2024 07:59:16 +0200 Subject: [PATCH] Adjust CORS related tests, add HEADERs with OPTION calls required by CORS. It has been unclear if the CORS / Allows logic in runtest is correct. At some some stage the understanding in the API spec with regards to CORS resulted in a misinterpreted implementation. This has been partially discussed in https://github.com/petterreinholdtsen/noark5-tester/issues/27 Other links relevantfor this change: https://www.baeldung.com/cs/cors-preflight-requests https://www.baeldung.com/spring-security-cors-preflight https://fetch.spec.whatwg.org/ A CORS response should include a 'Access-Control-Allow-Methods' and a 'Access-Control-Allow-Origin'. Others may be relevant headers may be relevant but these two are a minimum A preflight is discussed here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONS#preflighted_requests_in_cors In this branch we have not dealt with 'Access-Control-Request-Headers'. --- lib/n5core/endpoint.py | 14 +++++++++-- runtest | 57 +++++++++++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/lib/n5core/endpoint.py b/lib/n5core/endpoint.py index 97a21fc..948ecea 100644 --- a/lib/n5core/endpoint.py +++ b/lib/n5core/endpoint.py @@ -241,13 +241,23 @@ def json_get(self, path): content = content.decode("UTF-8") return (content, response) - def options(self, path): + def cors_options(self, path, headers=None, method=None): + if headers is None: + headers = {} + if method is None: + method = 'POST' + + headers['Access-Control-Request-Method'] = method + headers['Origin'] = 'Origin: http://localhost:3000' + return self.options(path, headers, method) + + def options(self, path, headers, method): url = self.expandurl(path) purl = urlparse(url) url = purl._replace(query=urllib .parse.quote_plus(purl.query)).geturl() opener = urllib.request.build_opener(urllib.request.HTTPHandler) - request = urllib.request.Request(url) + request = urllib.request.Request(url, None, headers) request.get_method = lambda: 'OPTIONS' response = opener.open(request) content = response.read() diff --git a/runtest b/runtest index b2b2c4b..4565354 100755 --- a/runtest +++ b/runtest @@ -476,16 +476,26 @@ class Noark5Tester (n5core.endpoint.Endpoint): self.urls.append(url) try: # Check if CORS can work for the URL - (ocontent, ores) = self.options(url) - allow = [] - allowstr = ores.getheader('Allow') - if allowstr: - allow = allowstr.split(',') - self.verify(-1 != allow.index('GET'), 'OPTIONS header Allow should include GET for %s' % url) + (ocontent, ores) = self.cors_options(url, method='GET') + allow_origin = ores.getheader('Access-Control-Allow-Origin') + if allow_origin: + self.verify(True, 'OPTIONS (CORS) header Access-Control-Allow-Origin is present for %s' % url) else: - self.failure("OPTIONS for URL %s missing Allow header" % url) + self.failure("OPTIONS (CORS) for URL %s missing Access-Control-Allow-Origin header" % url) + allow_methods = ores.getheader('Access-Control-Allow-Methods') + if allow_methods: + result = False + if '*' in allow_methods: + result = True + else: + allow = allow_methods.split(',') + result = 'GET' in allow + self.verify(result, 'OPTIONS (CORS) header Access-Control-Allow-Methods should include GET or (*) for %s' % url) + else: + self.failure("OPTIONS (CORS) for URL %s missing Access-Control-Allow-Methods header" % url) + except HTTPError as e: - self.failure("OPTIONS failed for URL %s: %s (%s)" % (url, str(e), e.read())) + self.failure("OPTIONS (CORS) failed for URL %s: %s (%s)" % (url, str(e), e.read())) except HTTPError as e: if self.isknownmissing(url): self.xfailure("unable to GET %s" % url) @@ -519,7 +529,7 @@ Test basis requirements for NOARK 5 Core. # Verify CORS support, # https://en.wikipedia.org/wiki/Cross-origin_resource_sharing try: - (ocontent, ores) = self.options('.') + (ocontent, ores) = self.cors_options('.') self.success("level 0 CORS - HTTP OPTIONS request worked") except HTTPError as e: self.failure("level 0 CORS - HTTP OPTIONS request not working") @@ -583,17 +593,30 @@ Test basis requirements for NOARK 5 Core. self.verify(etag is None, "no ETag from GET on %s" % url) try: # Verify OPTIONS announce POST support - (ocontent, ores) = self.options(url) + (ocontent, ores) = self.cors_options(url, method='POST') allow = [] - allowstr = ores.getheader('Allow') - if allowstr: - allow = allowstr.split(',') - verify('POST' in allow, 'OPTIONS header Allow should include POST for %s' % url) - verify('GET' in allow, 'OPTIONS header Allow should include GET for %s' % url) + allow_origin = ores.getheader('Access-Control-Allow-Methods') + if allow_origin: + allow = allow_origin.split(',') + verify('POST' in allow, 'OPTIONS (CORS) header Access-Control-Allow-Origin should include POST for %s' % url) + verify('GET' in allow, 'OPTIONS (CORS) header Access-Control-Allow-Origin should be present for %s' % url) else: - failure("OPTIONS for URL %s missing Allow header" % url) + failure("OPTIONS (CORS) for URL %s missing Access-Control-Allow-Origin header" % url) + + allow_methods = ores.getheader('Access-Control-Allow-Methods') + if allow_methods: + if '*' in allow_methods: + verify(True, 'OPTIONS (CORS) header Access-Control-Allow-Methods should include POST or (*) for %s' % url) + verify(True, 'OPTIONS (CORS) header Access-Control-Allow-Methods should include GET or (*) for %s' % url) + else: + allow = allow_methods.split(',') + verify('POST' in allow, 'OPTIONS (CORS) header Access-Control-Allow-Methods should include POST or (*) for %s' % url) + verify('GET' in allow, 'OPTIONS (CORS) header Access-Control-Allow-Methods should include GET or (*) for %s' % url) + else: + self.failure("OPTIONS (CORS) for URL %s missing Access-Control-Allow-Methods header" % url) + except HTTPError as e: - failure('unable to OPTIONS %s: %s' % (url, e.read())) + failure('unable to OPTIONS (CORS) %s: %s' % (url, e.read())) except HTTPError as e: failure('unable to GET %s: %s' % (url, e.read())) print("POST %s: %s" % (url, data))