diff --git a/core/trino-main/src/main/java/io/trino/security/GroupProviderManager.java b/core/trino-main/src/main/java/io/trino/security/GroupProviderManager.java index ae2ce87ab17f..d3fab1ef2ab6 100644 --- a/core/trino-main/src/main/java/io/trino/security/GroupProviderManager.java +++ b/core/trino-main/src/main/java/io/trino/security/GroupProviderManager.java @@ -22,6 +22,7 @@ import io.trino.spi.classloader.ThreadContextClassLoader; import io.trino.spi.security.GroupProvider; import io.trino.spi.security.GroupProviderFactory; +import io.trino.util.Case; import java.io.File; import java.io.IOException; @@ -35,9 +36,13 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.airlift.configuration.ConfigurationLoader.loadPropertiesFrom; +import static io.trino.util.Case.KEEP; import static java.lang.String.format; +import static java.util.Arrays.stream; import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.joining; public class GroupProviderManager implements GroupProvider @@ -45,9 +50,11 @@ public class GroupProviderManager private static final Logger log = Logger.get(GroupProviderManager.class); private static final File GROUP_PROVIDER_CONFIGURATION = new File("etc/group-provider.properties"); private static final String GROUP_PROVIDER_PROPERTY_NAME = "group-provider.name"; + private static final String GROUP_PROVIDER_PROPERTY_GROUP_CASE = "group-provider.group-case"; private final Map groupProviderFactories = new ConcurrentHashMap<>(); private final AtomicReference> configuredGroupProvider = new AtomicReference<>(Optional.empty()); private final SecretsResolver secretsResolver; + private Case groupCase = KEEP; @Inject public GroupProviderManager(SecretsResolver secretsResolver) @@ -83,6 +90,19 @@ void loadConfiguredGroupProvider(File groupProviderFile) checkArgument(!isNullOrEmpty(groupProviderName), "Group provider configuration %s does not contain %s", groupProviderFile.getAbsoluteFile(), GROUP_PROVIDER_PROPERTY_NAME); + String groupCase = properties.remove(GROUP_PROVIDER_PROPERTY_GROUP_CASE); + if (groupCase != null) { + this.groupCase = stream(Case.values()) + .map(Case::toString) + .filter(groupCase::equalsIgnoreCase) + .map(Case::valueOf) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException(format("Group provider configuration %s does not contain valid %s. Expected one of: %s", + groupProviderFile.getAbsoluteFile(), + GROUP_PROVIDER_PROPERTY_GROUP_CASE, + stream(Case.values()).map(Case::toString).collect(joining(", ", "[", "]"))))); + } + setConfiguredGroupProvider(groupProviderName, properties); } @@ -119,7 +139,7 @@ public Set getGroups(String user) requireNonNull(user, "user is null"); return configuredGroupProvider.get() .map(provider -> provider.getGroups(user)) - .map(ImmutableSet::copyOf) + .map(groups -> groups.stream().map(this.groupCase::transform).collect(toImmutableSet())) .orElse(ImmutableSet.of()); } } diff --git a/core/trino-main/src/main/java/io/trino/server/security/UserMapping.java b/core/trino-main/src/main/java/io/trino/server/security/UserMapping.java index 8256923393ff..10db6b045239 100644 --- a/core/trino-main/src/main/java/io/trino/server/security/UserMapping.java +++ b/core/trino-main/src/main/java/io/trino/server/security/UserMapping.java @@ -17,6 +17,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import io.trino.util.Case; import java.io.File; import java.util.List; @@ -26,9 +27,8 @@ import static com.google.common.base.Preconditions.checkArgument; import static io.trino.plugin.base.util.JsonUtils.parseJson; -import static io.trino.server.security.UserMapping.Case.KEEP; +import static io.trino.util.Case.KEEP; import static java.lang.Boolean.TRUE; -import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; public final class UserMapping @@ -86,33 +86,6 @@ public List getRules() } } - enum Case - { - KEEP { - @Override - public String transform(String value) - { - return value; - } - }, - LOWER { - @Override - public String transform(String value) - { - return value.toLowerCase(ENGLISH); - } - }, - UPPER { - @Override - public String transform(String value) - { - return value.toUpperCase(ENGLISH); - } - }; - - public abstract String transform(String value); - } - public static final class Rule { private final Pattern pattern; diff --git a/core/trino-main/src/main/java/io/trino/util/Case.java b/core/trino-main/src/main/java/io/trino/util/Case.java new file mode 100644 index 000000000000..c833f606bbb7 --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/util/Case.java @@ -0,0 +1,43 @@ +/* + * Licensed 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 io.trino.util; + +import static java.util.Locale.ENGLISH; + +public enum Case +{ + KEEP { + @Override + public String transform(String value) + { + return value; + } + }, + LOWER { + @Override + public String transform(String value) + { + return value.toLowerCase(ENGLISH); + } + }, + UPPER { + @Override + public String transform(String value) + { + return value.toUpperCase(ENGLISH); + } + }; + + public abstract String transform(String value); +} diff --git a/core/trino-main/src/test/java/io/trino/security/TestGroupProviderManager.java b/core/trino-main/src/test/java/io/trino/security/TestGroupProviderManager.java index 923b6451f849..30fa4461c0a1 100644 --- a/core/trino-main/src/test/java/io/trino/security/TestGroupProviderManager.java +++ b/core/trino-main/src/test/java/io/trino/security/TestGroupProviderManager.java @@ -20,13 +20,16 @@ import io.trino.spi.security.GroupProvider; import io.trino.spi.security.GroupProviderFactory; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.IOException; import java.nio.file.Files; import java.util.Map; -import static java.nio.charset.StandardCharsets.UTF_8; +import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestGroupProviderManager { @@ -46,17 +49,78 @@ public GroupProvider create(Map config) } }; - @Test - public void testGroupProviderIsLoaded() + @ParameterizedTest + @ValueSource(strings = {"", "group-provider.group-case=keep", "group-provider.group-case=KEEP"}) + public void testGroupProviderIsLoaded(String additional) throws IOException { try (TempFile tempFile = new TempFile()) { - Files.write(tempFile.path(), "group-provider.name=testGroupProvider".getBytes(UTF_8)); + Files.writeString(tempFile.path(), """ + group-provider.name=testGroupProvider + """ + additional); + + GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of())); + groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY); + groupProviderManager.loadConfiguredGroupProvider(tempFile.file()); + + assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("test", "Alice")); + assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("test", "Bob")); + } + } + + @Test + void setTestGroupProviderUpperCase() throws Exception + { + try (TempFile tempFile = new TempFile()) { + Files.writeString(tempFile.path(), """ + group-provider.name=testGroupProvider + group-provider.group-case=upper + """); + GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of())); groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY); groupProviderManager.loadConfiguredGroupProvider(tempFile.file()); - assertThat(groupProviderManager.getGroups("alice")).isEqualTo(ImmutableSet.of("test", "alice")); - assertThat(groupProviderManager.getGroups("bob")).isEqualTo(ImmutableSet.of("test", "bob")); + + assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("TEST", "ALICE")); + assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("TEST", "BOB")); + } + } + + @Test + void setTestGroupProviderLowerCase() throws Exception + { + try (TempFile tempFile = new TempFile()) { + Files.writeString(tempFile.path(), """ + group-provider.name=testGroupProvider + group-provider.group-case=lower + """); + + GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of())); + groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY); + groupProviderManager.loadConfiguredGroupProvider(tempFile.file()); + + assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("test", "alice")); + assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("test", "bob")); + } + } + + @Test + void setTestGroupProviderInvalidCase() throws Exception + { + try (TempFile tempFile = new TempFile()) { + Files.writeString(tempFile.path(), """ + group-provider.name=testGroupProvider + group-provider.group-case=invalid + """); + + GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of())); + groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY); + + assertThatThrownBy(() -> groupProviderManager.loadConfiguredGroupProvider(tempFile.file())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(format( + "Group provider configuration %s does not contain valid group-provider.group-case. Expected one of: [KEEP, LOWER, UPPER]", + tempFile.path().toAbsolutePath())); } } } diff --git a/core/trino-main/src/test/java/io/trino/server/security/TestUserMapping.java b/core/trino-main/src/test/java/io/trino/server/security/TestUserMapping.java index 9998d83515b7..30fef88f2f5c 100644 --- a/core/trino-main/src/test/java/io/trino/server/security/TestUserMapping.java +++ b/core/trino-main/src/test/java/io/trino/server/security/TestUserMapping.java @@ -22,8 +22,10 @@ import java.net.URISyntaxException; import java.util.Optional; -import static io.trino.server.security.UserMapping.Case.KEEP; import static io.trino.server.security.UserMapping.createUserMapping; +import static io.trino.util.Case.KEEP; +import static io.trino.util.Case.LOWER; +import static io.trino.util.Case.UPPER; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -121,7 +123,7 @@ public void testMultipleRule() public void testLowercaseUsernameRule() throws UserMappingException { - UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@EXAMPLE\\.COM", "$1", true, UserMapping.Case.LOWER))); + UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@EXAMPLE\\.COM", "$1", true, LOWER))); assertThat(userMapping.mapUser("TEST@EXAMPLE.COM")).isEqualTo("test"); } @@ -129,7 +131,7 @@ public void testLowercaseUsernameRule() public void testUppercaseUsernameRule() throws UserMappingException { - UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@example\\.com", "$1", true, UserMapping.Case.UPPER))); + UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@example\\.com", "$1", true, UPPER))); assertThat(userMapping.mapUser("test@example.com")).isEqualTo("TEST"); } diff --git a/docs/src/main/sphinx/release/release-344.md b/docs/src/main/sphinx/release/release-344.md index 476748dba416..c12ce77fef66 100644 --- a/docs/src/main/sphinx/release/release-344.md +++ b/docs/src/main/sphinx/release/release-344.md @@ -44,7 +44,7 @@ Please, read these release notes carefully. * Change file-based system and catalog access controls to only show catalogs, schemas, and tables a user has permissions on. ({issue}`5039`) * Change file-based catalog access control to deny permissions inspection and manipulation. ({issue}`5039`) -* Add [file-based group provider](/security/group-file). ({issue}`5028`) +* Add [file-based group provider](/security/group-mapping). ({issue}`5028`) ## Hive connector diff --git a/docs/src/main/sphinx/security.md b/docs/src/main/sphinx/security.md index 8cdb977066db..31912e98e47c 100644 --- a/docs/src/main/sphinx/security.md +++ b/docs/src/main/sphinx/security.md @@ -40,7 +40,7 @@ security/jwt :maxdepth: 1 security/user-mapping -security/group-file +security/group-mapping ``` (security-access-control)= diff --git a/docs/src/main/sphinx/security/group-file.md b/docs/src/main/sphinx/security/group-file.md deleted file mode 100644 index e773603aa77f..000000000000 --- a/docs/src/main/sphinx/security/group-file.md +++ /dev/null @@ -1,33 +0,0 @@ -# File group provider - -Trino can map usernames onto groups for easier access control and -resource group management. Group file resolves group membership using -a file on the coordinator. - -## Group file configuration - -Enable group file by creating an `etc/group-provider.properties` -file on the coordinator: - -```text -group-provider.name=file -file.group-file=/path/to/group.txt -``` - -The following configuration properties are available: - -| Property | Description | -| --------------------- | ----------------------------------------------------- | -| `file.group-file` | Path of the group file. | -| `file.refresh-period` | How often to reload the group file. Defaults to `5s`. | - -## Group files - -### File format - -The group file contains a list of groups and members, one per line, -separated by a colon. Users are separated by a comma. - -```text -group_name:user_1,user_2,user_3 -``` diff --git a/docs/src/main/sphinx/security/group-mapping.md b/docs/src/main/sphinx/security/group-mapping.md new file mode 100644 index 000000000000..e2e52b0cfcc0 --- /dev/null +++ b/docs/src/main/sphinx/security/group-mapping.md @@ -0,0 +1,79 @@ +# Group mapping + +Group providers in Trino to map usernames onto groups for easier access control +and resource group management. + +Configure a group provider by creating an `etc/group-provider.properties` file +on the coordinator: + +```properties +group-provider.name=file +``` +_The configuration of the chosen group provider should be appended to this +file_ + +:::{list-table} Group provider configuration +:widths: 40, 60 +:header-rows: 1 + +* - Property name + - Description + +* - `group-provider.name` + - Name of the group provider to use. + Supported values are: + + * `file`: [See configuration](file-group-provider) + * `ldap` +* - `group-provider.group-case` + - Optional transformation of the case of the group name. + Supported values are: + + * `keep`: default, no conversion + * `upper`: convert group name to _UPPERCASE_ + * `lower`: converts the group name to _lowercase_ + + Defaults to `keep`. +::: + +(file-group-provider)= +## File group provider + +Group file resolves group membership using a file on the coordinator. + +### Group file configuration + +Enable group file by creating an `etc/group-provider.properties` +file on the coordinator: + +```properties +group-provider.name=file +file.group-file=/path/to/group.txt +``` + +The following configuration properties are available: + +:::{list-table} File group provider configuration +:widths: 40, 60 +:header-rows: 1 + +* - Property name + - Description + +* - `file.group-file` + - Path of the group file +* - `file.refresh-period` + - [Duration](prop-type-duration) between refreshing the group mapping + configuration from the file. + Defaults to `5s`. + ::: + +### Group file format + +The group file contains a list of groups and members, one per line, +separated by a colon. Users are separated by a comma. + +```text +group_name:user_1,user_2,user_3 +``` + diff --git a/docs/src/main/sphinx/security/overview.md b/docs/src/main/sphinx/security/overview.md index 07ce2a9a5b0d..f244a19bcb7b 100644 --- a/docs/src/main/sphinx/security/overview.md +++ b/docs/src/main/sphinx/security/overview.md @@ -100,7 +100,7 @@ providers to Trino usernames. and allows for regular expression rules to be specified that map complex usernames from other systems (`alice@example.com`) to simple usernames (`alice`). -- {doc}`File group provider ` provides a way to assign a set +- {doc}`Group mapping ` provides ways to assign a set of usernames to a group name to ease access control. (cl-access-control)=