Skip to content

Change AccessControl filterCatalogs to have a SecurityContext #10673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dain merged 1 commit into from
Jan 21, 2022
Merged

Change AccessControl filterCatalogs to have a SecurityContext #10673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataListing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public static SortedMap<String, CatalogName> listCatalogs(Session session, Metad
Map.Entry::getKey,
entry -> entry.getValue().getConnectorCatalogName()));
}
Set<String> allowedCatalogs = accessControl.filterCatalogs(session.getIdentity(), catalogNames.keySet());
Set<String> allowedCatalogs = accessControl.filterCatalogs(session.toSecurityContext(), catalogNames.keySet());

ImmutableSortedMap.Builder<String, CatalogName> result = ImmutableSortedMap.naturalOrder();
for (Map.Entry<String, CatalogName> entry : catalogNames.entrySet()) {
Expand All @@ -79,7 +79,7 @@ public static SortedMap<String, CatalogName> listCatalogs(Session session, Metad
public static SortedMap<String, Catalog> getCatalogs(Session session, Metadata metadata, AccessControl accessControl)
{
Map<String, Catalog> catalogs = metadata.getCatalogs(session);
Set<String> allowedCatalogs = accessControl.filterCatalogs(session.getIdentity(), catalogs.keySet());
Set<String> allowedCatalogs = accessControl.filterCatalogs(session.toSecurityContext(), catalogs.keySet());

ImmutableSortedMap.Builder<String, Catalog> result = ImmutableSortedMap.naturalOrder();
for (Map.Entry<String, Catalog> entry : catalogs.entrySet()) {
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/security/AccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public interface AccessControl
/**
* Filter the list of catalogs to those visible to the identity.
*/
Set<String> filterCatalogs(Identity identity, Set<String> catalogs);
Set<String> filterCatalogs(SecurityContext context, Set<String> catalogs);

/**
* Check if identity is allowed to create the specified schema.
Expand Down
10 changes: 5 additions & 5 deletions core/trino-main/src/main/java/io/trino/security/AccessControlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,13 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner)
}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext securityContext, Set<String> catalogs)
{
requireNonNull(identity, "identity is null");
requireNonNull(securityContext, "securityContext is null");
requireNonNull(catalogs, "catalogs is null");

for (SystemAccessControl systemAccessControl : getSystemAccessControls()) {
catalogs = systemAccessControl.filterCatalogs(new SystemSecurityContext(identity, Optional.empty()), catalogs);
catalogs = systemAccessControl.filterCatalogs(securityContext.toSystemSecurityContext(), catalogs);
}
return catalogs;
}
Expand Down Expand Up @@ -373,7 +373,7 @@ public Set<String> filterSchemas(SecurityContext securityContext, String catalog
requireNonNull(catalogName, "catalogName is null");
requireNonNull(schemaNames, "schemaNames is null");

if (filterCatalogs(securityContext.getIdentity(), ImmutableSet.of(catalogName)).isEmpty()) {
if (filterCatalogs(securityContext, ImmutableSet.of(catalogName)).isEmpty()) {
return ImmutableSet.of();
}

Expand Down Expand Up @@ -527,7 +527,7 @@ public Set<SchemaTableName> filterTables(SecurityContext securityContext, String
requireNonNull(catalogName, "catalogName is null");
requireNonNull(tableNames, "tableNames is null");

if (filterCatalogs(securityContext.getIdentity(), ImmutableSet.of(catalogName)).isEmpty()) {
if (filterCatalogs(securityContext, ImmutableSet.of(catalogName)).isEmpty()) {
return ImmutableSet.of();
}

Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner)
}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext context, Set<String> catalogs)
{
return catalogs;
}
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner)
}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext context, Set<String> catalogs)
{
return ImmutableSet.of();
}
Expand Down
4 changes: 2 additions & 2 deletions core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner)
}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext context, Set<String> catalogs)
{
return delegate().filterCatalogs(identity, catalogs);
return delegate().filterCatalogs(context, catalogs);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/testing/AllowAllAccessControlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public Collection<Identity> filterQueriesOwnedBy(Identity identity, Collection<I
public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner) {}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext context, Set<String> catalogs)
{
return catalogs;
}
Expand Down
4 changes: 2 additions & 2 deletions core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ public void denyIdentityTable(BiPredicate<Identity, String> denyIdentityTable)
}

@Override
public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
public Set<String> filterCatalogs(SecurityContext securityContext, Set<String> catalogs)
{
return super.filterCatalogs(
identity,
securityContext,
catalogs.stream()
.filter(this.deniedCatalogs)
.collect(toImmutableSet()));
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void testReadOnlySystemAccessControl()
accessControlManager.checkCanGrantExecuteFunctionPrivilege(context, "function", Identity.ofUser("bob"), false);
accessControlManager.checkCanGrantExecuteFunctionPrivilege(context, "function", Identity.ofUser("bob"), true);
Set<String> catalogs = ImmutableSet.of("catalog");
assertEquals(accessControlManager.filterCatalogs(identity, catalogs), catalogs);
assertEquals(accessControlManager.filterCatalogs(context, catalogs), catalogs);
Set<String> schemas = ImmutableSet.of("schema");
assertEquals(accessControlManager.filterSchemas(context, "catalog", schemas), schemas);
Set<SchemaTableName> tableNames = ImmutableSet.of(new SchemaTableName("schema", "table"));
Expand Down
16 changes: 8 additions & 8 deletions core/trino-main/src/test/java/io/trino/security/TestFileBasedSystemAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ public void testCatalogOperations()

transaction(transactionManager, accessControlManager)
.execute(transactionId -> {
assertEquals(accessControlManager.filterCatalogs(admin, allCatalogs), allCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, admin, queryId), allCatalogs), allCatalogs);
Set<String> aliceCatalogs = ImmutableSet.of("open-to-all", "alice-catalog", "all-allowed", "staff-catalog");
assertEquals(accessControlManager.filterCatalogs(alice, allCatalogs), aliceCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, alice, queryId), allCatalogs), aliceCatalogs);
Set<String> bobCatalogs = ImmutableSet.of("open-to-all", "all-allowed", "staff-catalog");
assertEquals(accessControlManager.filterCatalogs(bob, allCatalogs), bobCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, bob, queryId), allCatalogs), bobCatalogs);
Set<String> nonAsciiUserCatalogs = ImmutableSet.of("open-to-all", "all-allowed", "\u0200\u0200\u0200");
assertEquals(accessControlManager.filterCatalogs(nonAsciiUser, allCatalogs), nonAsciiUserCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, nonAsciiUser, queryId), allCatalogs), nonAsciiUserCatalogs);
});
}

Expand All @@ -234,13 +234,13 @@ public void testCatalogOperationsReadOnly()

transaction(transactionManager, accessControlManager)
.execute(transactionId -> {
assertEquals(accessControlManager.filterCatalogs(admin, allCatalogs), allCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, admin, queryId), allCatalogs), allCatalogs);
Set<String> aliceCatalogs = ImmutableSet.of("open-to-all", "alice-catalog", "all-allowed");
assertEquals(accessControlManager.filterCatalogs(alice, allCatalogs), aliceCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, alice, queryId), allCatalogs), aliceCatalogs);
Set<String> bobCatalogs = ImmutableSet.of("open-to-all", "all-allowed");
assertEquals(accessControlManager.filterCatalogs(bob, allCatalogs), bobCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, bob, queryId), allCatalogs), bobCatalogs);
Set<String> nonAsciiUserCatalogs = ImmutableSet.of("open-to-all", "all-allowed", "\u0200\u0200\u0200");
assertEquals(accessControlManager.filterCatalogs(nonAsciiUser, allCatalogs), nonAsciiUserCatalogs);
assertEquals(accessControlManager.filterCatalogs(new SecurityContext(transactionId, nonAsciiUser, queryId), allCatalogs), nonAsciiUserCatalogs);
});
}

Expand Down