-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add Jwt cache. #14341
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 Jwt cache. #14341
Changes from 57 commits
4bf0213
07e3923
e89cf2c
6c5e822
23d6b6b
0d70dd7
433e4c8
35383c3
4d92197
6d9cbac
4d27b57
742220e
883de98
ac891a6
d69df58
eacfbc3
1335cf6
c8efe0e
9875c74
26b94cb
11e4057
1f36422
8964983
8a3cb9c
a2a31e5
a52ec67
bb42b8d
04335c7
cfcd530
f33850d
e11c695
c18856c
b986895
3460ec0
ef3ad74
3b95eee
ca3cbee
3d47511
e13209e
b96dc33
8a175d5
9419ddf
7ac42c0
07d45bb
d0e83f2
3f7c0c4
45d3132
13cb2b5
f5fb8a4
3e7768d
17fe231
5372efd
a1d3c72
4f7caeb
e4ceaac
783c207
a583ba7
1de642f
45f7298
fd2f636
a23bf86
4989b45
25f366a
efb4bb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,9 @@ class AuthenticatorImpl : public Logger::Loggable<Logger::Id::jwt>, | |
| // Verify with a specific public key. | ||
| void verifyKey(); | ||
|
|
||
| // Handle Good Jwt either Cache JWT or verified public key. | ||
| void handleGoodJwt(bool cache_hit); | ||
|
|
||
| // Calls the callback with status. | ||
| void doneWithStatus(const Status& status); | ||
|
|
||
|
|
@@ -79,10 +82,9 @@ class AuthenticatorImpl : public Logger::Loggable<Logger::Id::jwt>, | |
| std::vector<JwtLocationConstPtr> tokens_; | ||
| JwtLocationConstPtr curr_token_; | ||
| // The JWT object. | ||
| std::unique_ptr<::google::jwt_verify::Jwt> jwt_; | ||
| std::unique_ptr<::google::jwt_verify::Jwt> new_jwt_; | ||
|
wrowe marked this conversation as resolved.
Outdated
|
||
| // The JWKS data object | ||
| JwksCache::JwksData* jwks_data_{}; | ||
|
|
||
| // The HTTP request headers | ||
| Http::HeaderMap* headers_{}; | ||
| // The active span for the request | ||
|
|
@@ -98,6 +100,7 @@ class AuthenticatorImpl : public Logger::Loggable<Logger::Id::jwt>, | |
| const bool is_allow_failed_; | ||
| const bool is_allow_missing_; | ||
| TimeSource& time_source_; | ||
| ::google::jwt_verify::Jwt* jwt_{}; | ||
| }; | ||
|
|
||
| std::string AuthenticatorImpl::name() const { | ||
|
|
@@ -139,9 +142,20 @@ void AuthenticatorImpl::startVerify() { | |
| curr_token_ = std::move(tokens_.back()); | ||
| tokens_.pop_back(); | ||
|
|
||
| jwt_ = std::make_unique<::google::jwt_verify::Jwt>(); | ||
| if (provider_) { | ||
|
wrowe marked this conversation as resolved.
Outdated
|
||
| jwks_data_ = jwks_cache_.findByProvider(provider_.value()); | ||
| jwt_ = jwks_data_->getJwtCache().lookup(curr_token_->token()); | ||
| if (jwt_) { | ||
|
wrowe marked this conversation as resolved.
Outdated
|
||
| handleGoodJwt(/*cache_hit=*/true); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| ENVOY_LOG(debug, "{}: Parse Jwt {}", name(), curr_token_->token()); | ||
| Status status = jwt_->parseFromString(curr_token_->token()); | ||
| new_jwt_ = std::make_unique<::google::jwt_verify::Jwt>(); | ||
| Status status = new_jwt_->parseFromString(curr_token_->token()); | ||
| jwt_ = new_jwt_.get(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the value of Also is it possible to move
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't cache bad jwt. it is too early to move new_jwt_ to cache here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if status is not OK, new_jet_ has partial data. but jwt_ SHOULD not be used any way. but checking status, not to assign is better. |
||
|
|
||
|
mk46 marked this conversation as resolved.
|
||
| if (status != Status::Ok) { | ||
| doneWithStatus(status); | ||
| return; | ||
|
|
@@ -156,9 +170,10 @@ void AuthenticatorImpl::startVerify() { | |
| } | ||
| } | ||
|
|
||
| // Check the issuer is configured or not. | ||
| jwks_data_ = provider_ ? jwks_cache_.findByProvider(provider_.value()) | ||
| : jwks_cache_.findByIssuer(jwt_->iss_); | ||
| // Issuer is configured | ||
| if (!provider_) { | ||
| jwks_data_ = jwks_cache_.findByIssuer(jwt_->iss_); | ||
| } | ||
| // When `provider` is valid, findByProvider should never return nullptr. | ||
| // Only when `allow_missing` or `allow_failed` is used, `provider` is invalid, | ||
| // and this authenticator is checking tokens from all providers. In this case, | ||
|
|
@@ -199,6 +214,7 @@ void AuthenticatorImpl::startVerify() { | |
| // the key cached, if we do proceed to verify else try a new JWKS retrieval. | ||
| // JWTs without a kid header field in the JWS we might be best to get each | ||
| // time? This all only matters for remote JWKS. | ||
|
|
||
| verifyKey(); | ||
| return; | ||
| } | ||
|
|
@@ -245,11 +261,15 @@ void AuthenticatorImpl::onDestroy() { | |
| void AuthenticatorImpl::verifyKey() { | ||
| const Status status = | ||
| ::google::jwt_verify::verifyJwtWithoutTimeChecking(*jwt_, *jwks_data_->getJwksObj()); | ||
|
|
||
| if (status != Status::Ok) { | ||
| doneWithStatus(status); | ||
| return; | ||
| } | ||
| handleGoodJwt(/*cache_hit=*/false); | ||
| } | ||
|
|
||
| void AuthenticatorImpl::handleGoodJwt(bool cache_hit) { | ||
| // Forward the payload | ||
| const auto& provider = jwks_data_->getJwtProvider(); | ||
| if (!provider.forward_payload_header().empty()) { | ||
|
|
@@ -265,7 +285,10 @@ void AuthenticatorImpl::verifyKey() { | |
| if (set_payload_cb_ && !provider.payload_in_metadata().empty()) { | ||
| set_payload_cb_(provider.payload_in_metadata(), jwt_->payload_pb_); | ||
| } | ||
|
|
||
| if (provider_ && !cache_hit) { | ||
| // move the ownership of "new_jwt_" into the function. | ||
| jwks_data_->getJwtCache().insert(curr_token_->token(), std::move(new_jwt_)); | ||
| } | ||
| doneWithStatus(Status::Ok); | ||
| } | ||
|
|
||
|
|
@@ -292,6 +315,7 @@ void AuthenticatorImpl::doneWithStatus(const Status& status) { | |
| callback_ = nullptr; | ||
| return; | ||
| } | ||
|
|
||
| startVerify(); | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.