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

Make Authorization header checks tighter #849

Merged
merged 9 commits into from
May 9, 2023

Conversation

alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Apr 27, 2023

Include trailing space in checks for Basic and Bearer prefixes in Authorization headers. This space is already assumed to exist in subsequent parsing code.

An extra check is added for requests which provide the Authorization header, but in an unrecognised format. This ensures that a 401 header is still sent in preference to 404, as per #329. It also changes the response code for unsupported auth-schemes when they do not start with either Bearer or Basic.

@alxndrsn alxndrsn force-pushed the more-explicit-auth-header-checks branch from 9ba6ca2 to c024af1 Compare April 27, 2023 07:19
@alxndrsn alxndrsn marked this pull request as ready for review April 27, 2023 08:57
@matthew-white
Copy link
Member

It looks like the branch has a conflict. @alxndrsn, are you able to merge this PR sometime this week? We're hoping to begin QA for .3 next week.

alxndrsn added 2 commits May 9, 2023 04:28
Include trailing space in checks for Basic and Bearer prefixes in
Authorization headers.  This space is already assumed to exist in
subsequent parsing code.

An extra check is added for requests which _provide_ the Authorization
header, but in an unrecognised format.  This ensures that a 401 header
is still sent in preference to 404, as per getodk#329.
@alxndrsn alxndrsn force-pushed the more-explicit-auth-header-checks branch from 18cba8f to cf8a291 Compare May 9, 2023 04:32
@alxndrsn alxndrsn merged commit b9d5e4c into getodk:master May 9, 2023
@alxndrsn alxndrsn deleted the more-explicit-auth-header-checks branch May 9, 2023 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

3 participants