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.

Sign up for GitHub

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

Jump to bottom

Solve last CORS issues about duplicated headers #604

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Solve last CORS issues about duplicated headers #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f524e7c
Fix: Solve last CORS errors raised cause by duplication of headers re…
nesitor Apr 24, 2024
ed12122
Fix: Solve code quality issues.
nesitor Apr 24, 2024
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
4 changes: 3 additions & 1 deletion src/aleph/vm/orchestrator/resources.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pydantic import BaseModel, Field

from aleph.vm.conf import settings
from aleph.vm.utils import cors_allow_all


class Period(BaseModel):
Expand Down Expand Up @@ -92,6 +93,7 @@
)


@cors_allow_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the use of this decorator, not just here but in general, since a global cors config was added in supervisor.py?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a custom decorator that handles the CORS using this library: https://github.com/aio-libs/aiohttp-cors

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I have read the decorator code, but you realise it add the same cors config that you also added in supervisor.py in your previous PR? Doesn't that seems a bit strange to you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is a public endpoint used by the frontend, our frontend or other ones, so the endpoint should accept the same configuration as the other endpoints. As the decorator name says, allow all origins on CORS.

async def about_system_usage(_: web.Request):
"""Public endpoint to expose information about the system usage."""
period_start = datetime.now(timezone.utc).replace(second=0, microsecond=0)
Expand All @@ -116,7 +118,7 @@
),
properties=get_machine_properties(),
)
return web.json_response(text=usage.json(exclude_none=True), headers={"Access-Control-Allow-Origin:": "*"})
return web.json_response(text=usage.json(exclude_none=True))

Check warning on line 121 in src/aleph/vm/orchestrator/resources.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/resources.py#L121 

Added line #L121 was not covered by tests


class Allocation(BaseModel):
Expand Down
13 changes: 0 additions & 13 deletions src/aleph/vm/orchestrator/supervisor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,6 @@ async def server_version_middleware(
return resp


async def allow_cors_on_endpoint(request: web.Request):
"""Allow CORS on endpoints that VM owners use to control their machine."""
return web.Response(
status=200,
headers={
"Access-Control-Allow-Headers": "*",
"Access-Control-Allow-Methods": "*",
"Access-Control-Allow-Origin": "*",
"Allow": "POST",
},
)


async def http_not_found(request: web.Request):
"""Return a 404 error for unknown URLs."""
return web.HTTPNotFound()
Expand Down
18 changes: 5 additions & 13 deletions src/aleph/vm/orchestrator/views/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,9 @@
# "ipv6": await status.check_ipv6(session),
}

return web.json_response(
result, status=200 if all(result.values()) else 503, headers={"Access-Control-Allow-Origin": "*"}
)
return web.json_response(result, status=200 if all(result.values()) else 503)

Check warning on line 217 in src/aleph/vm/orchestrator/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/views/__init__.py#L217 

Added line #L217 was not covered by tests
except aiohttp.ServerDisconnectedError as error:
return web.json_response(
{"error": f"Server disconnected: {error}"}, status=503, headers={"Access-Control-Allow-Origin": "*"}
)
return web.json_response({"error": f"Server disconnected: {error}"}, status=503)

Check warning on line 219 in src/aleph/vm/orchestrator/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/views/__init__.py#L219 

Added line #L219 was not covered by tests


@cors_allow_all
Expand All @@ -246,7 +242,7 @@
},
}
result_status = 200 if all(result["ipv4"].values()) and all(result["ipv6"].values()) else 503
return web.json_response(result, status=result_status, headers={"Access-Control-Allow-Origin": "*"})
return web.json_response(result, status=result_status)

Check warning on line 245 in src/aleph/vm/orchestrator/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/views/__init__.py#L245 

Added line #L245 was not covered by tests


@cors_allow_all
Expand All @@ -260,7 +256,7 @@
vm_ipv6 = False

result = {"host": await check_host_egress_ipv6(), "vm": vm_ipv6}
return web.json_response(result, headers={"Access-Control-Allow-Origin": "*"})
return web.json_response(result)

Check warning on line 259 in src/aleph/vm/orchestrator/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/views/__init__.py#L259 

Added line #L259 was not covered by tests


@cors_allow_all
Expand All @@ -283,7 +279,6 @@
return web.Response(
status=200,
text=f"Up-to-date: version {current} >= {reference}",
headers={"Access-Control-Allow-Origin": "*"},
)
else:
return web.HTTPForbidden(text=f"Outdated: version {current} < {reference}")
Expand Down Expand Up @@ -327,7 +322,6 @@
},
},
dumps=dumps_for_json,
headers={"Access-Control-Allow-Origin": "*"},
)


Expand Down Expand Up @@ -436,9 +430,7 @@
except JSONDecodeError:
return web.HTTPBadRequest(reason="Body is not valid JSON")
except ValidationError as error:
return web.json_response(
data=error.json(), status=web.HTTPBadRequest.status_code, headers={"Access-Control-Allow-Origin": "*"}
)
return web.json_response(data=error.json(), status=web.HTTPBadRequest.status_code)

Check warning on line 433 in src/aleph/vm/orchestrator/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aleph/vm/orchestrator/views/__init__.py#L433 

Added line #L433 was not covered by tests

pubsub: PubSub = request.app["pubsub"]
pool: VmPool = request.app["vm_pool"]
Expand Down
2 changes: 0 additions & 2 deletions src/aleph/vm/orchestrator/views/authentication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,6 @@ async def wrapper(request):
return web.json_response(data={"error": e.reason}, status=e.status)

response = await handler(request, authenticated_sender)
# Allow browser clients to access the body of the response
response.headers.update({"Access-Control-Allow-Origin": request.headers.get("Origin", "")})
return response

return wrapper