Extract JWT claims as string lists under request.auth.claims[] for RBAC

include/istio/utils/attribute_names.h

@@ -90,7 +90,6 @@ 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[];

include/istio/utils/attributes_builder.h

@@ -99,11 +99,20 @@ class AttributesBuilder {
                        ->mutable_entries();
     entries->clear();
     for (const auto& field : struct_map.fields()) {
-      // Ignore all fields that are not string.
+      // Ignore all fields that are not string or string list.
       switch (field.second.kind_case()) {
         case google::protobuf::Value::kStringValue:
           (*entries)[field.first] = field.second.string_value();
           break;
+        case google::protobuf::Value::kListValue:
+          if (field.second.list_value().values_size() > 0) {
+            // Only uses the first item in the list as string
+            // TODO (lei-tang): the items in the list may be converted into a
+            // comma separated string
+            (*entries)[field.first] =
+                field.second.list_value().values().Get(0).string_value();
+          }
+          break;
         default:
           break;
       }

src/envoy/http/authn/authenticator_base_test.cc

@@ -44,7 +44,7 @@ const std::string kSecIstioAuthUserinfoHeaderValue =
      {
        "iss": "issuer@foo.com",
        "sub": "sub@foo.com",
-       "aud": "aud1",
+       "aud": ["aud1", "aud2"],
        "non-string-will-be-ignored": 1512754205,
        "some-other-string-claims": "some-claims-kept"
      }
@@ -273,15 +273,20 @@ TEST_F(ValidateJwtTest, JwtPayloadAvailable) {
       R"({
              "jwt": {
                "user": "issuer@foo.com/sub@foo.com",
-               "audiences": ["aud1"],
+               "audiences": ["aud1", "aud2"],
                "presenter": "",
                "claims": {
-                 "aud": "aud1",
                  "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   "
+               "raw_claims": "\n     {\n       \"iss\": \"issuer@foo.com\",\n       \"sub\": \"sub@foo.com\",\n       \"aud\": [\"aud1\", \"aud2\"],\n       \"non-string-will-be-ignored\": 1512754205,\n       \"some-other-string-claims\": \"some-claims-kept\"\n     }\n   ",
+               "claim_string_lists": {
+                 "aud": {"list": ["aud1", "aud2"]},
+                 "iss": {"list": ["issuer@foo.com"]},
+                 "some-other-string-claims": {"list": ["some-claims-kept"]},
+                 "sub": {"list": ["sub@foo.com"]},
+               },
              }
            }
         )",

src/envoy/http/authn/authn_utils.cc

@@ -25,9 +25,6 @@ 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(
@@ -52,24 +49,22 @@ void ExtractJwtAudience(
   }
 }
 
-// Extract JWT groups into the JwtPayload.
+// Extract JWT claim as a string list
 // This function should to be called after the claims are extracted.
-void ExtractJwtGroups(
-    const Envoy::Json::Object& obj,
+void ExtractStringList(
+    const std::string& key, 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.
+    std::vector<std::string>& list) {
   // First, try as string
   if (claims.count(key) > 0) {
-    payload->add_groups(claims.at(key));
+    list.push_back(claims.at(key));
     return;
   }
   // Next, try as string array
   try {
-    std::vector<std::string> group_vector = obj.getStringArray(key);
-    for (const std::string group : group_vector) {
-      payload->add_groups(group);
+    std::vector<std::string> vector = obj.getStringArray(key);
+    for (const std::string v : vector) {
+      list.push_back(v);
     }
   } catch (Json::Exception& e) {
     // Not convertable to string array
@@ -92,7 +87,7 @@ bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str,
   ::google::protobuf::Map< ::std::string, ::std::string>* claims =
       payload->mutable_claims();
 
-  // Extract claims
+  // Extract claims as strings
   json_obj->iterate(
       [payload](const std::string& key, const Json::Object& obj) -> bool {
         ::google::protobuf::Map< ::std::string, ::std::string>* claims =
@@ -110,8 +105,7 @@ bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str,
   // 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"]);
@@ -120,6 +114,18 @@ bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str,
   if (claims->count("azp") > 0) {
     payload->set_presenter((*claims)["azp"]);
   }
+
+  // Extract claims as lists of strings for RBAC list matcher
+  json_obj->iterate([json_obj, payload, claims](const std::string& key,
+                                                const Json::Object&) -> bool {
+    auto* claim_lists = payload->mutable_claim_string_lists();
+    std::vector<std::string> list;
+    ExtractStringList(key, *json_obj, *claims, list);
+    for (auto s : list) {
+      (*claim_lists)[key].add_list(s);
+    }
+    return true;
+  });
   return true;
 }
 

src/envoy/http/authn/authn_utils_test.cc

@@ -79,6 +79,22 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
+      claim_string_lists {
+        key: "aud"
+        value: {list: ["aud1"]}
+      }
+      claim_string_lists {
+        key: "iss"
+        value: {list: ["issuer@foo.com"]}
+      }
+      claim_string_lists {
+        key: "sub"
+        value: {list: ["sub@foo.com"]}
+      }
+      claim_string_lists {
+        key: "some-other-string-claims"
+        value: {list: ["some-claims-kept"]}
+      }
       raw_claims: ")" +
           StringUtil::escape(kSecIstioAuthUserinfoHeaderValue) + R"(")",
       &expected_payload));
@@ -107,6 +123,18 @@ TEST(AuthnUtilsTest, ProcessJwtPayloadWithNoAudTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
+      claim_string_lists {
+        key: "iss"
+        value: {list: ["issuer@foo.com"]}
+      }
+      claim_string_lists {
+        key: "sub"
+        value: {list: ["sub@foo.com"]}
+      }
+      claim_string_lists {
+        key: "some-other-string-claims"
+        value: {list: ["some-claims-kept"]}
+      }
       raw_claims: ")" +
           StringUtil::escape(kSecIstioAuthUserInfoHeaderWithNoAudValue) +
           R"(")",
@@ -139,16 +167,32 @@ TEST(AuthnUtilsTest, ProcessJwtPayloadWithTwoAudTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
+      claim_string_lists {
+        key: "aud"
+        value: {list: ["aud1", "aud2"]}
+      }
+      claim_string_lists {
+        key: "iss"
+        value: {list: ["issuer@foo.com"]}
+      }
+      claim_string_lists {
+        key: "sub"
+        value: {list: ["sub@foo.com"]}
+      }
+      claim_string_lists {
+        key: "some-other-string-claims"
+        value: {list: ["some-claims-kept"]}
+      }
       raw_claims: ")" +
           StringUtil::escape(kSecIstioAuthUserInfoHeaderWithTwoAudValue) +
           R"(")",
       &expected_payload));
-
   // The payload returned from ProcessJwtPayload() should be the same as
   // the expected. When the aud is a string array, the aud is not saved in the
   // claims.
   bool ret = AuthnUtils::ProcessJwtPayload(
       kSecIstioAuthUserInfoHeaderWithTwoAudValue, &payload);
+
   EXPECT_TRUE(ret);
   EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload));
 }

src/envoy/utils/authn.cc

@@ -57,15 +57,6 @@ void Authentication::SaveAuthAttributesToStruct(
       setKeyValue(data, istio::utils::AttributeName::kRequestAuthAudiences,
                   origin.audiences(0));
     }
-    if (!origin.groups().empty()) {
-      ::google::protobuf::ListValue* value;
-      value = (*data.mutable_fields())
-                  [istio::utils::AttributeName::kRequestAuthGroups]
-                      .mutable_list_value();
-      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,
                   origin.presenter());
@@ -82,6 +73,20 @@ void Authentication::SaveAuthAttributesToStruct(
       setKeyValue(data, istio::utils::AttributeName::kRequestAuthRawClaims,
                   origin.raw_claims());
     }
+    // Store the claims for RBAC
+    if (!origin.claim_string_lists().empty()) {
+      ::google::protobuf::Struct* s =
+          (*data.mutable_fields())
+              [istio::utils::AttributeName::kRequestAuthClaims]
+                  .mutable_struct_value();
+      for (auto const& e : origin.claim_string_lists()) {
+        ::google::protobuf::ListValue* value =
+            (*s->mutable_fields())[e.first].mutable_list_value();
+        for (const auto& v : e.second.list()) {
+          value->add_values()->set_string_value(v);
+        }
+      }
+    }
   }
 }
 

src/envoy/utils/authn_test.cc

@@ -46,8 +46,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");
+  (*origin->mutable_claim_string_lists())["groups"].add_list("group1");
+  (*origin->mutable_claim_string_lists())["groups"].add_list("group2");
   auto claim = origin->mutable_claims();
   (*claim)["key1"] = "value1";
   (*claim)["key2"] = "value2";
@@ -72,21 +72,26 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) {
                 .string_value(),
             "audiences0");
   EXPECT_EQ(data.fields()
-                .at(istio::utils::AttributeName::kRequestAuthGroups)
+                .at(istio::utils::AttributeName::kRequestAuthPresenter)
+                .string_value(),
+            "presenter");
+
+  auto auth_claims =
+      data.fields().at(istio::utils::AttributeName::kRequestAuthClaims);
+  EXPECT_EQ(auth_claims.struct_value()
+                .fields()
+                .at("groups")
                 .list_value()
                 .values(0)
                 .string_value(),
             "group1");
-  EXPECT_EQ(data.fields()
-                .at(istio::utils::AttributeName::kRequestAuthGroups)
+  EXPECT_EQ(auth_claims.struct_value()
+                .fields()
+                .at("groups")
                 .list_value()
                 .values(1)
                 .string_value(),
             "group2");
-  EXPECT_EQ(data.fields()
-                .at(istio::utils::AttributeName::kRequestAuthPresenter)
-                .string_value(),
-            "presenter");
 
   auto actual_claim =
       data.fields().at(istio::utils::AttributeName::kRequestAuthClaims);

src/istio/authn/context.proto

@@ -41,9 +41,8 @@ message JwtPayload {
   // map above.
   string raw_claims = 6;
 
-  // The groups claim in the JWT.
-  // Example: [‘group1’, ‘group2’]
-  repeated string groups = 7;
+  // Claims in JWT as lists of strings
+  map<string, StringList> claim_string_lists = 7;
 }
 
 // Container to hold authenticated attributes from X509 (mTLS).
@@ -74,3 +73,8 @@ message Result {
   // JwtPayload for origin authenticated attributes.
   JwtPayload origin = 3;
 }
+
+message StringList {
+  // A list of strings
+  repeated string list = 1;
+}

src/istio/utils/attribute_names.cc

@@ -85,7 +85,6 @@ const char AttributeName::kQuotaCacheHit[] = "quota.cache_hit";
 // Authentication attributes
 const char AttributeName::kRequestAuthPrincipal[] = "request.auth.principal";
 const char AttributeName::kRequestAuthAudiences[] = "request.auth.audiences";
-const char AttributeName::kRequestAuthGroups[] = "request.auth.groups";
 const char AttributeName::kRequestAuthPresenter[] = "request.auth.presenter";
 const char AttributeName::kRequestAuthClaims[] = "request.auth.claims";
 const char AttributeName::kRequestAuthRawClaims[] = "request.auth.raw_claims";