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

performBasicAuth: plz separate "Authorization" header read code and other stuff #1449

Closed
denizzzka opened this Issue Mar 8, 2016 · 1 comment

Comments

Projects
None yet
2 participants
@denizzzka
Contributor

denizzzka commented Mar 8, 2016

Two performBasicAuth functions contain code duplication and also without separate header read function it isn't possible to check auth data without different response cases.

(Faced out with it because I am writing server what uses auth info as parameter, but response isn't depends from availability of auth info.)

@brad-anderson

This comment has been minimized.

Show comment
Hide comment
@brad-anderson

brad-anderson Mar 8, 2016

Contributor

To rephrase it a little based on the conversation we are having in IRC I believe what is being asked for is to support optional verification of an auth request. For example, only requiring an auth request based on other factors of the request (such as what parameters are given). So basically something like enforceHTTP(/*some criteria that makes it not require auth */ || checkAuth(req), HTTPStatus.forbidden);

Contributor

brad-anderson commented Mar 8, 2016

To rephrase it a little based on the conversation we are having in IRC I believe what is being asked for is to support optional verification of an auth request. For example, only requiring an auth request based on other factors of the request (such as what parameters are given). So basically something like enforceHTTP(/*some criteria that makes it not require auth */ || checkAuth(req), HTTPStatus.forbidden);

s-ludwig added a commit that referenced this issue Feb 19, 2017

Add checkBasicAuth function. Fixes #1449.
Separates the authorization test from auhtorization enforcement.

@s-ludwig s-ludwig closed this in #1687 Feb 21, 2017

s-ludwig added a commit that referenced this issue Feb 21, 2017

Merge pull request #1687 from rejectedsoftware/issue1449-basic-auth-s…
…eparate

Add checkBasicAuth function. Fixes #1449.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment