Skip to content

Commit

Permalink
fix: checkPermission and checkPrivileges first check isAuthenticated (#…
Browse files Browse the repository at this point in the history
…1227)

affects: @esri/hub-common
  • Loading branch information
mjuniper authored Sep 25, 2023
1 parent cf4cc72 commit b292131
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 5 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions packages/common/src/permissions/_internal/checkOwner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
};
Expand Down
5 changes: 4 additions & 1 deletion packages/common/src/permissions/_internal/checkPrivileges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down
17 changes: 17 additions & 0 deletions packages/common/test/permissions/_internal/checkOwner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -19,6 +20,7 @@ describe("checkOwner:", () => {
});
it("returns granted if entity.owner = username is true", () => {
const ctx = {
isAuthenticated: true,
currentUser: {
username: "test",
},
Expand All @@ -35,6 +37,7 @@ describe("checkOwner:", () => {
});
it("returns not-owner if user is not owner", () => {
const ctx = {
isAuthenticated: true,
currentUser: {
username: "test",
},
Expand All @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions packages/common/test/permissions/_internal/checkPrivileges.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe("checkPrivileges:", () => {
});
it("returns check for each priv", () => {
const ctx = {
isAuthenticated: true,
currentUser: {
username: "test-user",
privileges: ["priv2"],
Expand All @@ -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");
});
});

0 comments on commit b292131

Please sign in to comment.