From 1491e4910db86a723b08752459cdb1c35316cc0d Mon Sep 17 00:00:00 2001 From: Diem Vu Date: Thu, 10 May 2018 14:54:10 -0700 Subject: [PATCH 1/2] Add raw_claims attribute to hold all original claims from JWT --- src/envoy/http/authn/authenticator_base_test.cc | 3 ++- src/envoy/http/authn/authn_utils.cc | 1 + src/envoy/http/authn/authn_utils_test.cc | 12 +++++++++--- src/envoy/http/authn/http_filter_integration_test.cc | 3 ++- src/istio/authn/context.proto | 5 +++++ src/istio/control/attribute_names.cc | 1 + src/istio/control/attribute_names.h | 1 + src/istio/control/http/attributes_builder.cc | 4 ++++ src/istio/control/http/attributes_builder_test.cc | 7 +++++++ 9 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index f223c0e9433..9c6d69c6510 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -307,7 +307,8 @@ TEST_F(ValidateJwtTest, JwtInHeader) { "iss": "issuer@foo.com", "sub": "sub@foo.com", "some-other-string-claims": "some-claims-kept" - } + }, + raw_claims: "\n {\n \"iss\": \"issuer@foo.com\",\n \"sub\": \"sub@foo.com\",\n \"aud\": \"aud1\",\n \"non-string-will-be-ignored\": 1512754205,\n \"some-other-string-claims\": \"some-claims-kept\"\n }\n " } } )", diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 2dc57bfed20..78c232e3108 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -68,6 +68,7 @@ bool AuthnUtils::GetJWTPayloadFromHeaders( jwt_payload_key.get(), value); return false; } + *payload->mutable_raw_claims() = payload_str; ::google::protobuf::Map< ::std::string, ::std::string>* claims = payload->mutable_claims(); Envoy::Json::ObjectSharedPtr json_obj; diff --git a/src/envoy/http/authn/authn_utils_test.cc b/src/envoy/http/authn/authn_utils_test.cc index fe9f3105445..3663476ab0d 100644 --- a/src/envoy/http/authn/authn_utils_test.cc +++ b/src/envoy/http/authn/authn_utils_test.cc @@ -14,6 +14,7 @@ */ #include "src/envoy/http/authn/authn_utils.h" #include "common/common/base64.h" +#include "common/common/utility.h" #include "src/envoy/http/authn/test_utils.h" #include "test/test_common/utility.h" @@ -91,7 +92,8 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) { key: "some-other-string-claims" value: "some-claims-kept" } - )", + raw_claims: ")" + + StringUtil::escape(kSecIstioAuthUserinfoHeaderValue) + R"(")", &expected_payload)); // The payload returned from GetJWTPayloadFromHeaders() should be the same as // the expected. @@ -121,7 +123,9 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) { key: "some-other-string-claims" value: "some-claims-kept" } - )", + raw_claims: ")" + + StringUtil::escape(kSecIstioAuthUserInfoHeaderWithNoAudValue) + + R"(")", &expected_payload)); // The payload returned from GetJWTPayloadFromHeaders() should be the same as // the expected. When there is no aud, the aud is not saved in the payload @@ -154,7 +158,9 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) { key: "some-other-string-claims" value: "some-claims-kept" } - )", + raw_claims: ")" + + StringUtil::escape(kSecIstioAuthUserInfoHeaderWithTwoAudValue) + + R"(")", &expected_payload)); // The payload returned from GetJWTPayloadFromHeaders() should be the same as diff --git a/src/envoy/http/authn/http_filter_integration_test.cc b/src/envoy/http/authn/http_filter_integration_test.cc index 3ec0c1197e1..5690ef7766c 100644 --- a/src/envoy/http/authn/http_filter_integration_test.cc +++ b/src/envoy/http/authn/http_filter_integration_test.cc @@ -214,7 +214,8 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckAuthnResultIsExpected) { "aud": "bookstore-esp-echo.cloudendpointsapis.com", "iss": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", "sub": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com" - } + }, + raw_claims: "{\"iss\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"sub\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"aud\":\"bookstore-esp-echo.cloudendpointsapis.com\",\"iat\":1512754205,\"exp\":5112754205}" } } )", diff --git a/src/istio/authn/context.proto b/src/istio/authn/context.proto index d87385b5d0f..5ea3fdb67d5 100644 --- a/src/istio/authn/context.proto +++ b/src/istio/authn/context.proto @@ -35,6 +35,11 @@ message JwtPayload { // Only raw JWT string claims are kept. map claims = 5; + + // All original claims in JsonString format, which can be parsed into json + // object (map) to access other claims that not cover with the string claims + // map above. + string raw_claims = 6; } // Container to hold authenticated attributes from X509 (mTLS). diff --git a/src/istio/control/attribute_names.cc b/src/istio/control/attribute_names.cc index 3f0326a84af..953957998a9 100644 --- a/src/istio/control/attribute_names.cc +++ b/src/istio/control/attribute_names.cc @@ -78,6 +78,7 @@ const char AttributeName::kRequestAuthPrincipal[] = "request.auth.principal"; const char AttributeName::kRequestAuthAudiences[] = "request.auth.audiences"; const char AttributeName::kRequestAuthPresenter[] = "request.auth.presenter"; const char AttributeName::kRequestAuthClaims[] = "request.auth.claims"; +const char AttributeName::kRequestAuthRawClaims[] = "request.auth.raw_claims"; } // namespace control } // namespace istio diff --git a/src/istio/control/attribute_names.h b/src/istio/control/attribute_names.h index 09bb1488e25..2ed3dfdf46a 100644 --- a/src/istio/control/attribute_names.h +++ b/src/istio/control/attribute_names.h @@ -85,6 +85,7 @@ struct AttributeName { static const char kRequestAuthAudiences[]; static const char kRequestAuthPresenter[]; static const char kRequestAuthClaims[]; + static const char kRequestAuthRawClaims[]; }; } // namespace control diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 4aac9a7d468..7be56696624 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -89,6 +89,10 @@ void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { builder.AddProtobufStringMap(AttributeName::kRequestAuthClaims, origin.claims()); } + if (!origin.raw_claims().empty()) { + builder.AddString(AttributeName::kRequestAuthRawClaims, + origin.raw_claims()); + } } return; } diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 958e4d3f5a2..a6e974d704b 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -398,6 +398,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { "thisisemail@email.com"; (*result->mutable_origin()->mutable_claims())["iat"] = "1512754205"; (*result->mutable_origin()->mutable_claims())["exp"] = "5112754205"; + result->mutable_origin()->set_raw_claims("test_raw_claims"); return true; })); @@ -414,6 +415,12 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); + // kCheckAttributes is also used in TestCheckAttributes, which is a deprecated way to construct + // mixer attribute (it was a fallback when authn filter is not available, which can be removed + // after 0.8). For now, modifying expected data manually for this test. + (*expected_attributes.mutable_attributes())[AttributeName::kRequestAuthRawClaims] + .set_string_value("test_raw_claims"); + EXPECT_TRUE( MessageDifferencer::Equals(request.attributes, expected_attributes)); } From 344c964ede87c3784d556acaad298fe1eb3929f7 Mon Sep 17 00:00:00 2001 From: Diem Vu Date: Thu, 10 May 2018 14:54:30 -0700 Subject: [PATCH 2/2] Clang --- src/istio/control/http/attributes_builder.cc | 2 +- src/istio/control/http/attributes_builder_test.cc | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 7be56696624..d85bc4f2bcf 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -91,7 +91,7 @@ void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { } if (!origin.raw_claims().empty()) { builder.AddString(AttributeName::kRequestAuthRawClaims, - origin.raw_claims()); + origin.raw_claims()); } } return; diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index a6e974d704b..9ac9019442e 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -415,10 +415,12 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); - // kCheckAttributes is also used in TestCheckAttributes, which is a deprecated way to construct - // mixer attribute (it was a fallback when authn filter is not available, which can be removed - // after 0.8). For now, modifying expected data manually for this test. - (*expected_attributes.mutable_attributes())[AttributeName::kRequestAuthRawClaims] + // kCheckAttributes is also used in TestCheckAttributes, which is a deprecated + // way to construct mixer attribute (it was a fallback when authn filter is + // not available, which can be removed after 0.8). For now, modifying expected + // data manually for this test. + (*expected_attributes + .mutable_attributes())[AttributeName::kRequestAuthRawClaims] .set_string_value("test_raw_claims"); EXPECT_TRUE(