Skip to content

Commit

Permalink
TST: Users cannot upgrade their own permissions.
Browse files Browse the repository at this point in the history
  • Loading branch information
epatters committed Jan 16, 2025
1 parent 1ba3670 commit 5f00d01
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 15 deletions.
6 changes: 3 additions & 3 deletions packages/backend/pkg/src/Permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export type Permissions = {
/**
* Base permission level for any person, logged in or not.
*/
anyone?: PermissionLevel,
anyone: PermissionLevel | null,
/**
* Permission level for the current user.
*/
user?: PermissionLevel,
user: PermissionLevel | null,
/** Permission levels for all other users.
Only owners of the document have access to this information.
*/
users?: Array<UserPermissions>, };
users: Array<UserPermissions> | null, };
3 changes: 0 additions & 3 deletions packages/backend/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ pub struct UserPermissions {
#[derive(Clone, Debug, Default, Serialize, TS)]
pub struct Permissions {
/// Base permission level for any person, logged in or not.
#[ts(optional)]
pub anyone: Option<PermissionLevel>,

/// Permission level for the current user.
#[ts(optional)]
pub user: Option<PermissionLevel>,

/** Permission levels for all other users.
Only owners of the document have access to this information.
*/
#[ts(optional)]
pub users: Option<Vec<UserPermissions>>,
}

Expand Down
4 changes: 3 additions & 1 deletion packages/backend/src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ pub async fn set_active_user_profile(ctx: AppCtx, profile: UserProfile) -> Resul
};
profile.validate().map_err(AppError::Invalid)?;

// Once set, a username cannot be unset, only changed to a new name.
// Once set, a username cannot be unset, only changed to a different name.
// This should be validated in the frontend, and it is enforced below by
// using `COALESCE`.
let query = sqlx::query!(
"
UPDATE users SET username = COALESCE($2, username), display_name = $3
Expand Down
18 changes: 15 additions & 3 deletions packages/frontend/src/api/user_rpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,31 @@ describe("Sharing documents between users", async () => {
});

await signOut(auth);

// Access the document as the sharee.
await signInWithEmailAndPassword(auth, shareeEmail, password);

const readonlyDoc = unwrap(await rpc.get_doc.query(refId));
test.sequential("should allow read-only document access when unauthenticated", () => {
test.sequential("should allow read-only document access when logged in", () => {
assert.strictEqual(readonlyDoc.tag, "Readonly");
assert.strictEqual(readonlyDoc.permissions.anyone, null);
assert.strictEqual(readonlyDoc.permissions.user, "Read");
});

const forbiddenResult1 = await rpc.set_permissions.mutate(refId, [
{
userId: shareeUser.uid,
level: "Write",
},
]);
test.sequential("should prohibit upgrading own permissions", () => {
assert.strictEqual(unwrapErr(forbiddenResult1).code, 403);
});

await signOut(auth);

const forbiddenResult = await rpc.get_doc.query(refId);
const forbiddenResult2 = await rpc.get_doc.query(refId);
test.sequential("should prohibit document access when logged out", () => {
assert.strictEqual(unwrapErr(forbiddenResult).code, 403);
assert.strictEqual(unwrapErr(forbiddenResult2).code, 403);
});
});
9 changes: 4 additions & 5 deletions packages/frontend/src/user/permissions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getAuth, signOut } from "firebase/auth";
import { useAuth, useFirebaseApp } from "solid-firebase";
import { type ComponentProps, Match, Show, Switch, createSignal } from "solid-js";

import type { Permissions } from "catcolab-api";
import type { PermissionLevel, Permissions } from "catcolab-api";
import { Dialog, IconButton } from "../components";
import { Login } from "./login";

Expand Down Expand Up @@ -129,7 +129,7 @@ const SharedPermissionsButton = (props: {
const tooltip = (permissions: Permissions) => (
<>
This document is <strong>{permissionAdjective(permissions.user)}</strong> by you and{" "}
{permissionAdjective()} by anyone.
{permissionAdjective(permissions.anyone)} by anyone.
</>
);
return (
Expand All @@ -155,9 +155,8 @@ const PrivatePermissionsButton = (props: {
);
};

type PermissionLevel = Required<Permissions>["user"];

const permissionAdjective = (level?: PermissionLevel) => permissionAdjectives[level ?? "Read"];
const permissionAdjective = (level: PermissionLevel | null) =>
level ? permissionAdjectives[level] : "not viewable";

const permissionAdjectives: { [level in PermissionLevel]: string } = {
Read: "viewable",
Expand Down

0 comments on commit 5f00d01

Please sign in to comment.