Skip to content

Commit

Permalink
jwt_authn: make from_cookies JWT removal behaviour similar to from_pa…
Browse files Browse the repository at this point in the history
…rams (#17985)

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>
  • Loading branch information
theshubhamp authored Sep 17, 2021
1 parent b963465 commit 5591dbc
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ message JwtProvider {

// If false, the JWT is removed in the request after a success verification. If true, the JWT is
// not removed in the request. Default value is false.
// caveat: only works for from_header & has no effect for JWTs extracted through from_params & from_cookies.
bool forward = 5;

// Two fields below define where to extract the JWT from an HTTP request.
Expand Down
1 change: 0 additions & 1 deletion source/extensions/filters/http/jwt_authn/extractor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ class JwtCookieLocation : public JwtLocationBase {

void removeJwt(Http::HeaderMap&) const override {
// TODO(theshubhamp): remove JWT from cookies.
NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
}
};

Expand Down
3 changes: 3 additions & 0 deletions test/extensions/filters/http/jwt_authn/extractor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -286,16 +286,19 @@ TEST_F(ExtractorTest, TestCookieToken) {
EXPECT_EQ(tokens[0]->token(), "token-cookie-value");
EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer9"));
EXPECT_FALSE(tokens[0]->isIssuerAllowed("issuer10"));
tokens[0]->removeJwt(headers);

// only issuer9 has specified "token-cookie-2" cookie location.
EXPECT_EQ(tokens[1]->token(), "token-cookie-value-2");
EXPECT_TRUE(tokens[1]->isIssuerAllowed("issuer9"));
EXPECT_FALSE(tokens[1]->isIssuerAllowed("issuer10"));
tokens[1]->removeJwt(headers);

// only issuer10 has specified "token-cookie-3" cookie location.
EXPECT_EQ(tokens[2]->token(), "token-cookie-value-3");
EXPECT_TRUE(tokens[2]->isIssuerAllowed("issuer10"));
EXPECT_FALSE(tokens[2]->isIssuerAllowed("issuer9"));
tokens[2]->removeJwt(headers);
}

// Test extracting multiple tokens.
Expand Down

0 comments on commit 5591dbc

Please sign in to comment.