roryqi commented on code in PR #11226: URL: https://github.com/apache/gravitino/pull/11226#discussion_r3309446085
########## docs/security/how-to-authenticate.md: ########## @@ -43,6 +43,45 @@ curl -v -X GET \ http://localhost:8090/api/version ``` +### Basic mode + +In Basic mode, Gravitino verifies HTTP Basic credentials against built-in IDP user metadata stored +in the relational entity store. + +To enable Basic mode: + +- Include the `idp-basic` plugin jar on the server classpath (included in the official + distribution). +- Set `gravitino.authenticators` to `basic`. +- Set `gravitino.entity.store` to `relational` and configure the relational JDBC backend. +- Set `gravitino.server.rest.extensionPackages` to `org.apache.gravitino.idp.web.rest.feature`. Review Comment: Is it correct? ########## docs/security/how-to-authenticate.md: ########## @@ -43,6 +43,45 @@ curl -v -X GET \ http://localhost:8090/api/version ``` +### Basic mode + +In Basic mode, Gravitino verifies HTTP Basic credentials against built-in IDP user metadata stored +in the relational entity store. + +To enable Basic mode: + +- Include the `idp-basic` plugin jar on the server classpath (included in the official + distribution). +- Set `gravitino.authenticators` to `basic`. Review Comment: Users don't need to set `gravitino.authenticators`. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpUserGroupManager.java: ########## @@ -100,6 +127,27 @@ public IdpUser getUser(String username) { return new IdpUser(userPO.getUsername(), USER_SERVICE.listGroupNamesByUsername(username)); } + /** + * Authenticates a built-in IdP user with the given plaintext password. + * + * @param username The username. + * @param password The plaintext password. + * @return The authenticated user with group memberships, or {@code null} when credentials are + * invalid. + */ + @Nullable + public IdpUser authenticate(String username, String password) { + try { + IdpUserPO userPO = USER_SERVICE.getIdpUserByUsername(username); + if (!passwordHasher.verify(password, userPO.getPasswordHash())) { + return null; + } + return new IdpUser(username, USER_SERVICE.listGroupNamesByUsername(username)); + } catch (NotFoundException e) { Review Comment: You return UnauthoirizationException. ########## docs/security/how-to-authenticate.md: ########## @@ -315,6 +354,49 @@ The signature algorithms that Gravitino supports follows: | PS384 | RSASSA-PSS using SHA-384 and MGF1 with SHA-384 | | PS512 | RSASSA-PSS using SHA-512 and MGF1 with SHA-512 | +### Example: Basic authentication + +This example shows how to enable built-in Basic authentication with the relational entity store. + +**Prerequisites:** + +- Gravitino distribution package (includes the idp-basic plugin on the server classpath) +- Relational entity store configured (H2, MySQL, or PostgreSQL) + +**Configuration:** + +Append the following to `conf/gravitino.conf`: + +```text +gravitino.entity.store = relational +gravitino.entity.store.relational = JDBCBackend +gravitino.authenticators = basic +gravitino.server.rest.extensionPackages = org.apache.gravitino.idp.web.rest.feature +gravitino.authorization.serviceAdmins = admin +``` + +On the first startup, if the `admin` service admin does not yet have a password in the store, +set initial passwords before starting the server: + +```bash +export GRAVITINO_INITIAL_ADMIN_PASSWORD='["admin:YourSecureGravitinoPassword"]' Review Comment: Could all the admin be the same admin password? ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/ServiceAdminInitializer.java: ########## @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.auth; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.auth.AuthenticatorType; +import org.apache.gravitino.idp.basic.IdpCredentialValidator; +import org.apache.gravitino.idp.basic.password.PasswordHasher; +import org.apache.gravitino.idp.basic.password.PasswordHasherFactory; +import org.apache.gravitino.idp.exception.NotFoundException; +import org.apache.gravitino.idp.storage.po.IdpUserPO; +import org.apache.gravitino.idp.storage.service.IdpUserMetaService; +import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.utils.POConverters; + +/** Initializes configured service admins in the built-in IdP during server startup. */ +public final class ServiceAdminInitializer { + + static final String INITIAL_ADMIN_PASSWORD_ENV = "GRAVITINO_INITIAL_ADMIN_PASSWORD"; + + private static final String BASIC_AUTHENTICATOR = AuthenticatorType.BASIC.name().toLowerCase(); + + private ServiceAdminInitializer() {} + + /** + * Initialize the service admins using the current runtime environment. + * + * @param config The configuration object to initialize the service admins. + */ + public static void initialize(Config config) throws IOException { + initialize( + config, + IdpUserMetaService.getInstance(), + PasswordHasherFactory.create(), + GravitinoEnv.getInstance().idGenerator(), + System.getenv(INITIAL_ADMIN_PASSWORD_ENV)); + } + + static void initialize( + Config config, + IdpUserMetaService userMetaService, + PasswordHasher passwordHasher, + IdGenerator idGenerator, + @Nullable String initialAdminPasswords) + throws IOException { + List<String> authenticators = config.get(Configs.AUTHENTICATORS); + if (authenticators == null || !authenticators.contains(BASIC_AUTHENTICATOR)) { + return; + } + + List<String> serviceAdmins = config.get(Configs.SERVICE_ADMINS); + if (serviceAdmins == null || serviceAdmins.isEmpty()) { + return; + } + + Map<String, String> passwordsByAdmin = + parseInitialAdminPasswords(serviceAdmins, initialAdminPasswords); + for (String serviceAdmin : serviceAdmins) { + IdpCredentialValidator.validateUsername(serviceAdmin); + if (userExists(userMetaService, serviceAdmin)) { + continue; + } + + String password = passwordsByAdmin.get(serviceAdmin); + Preconditions.checkArgument( + StringUtils.isNotBlank(password), + "Missing initial password for configured service admin %s; declare %s", + serviceAdmin, + INITIAL_ADMIN_PASSWORD_ENV); + userMetaService.insertIdpUser( + newServiceAdminUser(idGenerator, passwordHasher, serviceAdmin, password)); + } + } + + private static IdpUserPO newServiceAdminUser( + IdGenerator idGenerator, PasswordHasher passwordHasher, String username, String password) { + return IdpUserPO.builder() + .withUserId(idGenerator.nextId()) + .withUsername(username) + .withPasswordHash(passwordHasher.hash(password)) + .withCurrentVersion(POConverters.INIT_VERSION) + .withLastVersion(POConverters.INIT_VERSION) + .withDeletedAt(POConverters.DEFAULT_DELETED_AT) + .build(); + } + + private static boolean userExists(IdpUserMetaService userMetaService, String username) { + try { + userMetaService.getIdpUserByUsername(username); + return true; + } catch (NotFoundException e) { + return false; + } + } + + private static Map<String, String> parseInitialAdminPasswords( + List<String> serviceAdmins, @Nullable String initialAdminPasswords) { Review Comment: Why is`initialAdminPasswords` nullable? ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/ServiceAdminInitializer.java: ########## @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.auth; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.auth.AuthenticatorType; +import org.apache.gravitino.idp.basic.IdpCredentialValidator; +import org.apache.gravitino.idp.basic.password.PasswordHasher; +import org.apache.gravitino.idp.basic.password.PasswordHasherFactory; +import org.apache.gravitino.idp.exception.NotFoundException; +import org.apache.gravitino.idp.storage.po.IdpUserPO; +import org.apache.gravitino.idp.storage.service.IdpUserMetaService; +import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.utils.POConverters; + +/** Initializes configured service admins in the built-in IdP during server startup. */ +public final class ServiceAdminInitializer { Review Comment: Could u put this logic into the user group manager? ########## server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticatorFactory.java: ########## @@ -40,7 +40,9 @@ public class AuthenticatorFactory { AuthenticatorType.OAUTH.name().toLowerCase(), OAuth2TokenAuthenticator.class.getCanonicalName(), AuthenticatorType.KERBEROS.name().toLowerCase(), - KerberosAuthenticator.class.getCanonicalName()); + KerberosAuthenticator.class.getCanonicalName(), + AuthenticatorType.BASIC.name().toLowerCase(), Review Comment: No need. You can remove this. We shouldn't let users to configure basic mode in the server side. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/storage/relational/IdpGarbageCollector.java: ########## @@ -41,6 +41,8 @@ public final class IdpGarbageCollector implements Closeable { private static final IdpUserMetaService USER_SERVICE = IdpUserMetaService.getInstance(); private static final IdpGroupMetaService GROUP_SERVICE = IdpGroupMetaService.getInstance(); + private static volatile IdpGarbageCollector instance; Review Comment: You don't need to use the singleton design mode here. The singleton design mode isn't easy to test. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/ServiceAdminInitializer.java: ########## @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.auth; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.auth.AuthenticatorType; +import org.apache.gravitino.idp.basic.IdpCredentialValidator; +import org.apache.gravitino.idp.basic.password.PasswordHasher; +import org.apache.gravitino.idp.basic.password.PasswordHasherFactory; +import org.apache.gravitino.idp.exception.NotFoundException; +import org.apache.gravitino.idp.storage.po.IdpUserPO; +import org.apache.gravitino.idp.storage.service.IdpUserMetaService; +import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.utils.POConverters; + +/** Initializes configured service admins in the built-in IdP during server startup. */ +public final class ServiceAdminInitializer { + + static final String INITIAL_ADMIN_PASSWORD_ENV = "GRAVITINO_INITIAL_ADMIN_PASSWORD"; + + private static final String BASIC_AUTHENTICATOR = AuthenticatorType.BASIC.name().toLowerCase(); + + private ServiceAdminInitializer() {} + + /** + * Initialize the service admins using the current runtime environment. + * + * @param config The configuration object to initialize the service admins. + */ + public static void initialize(Config config) throws IOException { + initialize( + config, + IdpUserMetaService.getInstance(), + PasswordHasherFactory.create(), + GravitinoEnv.getInstance().idGenerator(), + System.getenv(INITIAL_ADMIN_PASSWORD_ENV)); + } + + static void initialize( + Config config, + IdpUserMetaService userMetaService, + PasswordHasher passwordHasher, + IdGenerator idGenerator, + @Nullable String initialAdminPasswords) + throws IOException { + List<String> authenticators = config.get(Configs.AUTHENTICATORS); + if (authenticators == null || !authenticators.contains(BASIC_AUTHENTICATOR)) { Review Comment: You should set your authenticator class to the authentocators configuration option. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/idp/auth/ServiceAdminInitializer.java: ########## @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.idp.auth; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.auth.AuthenticatorType; +import org.apache.gravitino.idp.basic.IdpCredentialValidator; +import org.apache.gravitino.idp.basic.password.PasswordHasher; +import org.apache.gravitino.idp.basic.password.PasswordHasherFactory; +import org.apache.gravitino.idp.exception.NotFoundException; +import org.apache.gravitino.idp.storage.po.IdpUserPO; +import org.apache.gravitino.idp.storage.service.IdpUserMetaService; +import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.storage.IdGenerator; +import org.apache.gravitino.storage.relational.utils.POConverters; + +/** Initializes configured service admins in the built-in IdP during server startup. */ +public final class ServiceAdminInitializer { + + static final String INITIAL_ADMIN_PASSWORD_ENV = "GRAVITINO_INITIAL_ADMIN_PASSWORD"; + + private static final String BASIC_AUTHENTICATOR = AuthenticatorType.BASIC.name().toLowerCase(); + + private ServiceAdminInitializer() {} + + /** + * Initialize the service admins using the current runtime environment. + * + * @param config The configuration object to initialize the service admins. + */ + public static void initialize(Config config) throws IOException { + initialize( + config, + IdpUserMetaService.getInstance(), + PasswordHasherFactory.create(), + GravitinoEnv.getInstance().idGenerator(), + System.getenv(INITIAL_ADMIN_PASSWORD_ENV)); + } + + static void initialize( + Config config, + IdpUserMetaService userMetaService, + PasswordHasher passwordHasher, + IdGenerator idGenerator, + @Nullable String initialAdminPasswords) + throws IOException { + List<String> authenticators = config.get(Configs.AUTHENTICATORS); + if (authenticators == null || !authenticators.contains(BASIC_AUTHENTICATOR)) { + return; + } + + List<String> serviceAdmins = config.get(Configs.SERVICE_ADMINS); + if (serviceAdmins == null || serviceAdmins.isEmpty()) { + return; + } + + Map<String, String> passwordsByAdmin = + parseInitialAdminPasswords(serviceAdmins, initialAdminPasswords); + for (String serviceAdmin : serviceAdmins) { + IdpCredentialValidator.validateUsername(serviceAdmin); + if (userExists(userMetaService, serviceAdmin)) { + continue; + } + + String password = passwordsByAdmin.get(serviceAdmin); + Preconditions.checkArgument( + StringUtils.isNotBlank(password), + "Missing initial password for configured service admin %s; declare %s", + serviceAdmin, + INITIAL_ADMIN_PASSWORD_ENV); + userMetaService.insertIdpUser( + newServiceAdminUser(idGenerator, passwordHasher, serviceAdmin, password)); + } + } + + private static IdpUserPO newServiceAdminUser( Review Comment: `addServiceAdminUser` ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
