Skip to content

Commit

Permalink
Fix listing privileges when admin role is set
Browse files Browse the repository at this point in the history 
Presto currently lists only privilges of the
tables owned by the current user, even after the
admin role is set. This commit fixes this and lists all
privileges for admins.

Extracted-From: prestodb/presto#10904
  • Loading branch information
@anusudarsan @sopel39
anusudarsan authored and sopel39 committed Jan 29, 2019
1 parent fb93b6e commit 20cf120
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 51 deletions.
59 changes: 39 additions & 20 deletions presto-hive/src/main/java/io/prestosql/plugin/hive/HiveMetadata.java
View file
Expand Up @@ -184,6 +184,7 @@
import static io.prestosql.plugin.hive.metastore.StorageFormat.VIEW_STORAGE_FORMAT;
import static io.prestosql.plugin.hive.metastore.StorageFormat.fromHiveStorageFormat;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.listEnabledPrincipals;
import static io.prestosql.plugin.hive.security.SqlStandardAccessControl.ADMIN_ROLE_NAME;
import static io.prestosql.plugin.hive.util.ConfigurationUtils.toJobConf;
import static io.prestosql.plugin.hive.util.Statistics.ReduceOperator.ADD;
import static io.prestosql.plugin.hive.util.Statistics.createComputedStatisticsToPartitionMap;
Expand Down Expand Up @@ -837,13 +838,13 @@ private static Table buildTableObject(

private static PrincipalPrivileges buildInitialPrivilegeSet(String tableOwner)
{
PrestoPrincipal grantor = new PrestoPrincipal(USER, tableOwner);
PrestoPrincipal owner = new PrestoPrincipal(USER, tableOwner);
return new PrincipalPrivileges(
ImmutableMultimap.<String, HivePrivilegeInfo>builder()
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.SELECT, true, grantor))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.INSERT, true, grantor))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.UPDATE, true, grantor))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.DELETE, true, grantor))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.SELECT, true, owner, owner))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.INSERT, true, owner, owner))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.UPDATE, true, owner, owner))
.put(tableOwner, new HivePrivilegeInfo(HivePrivilege.DELETE, true, owner, owner))
.build(),
ImmutableMultimap.of());
}
Expand Down Expand Up @@ -1823,7 +1824,7 @@ public void grantTablePrivileges(ConnectorSession session, SchemaTableName schem
String tableName = schemaTableName.getTableName();

Set<HivePrivilegeInfo> hivePrivilegeInfos = privileges.stream()
.map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption, new PrestoPrincipal(USER, session.getUser())))
.map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption, new PrestoPrincipal(USER, session.getUser()), new PrestoPrincipal(USER, session.getUser())))
.collect(toSet());

metastore.grantTablePrivileges(schemaName, tableName, grantee, hivePrivilegeInfos);
Expand All @@ -1836,7 +1837,7 @@ public void revokeTablePrivileges(ConnectorSession session, SchemaTableName sche
String tableName = schemaTableName.getTableName();

Set<HivePrivilegeInfo> hivePrivilegeInfos = privileges.stream()
.map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption, new PrestoPrincipal(USER, session.getUser())))
.map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption, new PrestoPrincipal(USER, session.getUser()), new PrestoPrincipal(USER, session.getUser())))
.collect(toSet());

metastore.revokeTablePrivileges(schemaName, tableName, grantee, hivePrivilegeInfos);
Expand All @@ -1847,27 +1848,45 @@ public List<GrantInfo> listTablePrivileges(ConnectorSession session, SchemaTable
{
Set<PrestoPrincipal> principals = listEnabledPrincipals(metastore, session.getIdentity())
.collect(toImmutableSet());
boolean isAdminRoleSet = hasAdminRole(principals);
ImmutableList.Builder<GrantInfo> result = ImmutableList.builder();
for (SchemaTableName tableName : listTables(session, schemaTablePrefix)) {
for (PrestoPrincipal grantee : principals) {
Set<HivePrivilegeInfo> hivePrivileges = metastore.listTablePrivileges(tableName.getSchemaName(), tableName.getTableName(), grantee);
for (HivePrivilegeInfo hivePrivilege : hivePrivileges) {
Set<PrivilegeInfo> prestoPrivileges = hivePrivilege.toPrivilegeInfo();
for (PrivilegeInfo prestoPrivilege : prestoPrivileges) {
GrantInfo grant = new GrantInfo(
prestoPrivilege,
grantee,
tableName,
Optional.of(hivePrivilege.getGrantor()),
Optional.empty());
result.add(grant);
}
if (isAdminRoleSet) {
result.addAll(buildGrants(tableName, null));
}
else {
for (PrestoPrincipal grantee : principals) {
result.addAll(buildGrants(tableName, grantee));
}
}
}
return result.build();
}

private List<GrantInfo> buildGrants(SchemaTableName tableName, PrestoPrincipal principal)
{
ImmutableList.Builder<GrantInfo> result = ImmutableList.builder();
Set<HivePrivilegeInfo> hivePrivileges = metastore.listTablePrivileges(tableName.getSchemaName(), tableName.getTableName(), principal);
for (HivePrivilegeInfo hivePrivilege : hivePrivileges) {
Set<PrivilegeInfo> prestoPrivileges = hivePrivilege.toPrivilegeInfo();
for (PrivilegeInfo prestoPrivilege : prestoPrivileges) {
GrantInfo grant = new GrantInfo(
prestoPrivilege,
hivePrivilege.getGrantee(),
tableName,
Optional.of(hivePrivilege.getGrantor()),
Optional.empty());
result.add(grant);
}
}
return result.build();
}

private static boolean hasAdminRole(Set<PrestoPrincipal> roles)
{
return roles.stream().anyMatch(principal -> principal.getName().equalsIgnoreCase(ADMIN_ROLE_NAME));
}

private void verifyJvmTimeZone()
{
if (!allowCorruptWritesForTesting && !timeZone.equals(DateTimeZone.getDefault())) {
Expand Down
21 changes: 16 additions & 5 deletions presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/HivePrivilegeInfo.java
View file
Expand Up @@ -22,10 +22,8 @@

import javax.annotation.concurrent.Immutable;

import java.util.Arrays;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

import static com.google.common.base.MoreObjects.toStringHelper;
import static io.prestosql.plugin.hive.metastore.HivePrivilegeInfo.HivePrivilege.DELETE;
Expand All @@ -45,16 +43,19 @@ public enum HivePrivilege
private final HivePrivilege hivePrivilege;
private final boolean grantOption;
private final PrestoPrincipal grantor;
private final PrestoPrincipal grantee;

@JsonCreator
public HivePrivilegeInfo(
@JsonProperty("hivePrivilege") HivePrivilege hivePrivilege,
@JsonProperty("grantOption") boolean grantOption,
@JsonProperty("grantor") PrestoPrincipal grantor)
@JsonProperty("grantor") PrestoPrincipal grantor,
@JsonProperty("grantee") PrestoPrincipal grantee)
{
this.hivePrivilege = requireNonNull(hivePrivilege, "hivePrivilege is null");
this.grantOption = grantOption;
this.grantor = requireNonNull(grantor, "grantor is null");
this.grantee = requireNonNull(grantee, "grantee is null");
}

@JsonProperty
Expand All @@ -75,6 +76,12 @@ public PrestoPrincipal getGrantor()
return grantor;
}

@JsonProperty
public PrestoPrincipal getGrantee()
{
return grantee;
}

public static HivePrivilege toHivePrivilege(Privilege privilege)
{
switch (privilege) {
Expand Down Expand Up @@ -119,7 +126,7 @@ public Set<PrivilegeInfo> toPrivilegeInfo()
@Override
public int hashCode()
{
return Objects.hash(hivePrivilege, grantOption);
return Objects.hash(hivePrivilege, grantOption, grantor, grantee);
}

@Override
Expand All @@ -133,7 +140,9 @@ public boolean equals(Object o)
}
HivePrivilegeInfo hivePrivilegeInfo = (HivePrivilegeInfo) o;
return Objects.equals(hivePrivilege, hivePrivilegeInfo.hivePrivilege) &&
Objects.equals(grantOption, hivePrivilegeInfo.grantOption);
Objects.equals(grantOption, hivePrivilegeInfo.grantOption) &&
Objects.equals(grantor, hivePrivilegeInfo.grantor) &&
Objects.equals(grantee, hivePrivilegeInfo.grantee);
}

@Override
Expand All @@ -142,6 +151,8 @@ public String toString()
return toStringHelper(this)
.add("privilege", hivePrivilege)
.add("grantOption", grantOption)
.add("grantor", grantor)
.add("grantee", grantee)
.toString();
}
}
2 changes: 1 addition & 1 deletion ...hive/src/main/java/io/prestosql/plugin/hive/metastore/SemiTransactionalHiveMetastore.java
View file
Expand Up @@ -789,7 +789,7 @@ public synchronized Set<HivePrivilegeInfo> listTablePrivileges(String databaseNa
Collection<HivePrivilegeInfo> privileges = tableAction.getData().getPrincipalPrivileges().getUserPrivileges().get(principal.getName());
return ImmutableSet.<HivePrivilegeInfo>builder()
.addAll(privileges)
.add(new HivePrivilegeInfo(OWNERSHIP, true, new PrestoPrincipal(USER, principal.getName())))
.add(new HivePrivilegeInfo(OWNERSHIP, true, new PrestoPrincipal(USER, principal.getName()), new PrestoPrincipal(USER, principal.getName())))
.build();
}
case INSERT_EXISTING:
Expand Down
3 changes: 2 additions & 1 deletion presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/UserTableKey.java
View file
Expand Up @@ -34,7 +34,8 @@ public class UserTableKey
@JsonCreator
public UserTableKey(@JsonProperty("principal") PrestoPrincipal principal, @JsonProperty("database") String database, @JsonProperty("table") String table)
{
this.principal = requireNonNull(principal, "user is null");
// principal can be null when we want to list all privileges for admins
this.principal = principal;
this.database = requireNonNull(database, "database is null");
this.table = requireNonNull(table, "table is null");
}
Expand Down
2 changes: 1 addition & 1 deletion presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/file/FileHiveMetastore.java
View file
Expand Up @@ -944,7 +944,7 @@ public synchronized Set<HivePrivilegeInfo> listTablePrivileges(String databaseNa
ImmutableSet.Builder<HivePrivilegeInfo> result = ImmutableSet.builder();
Table table = getRequiredTable(databaseName, tableName);
if (principal.getType() == USER && table.getOwner().equals(principal.getName())) {
result.add(new HivePrivilegeInfo(OWNERSHIP, true, principal));
result.add(new HivePrivilegeInfo(OWNERSHIP, true, principal, principal));
}
Path permissionFilePath = getPermissionsPath(getPermissionsDirectory(table), principal);
result.addAll(readFile("permissions", permissionFilePath, permissionsCodec).orElse(ImmutableList.of()).stream()
Expand Down
2 changes: 1 addition & 1 deletion presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/file/PermissionMetadata.java
View file
Expand Up @@ -56,6 +56,6 @@ public boolean isGrantOption()

public HivePrivilegeInfo toHivePrivilegeInfo()
{
return new HivePrivilegeInfo(permission, grantOption, new PrestoPrincipal(USER, "admin"));
return new HivePrivilegeInfo(permission, grantOption, new PrestoPrincipal(USER, "admin"), new PrestoPrincipal(USER, "admin"));
}
}
30 changes: 21 additions & 9 deletions presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/thrift/ThriftHiveMetastore.java
View file
Expand Up @@ -86,6 +86,7 @@
import static io.prestosql.plugin.hive.metastore.HivePrivilegeInfo.HivePrivilege;
import static io.prestosql.plugin.hive.metastore.HivePrivilegeInfo.HivePrivilege.OWNERSHIP;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.createMetastoreColumnStatistics;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.fromMetastoreApiPrincipalType;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.fromMetastoreApiTable;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.fromPrestoPrincipalType;
import static io.prestosql.plugin.hive.metastore.thrift.ThriftMetastoreUtil.fromRolePrincipalGrants;
Expand Down Expand Up @@ -1168,7 +1169,7 @@ public void grantTablePrivileges(String databaseName, String tableName, PrestoPr
Set<PrivilegeGrantInfo> privilegesToGrant = new HashSet<>(requestedPrivileges);
Iterator<PrivilegeGrantInfo> iterator = privilegesToGrant.iterator();
while (iterator.hasNext()) {
HivePrivilegeInfo requestedPrivilege = getOnlyElement(parsePrivilege(iterator.next()));
HivePrivilegeInfo requestedPrivilege = getOnlyElement(parsePrivilege(iterator.next(), Optional.empty()));

for (HivePrivilegeInfo existingPrivilege : existingPrivileges) {
if ((requestedPrivilege.isContainedIn(existingPrivilege))) {
Expand Down Expand Up @@ -1219,7 +1220,7 @@ public void revokeTablePrivileges(String databaseName, String tableName, PrestoP
.collect(toSet());

Set<PrivilegeGrantInfo> privilegesToRevoke = requestedPrivileges.stream()
.filter(privilegeGrantInfo -> existingHivePrivileges.contains(getOnlyElement(parsePrivilege(privilegeGrantInfo)).getHivePrivilege()))
.filter(privilegeGrantInfo -> existingHivePrivileges.contains(getOnlyElement(parsePrivilege(privilegeGrantInfo, Optional.empty())).getHivePrivilege()))
.collect(toSet());

if (privilegesToRevoke.isEmpty()) {
Expand Down Expand Up @@ -1249,15 +1250,26 @@ public Set<HivePrivilegeInfo> listTablePrivileges(String databaseName, String ta
try (HiveMetastoreClient client = clientProvider.createMetastoreClient()) {
Table table = client.getTable(databaseName, tableName);
ImmutableSet.Builder<HivePrivilegeInfo> privileges = ImmutableSet.builder();
if (principal.getType() == USER && table.getOwner().equals(principal.getName())) {
privileges.add(new HivePrivilegeInfo(OWNERSHIP, true, principal));
List<HiveObjectPrivilege> hiveObjectPrivilegeList;
// principal can be null when we want to list all privileges for admins
if (principal == null) {
hiveObjectPrivilegeList = client.listPrivileges(
null,
null,
new HiveObjectRef(TABLE, databaseName, tableName, null, null));
}
else {
if (principal.getType() == USER && table.getOwner().equals(principal.getName())) {
privileges.add(new HivePrivilegeInfo(OWNERSHIP, true, principal, principal));
}
hiveObjectPrivilegeList = client.listPrivileges(
principal.getName(),
fromPrestoPrincipalType(principal.getType()),
new HiveObjectRef(TABLE, databaseName, tableName, null, null));
}
List<HiveObjectPrivilege> hiveObjectPrivilegeList = client.listPrivileges(
principal.getName(),
fromPrestoPrincipalType(principal.getType()),
new HiveObjectRef(TABLE, databaseName, tableName, null, null));
for (HiveObjectPrivilege hiveObjectPrivilege : hiveObjectPrivilegeList) {
privileges.addAll(parsePrivilege(hiveObjectPrivilege.getGrantInfo()));
PrestoPrincipal grantee = new PrestoPrincipal(fromMetastoreApiPrincipalType(hiveObjectPrivilege.getPrincipalType()), hiveObjectPrivilege.getPrincipalName());
privileges.addAll(parsePrivilege(hiveObjectPrivilege.getGrantInfo(), Optional.of(grantee)));
}
return privileges.build();
}
Expand Down
16 changes: 8 additions & 8 deletions presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java
View file
Expand Up @@ -715,26 +715,26 @@ private static StorageDescriptor makeStorageDescriptor(String tableName, List<Co
return sd;
}

public static Set<HivePrivilegeInfo> parsePrivilege(PrivilegeGrantInfo userGrant)
public static Set<HivePrivilegeInfo> parsePrivilege(PrivilegeGrantInfo userGrant, Optional<PrestoPrincipal> grantee)
{
boolean withGrantOption = userGrant.isGrantOption();
String name = userGrant.getPrivilege().toUpperCase(ENGLISH);
PrestoPrincipal grantor = new PrestoPrincipal(ThriftMetastoreUtil.fromMetastoreApiPrincipalType(userGrant.getGrantorType()), userGrant.getGrantor());
PrestoPrincipal grantor = new PrestoPrincipal(fromMetastoreApiPrincipalType(userGrant.getGrantorType()), userGrant.getGrantor());
switch (name) {
case "ALL":
return Arrays.stream(HivePrivilegeInfo.HivePrivilege.values())
.map(hivePrivilege -> new HivePrivilegeInfo(hivePrivilege, withGrantOption, grantor))
.map(hivePrivilege -> new HivePrivilegeInfo(hivePrivilege, withGrantOption, grantor, grantee.orElse(grantor)))
.collect(toImmutableSet());
case "SELECT":
return ImmutableSet.of(new HivePrivilegeInfo(SELECT, withGrantOption, grantor));
return ImmutableSet.of(new HivePrivilegeInfo(SELECT, withGrantOption, grantor, grantee.orElse(grantor)));
case "INSERT":
return ImmutableSet.of(new HivePrivilegeInfo(INSERT, withGrantOption, grantor));
return ImmutableSet.of(new HivePrivilegeInfo(INSERT, withGrantOption, grantor, grantee.orElse(grantor)));
case "UPDATE":
return ImmutableSet.of(new HivePrivilegeInfo(UPDATE, withGrantOption, grantor));
return ImmutableSet.of(new HivePrivilegeInfo(UPDATE, withGrantOption, grantor, grantee.orElse(grantor)));
case "DELETE":
return ImmutableSet.of(new HivePrivilegeInfo(DELETE, withGrantOption, grantor));
return ImmutableSet.of(new HivePrivilegeInfo(DELETE, withGrantOption, grantor, grantee.orElse(grantor)));
case "OWNERSHIP":
return ImmutableSet.of(new HivePrivilegeInfo(OWNERSHIP, withGrantOption, grantor));
return ImmutableSet.of(new HivePrivilegeInfo(OWNERSHIP, withGrantOption, grantor, grantee.orElse(grantor)));
default:
throw new IllegalArgumentException("Unsupported privilege name: " + name);
}
Expand Down

0 comments on commit 20cf120

Please sign in to comment.