jwt_authn: make from_cookies JWT removal behaviour similar to from_params #17985
Conversation
…rams Removal of params or cookies after authentication is not implemented as of today. authenticator.cc calls the removeJwt(...) if forward is set to false (default) and this leads to an assertion failures caused by NOT_IMPLEMENTED_GCOVR_EXCL_LINE. Changed removeJwt(...) for JwtCookieLocation to be empty, added test coverage and updated proto doc to call-out this caveat. Signed-off-by: Shubham Patil <theshubhamp@gmail.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/lgtm api |
|
cc @lizan, PTAL whenever you can! |
…drop-cookie-remove-panic Signed-off-by: Shubham Patil <theshubhamp@gmail.com>
|
Ping! Can someone review this PR ? |
|
First commit is good. The second one after the merge seems getting a lot of unrelated changes. Please fix it. |
|
Thanks for taking a look!
The second commit shows unrelated changes because it is a merge commit. I had to merge Only the 3 files changed in the PR should end up on main as a single commit after merge. |
|
LGTM |
jwt_authn: make from_cookies JWT removal behaviour similar to from_params
Removal of params or cookies after authentication is not implemented as of today.
authenticator.cc calls the removeJwt(...) if forward is set to false (default)
and this leads to an assertion failures caused by NOT_IMPLEMENTED_GCOVR_EXCL_LINE.
Changed removeJwt(...) for JwtCookieLocation to be empty, added test coverage
and updated proto doc to call-out this caveat.
Risk Level: Low
Testing: Tests
Docs Changes: Added caveat to proto docs.
Release Notes: None.
Platform Specific Features: None
Follow up for #17424
Signed-off-by: Shubham Patil theshubhamp@gmail.com