small fixes

Author: zvonand

docs/en/operations/external-authenticators/tokens.md

@@ -13,25 +13,21 @@ OAuth 2.0 access tokens can be used to authenticate ClickHouse users. This works
 
 Though this authentication method is different from JWT authentication, it works under the same authentication method to maintain better compatibility. 
 
-For both of these approaches a definition of `access_token_processors` is mandatory.
+For both of these approaches a definition of `token_processors` is mandatory.
 
 ## Access Token Processors
 
-To define an access token processor, add `access_token_processors` section to `config.xml`. Example:
+To define an access token processor, add `token_processors` section to `config.xml`. Example:
 ```xml
 <clickhouse>
-    <access_token_processors>
-        <gogoogle>
-            <provider>Google</provider>
-            <email_filter>^[A-Za-z0-9._%+-]+@example\.com$</email_filter>
-            <cache_lifetime>600</cache_lifetime>
-        </gogoogle>
+    <token_processors>
         <azuure>
             <provider>azure</provider>
+            <username_claim>claim_name</username_claim>
             <client_id>CLIENT_ID</client_id>
             <tenant_id>TENANT_ID</tenant_id>
         </azuure>
-    </access_token_processors>
+    </token_processors>
 </clickhouse>
 ```
 
@@ -41,11 +37,21 @@ Different providers have different sets of parameters.
 
 **Parameters**
 
-- `provider` -- name of identity provider. Mandatory, case-insensitive. Supported options: "Google", "Azure".
+- `provider` -- name of identity provider. Mandatory, case-insensitive. Supported options: "Google", "Azure", "OpenID".
+- `username_claim` -- name of claim (field) that will be treated as ClickHouse user name. Optional, default: "sub".
 - `cache_lifetime` --  maximum lifetime of cached token (in seconds). Optional, default: 3600.
 - `email_filter` -- Regex for validation of user emails. Optional parameter, only for Google IdP.
-- `client_id` -- Azure AD (Entra ID) client ID. Optional parameter, only for Azure IdP.
-- `tenant_id` -- Azure AD (Entra ID) tenant ID. Optional parameter, only for Azure IdP.
+- `client_id` -- Azure AD (Entra ID) client ID. Optional parameter, used only for Azure IdP.
+- `tenant_id` -- Azure AD (Entra ID) tenant ID. Optional parameter, used only for Azure IdP.
+- `groups_claim` -- Name of claim (field) that contains list of groups user belongs to. This claim will be looked up in the token itself (in case token is a valid JWT, e.g. in Keycloak) or in response from `/userinfo`. Optional parameter.
+- `configuration_endpoint` -- URI of `.well-known/openid-configuration`. Optional parameter, useful only for OIDC-compliant providers (e.g. Keycloak).
+- `userinfo_endpoint` -- URI of userinfo endpoint. Optional parameter.
+- `token_introspection_endpoint` -- URI of token introspection endpoint. Optional parameter.
+  
+:::note
+Either `configuration_endpoint` or both `userinfo_endpoint` and `token_introspection_endpoint` shall be set. If none of them are set or all three are set, this is invalid configuration, it will not be parsed.
+:::
+
 
 ### Tokens cache
 To reduce number of requests to IdP, tokens are cached internally for no longer then `cache_lifetime` seconds.
@@ -58,17 +64,20 @@ Locally defined users can be authenticated with an access token. To allow this,
 
 ```xml
 <clickhouse>
-    <!- ... -->
     <users>
-        <!- ... -->
         <my_user>
-            <!- ... -->
+            <jwt>
+                <allowed_processors>
+                    <azuure />
+                </allowed_processors>
             </jwt>
         </my_user>
     </users>
 </clickhouse>
 ```
 
+Inside `jwt` one or more specific access token processors names can be specified -- only those processors will be tried when authenticating. If no processors are specified, _all_ processors will be tried.
+
 At each login attempt, ClickHouse will attempt to validate token and get user info against every defined access token provider.
 
 When SQL-driven [Access Control and Account Management](/docs/en/guides/sre/user-management/index.md#access-control) is enabled, users that are authenticated with tokens can also be created using the [CREATE USER](/docs/en/sql-reference/statements/create/user.md#create-user-statement) statement.
@@ -85,24 +94,32 @@ If there is no suitable user pre-defined in ClickHouse, authentication is still
 To allow this, add `token` section to the `users_directories` section of the `config.xml` file. 
 
 At each login attempt, ClickHouse tries to find the user definition locally and authenticate it as usual.
-If the user is not defined, ClickHouse will treat user as externally defined, and will try to validate the token and get user information from the specified processor.
+If the user is not defined, ClickHouse will treat the user as externally defined and will try to validate the token and get user information from the specified processor.
 If validated successfully, the user will be considered existing and authenticated. The user will be assigned roles from the list specified in the `roles` section. 
 All this implies that the SQL-driven [Access Control and Account Management](/docs/en/guides/sre/user-management/index.md#access-control) is enabled and roles are created using the [CREATE ROLE](/docs/en/sql-reference/statements/create/role.md#create-role-statement) statement.
 
 **Example**
 
 ```xml
 <clickhouse>
-    <token>
-        <processor>gogoogle</processor>
-        <roles>
-            <token_test_role_1 />
-        </roles>
-    </token>
+    <user_directories>
+        <token>
+            <processor>processor_name</processor>
+            <common_roles>
+                <token_test_role_1 />
+            </common_roles>
+            <roles_filter></roles_filter>
+        </token>
+    </user_directories>
 </clickhouse>
 ```
 
+:::note
+For now, no more than one `token` section can be defined inside `user_directories`. This _may_ change in future.
+:::
+
 **Parameters**
 
-- `server` — Name of one of processors defined in `access_token_processors` config section described above. This parameter is mandatory and cannot be empty.
-- `roles` — Section with a list of locally defined roles that will be assigned to each user retrieved from the IdP.
+- `processor` — Name of one of processors defined in `token_processors` config section described above. This parameter is mandatory and cannot be empty.
+- `common_roles` — Section with a list of locally defined roles that will be assigned to each user retrieved from the IdP. Optional.
+- `roles_filter` — Regex string for groups filtering. Only groups matching this regex will be mapped to roles. Optional.

src/Access/AccessControl.cpp

@@ -42,6 +42,7 @@ namespace ErrorCodes
     extern const int REQUIRED_PASSWORD;
     extern const int CANNOT_COMPILE_REGEXP;
     extern const int BAD_ARGUMENTS;
+    extern const int INVALID_CONFIG_PARAMETER;
 }
 
 namespace
@@ -440,6 +441,8 @@ void AccessControl::addStoragesFromUserDirectoriesConfig(
     Strings keys_in_user_directories;
     config.keys(key, keys_in_user_directories);
 
+    bool has_token_storage = false;
+
     for (const String & key_in_user_directories : keys_in_user_directories)
     {
         String prefix = key + "." + key_in_user_directories;
@@ -490,7 +493,11 @@ void AccessControl::addStoragesFromUserDirectoriesConfig(
         }
         else if (type == TokenAccessStorage::STORAGE_TYPE)
         {
+            if (has_token_storage)
+                throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Only one `token` section can be defined.");
+
             addTokenStorage(name, config, prefix);
+            has_token_storage = true;
         }
         else
             throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, "Unknown storage type '{}' at {} in config", type, prefix);

src/Access/AccessTokenProcessor.cpp

@@ -92,15 +92,14 @@ std::unique_ptr<IAccessTokenProcessor> IAccessTokenProcessor::parseTokenProcesso
     const String & prefix,
     const String & name)
 {
+    /// TODO: maybe bind external user to the processor it was created with?
     if (config.hasProperty(prefix + ".provider"))
     {
         String provider = Poco::toLower(config.getString(prefix + ".provider"));
 
-        String email_regex_str = config.hasProperty(prefix + ".email_filter") ? config.getString(
-                prefix + ".email_filter") : "";
-
-        UInt64 cache_lifetime = config.hasProperty(prefix + ".cache_lifetime") ? config.getUInt64(
-                prefix + ".cache_lifetime") : 3600;
+        String email_regex_str = config.getString(prefix + ".email_filter", "");
+        UInt64 cache_lifetime = config.getUInt64(prefix + ".cache_lifetime", 3600);
+        String username_claim = config.getString(prefix + ".username_claim", "sub");
 
         if (provider == "google")
         {
@@ -129,11 +128,21 @@ std::unique_ptr<IAccessTokenProcessor> IAccessTokenProcessor::parseTokenProcesso
 
             if (is_auto && !is_manual)
             {
-                return std::make_unique<OpenIDAccessTokenProcessor>(name, cache_lifetime, email_regex_str, config.getString(prefix + ".configuration_endpoint"), config.getString(prefix + ".groups_claim_name", ""));
+                return std::make_unique<OpenIDAccessTokenProcessor>(name,
+                                                                    cache_lifetime,
+                                                                    email_regex_str,
+                                                                    config.getString(prefix + ".configuration_endpoint"),
+                                                                    config.getString(prefix + ".groups_claim", ""));
             }
             else if (!is_auto && is_manual)
             {
-                return std::make_unique<OpenIDAccessTokenProcessor>(name, cache_lifetime, email_regex_str, config.getString(prefix + ".userinfo_endpoint"), config.getString(prefix + ".token_introspection_endpoint"));
+                return std::make_unique<OpenIDAccessTokenProcessor>(name,
+                                                                    cache_lifetime,
+                                                                    email_regex_str,
+                                                                    config.getString(prefix + ".userinfo_endpoint"),
+                                                                    config.getString(prefix + ".token_introspection_endpoint"),
+                                                                    config.getString(prefix + ".jwks_uri"),
+                                                                    config.getString(prefix + ".groups_claim", ""));
             }
 
             throw Exception(ErrorCodes::INVALID_CONFIG_PARAMETER, "Could not parse access token processor {}: "
@@ -154,7 +163,7 @@ bool GoogleAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cre
     const String & token = credentials.getToken();
 
     auto user_info = getUserInfo(token);
-    String user_name = user_info["sub"];
+    String user_name = user_info[username_claim];
     bool has_email = user_info.contains("email");
 
     if (email_regex.ok())
@@ -238,7 +247,7 @@ std::unordered_map<String, String> GoogleAccessTokenProcessor::getUserInfo(const
     try
     {
         user_info_map["email"] = getValueByKey(user_info_json, "email");
-        user_info_map["sub"] = getValueByKey(user_info_json, "sub");
+        user_info_map[username_claim] = getValueByKey(user_info_json, username_claim);
         return user_info_map;
     }
     catch (std::runtime_error & e)
@@ -254,7 +263,7 @@ bool AzureAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cred
     /// We will not trust user data in this token except for 'exp' value to determine caching duration.
     /// Explanation here: https://stackoverflow.com/questions/60778634/failing-signature-validation-of-jwt-tokens-from-azure-ad
     /// Let Azure validate it: only valid tokens will be accepted.
-    /// Use GET https://graph.microsoft.com/oidc/userinfo to verify token and get sub at the same time
+    /// Use GET https://graph.microsoft.com/oidc/userinfo to verify token and get user info at the same time
 
     const String & token = credentials.getToken();
 
@@ -333,7 +342,7 @@ bool AzureAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cred
 String AzureAccessTokenProcessor::validateTokenAndGetUsername(const String & token) const
 {
     picojson::object user_info_json = getObjectFromURI(user_info_uri, token);
-    return getValueByKey(user_info_json, "sub");
+    return getValueByKey(user_info_json, username_claim);
 }
 
 
@@ -343,12 +352,13 @@ OpenIDAccessTokenProcessor::OpenIDAccessTokenProcessor(const String & name_,
                            const String & userinfo_endpoint_,
                            const String & token_introspection_endpoint_,
                            const String & jwks_uri_,
-                           const String & groups_claim_name_)
+                           const String & groups_claim_)
     : IAccessTokenProcessor(name_, cache_invalidation_interval_, email_regex_str),
-    userinfo_endpoint(userinfo_endpoint_), token_introspection_endpoint(token_introspection_endpoint_), groups_claim_name(groups_claim_name_)
+    userinfo_endpoint(userinfo_endpoint_), token_introspection_endpoint(token_introspection_endpoint_), groups_claim(groups_claim_)
 {
     if (!jwks_uri_.empty())
     {
+        LOG_TRACE(getLogger("AccessTokenProcessor"), "{}: JWKS URI set, local JWT processing will be attempted", name);
         jwt_validator.emplace(name_ + "jwks_val", jwks_uri_, cache_invalidation_interval_);
     }
 }
@@ -357,7 +367,7 @@ OpenIDAccessTokenProcessor::OpenIDAccessTokenProcessor(const String & name_,
                            const UInt64 cache_invalidation_interval_,
                            const String & email_regex_str,
                            const String & openid_config_endpoint_,
-                           const String & groups_claim_name_)
+                           const String & groups_claim_)
     : IAccessTokenProcessor(name_, cache_invalidation_interval_, email_regex_str)
 {
     const picojson::object openid_config = getObjectFromURI(Poco::URI(openid_config_endpoint_));
@@ -371,7 +381,7 @@ OpenIDAccessTokenProcessor::OpenIDAccessTokenProcessor(const String & name_,
                                getValueByKey(openid_config, "userinfo_endpoint"),
                                getValueByKey(openid_config, "introspection_endpoint"),
                                openid_config.contains("jwks_uri") ? getValueByKey(openid_config, "jwks_uri") : "",
-                               groups_claim_name_);
+                               groups_claim_);
 }
 
 bool OpenIDAccessTokenProcessor::resolveAndValidate(const TokenCredentials & credentials)
@@ -382,7 +392,6 @@ bool OpenIDAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cre
 
     if (jwt_validator.has_value() && jwt_validator.value().validate("", token, username))
     {
-
         try
         {
             auto decoded_token = jwt::decode(token);
@@ -404,7 +413,7 @@ bool OpenIDAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cre
         try
         {
             user_info_json = getObjectFromURI(userinfo_endpoint, token);
-            username = getValueByKey(user_info_json, "sub");
+            username = getValueByKey(user_info_json, username_claim);
         }
         catch (...)
         {
@@ -429,9 +438,9 @@ bool OpenIDAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cre
 
     /// For now, list of groups is expected in a claim with specified name either in token itself or in userinfo response (Keycloak works this way)
     /// TODO: add support for custom endpoints for retrieving groups. Keycloak lists groups in /userinfo and token itself, which is not always the case.
-    if (!groups_claim_name.empty() && user_info_json.contains(groups_claim_name))
+    if (!groups_claim.empty() && user_info_json.contains(groups_claim))
     {
-        if (!user_info_json[groups_claim_name].is<picojson::array>())
+        if (!user_info_json[groups_claim].is<picojson::array>())
         {
             LOG_TRACE(getLogger("AccessTokenProcessor"),
                       "{}: Failed to extract groups: invalid content in user data", name);
@@ -440,7 +449,7 @@ bool OpenIDAccessTokenProcessor::resolveAndValidate(const TokenCredentials & cre
 
         std::set<String> external_groups_names;
 
-        picojson::array groups_array = user_info_json[groups_claim_name].get<picojson::array>();
+        picojson::array groups_array = user_info_json[groups_claim].get<picojson::array>();
         for (const auto & group: groups_array)
         {
             if (group.is<std::string>())

src/Access/AccessTokenProcessor.h

@@ -55,6 +55,7 @@ class IAccessTokenProcessor
 
 protected:
     const String name;
+    const String username_claim = "sub";
     const UInt64 cache_invalidation_interval;
     re2::RE2 email_regex;
 
@@ -107,7 +108,7 @@ class OpenIDAccessTokenProcessor : public IAccessTokenProcessor
                               const UInt64 cache_invalidation_interval_,
                               const String & email_regex_str,
                               const String & openid_config_endpoint_,
-                              const String & groups_claim_name_);
+                              const String & groups_claim_);
 
     /// Specify endpoints manually
     OpenIDAccessTokenProcessor(const String & name_,
@@ -116,7 +117,7 @@ class OpenIDAccessTokenProcessor : public IAccessTokenProcessor
                                const String & userinfo_endpoint_,
                                const String & token_introspection_endpoint_,
                                const String & jwks_uri_,
-                               const String & groups_claim_name_);
+                               const String & groups_claim_);
 
     bool resolveAndValidate(const TokenCredentials & credentials) override;
 private:
@@ -127,7 +128,7 @@ class OpenIDAccessTokenProcessor : public IAccessTokenProcessor
     std::optional<JWKSValidator> jwt_validator = std::nullopt;
 
     /// groups are expected under /userinfo endpoint under specified name
-    const String groups_claim_name;
+    const String groups_claim;
 };
 
 }

src/Access/AuthenticationData.h

@@ -82,6 +82,9 @@ class AuthenticationData
     const String & getJWTClaims() const { return jwt_claims; }
     void setJWTClaims(const String & jwt_claims_) { jwt_claims = jwt_claims_; }
 
+    const String & getTokenProcessorName() const { return token_processor_name; }
+    void setTokenProcessorName(const String & token_processor_name_) { token_processor_name = token_processor_name_; }
+
     friend bool operator ==(const AuthenticationData & lhs, const AuthenticationData & rhs);
     friend bool operator !=(const AuthenticationData & lhs, const AuthenticationData & rhs) { return !(lhs == rhs); }
 
@@ -121,6 +124,7 @@ class AuthenticationData
     HTTPAuthenticationScheme http_auth_scheme = HTTPAuthenticationScheme::BASIC;
     time_t valid_until = 0;
     String jwt_claims;
+    String token_processor_name;
 };
 
 }