[SC-5613] Better error message when authorization denied

## What changes were proposed in this pull request?

Fixed the error message so it shows the permission that is required

## How was this patch tested?

Existing unit tests

Author: Srinath Shankar <srinath@databricks.com>

Closes apache#172 from srinathshankar/aclerrormess.

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}"
+  }
 }

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"
 }
 
 /**

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

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

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: