Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR-5790 Add CORS preflight OPTIONS request #803

Merged
merged 2 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ def test_do_auth_and_return(mock_get_urs_url, monkeypatch):
assert response.headers == {"Location": "%2Fsome%2Fpath"}


def test_send_cors_header(current_request, monkeypatch):
def test_add_cors_headers(current_request, monkeypatch):
current_request.headers = {}
headers = {"foo": "bar"}
app.add_cors_headers(headers)
Expand Down Expand Up @@ -1093,6 +1093,41 @@ def test_try_download_head_error(
)


@mock.patch(f"{MODULE}.b_map", None)
def test_dynamic_url_options(current_request, monkeypatch):
monkeypatch.setenv("CORS_ORIGIN", "example.com")
current_request.headers = {
"origin": "example.com",
"access-control-request-method": "GET",
}

current_request.uri_params = {"proxy": "DATA-TYPE-1/PLATFORM-A/OBJECT_1"}

# Can't use the chalice test client here as it doesn't seem to understand the `{proxy+}` route
response = app.dynamic_url_options()

assert response.body == ""
assert response.status_code == 204
assert response.headers == {
"Access-Control-Allow-Origin": "example.com",
"Access-Control-Allow-Credentials": "true",
"Access-Control-Allow-Methods": "GET, HEAD, OPTIONS",
"Access-Control-Allow-Headers": "Authorization, Origin, X-Requested-With",
}


@mock.patch(f"{MODULE}.b_map", None)
def test_dynamic_url_options_error(current_request):
current_request.uri_params = {"proxy": "DATA-TYPE-1/PLATFORM-A/OBJECT_1"}

# Can't use the chalice test client here as it doesn't seem to understand the `{proxy+}` route
response = app.dynamic_url_options()

assert response.body == "Method Not Allowed"
assert response.status_code == 405
assert response.headers == {}


@mock.patch(f"{MODULE}.get_yaml_file", autospec=True)
@mock.patch(f"{MODULE}.try_download_head", autospec=True)
@mock.patch(f"{MODULE}.b_map", None)
Expand Down
65 changes: 61 additions & 4 deletions tests_e2e/test_cors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,73 @@ def test_cors(urls, auth_cookies):
origin_host = "https://something.asf.alaska.edu"

url = urls.join(urls.METADATA_FILE_CH)
origin_headers = {"origin": origin_host}
request_headers = {"origin": origin_host}

r = requests.get(url, cookies=auth_cookies, headers=origin_headers, allow_redirects=False)
r = requests.get(
url,
cookies=auth_cookies,
headers=request_headers,
allow_redirects=False,
)
headers = dict(r.headers)

assert headers.get("Access-Control-Allow-Origin") == origin_host
assert headers.get("Access-Control-Allow-Credentials") == "true"

headers = {"origin": "null"}
r = requests.get(url, cookies=auth_cookies, headers=headers, allow_redirects=False)

def test_cors_origin_null(urls, auth_cookies):
url = urls.join(urls.METADATA_FILE_CH)
request_headers = {"origin": "null"}
r = requests.get(
url,
cookies=auth_cookies,
headers=request_headers,
allow_redirects=False,
)
headers = dict(r.headers)

assert headers.get("Access-Control-Allow-Origin") == "null"


def test_cors_preflight_options(urls, auth_cookies):
origin_host = "https://something.asf.alaska.edu"

url = urls.join(urls.METADATA_FILE_CH)
request_headers = {
"Origin": origin_host,
"Access-Control-Request-Method": "GET"
}

r = requests.options(
url,
cookies=auth_cookies,
headers=request_headers,
allow_redirects=False,
)
headers = dict(r.headers)

assert r.status_code == 204
assert headers["Access-Control-Allow-Origin"] == origin_host
assert set(headers["Access-Control-Allow-Methods"].split(", ")) >= {"GET", "HEAD", "OPTIONS"}
assert set(headers["Access-Control-Allow-Headers"].split(", ")) >= {"Authorization", "Origin"}


def test_cors_preflight_options_origin_null(urls, auth_cookies):
url = urls.join(urls.METADATA_FILE_CH)
request_headers = {
"Origin": "null",
"Access-Control-Request-Method": "GET"
}

r = requests.options(
url,
cookies=auth_cookies,
headers=request_headers,
allow_redirects=False,
)
headers = dict(r.headers)

assert r.status_code == 204
assert headers["Access-Control-Allow-Origin"] == "null"
assert set(headers["Access-Control-Allow-Methods"].split(", ")) >= {"GET", "HEAD", "OPTIONS"}
assert set(headers["Access-Control-Allow-Headers"].split(", ")) >= {"Authorization", "Origin"}
78 changes: 68 additions & 10 deletions thin_egress_app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,17 +459,34 @@ def add_cors_headers(headers):

# send CORS headers if we're configured to use them
origin_header = app.current_request.headers.get("origin")
if origin_header is not None:
if is_cors_allowed():
headers["Access-Control-Allow-Origin"] = origin_header
headers["Access-Control-Allow-Credentials"] = "true"
else:
cors_origin = os.getenv("CORS_ORIGIN")
if cors_origin and (origin_header.endswith(cors_origin) or origin_header.lower() == "null"):
headers["Access-Control-Allow-Origin"] = origin_header
headers["Access-Control-Allow-Credentials"] = "true"
else:
log.warning(
"Origin %s is not an approved CORS host: %s",
origin_header,
cors_origin,
)
log.warning(
"Origin %s is not an approved CORS host: %s",
origin_header,
cors_origin,
)


def is_cors_allowed():
assert app.current_request is not None

# send CORS headers if we're configured to use them
origin_header = app.current_request.headers.get("origin")
cors_origin = os.getenv("CORS_ORIGIN")

log.debug("origin_header: %r, cors_origin: %r", origin_header, cors_origin)
return bool(
origin_header
and cors_origin
and (
origin_header.endswith(cors_origin)
or origin_header.lower() == "null"
)
)


@with_trace()
Expand Down Expand Up @@ -942,6 +959,47 @@ def try_download_head(bucket, filename):
return make_redirect(presigned_url, {}, 303)


@app.route("/{proxy+}", methods=["OPTIONS"])
@with_trace(context={})
def dynamic_url_options():
allowed_methods = [
"GET",
"HEAD",
"OPTIONS",
]
allowed_headers = [
"Authorization",
"Origin",
"X-Requested-With",
]
request_method = app.current_request.headers.get(
"access-control-request-method",
"",
).strip()
log.info("Received CORS preflight request for method: %r", request_method)

log.debug("is_cors_allowed: %s", is_cors_allowed())
log.debug("request_method in allowed_methods: %s", request_method in allowed_methods)
if is_cors_allowed() and request_method in allowed_methods:
headers = {
"Access-Control-Allow-Methods": ", ".join(allowed_methods),
"Access-Control-Allow-Headers": ", ".join(allowed_headers),
}
add_cors_headers(headers)
log.info("Returning success response")
return Response(
body="",
headers=headers,
status_code=204,
)

log.info("Returning error response")
return Response(
body="Method Not Allowed",
status_code=405,
)


# Attempt to validate HEAD request
@app.route("/{proxy+}", methods=["HEAD"])
@with_trace(context={})
Expand Down
Loading