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

fix(basic-auth): add missing www-authenticate headers #11795

Merged

Conversation

nowNick
Copy link
Contributor

@nowNick nowNick commented Oct 19, 2023

Summary

When kong returns 401 Unauthorized response it should return WWW-Authenticate header with proper challenge. Basic auth was missing this header on some responses. Previously it only returned this header when Authorization or Proxy-Authorization was missing but the RFC states that it should return it for every 401 response. This PR also adds a possibility to configure a parameter - realm (defaults to service).

Related PRs:

RFCs & Materials

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • N/A There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

  • add WWW-Authenticate header to all basic-auth 401 response

Issue reference

@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from e451320 to de31747 Compare November 8, 2023 16:46
@pull-request-size pull-request-size bot added size/M and removed size/S labels Nov 8, 2023
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch 2 times, most recently from 6fb3c59 to 1aa4983 Compare November 8, 2023 17:23
@nowNick nowNick marked this pull request as ready for review November 8, 2023 17:23
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from 1aa4983 to ccd2a94 Compare November 8, 2023 17:35
@nowNick nowNick requested a review from kikito November 9, 2023 11:29
@kikito kikito force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from 56493e3 to cf4197d Compare November 13, 2023 13:24
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from cf4197d to f163e3a Compare November 14, 2023 16:28
Copy link
Member

@kikito kikito left a comment

Choose a reason for hiding this comment

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

Approved with a small comment

@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from f163e3a to b543952 Compare December 5, 2023 08:30
@nowNick nowNick requested a review from bungle December 5, 2023 13:35
@locao
Copy link
Contributor

locao commented Dec 19, 2023

@nowNick there are conflicts in this PR, can you check please?

@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from b543952 to abd13e4 Compare January 2, 2024 14:33
@nowNick nowNick marked this pull request as draft January 9, 2024 09:26
When server returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
Not all basic auth 401 responses had this header. It also allows
to configure the protected resource realm via plugin config.

Fix: #7772
KAG-321
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-basic-auth branch from 4a2974e to 54bdfb8 Compare January 11, 2024 14:08
@nowNick nowNick marked this pull request as ready for review January 11, 2024 15:19
@kikito kikito merged commit b04aa72 into master Jan 22, 2024
23 checks passed
@kikito kikito deleted the feat/implement-missing-www-authenticate-headers-basic-auth branch January 22, 2024 12:23
@nowNick nowNick added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 25, 2024
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-11795-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-11795-to-master-to-upstream
git checkout -b cherry-pick-11795-to-master-to-upstream
ancref=$(git merge-base 15d6f4cec8f6253ff73f157cb37d1a2cdce8cb94 54bdfb8494166ff357f55486b15e3965101597b6)
git cherry-pick -x $ancref..54bdfb8494166ff357f55486b15e3965101597b6

flrgh added a commit to Kong/go-kong that referenced this pull request Mar 26, 2024
A new `realm` field was recently added to the basic-auth plugin:

Kong/kong#11795

To address this I added version awareness to the test cases.
flrgh added a commit to Kong/go-kong that referenced this pull request Apr 1, 2024
A new `realm` field was recently added to the basic-auth plugin:

Kong/kong#11795

To address this I added version awareness to the test cases.
flrgh added a commit to Kong/go-kong that referenced this pull request Apr 1, 2024
A new `realm` field was recently added to the basic-auth plugin:

Kong/kong#11795

To address this I added version awareness to the test cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WWW-Authenticate header not present when Kong and plugins return HTTP status 401
5 participants