Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: checkPermission and checkPrivileges first check isAuthenticated #1227

Merged
merged 1 commit into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
});
});