-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add 'WWW-Authenticate' header for reponses w/ status code 401 Unauthorized. #588
Conversation
Lokks ike the tests are failing on master too? |
@@ -61,6 +61,8 @@ local function send_response(status_code) | |||
ngx.log(ngx.ERR, tostring(content)) | |||
end | |||
ngx.ctx.stop_phases = true -- interrupt other phases of this request | |||
elseif status_code == _M.status_codes.HTTP_UNAUTHORIZED then | |||
ngx.header["WWW-Authenticate"] = "Key realm=\""..constants.NAME.."\"" -- respond with a challenge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cannot be hard-coded here since 401 is also returned for Basic authentication. This should probably be handled in the access.lua file of authentication plugins. Feel free to disagree or update the PR, otherwise I'll change it myself, but we appreciate contributions so... :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thibaultcha definitely, I'm so new to lua/kong land. Just made this PR after 30 mins of going through code, I'll update the PR soon, thanks so much for pointing in the right direction!
9fb994a
to
c07893e
Compare
@thibaultcha let me know if it looks alright now. |
@thibaultcha removing the header check, not sure how to get headers from |
On mobile now, but the headers are the third returned argument I think. It is used many times in the tests. Otherwise this looks good, just a test on each would be amazing. |
@thibaultcha looks like the table returned from |
Only because it ignores the third one
|
@thibaultcha alright done, let me know if all looks good and I'll squash my commits to just one. |
@thibaultcha ping! |
@ybv looks good to me, can you squash? |
e9deaa8
to
3f67edc
Compare
@thefosk @thibaultcha done! |
Thank you! |
Add 'WWW-Authenticate' header for reponses w/ status code 401 Unauthorized.
Adds a new header 'WWW-Authenticate' for 401 responses, the header value typically should include a challenge with some information about the authentication scheme (key-auth) #587