Added group name case conversion

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,19 +36,25 @@
 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
 {
     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<String, GroupProviderFactory> groupProviderFactories = new ConcurrentHashMap<>();
     private final AtomicReference<Optional<GroupProvider>> 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<String> 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());
     }
 }

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<Rule> 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;

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);
+}

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<String, String> 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()));
         }
     }
 }

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,15 +123,15 @@ 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("[email protected]")).isEqualTo("test");
     }
 
     @Test
     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("[email protected]")).isEqualTo("TEST");
     }
 

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
 

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)=