Skip to content

Commit

Permalink
Deprecate getting groups in Oauth2 authentication
Browse files Browse the repository at this point in the history
This feature is not working properly and it is not possible to fix it.
The problem is widely understood impersonation:
 - view SECURITY DEFINER
 - sessionUser JDBC parameter
 - identity from view expression for column masking and row filtering

Using Oauth2 is able to provide groups only during authentication.
However it is unable to provide groups for any impersonated user.
  • Loading branch information
kokosing committed Feb 22, 2023
1 parent 275ea26 commit 65a8f11
Show file tree
Hide file tree
Showing 11 changed files with 4 additions and 27 deletions.
Expand Up @@ -157,8 +157,8 @@ public Optional<String> getGroupsField()
return groupsField;
}

@Config("http-server.authentication.oauth2.groups-field")
@ConfigDescription("Groups field in the claim")
@Config("deprecated.http-server.authentication.oauth2.groups-field")
@ConfigDescription("Groups field in the claim. This configuration is scheduled for removal.")
public OAuth2Config setGroupsField(String groupsField)
{
this.groupsField = Optional.ofNullable(groupsField);
Expand Down
Expand Up @@ -784,7 +784,7 @@ public void testOAuth2Groups(Optional<Set<String>> groups)
.put("web-ui.enabled", "true")
.put("http-server.authentication.type", "oauth2")
.putAll(getOAuth2Properties(tokenServer))
.put("http-server.authentication.oauth2.groups-field", GROUPS_CLAIM)
.put("deprecated.http-server.authentication.oauth2.groups-field", GROUPS_CLAIM)
.buildOrThrow())
.setAdditionalModule(oauth2Module(tokenServer))
.build()) {
Expand Down
Expand Up @@ -64,7 +64,7 @@ public void testExplicitPropertyMappings()
.put("http-server.authentication.oauth2.client-secret", "consumer-secret")
.put("http-server.authentication.oauth2.scopes", "email,offline")
.put("http-server.authentication.oauth2.principal-field", "some-field")
.put("http-server.authentication.oauth2.groups-field", "groups")
.put("deprecated.http-server.authentication.oauth2.groups-field", "groups")
.put("http-server.authentication.oauth2.additional-audiences", "test-aud1,test-aud2")
.put("http-server.authentication.oauth2.challenge-timeout", "90s")
.put("http-server.authentication.oauth2.max-clock-skew", "15s")
Expand Down
2 changes: 0 additions & 2 deletions docs/src/main/sphinx/security/oauth2.rst
Expand Up @@ -148,8 +148,6 @@ The following configuration properties are available:
for more information.
* - ``http-server.authentication.oauth2.principal-field``
- The field of the access token used for the Trino user principal. Defaults to ``sub``. Other commonly used fields include ``sAMAccountName``, ``name``, ``upn``, and ``email``.
* - ``http-server.authentication.oauth2.groups-field``
- Array-based field in the access token used to list group information for a user.
* - ``http-server.authentication.oauth2.oidc.discovery``
- Enable reading the `OIDC provider metadata <https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata>`_.
Default is ``true``.
Expand Down
Expand Up @@ -20,7 +20,6 @@ http-server.authentication.oauth2.jwks-url=http://hydra:4444/.well-known/jwks.js
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.oidc.discovery=false
oauth2-jwk.http-client.trust-store-path=/docker/presto-product-tests/conf/presto/etc/hydra.pem
oauth2-jwk.http-client.http-proxy=proxy:8888
Expand Down
Expand Up @@ -20,7 +20,6 @@ http-server.authentication.oauth2.jwks-url=http://hydra:4444/.well-known/jwks.js
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.oidc.discovery=false
oauth2-jwk.http-client.trust-store-path=/docker/presto-product-tests/conf/presto/etc/cert/truststore.jks
oauth2-jwk.http-client.trust-store-password=123456
Expand Down
Expand Up @@ -21,7 +21,6 @@ http-server.authentication.oauth2.jwks-url=http://hydra:4444/.well-known/jwks.js
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.refresh-tokens=true
http-server.authentication.oauth2.refresh-tokens.issued-token.timeout=30s
http-server.authentication.oauth2.oidc.discovery=false
Expand Down
Expand Up @@ -20,7 +20,6 @@ http-server.authentication.oauth2.jwks-url=http://hydra:4444/.well-known/jwks.js
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.oidc.discovery=false
oauth2-jwk.http-client.trust-store-path=/docker/presto-product-tests/conf/presto/etc/hydra.pem

Expand Down
Expand Up @@ -18,7 +18,6 @@ http-server.authentication.oauth2.scopes=openid,offline
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.refresh-tokens=true
http-server.authentication.oauth2.refresh-tokens.issued-token.timeout=30s
http-server.authentication.oauth2.oidc.use-userinfo-endpoint=false
Expand Down
Expand Up @@ -17,7 +17,6 @@ http-server.authentication.oauth2.issuer=http://hydra:4444/
http-server.authentication.oauth2.client-id=trinodb_client_id
http-server.authentication.oauth2.client-secret=trinodb_client_secret
http-server.authentication.oauth2.user-mapping.pattern=(.*)(@.*)?
http-server.authentication.oauth2.groups-field=groups
http-server.authentication.oauth2.oidc.use-userinfo-endpoint=false
oauth2-jwk.http-client.trust-store-path=/docker/presto-product-tests/conf/presto/etc/hydra.pem

Expand Down
Expand Up @@ -13,7 +13,6 @@
*/
package io.trino.tests.product.jdbc;

import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import io.trino.jdbc.TestingRedirectHandlerInjector;
Expand All @@ -39,10 +38,8 @@
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

import static com.google.common.base.Preconditions.checkState;
import static io.trino.tempto.assertions.QueryAssert.Row.row;
import static io.trino.tempto.assertions.QueryAssert.assertThat;
import static io.trino.tempto.query.QueryResult.forResultSet;
import static io.trino.tests.product.TestGroups.OAUTH2;
Expand Down Expand Up @@ -123,18 +120,6 @@ public void shouldAuthenticateAfterTokenExpires()
}
}

@Test(groups = {OAUTH2, PROFILE_SPECIFIC_TESTS})
public void shouldReturnGroups()
throws SQLException
{
prepareHandler();
try (Connection connection = DriverManager.getConnection(jdbcUrl);
PreparedStatement statement = connection.prepareStatement("SELECT array_sort(current_groups())");
ResultSet rs = statement.executeQuery()) {
assertThat(forResultSet(rs)).containsOnly(row(ImmutableList.of("admin", "public")));
}
}

private void prepareHandler()
{
TestingRedirectHandlerInjector.setRedirectHandler(uri -> {
Expand Down

0 comments on commit 65a8f11

Please sign in to comment.