Skip to content

Commit e1b8c2a

Browse files
gertjanalgertjanal
committed
Add support for group name case conversion
Add configuration property group-provider.group-case={keep/upper/lower}
1 parent acc308c commit e1b8c2a

File tree

11 files changed

+191
-45
lines changed

11 files changed

+191
-45
lines changed

‎core/trino-main/src/main/java/io/trino/security/GroupProviderManager.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.trino.spi.classloader.ThreadContextClassLoader;
2323
import io.trino.spi.security.GroupProvider;
2424
import io.trino.spi.security.GroupProviderFactory;
25+
import io.trino.util.Case;
2526

2627
import java.io.File;
2728
import java.io.IOException;
@@ -35,19 +36,25 @@
3536
import static com.google.common.base.Preconditions.checkArgument;
3637
import static com.google.common.base.Preconditions.checkState;
3738
import static com.google.common.base.Strings.isNullOrEmpty;
39+
import static com.google.common.collect.ImmutableSet.toImmutableSet;
3840
import static io.airlift.configuration.ConfigurationLoader.loadPropertiesFrom;
41+
import static io.trino.util.Case.KEEP;
3942
import static java.lang.String.format;
43+
import static java.util.Arrays.stream;
4044
import static java.util.Objects.requireNonNull;
45+
import static java.util.stream.Collectors.joining;
4146

4247
public class GroupProviderManager
4348
implements GroupProvider
4449
{
4550
private static final Logger log = Logger.get(GroupProviderManager.class);
4651
private static final File GROUP_PROVIDER_CONFIGURATION = new File("etc/group-provider.properties");
4752
private static final String GROUP_PROVIDER_PROPERTY_NAME = "group-provider.name";
53+
private static final String GROUP_PROVIDER_PROPERTY_GROUP_CASE = "group-provider.group-case";
4854
private final Map<String, GroupProviderFactory> groupProviderFactories = new ConcurrentHashMap<>();
4955
private final AtomicReference<Optional<GroupProvider>> configuredGroupProvider = new AtomicReference<>(Optional.empty());
5056
private final SecretsResolver secretsResolver;
57+
private Case groupCase = KEEP;
5158

5259
@Inject
5360
public GroupProviderManager(SecretsResolver secretsResolver)
@@ -83,6 +90,19 @@ void loadConfiguredGroupProvider(File groupProviderFile)
8390
checkArgument(!isNullOrEmpty(groupProviderName),
8491
"Group provider configuration %s does not contain %s", groupProviderFile.getAbsoluteFile(), GROUP_PROVIDER_PROPERTY_NAME);
8592

93+
String groupCase = properties.remove(GROUP_PROVIDER_PROPERTY_GROUP_CASE);
94+
if (groupCase != null) {
95+
this.groupCase = stream(Case.values())
96+
.map(Case::toString)
97+
.filter(groupCase::equalsIgnoreCase)
98+
.map(Case::valueOf)
99+
.findFirst()
100+
.orElseThrow(() -> new IllegalArgumentException(format("Group provider configuration %s does not contain valid %s. Expected one of: %s",
101+
groupProviderFile.getAbsoluteFile(),
102+
GROUP_PROVIDER_PROPERTY_GROUP_CASE,
103+
stream(Case.values()).map(Case::toString).collect(joining(", ", "[", "]")))));
104+
}
105+
86106
setConfiguredGroupProvider(groupProviderName, properties);
87107
}
88108

@@ -119,7 +139,7 @@ public Set<String> getGroups(String user)
119139
requireNonNull(user, "user is null");
120140
return configuredGroupProvider.get()
121141
.map(provider -> provider.getGroups(user))
122-
.map(ImmutableSet::copyOf)
142+
.map(groups -> groups.stream().map(this.groupCase::transform).collect(toImmutableSet()))
123143
.orElse(ImmutableSet.of());
124144
}
125145
}

‎core/trino-main/src/main/java/io/trino/server/security/UserMapping.java‎

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.fasterxml.jackson.annotation.JsonProperty;
1818
import com.google.common.annotations.VisibleForTesting;
1919
import com.google.common.collect.ImmutableList;
20+
import io.trino.util.Case;
2021

2122
import java.io.File;
2223
import java.util.List;
@@ -26,9 +27,8 @@
2627

2728
import static com.google.common.base.Preconditions.checkArgument;
2829
import static io.trino.plugin.base.util.JsonUtils.parseJson;
29-
import static io.trino.server.security.UserMapping.Case.KEEP;
30+
import static io.trino.util.Case.KEEP;
3031
import static java.lang.Boolean.TRUE;
31-
import static java.util.Locale.ENGLISH;
3232
import static java.util.Objects.requireNonNull;
3333

3434
public final class UserMapping
@@ -86,33 +86,6 @@ public List<Rule> getRules()
8686
}
8787
}
8888

89-
enum Case
90-
{
91-
KEEP {
92-
@Override
93-
public String transform(String value)
94-
{
95-
return value;
96-
}
97-
},
98-
LOWER {
99-
@Override
100-
public String transform(String value)
101-
{
102-
return value.toLowerCase(ENGLISH);
103-
}
104-
},
105-
UPPER {
106-
@Override
107-
public String transform(String value)
108-
{
109-
return value.toUpperCase(ENGLISH);
110-
}
111-
};
112-
113-
public abstract String transform(String value);
114-
}
115-
11689
public static final class Rule
11790
{
11891
private final Pattern pattern;

‎core/trino-main/src/main/java/io/trino/util/Case.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package io.trino.util;
15+
16+
import static java.util.Locale.ROOT;
17+
18+
public enum Case
19+
{
20+
KEEP {
21+
@Override
22+
public String transform(String value)
23+
{
24+
return value;
25+
}
26+
},
27+
LOWER {
28+
@Override
29+
public String transform(String value)
30+
{
31+
return value.toLowerCase(ROOT);
32+
}
33+
},
34+
UPPER {
35+
@Override
36+
public String transform(String value)
37+
{
38+
return value.toUpperCase(ROOT);
39+
}
40+
};
41+
42+
public abstract String transform(String value);
43+
}

‎core/trino-main/src/test/java/io/trino/security/TestGroupProviderManager.java‎

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@
2020
import io.trino.spi.security.GroupProvider;
2121
import io.trino.spi.security.GroupProviderFactory;
2222
import org.junit.jupiter.api.Test;
23+
import org.junit.jupiter.params.ParameterizedTest;
24+
import org.junit.jupiter.params.provider.ValueSource;
2325

2426
import java.io.IOException;
2527
import java.nio.file.Files;
2628
import java.util.Map;
2729

28-
import static java.nio.charset.StandardCharsets.UTF_8;
30+
import static java.lang.String.format;
2931
import static org.assertj.core.api.Assertions.assertThat;
32+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3033

3134
public class TestGroupProviderManager
3235
{
@@ -46,17 +49,78 @@ public GroupProvider create(Map<String, String> config)
4649
}
4750
};
4851

49-
@Test
50-
public void testGroupProviderIsLoaded()
52+
@ParameterizedTest
53+
@ValueSource(strings = {"", "group-provider.group-case=keep", "group-provider.group-case=KEEP"})
54+
public void testGroupProviderIsLoaded(String additional)
5155
throws IOException
5256
{
5357
try (TempFile tempFile = new TempFile()) {
54-
Files.write(tempFile.path(), "group-provider.name=testGroupProvider".getBytes(UTF_8));
58+
Files.writeString(tempFile.path(), """
59+
group-provider.name=testGroupProvider
60+
""" + additional);
61+
62+
GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of()));
63+
groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY);
64+
groupProviderManager.loadConfiguredGroupProvider(tempFile.file());
65+
66+
assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("test", "Alice"));
67+
assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("test", "Bob"));
68+
}
69+
}
70+
71+
@Test
72+
void setTestGroupProviderUpperCase() throws Exception
73+
{
74+
try (TempFile tempFile = new TempFile()) {
75+
Files.writeString(tempFile.path(), """
76+
group-provider.name=testGroupProvider
77+
group-provider.group-case=upper
78+
""");
79+
5580
GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of()));
5681
groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY);
5782
groupProviderManager.loadConfiguredGroupProvider(tempFile.file());
58-
assertThat(groupProviderManager.getGroups("alice")).isEqualTo(ImmutableSet.of("test", "alice"));
59-
assertThat(groupProviderManager.getGroups("bob")).isEqualTo(ImmutableSet.of("test", "bob"));
83+
84+
assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("TEST", "ALICE"));
85+
assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("TEST", "BOB"));
86+
}
87+
}
88+
89+
@Test
90+
void setTestGroupProviderLowerCase() throws Exception
91+
{
92+
try (TempFile tempFile = new TempFile()) {
93+
Files.writeString(tempFile.path(), """
94+
group-provider.name=testGroupProvider
95+
group-provider.group-case=lower
96+
""");
97+
98+
GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of()));
99+
groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY);
100+
groupProviderManager.loadConfiguredGroupProvider(tempFile.file());
101+
102+
assertThat(groupProviderManager.getGroups("Alice")).isEqualTo(ImmutableSet.of("test", "alice"));
103+
assertThat(groupProviderManager.getGroups("Bob")).isEqualTo(ImmutableSet.of("test", "bob"));
104+
}
105+
}
106+
107+
@Test
108+
void setTestGroupProviderInvalidCase() throws Exception
109+
{
110+
try (TempFile tempFile = new TempFile()) {
111+
Files.writeString(tempFile.path(), """
112+
group-provider.name=testGroupProvider
113+
group-provider.group-case=invalid
114+
""");
115+
116+
GroupProviderManager groupProviderManager = new GroupProviderManager(new SecretsResolver(ImmutableMap.of()));
117+
groupProviderManager.addGroupProviderFactory(TEST_GROUP_PROVIDER_FACTORY);
118+
119+
assertThatThrownBy(() -> groupProviderManager.loadConfiguredGroupProvider(tempFile.file()))
120+
.isInstanceOf(IllegalArgumentException.class)
121+
.hasMessageContaining(format(
122+
"Group provider configuration %s does not contain valid group-provider.group-case. Expected one of: [KEEP, LOWER, UPPER]",
123+
tempFile.path().toAbsolutePath()));
60124
}
61125
}
62126
}

‎core/trino-main/src/test/java/io/trino/server/security/TestUserMapping.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
import java.net.URISyntaxException;
2323
import java.util.Optional;
2424

25-
import static io.trino.server.security.UserMapping.Case.KEEP;
2625
import static io.trino.server.security.UserMapping.createUserMapping;
26+
import static io.trino.util.Case.KEEP;
27+
import static io.trino.util.Case.LOWER;
28+
import static io.trino.util.Case.UPPER;
2729
import static org.assertj.core.api.Assertions.assertThat;
2830
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2931

@@ -121,15 +123,15 @@ public void testMultipleRule()
121123
public void testLowercaseUsernameRule()
122124
throws UserMappingException
123125
{
124-
UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@EXAMPLE\\.COM", "$1", true, UserMapping.Case.LOWER)));
126+
UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@EXAMPLE\\.COM", "$1", true, LOWER)));
125127
assertThat(userMapping.mapUser("[email protected]")).isEqualTo("test");
126128
}
127129

128130
@Test
129131
public void testUppercaseUsernameRule()
130132
throws UserMappingException
131133
{
132-
UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@example\\.com", "$1", true, UserMapping.Case.UPPER)));
134+
UserMapping userMapping = new UserMapping(ImmutableList.of(new Rule("(.*)@example\\.com", "$1", true, UPPER)));
133135
assertThat(userMapping.mapUser("[email protected]")).isEqualTo("TEST");
134136
}
135137

‎docs/src/main/sphinx/develop/group-provider.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,22 @@ must be wrapped as a Trino plugin and installed on the cluster.
2020

2121
After a plugin that implements `GroupProviderFactory` has been installed on the coordinator,
2222
it is configured using an `etc/group-provider.properties` file.
23-
All the properties other than `group-provider.name` are specific to
23+
All the properties other than `group-provider.name` and `group-provider.group-case` are specific to
2424
the `GroupProviderFactory` implementation.
2525

26-
The `group-provider.name` property is used by Trino to find a registered
26+
* The `group-provider.name` property is used by Trino to find a registered
2727
`GroupProviderFactory` based on the name returned by `GroupProviderFactory.getName()`.
28-
The remaining properties are passed as a map to
28+
* The `group-provider.group-case` property can be used to transform the case of groups. Allowed values
29+
are `[keep, upper, lower]`. This property is not mandatory and the default is `keep`, which does not
30+
transform the group name.
31+
* The remaining properties are passed as a map to
2932
`GroupProviderFactory.create(Map<String, String>)`.
3033

3134
Example configuration file:
3235

3336
```text
3437
group-provider.name=custom-group-provider
38+
group-provider.group-case=keep
3539
custom-property1=custom-value1
3640
custom-property2=custom-value2
3741
```

‎docs/src/main/sphinx/release/release-344.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Please, read these release notes carefully.
4444
* Change file-based system and catalog access controls to only show catalogs, schemas, and tables a user
4545
has permissions on. ({issue}`5039`)
4646
* Change file-based catalog access control to deny permissions inspection and manipulation. ({issue}`5039`)
47-
* Add [file-based group provider](/security/group-file). ({issue}`5028`)
47+
* Add [file-based group provider](/security/group-mapping/file). ({issue}`5028`)
4848

4949
## Hive connector
5050

‎docs/src/main/sphinx/security.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ security/jwt
4040
:maxdepth: 1
4141
4242
security/user-mapping
43-
security/group-file
43+
security/group-mapping
4444
```
4545

4646
(security-access-control)=

‎docs/src/main/sphinx/security/group-mapping.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Group mapping
2+
3+
This section describes the group providers available in Trino to map usernames
4+
onto groups for easier access control and resource group management.
5+
6+
Configure a group provider by creating an `etc/group-provider.properties` file on the coordinator:
7+
8+
```text
9+
group-provider.name=provider-name
10+
group-provider.group-case=keep
11+
```
12+
_The configuration of the chosen group provider should be appended to this file_
13+
14+
:::{list-table} Group provider configuration
15+
:widths: 35, 50, 15
16+
:header-rows: 1
17+
18+
* - Property Name
19+
- Description
20+
- Default
21+
22+
* - `group-provider.name`
23+
- Name of the group provider to use.
24+
-
25+
* - `group-provider.group-case`
26+
- Optional transformation of the case of the group name.
27+
Supported values are:
28+
29+
* `keep`: default, no conversion
30+
* `upper`: convert group name to _UPPERCASE_
31+
* `lower`: converts the group name to _lowercase_
32+
- `keep`
33+
:::
34+
35+
The available group providers are:
36+
```{toctree}
37+
:maxdepth: 1
38+
39+
File group provider <group-mapping/file>
40+
```

‎docs/src/main/sphinx/security/group-file.md‎ renamed to ‎docs/src/main/sphinx/security/group-mapping/file.md‎

File renamed without changes.

0 commit comments

Comments
 (0)