kevdoran commented on a change in pull request #232: URL: https://github.com/apache/nifi-registry/pull/232#discussion_r462516281
########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/identity/IdentityMapper.java ########## @@ -0,0 +1,40 @@ +/* + * 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.nifi.registry.security.identity; + + +public interface IdentityMapper { Review comment: I really like introducing this interface / bean. Simplifies the usage throughout. NiFi (or std libs) should do the same eventually. ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/util/InitialPolicies.java ########## @@ -0,0 +1,99 @@ +/* + * 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.nifi.registry.security.authorization.util; + +import org.apache.nifi.registry.security.authorization.RequestAction; +import org.apache.nifi.registry.security.authorization.resource.ResourceFactory; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Defines the initial policies to be created for the initial users. + */ +public final class InitialPolicies { Review comment: Yeah this new class is a big improvement over the old hard-coded stuff in FileAPP. Nice refactoring. ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/file/FileAccessPolicyProvider.java ########## @@ -70,17 +69,13 @@ import java.util.Collections; import java.util.Comparator; import java.util.Date; -import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -public class FileAccessPolicyProvider implements ConfigurableAccessPolicyProvider { +public class FileAccessPolicyProvider extends AbstractConfigurableAccessPolicyProvider { Review comment: wow this class got a lot simpler 👍 ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/util/UserGroupProviderUtils.java ########## @@ -0,0 +1,51 @@ +/* + * 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.nifi.registry.security.authorization.util; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.registry.security.authorization.AuthorizerConfigurationContext; +import org.apache.nifi.registry.security.identity.IdentityMapper; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Utility methods related to access policies for use by various {@link org.apache.nifi.registry.security.authorization.UserGroupProvider} implementations. + */ +public final class UserGroupProviderUtils { + + public static final String PROP_INITIAL_USER_IDENTITY_PREFIX = "Initial User Identity "; + public static final Pattern INITIAL_USER_IDENTITY_PATTERN = Pattern.compile(PROP_INITIAL_USER_IDENTITY_PREFIX + "\\S+"); + + public static Set<String> getInitialUserIdentities(final AuthorizerConfigurationContext configurationContext, final IdentityMapper identityMapper) { + final Set<String> initialUserIdentities = new HashSet<>(); + for (Map.Entry<String,String> entry : configurationContext.getProperties().entrySet()) { + Matcher matcher = UserGroupProviderUtils.INITIAL_USER_IDENTITY_PATTERN.matcher(entry.getKey()); + if (matcher.matches() && !StringUtils.isBlank(entry.getValue())) { + initialUserIdentities.add(identityMapper.mapUser(entry.getValue())); Review comment: Same question here as my comment in APPUtils regarding the use of IdentityMapper to map user-provided values. ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/database/DatabaseUserGroupProvider.java ########## @@ -0,0 +1,387 @@ +/* + * 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.nifi.registry.security.authorization.database; + +import org.apache.commons.lang3.Validate; +import org.apache.nifi.registry.security.authorization.AuthorizerConfigurationContext; +import org.apache.nifi.registry.security.authorization.ConfigurableUserGroupProvider; +import org.apache.nifi.registry.security.authorization.Group; +import org.apache.nifi.registry.security.authorization.User; +import org.apache.nifi.registry.security.authorization.UserAndGroups; +import org.apache.nifi.registry.security.authorization.UserGroupProviderInitializationContext; +import org.apache.nifi.registry.security.authorization.annotation.AuthorizerContext; +import org.apache.nifi.registry.security.authorization.database.entity.DatabaseGroup; +import org.apache.nifi.registry.security.authorization.database.entity.DatabaseUser; +import org.apache.nifi.registry.security.authorization.database.mapper.DatabaseGroupRowMapper; +import org.apache.nifi.registry.security.authorization.database.mapper.DatabaseUserRowMapper; +import org.apache.nifi.registry.security.authorization.exception.AuthorizationAccessException; +import org.apache.nifi.registry.security.authorization.exception.UninheritableAuthorizationsException; +import org.apache.nifi.registry.security.authorization.util.UserGroupProviderUtils; +import org.apache.nifi.registry.security.exception.SecurityProviderCreationException; +import org.apache.nifi.registry.security.exception.SecurityProviderDestructionException; +import org.apache.nifi.registry.security.identity.IdentityMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.dao.EmptyResultDataAccessException; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowMapper; + +import javax.sql.DataSource; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Implementation of {@link org.apache.nifi.registry.security.authorization.ConfigurableUserGroupProvider} backed by a relational database. + */ +public class DatabaseUserGroupProvider implements ConfigurableUserGroupProvider { + + private static final Logger LOGGER = LoggerFactory.getLogger(DatabaseUserGroupProvider.class); + + private DataSource dataSource; + private IdentityMapper identityMapper; + + private JdbcTemplate jdbcTemplate; + + @AuthorizerContext + public void setDataSource(final DataSource dataSource) { + this.dataSource = dataSource; + } + + @AuthorizerContext + public void setIdentityMapper(final IdentityMapper identityMapper) { + this.identityMapper = identityMapper; + } + + @Override + public void initialize(final UserGroupProviderInitializationContext initializationContext) throws SecurityProviderCreationException { + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + + @Override + public void onConfigured(final AuthorizerConfigurationContext configurationContext) throws SecurityProviderCreationException { + final Set<String> initialUserIdentities = UserGroupProviderUtils.getInitialUserIdentities(configurationContext, identityMapper); + + for (final String initialUserIdentity : initialUserIdentities) { + final User existingUser = getUserByIdentity(initialUserIdentity); + if (existingUser == null) { + final User initialUser = new User.Builder() + .identifierGenerateFromSeed(initialUserIdentity) + .identity(initialUserIdentity) + .build(); + addUser(initialUser); + LOGGER.info("Created initial user with identity {}", new Object[]{initialUserIdentity}); + } else { + LOGGER.debug("User already exists with identity {}", new Object[]{initialUserIdentity}); + } + } + } + + @Override + public void preDestruction() throws SecurityProviderDestructionException { + + } + + //-- fingerprint methods + + @Override + public String getFingerprint() throws AuthorizationAccessException { + throw new UnsupportedOperationException("Fingerprinting is not supported by this provider"); + } + + @Override + public void inheritFingerprint(final String fingerprint) throws AuthorizationAccessException { + throw new UnsupportedOperationException("Fingerprinting is not supported by this provider"); + } + + @Override + public void checkInheritability(final String proposedFingerprint) throws AuthorizationAccessException, UninheritableAuthorizationsException { + throw new UnsupportedOperationException("Fingerprinting is not supported by this provider"); + } + + //-- User CRUD + + @Override + public User addUser(final User user) throws AuthorizationAccessException { + Validate.notNull(user); + final String sql = "INSERT INTO UGP_USER(IDENTIFIER, IDENTITY) VALUES (?, ?)"; + jdbcTemplate.update(sql, new Object[] {user.getIdentifier(), user.getIdentity()}); + return user; + } + + @Override + public User updateUser(final User user) throws AuthorizationAccessException { + Validate.notNull(user); + + // update the user identity + final String sql = "UPDATE UGP_USER SET IDENTITY = ? WHERE IDENTIFIER = ?"; + final int updated = jdbcTemplate.update(sql, user.getIdentity(), user.getIdentifier()); + + // if no rows were updated then there is no user with the given identifier, so return null + if (updated <= 0) { + return null; + } + + return user; + } + + @Override + public Set<User> getUsers() throws AuthorizationAccessException { + final String sql = "SELECT * FROM UGP_USER"; + final List<DatabaseUser> databaseUsers = jdbcTemplate.query(sql, new DatabaseUserRowMapper()); + + final Set<User> users = new HashSet<>(); + databaseUsers.forEach(u -> { + users.add(mapToUser(u)); + }); + return users; + } + + @Override + public User getUser(final String identifier) throws AuthorizationAccessException { + Validate.notBlank(identifier); + + final DatabaseUser databaseUser = getDatabaseUser(identifier); + if (databaseUser == null) { + return null; + } + + return mapToUser(databaseUser); + } + + @Override + public User getUserByIdentity(final String identity) throws AuthorizationAccessException { + Validate.notBlank(identity); + + final String sql = "SELECT * FROM UGP_USER WHERE IDENTITY = ?"; + final DatabaseUser databaseUser = queryForObject(sql, new Object[] {identity}, new DatabaseUserRowMapper()); + if (databaseUser == null) { + return null; + } + + return mapToUser(databaseUser); + } + + @Override + public UserAndGroups getUserAndGroups(final String userIdentity) throws AuthorizationAccessException { + Validate.notBlank(userIdentity); + + // retrieve the user + final User user = getUserByIdentity(userIdentity); + + // if the user exists, then retrieve the groups for the user + final Set<Group> groups; + if (user == null) { + groups = null; + } else { + final String userGroupSql = + "SELECT " + + "G.IDENTIFIER AS IDENTIFIER, " + + "G.IDENTITY AS IDENTITY " + + "FROM " + + "UGP_GROUP AS G, " + + "UGP_USER_GROUP AS UG " + + "WHERE " + + "G.IDENTIFIER = UG.GROUP_IDENTIFIER AND " + + "UG.USER_IDENTIFIER = ?"; Review comment: Thanks for maintaining al these JDBC templates 🙂 ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/util/AccessPolicyProviderUtils.java ########## @@ -0,0 +1,143 @@ +/* + * 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.nifi.registry.security.authorization.util; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.registry.security.authorization.AuthorizerConfigurationContext; +import org.apache.nifi.registry.security.authorization.Group; +import org.apache.nifi.registry.security.authorization.UserGroupProvider; +import org.apache.nifi.registry.security.exception.SecurityProviderCreationException; +import org.apache.nifi.registry.security.identity.IdentityMapper; +import org.apache.nifi.registry.util.PropertyValue; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Utility methods related to access policies for use by various {@link org.apache.nifi.registry.security.authorization.AccessPolicyProvider} implementations. + */ +public final class AccessPolicyProviderUtils { + + private static final Logger LOGGER = LoggerFactory.getLogger(AccessPolicyProviderUtils.class); + + /** + * The prefix of a property from an AuthorizerConfigurationContext that specifies a NiFi Identity. + */ + public static final String PROP_NIFI_IDENTITY_PREFIX = "NiFi Identity "; + + /** + * The name of the property from an AuthorizerConfigurationContext that specifies the initial admin identity. + */ + public static final String PROP_INITIAL_ADMIN_IDENTITY = "Initial Admin Identity"; + + /** + * A Pattern for identifying properties that represent NiFi Identities. + */ + public static final Pattern NIFI_IDENTITY_PATTERN = Pattern.compile(PROP_NIFI_IDENTITY_PREFIX + "\\S+"); + + /** + * The name of the property from AuthorizerConfigurationContext that specifies a name of a group for NiFi Identities. + */ + public static final String PROP_NIFI_GROUP_NAME = "NiFi Group Name"; + + /** + * Returns the value of the 'Initial Admin Identity' property with any identity mappings applied. + * + * @param configurationContext the configuration context + * @param identityMapper the identity mapper + * @return the value for the initial admin identity + */ + public static String getInitialAdminIdentity(final AuthorizerConfigurationContext configurationContext, final IdentityMapper identityMapper) { + final PropertyValue initialAdminIdentityProp = configurationContext.getProperty(PROP_INITIAL_ADMIN_IDENTITY); + return initialAdminIdentityProp.isSet() ? identityMapper.mapUser(initialAdminIdentityProp.getValue()) : null; Review comment: It's not obvious to me that the user-provided values here (and elsewhere in this file / other property readers) should be mapped using the identity mapper. Can you explain that to me? Is that how it works in NiFi? Not saying it is wrong, just wondering what a typical user might be more likely to enter... the pre-mapped identity or expected post-mapped identity. If they enter the post-mapped identity, I suppose it still works, right? If so, this is not a breaking change for existing configurations, right? ########## File path: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/util/AccessPolicyProviderUtils.java ########## @@ -0,0 +1,143 @@ +/* + * 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.nifi.registry.security.authorization.util; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.registry.security.authorization.AuthorizerConfigurationContext; +import org.apache.nifi.registry.security.authorization.Group; +import org.apache.nifi.registry.security.authorization.UserGroupProvider; +import org.apache.nifi.registry.security.exception.SecurityProviderCreationException; +import org.apache.nifi.registry.security.identity.IdentityMapper; +import org.apache.nifi.registry.util.PropertyValue; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Utility methods related to access policies for use by various {@link org.apache.nifi.registry.security.authorization.AccessPolicyProvider} implementations. + */ +public final class AccessPolicyProviderUtils { + + private static final Logger LOGGER = LoggerFactory.getLogger(AccessPolicyProviderUtils.class); + + /** + * The prefix of a property from an AuthorizerConfigurationContext that specifies a NiFi Identity. + */ + public static final String PROP_NIFI_IDENTITY_PREFIX = "NiFi Identity "; + + /** + * The name of the property from an AuthorizerConfigurationContext that specifies the initial admin identity. + */ + public static final String PROP_INITIAL_ADMIN_IDENTITY = "Initial Admin Identity"; + + /** + * A Pattern for identifying properties that represent NiFi Identities. + */ + public static final Pattern NIFI_IDENTITY_PATTERN = Pattern.compile(PROP_NIFI_IDENTITY_PREFIX + "\\S+"); + + /** + * The name of the property from AuthorizerConfigurationContext that specifies a name of a group for NiFi Identities. + */ + public static final String PROP_NIFI_GROUP_NAME = "NiFi Group Name"; + + /** + * Returns the value of the 'Initial Admin Identity' property with any identity mappings applied. + * + * @param configurationContext the configuration context + * @param identityMapper the identity mapper + * @return the value for the initial admin identity + */ + public static String getInitialAdminIdentity(final AuthorizerConfigurationContext configurationContext, final IdentityMapper identityMapper) { + final PropertyValue initialAdminIdentityProp = configurationContext.getProperty(PROP_INITIAL_ADMIN_IDENTITY); + return initialAdminIdentityProp.isSet() ? identityMapper.mapUser(initialAdminIdentityProp.getValue()) : null; Review comment: Yeah that makes sense. Thanks for clarifying that this preserves existing behavior. I checked but had missed that. This is good for me. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
