From b29213183ef9362a3517dbcaca8ec854b4b80537 Mon Sep 17 00:00:00 2001
From: Mike Juniper <mjuniper@esri.com>
Date: Mon, 25 Sep 2023 11:56:33 -0600
Subject: [PATCH] fix: checkPermission and checkPrivileges first check
 isAuthenticated (#1227)

affects: @esri/hub-common
---
 package-lock.json                               |  4 ++--
 .../src/permissions/_internal/checkOwner.ts     |  6 ++++--
 .../permissions/_internal/checkPrivileges.ts    |  5 ++++-
 .../permissions/_internal/checkOwner.test.ts    | 17 +++++++++++++++++
 .../_internal/checkPrivileges.test.ts           | 16 ++++++++++++++++
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 9a897398077..89a7ebb47af 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -64981,7 +64981,7 @@
 		},
 		"packages/common": {
 			"name": "@esri/hub-common",
-			"version": "14.20.0",
+			"version": "14.21.0",
 			"license": "Apache-2.0",
 			"dependencies": {
 				"abab": "^2.0.5",
@@ -65025,7 +65025,7 @@
 		},
 		"packages/downloads": {
 			"name": "@esri/hub-downloads",
-			"version": "14.0.0",
+			"version": "14.1.0",
 			"license": "Apache-2.0",
 			"dependencies": {
 				"eventemitter3": "^4.0.4",
diff --git a/packages/common/src/permissions/_internal/checkOwner.ts b/packages/common/src/permissions/_internal/checkOwner.ts
index dc7412b5559..5c5f67721d2 100644
--- a/packages/common/src/permissions/_internal/checkOwner.ts
+++ b/packages/common/src/permissions/_internal/checkOwner.ts
@@ -19,7 +19,9 @@ export function checkOwner(
   if (policy.entityOwner) {
     let response: PolicyResponse = "granted";
     let name = "entity owner required";
-    if (!entity) {
+    if (!context.isAuthenticated) {
+      response = "not-authenticated";
+    } else if (!entity) {
       // fail b/c no entity
       response = "entity-required";
     } else {
@@ -32,7 +34,7 @@ export function checkOwner(
     // create the check
     const check: IPolicyCheck = {
       name,
-      value: `current user: ${context.currentUser.username}`,
+      value: `current user: ${context.currentUser?.username}`,
       code: getPolicyResponseCode(response),
       response,
     };
diff --git a/packages/common/src/permissions/_internal/checkPrivileges.ts b/packages/common/src/permissions/_internal/checkPrivileges.ts
index 144e15ae549..ba4fe72616f 100644
--- a/packages/common/src/permissions/_internal/checkPrivileges.ts
+++ b/packages/common/src/permissions/_internal/checkPrivileges.ts
@@ -20,7 +20,10 @@ export function checkPrivileges(
     checks = policy.privileges.map((privilege) => {
       let response: PolicyResponse = "granted";
       let value = "privilege present";
-      if (!context.currentUser.privileges.includes(privilege)) {
+      if (!context.isAuthenticated) {
+        response = "not-authenticated";
+        value = "not authenticated";
+      } else if (!context.currentUser.privileges.includes(privilege)) {
         response = "privilege-required";
         value = "privilege missing";
       }
diff --git a/packages/common/test/permissions/_internal/checkOwner.test.ts b/packages/common/test/permissions/_internal/checkOwner.test.ts
index 15bade7b7a7..6c01648686e 100644
--- a/packages/common/test/permissions/_internal/checkOwner.test.ts
+++ b/packages/common/test/permissions/_internal/checkOwner.test.ts
@@ -5,6 +5,7 @@ import { checkOwner } from "../../../src/permissions/_internal/checkOwner";
 describe("checkOwner:", () => {
   it("returns entity-required if not passed entity", () => {
     const ctx = {
+      isAuthenticated: true,
       currentUser: {
         username: "test-user",
       },
@@ -19,6 +20,7 @@ describe("checkOwner:", () => {
   });
   it("returns granted if entity.owner = username is true", () => {
     const ctx = {
+      isAuthenticated: true,
       currentUser: {
         username: "test",
       },
@@ -35,6 +37,7 @@ describe("checkOwner:", () => {
   });
   it("returns not-owner if user is not owner", () => {
     const ctx = {
+      isAuthenticated: true,
       currentUser: {
         username: "test",
       },
@@ -49,6 +52,20 @@ describe("checkOwner:", () => {
     expect(chks.length).toBe(1);
     expect(chks[0].response).toBe("not-owner");
   });
+  it("returns not-authenticated if not authenticated", () => {
+    const ctx = {
+      isAuthenticated: false,
+    } as unknown as IArcGISContext;
+    const policy = {
+      entityOwner: true,
+    } as unknown as IPermissionPolicy;
+    const entity = {
+      owner: "not-test",
+    } as unknown as HubEntity;
+    const chks = checkOwner(policy, ctx, entity);
+    expect(chks.length).toBe(1);
+    expect(chks[0].response).toBe("not-authenticated");
+  });
   it("returns empty array if policy does not specify entityOwner", () => {
     const ctx = {} as unknown as IArcGISContext;
     const policy = {} as unknown as IPermissionPolicy;
diff --git a/packages/common/test/permissions/_internal/checkPrivileges.test.ts b/packages/common/test/permissions/_internal/checkPrivileges.test.ts
index dc62b95e28a..a9009464396 100644
--- a/packages/common/test/permissions/_internal/checkPrivileges.test.ts
+++ b/packages/common/test/permissions/_internal/checkPrivileges.test.ts
@@ -19,6 +19,7 @@ describe("checkPrivileges:", () => {
   });
   it("returns check for each priv", () => {
     const ctx = {
+      isAuthenticated: true,
       currentUser: {
         username: "test-user",
         privileges: ["priv2"],
@@ -35,4 +36,19 @@ describe("checkPrivileges:", () => {
     expect(chks[1].response).toBe("granted");
     expect(chks[1].value).toBe("privilege present");
   });
+  it("returns unauthed message", () => {
+    const ctx = {
+      isAuthenticated: false,
+    } as unknown as IArcGISContext;
+    const policy = {
+      privileges: ["priv1", "priv2"],
+    } as unknown as IPermissionPolicy;
+
+    const chks = checkPrivileges(policy, ctx);
+    expect(chks.length).toBe(2);
+    expect(chks[0].response).toBe("not-authenticated");
+    expect(chks[0].value).toBe("not authenticated");
+    expect(chks[1].response).toBe("not-authenticated");
+    expect(chks[1].value).toBe("not authenticated");
+  });
 });