diff --git a/sql/core/src/main/scala/com/databricks/sql/acl/CheckPermissions.scala b/sql/core/src/main/scala/com/databricks/sql/acl/CheckPermissions.scala index 993cc0e742c4a..4ac949dae530a 100644 --- a/sql/core/src/main/scala/com/databricks/sql/acl/CheckPermissions.scala +++ b/sql/core/src/main/scala/com/databricks/sql/acl/CheckPermissions.scala @@ -32,8 +32,10 @@ class CheckPermissions(catalog: PublicCatalog, aclClient: AclClient) val permissionChecker = new PermissionChecker(aclClient) def apply(plan: LogicalPlan): Unit = { - if (!getRequestsToCheck(plan).forall(permissionChecker)) { - throw new SecurityException("Principal is not authorized to execute the given query") + getRequestsToCheck(plan).foreach { request => + if (!permissionChecker(request)) { + throw new SecurityException(toErrorMessageForFailedRequest(request)) + } } } @@ -361,4 +363,13 @@ class CheckPermissions(catalog: PublicCatalog, aclClient: AclClient) Request(securable, ReadMetadata, toExplainRequests(children.toSeq).toSet) } } + + private def toErrorMessageForFailedRequest(request: Request): String = { + val actionString = request.action match { + case Own => "own" + case _ => s"have permission ${request.action} on" + } + + s"User does not ${actionString} ${request.securable.prettyString}" + } } diff --git a/sql/core/src/main/scala/com/databricks/sql/acl/interfaces.scala b/sql/core/src/main/scala/com/databricks/sql/acl/interfaces.scala index ff275a851e2cc..62f7dab2d9b32 100644 --- a/sql/core/src/main/scala/com/databricks/sql/acl/interfaces.scala +++ b/sql/core/src/main/scala/com/databricks/sql/acl/interfaces.scala @@ -55,6 +55,7 @@ sealed trait Securable { val name: String val key: AnyRef def typeName: String = getClass.getSimpleName.toUpperCase + def prettyString: String } object Securable { @@ -159,6 +160,7 @@ object Securable { case object Catalog extends Securable { override val key: Option[Nothing] = None override val name: String = "/CATALOG/`default`" + override def prettyString: String = "CATALOG" } case class Database(key: String) extends Securable { @@ -167,6 +169,7 @@ case class Database(key: String) extends Securable { val name = Securable.addBackticks(key) s"${Catalog.name}/DATABASE/$name" } + override def prettyString: String = s"database ${Securable.addBackticks(key)}" } sealed trait DatabaseSecurable extends Securable { @@ -181,6 +184,7 @@ sealed trait DatabaseSecurable extends Securable { case class Table(key: TableIdentifier) extends DatabaseSecurable { assert(key != null) + override def prettyString: String = s"table ${key.quotedString}" } object Table { @@ -192,6 +196,7 @@ sealed trait SecurableFunction extends Securable case class Function(key: FunctionIdentifier) extends DatabaseSecurable with SecurableFunction { assert(key != null) + override def prettyString: String = s"function ${key.quotedString}" } object Function { @@ -203,12 +208,14 @@ case object AnonymousFunction extends SecurableFunction { override val key: Option[Nothing] = None override def typeName: String = "ANONYMOUS_FUNCTION" override val name: String = "/" + typeName + override def prettyString: String = s"anonymous function" } case object AnyFile extends Securable { override val key: Option[Nothing] = None override def typeName: String = "ANY_FILE" override val name: String = "/" + typeName + override def prettyString: String = s"any file" } /** diff --git a/sql/hive-thriftserver/src/test/resources/acl-tests/results/base.sql.out b/sql/hive-thriftserver/src/test/resources/acl-tests/results/base.sql.out index 93aa5556511e1..47d125f2e03d7 100644 --- a/sql/hive-thriftserver/src/test/resources/acl-tests/results/base.sql.out +++ b/sql/hive-thriftserver/src/test/resources/acl-tests/results/base.sql.out @@ -48,7 +48,7 @@ usr1 -- !query 3 schema struct<> -- !query 3 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission SELECT on table `db1`.`vw1` -- !query 4 @@ -147,7 +147,7 @@ usr1 -- !query 12 schema struct<> -- !query 12 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on database `db1` -- !query 13 @@ -157,7 +157,7 @@ usr1 -- !query 13 schema struct<> -- !query 13 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission SELECT on table `db1`.`vw1` -- !query 14 diff --git a/sql/hive-thriftserver/src/test/resources/acl-tests/results/explain.sql.out b/sql/hive-thriftserver/src/test/resources/acl-tests/results/explain.sql.out index fa100501064d1..189e181a66d05 100644 --- a/sql/hive-thriftserver/src/test/resources/acl-tests/results/explain.sql.out +++ b/sql/hive-thriftserver/src/test/resources/acl-tests/results/explain.sql.out @@ -80,7 +80,7 @@ usr1 -- !query 7 schema struct<> -- !query 7 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on function `perm`.`fn1` -- !query 8 @@ -90,7 +90,7 @@ usr1 -- !query 8 schema struct<> -- !query 8 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on function `perm`.`fn1` -- !query 9 @@ -100,7 +100,7 @@ usr1 -- !query 9 schema struct<> -- !query 9 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on table `perm`.`vw1` -- !query 10 @@ -182,7 +182,7 @@ usr1 -- !query 16 schema struct<> -- !query 16 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on function `perm`.`fn1` -- !query 17 @@ -192,7 +192,7 @@ usr1 -- !query 17 schema struct<> -- !query 17 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission SELECT on function `perm`.`fn1` -- !query 18 @@ -202,7 +202,7 @@ usr1 -- !query 18 schema struct<> -- !query 18 output -java.lang.SecurityException: Principal is not authorized to execute the given query +java.lang.SecurityException: User does not have permission READ_METADATA on function `perm`.`fn1` -- !query 19 diff --git a/sql/hive-thriftserver/src/test/scala/com/databricks/sql/acl/ThriftEndToEndAclTestSuite.scala b/sql/hive-thriftserver/src/test/scala/com/databricks/sql/acl/ThriftEndToEndAclTestSuite.scala index 5b28fa537dc38..50d9fbb3c4156 100644 --- a/sql/hive-thriftserver/src/test/scala/com/databricks/sql/acl/ThriftEndToEndAclTestSuite.scala +++ b/sql/hive-thriftserver/src/test/scala/com/databricks/sql/acl/ThriftEndToEndAclTestSuite.scala @@ -45,7 +45,7 @@ import org.apache.spark.sql.jdbc.JdbcDialects * * To re-generate golden files, run: * {{{ - * SPARK_GENERATE_GOLDEN_FILES=1 build/sbt -Phive -Phive-thriftserver "thriftserver/test-only *ThriftEndToEndAclTestSuite" + * SPARK_GENERATE_GOLDEN_FILES=1 build/sbt -Phive -Phive-thriftserver "hive-thriftserver/test-only *ThriftEndToEndAclTestSuite" * }}} * * The format for input files is simple: