Skip to content

fix: missing headers return 400 bad request #26

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

Merged
merged 1 commit into from
Nov 4, 2022
Merged

fix: missing headers return 400 bad request #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/pull-request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- name: Set up Python 3
uses: actions/setup-python@v2
with:
python-version: 3.X
python-version: "3.10"
Copy link
Author

@pilgd-aws pilgd-aws Nov 3, 2022

Choose a reason for hiding this comment

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

This is to avoid a module 'asyncio' has no attribute 'coroutine' error when using Python 3.11 to build. The previous #25 used 3.10 and was successfully deployed.


- name: Install dependencies
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-and-publish.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- name: Set up Python 3
uses: actions/setup-python@v2
with:
python-version: 3.X
python-version: "3.10"

- name: Install dependencies
run: |
Expand Down
9 changes: 6 additions & 3 deletions aws_jupyter_proxy/awsproxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,12 @@ def _get_signed_canonical_headers(self) -> Tuple[str, str]:
canonical_headers = {}

for signed_header in self.upstream_auth_info.signed_headers:
canonical_headers[signed_header] = self.upstream_request.headers[
signed_header
]
try:
canonical_headers[signed_header] = self.upstream_request.headers[
signed_header
]
except KeyError:
raise HTTPError(400, message=f"Bad Request")
Copy link
Contributor

@declanvk declanvk Nov 4, 2022

Choose a reason for hiding this comment

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

Can we provide more information about the failure that happened, like the fact that the Authorization header had expected the presence of other headers and which header was missing?

Would that be too much information or would it be a security issue to provide?

Copy link
Author

@pilgd-aws pilgd-aws Nov 4, 2022

Choose a reason for hiding this comment

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

We don't expect a keyerror unless it is intentionally malformed request so we chose to return a generic 400. We don't expect customers to malform request, so this is fine

Copy link
Contributor

@declanvk declanvk Nov 4, 2022

Choose a reason for hiding this comment

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

Who is generating the Authorization header attached to the request?

Copy link
Author

@pilgd-aws pilgd-aws Nov 4, 2022

Choose a reason for hiding this comment

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

Tracking improvements of better error message with header specification in https://t.corp.amazon.com/P74816551

Copy link
Contributor

@declanvk declanvk Nov 4, 2022

Choose a reason for hiding this comment

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

Discussed with Daniel on a call, this will be taken as a followup item to either:

  1. Add a log statement which captures which header was missing for the customer
  2. Update the error message to communicate which header was expected and not present


base_service_url = urlparse(self.service_info.endpoint_url)
canonical_headers["host"] = base_service_url.netloc
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

setuptools.setup(
name="aws_jupyter_proxy",
version="0.3.5",
version="0.3.6",
url="https://github.com/aws/aws-jupyter-proxy",
author="Amazon Web Services",
description="A Jupyter server extension to proxy requests with AWS SigV4 authentication",
Expand Down