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(jwt): add missing www-authenticate headers #11792

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. JWT auth was missing this header.

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 headxer to jwt 401 response

Issue reference

@brentos
Copy link
Contributor

brentos commented Oct 24, 2023

Should we also be setting realm and/or the error in the www-authenticate header? Example:

     WWW-Authenticate: Bearer realm="example",
                       error="invalid_token",
                       error_description="The access token expired"

@nowNick nowNick marked this pull request as draft October 31, 2023 09:34
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch from 10e6baa to 07e3f93 Compare April 30, 2024 15:11
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 30, 2024
@github-actions github-actions bot added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Apr 30, 2024
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch 3 times, most recently from 36df460 to f972259 Compare May 8, 2024 11:32
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch from f972259 to 8c71ea8 Compare May 8, 2024 14:17

local function do_authentication(conf)
local token, err = retrieve_tokens(conf)
if err then
return error(err)
end

local www_authenticate_base = conf.realm and fmt('Bearer realm="%s"', conf.realm) or 'Bearer'
local www_authenticate_with_error = www_authenticate_base .. ' error="invalid_token"'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The RFC6750 - The OAuth 2.0 Authorization Framework: Bearer Token Usage describes that:

All challenges defined by this specification MUST use the auth-scheme
value "Bearer". This scheme MUST be followed by one or more
auth-param values. The auth-param attributes used or defined by this
specification are as follows. Other auth-param attributes MAY be
used as well.

So we MUST pass at least one auth-param value. Therefore I added error="invalid_token". However it also states that for requests without JWT token at all it should not add error code.

If the request lacks any authentication information (e.g., the client
was unaware that authentication is necessary or attempted using an
unsupported authentication method), the resource server SHOULD NOT
include an error code or other error information.

So I need two variables: www_authenticate_base to return for requests without jwt token and www_authenticate_with_error.

@nowNick nowNick marked this pull request as ready for review May 8, 2024 15:26
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch 2 times, most recently from f4793ed to b558b3e Compare May 9, 2024 12:52
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch from b558b3e to 02956be Compare May 21, 2024 16:04
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch from 02956be to f739ecd Compare May 21, 2024 16:30
When serve returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
JWT auth was missing this header.

Fix: #7772
KAG-321
@nowNick nowNick force-pushed the feat/implement-missing-www-authenticate-headers-jwt branch from f739ecd to efe088e Compare June 17, 2024 13:41
@nowNick nowNick removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jun 18, 2024
@hanshuebner hanshuebner merged commit a3f5410 into master Jun 18, 2024
30 checks passed
@hanshuebner hanshuebner deleted the feat/implement-missing-www-authenticate-headers-jwt branch June 18, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
4 participants