-
Notifications
You must be signed in to change notification settings - Fork 5.4k
jwt_authn: write JWT payload to dynamic metadata #4698
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,11 +48,21 @@ class ContextImpl : public Verifier::Context { | |
| // Stores an authenticator object for this request. | ||
| void storeAuth(AuthenticatorPtr&& auth) { auths_.emplace_back(std::move(auth)); } | ||
|
|
||
| // Add a pair of (issuer, payload), called by Authenticator | ||
| void addPaylod(const std::string& issuer, const std::string& payload) { | ||
|
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. addPayload |
||
| payload_pairs_.push_back({issuer, payload}); | ||
| } | ||
|
|
||
| bool hasPayload() const { return !payload_pairs_.empty(); } | ||
|
|
||
| ProtobufWkt::Struct getPayload() const { return MessageUtil::stringPairStruct(payload_pairs_); } | ||
|
|
||
| private: | ||
| Http::HeaderMap& headers_; | ||
| Verifier::Callbacks* callback_; | ||
| std::unordered_map<const Verifier*, CompletionState> completion_states_; | ||
| std::vector<AuthenticatorPtr> auths_; | ||
| std::vector<std::pair<std::string, std::string>> payload_pairs_; | ||
|
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. Just use ProbotufWkt::Struct here?
Contributor
Author
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. I am looking for a utility code to convert JSON to protobuf::Struct. Do you know any?
Contributor
Author
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. How about just use MessageUtil::loadFromJson() to convert it to Struct. Not sure what it will look like? |
||
| }; | ||
|
|
||
| // base verifier for provider_name, provider_and_audiences, and allow_missing_or_failed. | ||
|
|
@@ -65,6 +75,10 @@ class BaseVerifierImpl : public Verifier { | |
| return parent_->onComplete(status, context); | ||
| } | ||
|
|
||
| if (Status::Ok == status && context.hasPayload()) { | ||
| context.callback()->setPayload(context.getPayload()); | ||
| } | ||
|
|
||
| context.callback()->onComplete(status); | ||
| context.cancel(); | ||
| } | ||
|
|
@@ -97,6 +111,9 @@ class ProviderVerifierImpl : public BaseVerifierImpl { | |
| auto auth = auth_factory_.create(getAudienceChecker(), provider_name_, false); | ||
| extractor_->sanitizePayloadHeaders(ctximpl.headers()); | ||
| auth->verify(ctximpl.headers(), extractor_->extract(ctximpl.headers()), | ||
| [&ctximpl](const std::string& issuer, const std::string& payload) { | ||
| ctximpl.addPaylod(issuer, payload); | ||
| }, | ||
| [this, context](const Status& status) { | ||
| onComplete(status, static_cast<ContextImpl&>(*context)); | ||
| }); | ||
|
|
@@ -143,6 +160,9 @@ class AllowFailedVerifierImpl : public BaseVerifierImpl { | |
| auto auth = auth_factory_.create(nullptr, absl::nullopt, true); | ||
| extractor_.sanitizePayloadHeaders(ctximpl.headers()); | ||
| auth->verify(ctximpl.headers(), extractor_.extract(ctximpl.headers()), | ||
| [&ctximpl](const std::string& issuer, const std::string& payload) { | ||
| ctximpl.addPaylod(issuer, payload); | ||
| }, | ||
| [this, context](const Status& status) { | ||
| onComplete(status, static_cast<ContextImpl&>(*context)); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set the non-decoded version? It save a call to base64 decode in the consumer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually can we parse the JSON payload into a ProtobufWkt::Struct? otherwise you won't be able to match each claim in RBAC.
Also rather than issuer as key, provider_name would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For allow_missing_or_fail rule, it doesn't need to specify provider, so there is not provider_name. In theory, there could be two ProviderInfo with same same issuer. It is a useful feature for normal cases. But for allow_missing_or_fail case, we choose the first one matched with the issuer from the JWT token, it is better not to have two providers points to the same issuer in the config.
Overall, writing issuer works for all cases, but not provider name.