fix groups in azure

fix reading values from jsons

Author: zvonand

src/Access/TokenProcessorsOpaque.cpp

@@ -30,17 +30,23 @@ namespace
         return jsonValue.get<picojson::object>();
     }
 
-    template<typename ValueType = std::string>
-    ValueType getValueByKey(const picojson::object & jsonObject, const std::string & key) {
+    template<typename ValueType = std::string, bool throw_on_exception = true>
+    std::optional<ValueType> getValueByKey(const picojson::object & jsonObject, const std::string & key) {
         auto it = jsonObject.find(key); // Find the key in the object
         if (it == jsonObject.end())
         {
-            throw std::runtime_error("Key not found: " + key);
+            if constexpr (throw_on_exception)
+                throw std::runtime_error("Key not found: " + key);
+            else
+                return std::nullopt;
         }
 
         const picojson::value & value = it->second;
         if (!value.is<ValueType>()) {
-            throw std::runtime_error("Value for key '" + key + "' has incorrect type.");
+            if constexpr (throw_on_exception)
+                throw std::runtime_error("Value for key '" + key + "' has incorrect type.");
+            else
+                return std::nullopt;
         }
 
         return value.get<ValueType>();
@@ -94,11 +100,9 @@ bool GoogleTokenProcessor::resolveAndValidate(const TokenCredentials & credentia
         throw Exception(ErrorCodes::AUTHENTICATION_FAILED,
                         "{}: Specified username_claim {} not found in token", processor_name, username_claim);
 
-    bool has_email = user_info_json.contains("email");
-    if (has_email)
-        user_info["email"] = getValueByKey(user_info_json, "email");
+    user_info["email"] = getValueByKey<std::string, false>(user_info_json, "email").value_or("");
 
-    user_info[username_claim] = getValueByKey(user_info_json, username_claim);
+    user_info[username_claim] = getValueByKey(user_info_json, username_claim).value();
 
     String user_name = user_info[username_claim];
 
@@ -109,11 +113,11 @@ bool GoogleTokenProcessor::resolveAndValidate(const TokenCredentials & credentia
 
     auto token_info = getObjectFromURI(Poco::URI("https://www.googleapis.com/oauth2/v3/tokeninfo"), token);
     if (token_info.contains("exp"))
-        const_cast<TokenCredentials &>(credentials).setExpiresAt(std::chrono::system_clock::from_time_t((getValueByKey<time_t>(token_info, "exp"))));
+        const_cast<TokenCredentials &>(credentials).setExpiresAt(std::chrono::system_clock::from_time_t((getValueByKey<time_t>(token_info, "exp").value())));
 
     /// Groups info can only be retrieved if user email is known.
     /// If no email found in user info, we skip this step and there are no external roles for the user.
-    if (has_email)
+    if (!user_info["email"].empty())
     {
         std::set<String> external_groups_names;
         const Poco::URI get_groups_uri = Poco::URI("https://cloudidentity.googleapis.com/v1/groups/-/memberships:searchDirectGroups?query=member_key_id==" + user_info["email"] + "'");
@@ -139,10 +143,13 @@ bool GoogleTokenProcessor::resolveAndValidate(const TokenCredentials & credentia
                 }
 
                 auto group_data = group.get<picojson::object>();
-                String group_name = getValueByKey(group_data["groupKey"].get<picojson::object>(), "id");
-                external_groups_names.insert(group_name);
-                LOG_TRACE(getLogger("TokenAuthentication"),
-                          "{}: User {}: new external group {}", processor_name, user_name, group_name);
+                String group_name = getValueByKey<std::string, false>(group_data["groupKey"].get<picojson::object>(), "id").value_or("");
+                if (!group_name.empty())
+                {
+                    external_groups_names.insert(group_name);
+                    LOG_TRACE(getLogger("TokenAuthentication"),
+                              "{}: User {}: new external group {}", processor_name, user_name, group_name);
+                }
             }
 
             const_cast<TokenCredentials &>(credentials).setGroups(external_groups_names);
@@ -172,7 +179,7 @@ bool AzureTokenProcessor::resolveAndValidate(const TokenCredentials & credential
     try
     {
         picojson::object user_info_json = getObjectFromURI(Poco::URI("https://graph.microsoft.com/oidc/userinfo"), token);
-        String username = getValueByKey(user_info_json, username_claim);
+        String username = getValueByKey(user_info_json, username_claim).value();
         if (!username.empty())
         {
             /// Credentials are passed as const everywhere up the flow, so we have to comply,
@@ -224,9 +231,15 @@ bool AzureTokenProcessor::resolveAndValidate(const TokenCredentials & credential
             }
 
             auto group_data = group.get<picojson::object>();
-            String group_name = getValueByKey(group_data, "id");
-            external_groups_names.insert(group_name);
-            LOG_TRACE(getLogger("TokenAuthentication"), "{}: User {}: new external group {}", processor_name, credentials.getUserName(), group_name);
+            if (!group_data.contains("displayName"))
+                continue;
+
+            String group_name = getValueByKey<std::string, false>(group_data, "displayName").value_or("");
+            if (!group_name.empty())
+            {
+                external_groups_names.insert(group_name);
+                LOG_TRACE(getLogger("TokenAuthentication"), "{}: User {}: new external group {}", processor_name, credentials.getUserName(), group_name);
+            }
         }
     }
     catch (const Exception & e)
@@ -282,7 +295,7 @@ OpenIdTokenProcessor::OpenIdTokenProcessor(const String & processor_name_,
     if (!openid_config.contains("userinfo_endpoint") || !openid_config.contains("introspection_endpoint"))
         throw Exception(ErrorCodes::AUTHENTICATION_FAILED, "{}: Cannot extract userinfo_endpoint or introspection_endpoint from OIDC configuration, consider manual configuration.", processor_name);
 
-    if (!openid_config.contains("jwks_uri"))
+    if (openid_config.contains("jwks_uri"))
     {
         LOG_TRACE(getLogger("TokenAuthentication"), "{}: JWKS URI set, local JWT processing will be attempted", processor_name_);
         jwt_validator.emplace(processor_name_ + "jwks_val",
@@ -291,7 +304,7 @@ OpenIdTokenProcessor::OpenIdTokenProcessor(const String & processor_name_,
                               groups_claim_,
                               "",
                               verifier_leeway_,
-                              getValueByKey(openid_config, "jwks_uri"),
+                              getValueByKey(openid_config, "jwks_uri").value(),
                               jwks_cache_lifetime_);
     }
 }
@@ -308,7 +321,7 @@ bool OpenIdTokenProcessor::resolveAndValidate(const TokenCredentials & credentia
         {
             auto decoded_token = jwt::decode(token);
             user_info_json = decoded_token.get_payload_json();
-            username = getValueByKey(user_info_json, username_claim);
+            username = getValueByKey(user_info_json, username_claim).value();
 
             /// TODO: Now we work only with Keycloak -- and it provides expires_at in token itself. Need to add actual token introspection logic for other OIDC providers.
             if (decoded_token.has_expires_at())
@@ -326,7 +339,7 @@ bool OpenIdTokenProcessor::resolveAndValidate(const TokenCredentials & credentia
         try
         {
             user_info_json = getObjectFromURI(userinfo_endpoint, token);
-            username = getValueByKey(user_info_json, username_claim);
+            username = getValueByKey(user_info_json, username_claim).value();
         }
         catch (...)
         {