From 6ca1fe92b5b176a43ae7631d36a01baf4b6d8b75 Mon Sep 17 00:00:00 2001 From: Lei Tang <32078630+lei-tang@users.noreply.github.com> Date: Mon, 6 Aug 2018 18:53:45 -0700 Subject: [PATCH 1/7] Add the groups JWT claims to the attribute request.auth.groups --- include/istio/utils/attribute_names.h | 1 + src/envoy/http/authn/authn_utils.cc | 29 +++++++++++++++++++++++++++ src/envoy/utils/authn.cc | 16 +++++++++++++++ src/istio/authn/context.proto | 4 ++++ src/istio/utils/attribute_names.cc | 1 + 5 files changed, 51 insertions(+) diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h index 462f957ed46..ed93247efd6 100644 --- a/include/istio/utils/attribute_names.h +++ b/include/istio/utils/attribute_names.h @@ -87,6 +87,7 @@ struct AttributeName { // Authentication attributes static const char kRequestAuthPrincipal[]; static const char kRequestAuthAudiences[]; + static const char kRequestAuthGroups[]; static const char kRequestAuthPresenter[]; static const char kRequestAuthClaims[]; static const char kRequestAuthRawClaims[]; diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 78c232e3108..2aaba7bb981 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -25,6 +25,9 @@ namespace { // The JWT audience key name static const std::string kJwtAudienceKey = "aud"; +// The JWT groups key name +static const std::string kJwtGroupsKey = "groups"; + // Extract JWT audience into the JwtPayload. // This function should to be called after the claims are extracted. void ExtractJwtAudience( @@ -48,6 +51,30 @@ void ExtractJwtAudience( // Not convertable to string array } } + +// Extract JWT groups into the JwtPayload. +// This function should to be called after the claims are extracted. +void ExtractJwtGroups( +const Envoy::Json::Object& obj, +const ::google::protobuf::Map< ::std::string, ::std::string>& claims, +istio::authn::JwtPayload* payload) { + const std::string& key = kJwtGroupsKey; + // "groups" can be either string array or string. + // First, try as string + if (claims.count(key) > 0) { + payload->add_groups(claims.at(key)); + return; + } + // Next, try as string array + try { + std::vector group_vector = obj.getStringArray(key); + for (const std::string group : group_vector) { + payload->add_groups(group); + } + } catch (Json::Exception& e) { + // Not convertable to string array + } +} }; // namespace // Retrieve the JwtPayload from the HTTP headers with the key @@ -98,6 +125,8 @@ bool AuthnUtils::GetJWTPayloadFromHeaders( // Extract audience // ExtractJwtAudience() should be called after claims are extracted. ExtractJwtAudience(*json_obj, payload->claims(), payload); + // ExtractJwtGroups() should be called after claims are extracted. + ExtractJwtGroups(*json_obj, payload->claims(), payload); // Build user if (claims->count("iss") > 0 && claims->count("sub") > 0) { payload->set_user((*claims)["iss"] + "/" + (*claims)["sub"]); diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index a9b03bd67df..c99132e8145 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -34,6 +34,12 @@ static void setKeyValue(::google::protobuf::Struct& data, std::string key, (*data.mutable_fields())[key].set_string_value(value); } +// Helper function to set a key list pair into Struct. +static void setListValue(::google::protobuf::Struct& data, std::string key, + ::google::protobuf::ListValue* value) { + (*data.mutable_fields())[key].set_allocated_list_value(value); +} + } // namespace bool Authentication::SaveResultToHeader(const istio::authn::Result& result, @@ -74,6 +80,16 @@ void Authentication::SaveAuthAttributesToStruct( setKeyValue(data, istio::utils::AttributeName::kRequestAuthAudiences, origin.audiences(0)); } + if (!origin.groups().empty()) { + std::vector groups; + for(int i=0; i Date: Mon, 6 Aug 2018 19:10:39 -0700 Subject: [PATCH 2/7] Fix lint errors --- src/envoy/http/authn/authn_utils.cc | 6 +++--- src/envoy/utils/authn.cc | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 2aaba7bb981..76ea0365e73 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -55,9 +55,9 @@ void ExtractJwtAudience( // Extract JWT groups into the JwtPayload. // This function should to be called after the claims are extracted. void ExtractJwtGroups( -const Envoy::Json::Object& obj, -const ::google::protobuf::Map< ::std::string, ::std::string>& claims, -istio::authn::JwtPayload* payload) { + const Envoy::Json::Object& obj, + const ::google::protobuf::Map< ::std::string, ::std::string>& claims, + istio::authn::JwtPayload* payload) { const std::string& key = kJwtGroupsKey; // "groups" can be either string array or string. // First, try as string diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index c99132e8145..5b3c607aa1b 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -36,7 +36,7 @@ static void setKeyValue(::google::protobuf::Struct& data, std::string key, // Helper function to set a key list pair into Struct. static void setListValue(::google::protobuf::Struct& data, std::string key, - ::google::protobuf::ListValue* value) { + ::google::protobuf::ListValue* value) { (*data.mutable_fields())[key].set_allocated_list_value(value); } @@ -82,13 +82,13 @@ void Authentication::SaveAuthAttributesToStruct( } if (!origin.groups().empty()) { std::vector groups; - for(int i=0; i Date: Mon, 6 Aug 2018 19:39:19 -0700 Subject: [PATCH 3/7] Simplify the code --- src/envoy/utils/authn.cc | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index 5b3c607aa1b..0cb11100536 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -34,12 +34,6 @@ static void setKeyValue(::google::protobuf::Struct& data, std::string key, (*data.mutable_fields())[key].set_string_value(value); } -// Helper function to set a key list pair into Struct. -static void setListValue(::google::protobuf::Struct& data, std::string key, - ::google::protobuf::ListValue* value) { - (*data.mutable_fields())[key].set_allocated_list_value(value); -} - } // namespace bool Authentication::SaveResultToHeader(const istio::authn::Result& result, @@ -85,10 +79,9 @@ void Authentication::SaveAuthAttributesToStruct( for (int i = 0; i < origin.groups().size(); i++) { groups.push_back(origin.groups(i)); } - ::google::protobuf::ListValue value; - value.ParsePartialFromArray(groups.data(), groups.size()); - setListValue(data, istio::utils::AttributeName::kRequestAuthGroups, - &value); + ::google::protobuf::ListValue *value; + value = (*data.mutable_fields())[istio::utils::AttributeName::kRequestAuthGroups].mutable_list_value(); + value->ParseFromArray(groups.data(), groups.size()); } if (!origin.presenter().empty()) { setKeyValue(data, istio::utils::AttributeName::kRequestAuthPresenter, From ad26f531e365aeb995c35abc1e533a1422d3889a Mon Sep 17 00:00:00 2001 From: Lei Tang <32078630+lei-tang@users.noreply.github.com> Date: Mon, 6 Aug 2018 19:41:41 -0700 Subject: [PATCH 4/7] Fix lint error --- src/envoy/utils/authn.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index 0cb11100536..9ee4c354b04 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -79,8 +79,10 @@ void Authentication::SaveAuthAttributesToStruct( for (int i = 0; i < origin.groups().size(); i++) { groups.push_back(origin.groups(i)); } - ::google::protobuf::ListValue *value; - value = (*data.mutable_fields())[istio::utils::AttributeName::kRequestAuthGroups].mutable_list_value(); + ::google::protobuf::ListValue* value; + value = (*data.mutable_fields()) + [istio::utils::AttributeName::kRequestAuthGroups] + .mutable_list_value(); value->ParseFromArray(groups.data(), groups.size()); } if (!origin.presenter().empty()) { From 9fa8cf59b1b228f558b2b2fc071b48211916f11a Mon Sep 17 00:00:00 2001 From: Lei Tang <32078630+lei-tang@users.noreply.github.com> Date: Mon, 6 Aug 2018 19:51:53 -0700 Subject: [PATCH 5/7] Simplify the code --- src/envoy/utils/authn.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index 9ee4c354b04..697b3fff129 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -75,15 +75,13 @@ void Authentication::SaveAuthAttributesToStruct( origin.audiences(0)); } if (!origin.groups().empty()) { - std::vector groups; - for (int i = 0; i < origin.groups().size(); i++) { - groups.push_back(origin.groups(i)); - } ::google::protobuf::ListValue* value; value = (*data.mutable_fields()) [istio::utils::AttributeName::kRequestAuthGroups] .mutable_list_value(); - value->ParseFromArray(groups.data(), groups.size()); + for (int i = 0; i < origin.groups().size(); i++) { + value->add_values()->set_string_value(origin.groups(i)); + } } if (!origin.presenter().empty()) { setKeyValue(data, istio::utils::AttributeName::kRequestAuthPresenter, From 0b604c8fac690a6c24a1d46f91748d7b364f191b Mon Sep 17 00:00:00 2001 From: Lei Tang <32078630+lei-tang@users.noreply.github.com> Date: Mon, 6 Aug 2018 20:13:06 -0700 Subject: [PATCH 6/7] Add a test --- src/envoy/utils/authn_test.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/envoy/utils/authn_test.cc b/src/envoy/utils/authn_test.cc index 4e75067d615..09fb8c50bac 100644 --- a/src/envoy/utils/authn_test.cc +++ b/src/envoy/utils/authn_test.cc @@ -69,6 +69,8 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { origin->add_audiences("audiences0"); origin->add_audiences("audiences1"); origin->set_presenter("presenter"); + origin->add_groups("group1"); + origin->add_groups("group2"); auto claim = origin->mutable_claims(); (*claim)["key1"] = "value1"; (*claim)["key2"] = "value2"; @@ -92,6 +94,16 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { .at(istio::utils::AttributeName::kRequestAuthAudiences) .string_value(), "audiences0"); + EXPECT_EQ(data.fields() + .at(istio::utils::AttributeName::kRequestAuthGroups) + .list_value() + .values(0), + "group1"); + EXPECT_EQ(data.fields() + .at(istio::utils::AttributeName::kRequestAuthGroups) + .list_value() + .values(1), + "group2"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthPresenter) .string_value(), From 1a5a19a1754ed4e374335a238e013ebc332bf8bf Mon Sep 17 00:00:00 2001 From: Lei Tang <32078630+lei-tang@users.noreply.github.com> Date: Mon, 6 Aug 2018 20:38:04 -0700 Subject: [PATCH 7/7] Fix the test error --- src/envoy/utils/authn_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/envoy/utils/authn_test.cc b/src/envoy/utils/authn_test.cc index 09fb8c50bac..1fd142641dd 100644 --- a/src/envoy/utils/authn_test.cc +++ b/src/envoy/utils/authn_test.cc @@ -97,12 +97,14 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthGroups) .list_value() - .values(0), + .values(0) + .string_value(), "group1"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthGroups) .list_value() - .values(1), + .values(1) + .string_value(), "group2"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthPresenter)