From 95284162d6f0230f74b621c4b9bf4a9b3494b1a6 Mon Sep 17 00:00:00 2001
From: Mike Lewis <mtlewis@users.noreply.github.com>
Date: Thu, 3 Mar 2022 17:56:37 +0000
Subject: [PATCH] permission-common: introduce ResourcePermission type

Signed-off-by: Mike Lewis <mtlewis@users.noreply.github.com>
---
 .changeset/unlucky-schools-heal.md            |  5 ++++
 .changeset/warm-mangos-compete.md             |  5 ++++
 .../permission-backend/src/service/router.ts  |  6 +++++
 plugins/permission-common/api-report.md       | 17 +++++++++----
 plugins/permission-common/src/types/index.ts  |  2 ++
 .../permission-common/src/types/permission.ts | 24 +++++++++++++++----
 6 files changed, 49 insertions(+), 10 deletions(-)
 create mode 100644 .changeset/unlucky-schools-heal.md
 create mode 100644 .changeset/warm-mangos-compete.md

diff --git a/.changeset/unlucky-schools-heal.md b/.changeset/unlucky-schools-heal.md
new file mode 100644
index 0000000000..1f2930b29a
--- /dev/null
+++ b/.changeset/unlucky-schools-heal.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-permission-backend': patch
+---
+
+Add more specific check for policies which return conditional decisions for non-resource permissions.
diff --git a/.changeset/warm-mangos-compete.md b/.changeset/warm-mangos-compete.md
new file mode 100644
index 0000000000..3ad688c372
--- /dev/null
+++ b/.changeset/warm-mangos-compete.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-permission-common': patch
+---
+
+Add more specific `Permission` types
diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts
index 48f7b1a2e5..51e3ce960e 100644
--- a/plugins/permission-backend/src/service/router.ts
+++ b/plugins/permission-backend/src/service/router.ts
@@ -109,6 +109,12 @@ const handleRequest = async (
           };
         }
 
+        if (!('resourceType' in request.permission)) {
+          throw new Error(
+            `Conditional decision returned from permission policy for non-resource permission ${request.permission.name}`,
+          );
+        }
+
         if (decision.resourceType !== request.permission.resourceType) {
           throw new Error(
             `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`,
diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md
index db05766dc0..fcd1e1251b 100644
--- a/plugins/permission-common/api-report.md
+++ b/plugins/permission-common/api-report.md
@@ -53,6 +53,12 @@ export enum AuthorizeResult {
   DENY = 'DENY',
 }
 
+// @public
+export type BasicPermission = {
+  name: string;
+  attributes: PermissionAttributes;
+};
+
 // @public
 export type DiscoveryApi = {
   getBaseUrl(pluginId: string): Promise<string>;
@@ -81,11 +87,7 @@ export type NotCriteria<TQuery> = {
 };
 
 // @public
-export type Permission = {
-  name: string;
-  attributes: PermissionAttributes;
-  resourceType?: string;
-};
+export type Permission = BasicPermission | ResourcePermission;
 
 // @public
 export type PermissionAttributes = {
@@ -122,4 +124,9 @@ export type PermissionCriteria<TQuery> =
   | AnyOfCriteria<TQuery>
   | NotCriteria<TQuery>
   | TQuery;
+
+// @public
+export type ResourcePermission<T extends string = string> = BasicPermission & {
+  resourceType: T;
+};
 ```
diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts
index fa98de73f1..2fdb553f09 100644
--- a/plugins/permission-common/src/types/index.ts
+++ b/plugins/permission-common/src/types/index.ts
@@ -29,8 +29,10 @@ export type {
 } from './api';
 export type { DiscoveryApi } from './discovery';
 export type {
+  BasicPermission,
   PermissionAttributes,
   Permission,
   PermissionAuthorizer,
+  ResourcePermission,
   AuthorizeRequestOptions,
 } from './permission';
diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts
index 16934d8771..0777e24eb4 100644
--- a/plugins/permission-common/src/types/permission.ts
+++ b/plugins/permission-common/src/types/permission.ts
@@ -28,6 +28,8 @@ export type PermissionAttributes = {
 /**
  * A permission that can be checked through authorization.
  *
+ * @remarks
+ *
  * Permissions are the "what" part of authorization, the action to be performed. This may be reading
  * an entity from the catalog, executing a software template, or any other action a plugin author
  * may wish to protect.
@@ -36,7 +38,13 @@ export type PermissionAttributes = {
  * evaluated using an authorization policy.
  * @public
  */
-export type Permission = {
+export type Permission = BasicPermission | ResourcePermission;
+
+/**
+ * A standard {@link Permission} with no additional capabilities or restrictions.
+ * @public
+ */
+export type BasicPermission = {
   /**
    * The name of the permission.
    */
@@ -47,13 +55,19 @@ export type Permission = {
    * all by name.
    */
   attributes: PermissionAttributes;
+};
+
+/**
+ * ResourcePermissions are {@link Permission}s that can be authorized based on
+ * characteristics of a resource such a catalog entity.
+ * @public
+ */
+export type ResourcePermission<T extends string = string> = BasicPermission & {
   /**
-   * Some permissions can be authorized based on characteristics of a resource
-   * such a catalog entity. For these permissions, the resourceType field
-   * denotes the type of the resource whose resourceRef should be passed when
+   * Denotes the type of the resource whose resourceRef should be passed when
    * authorizing.
    */
-  resourceType?: string;
+  resourceType: T;
 };
 
 /**