Supabase constraints (#343)

YousefED · web-flow · commit bc826cf64bc9 · 2023-07-12T22:58:03.000+02:00
* add constraints

* fix tests

* fix

* fix

* fix

* fix rls

* fix RLS

* misc fixes

* remove useless test
diff --git a/packages/editor/src/app/main/components/Navigation.tsx b/packages/editor/src/app/main/components/Navigation.tsx
@@ -1,15 +1,13 @@
 /* eslint-disable jsx-a11y/anchor-is-valid */
 
-import {
-  AtlassianNavigation,
-  PrimaryButton,
-} from "@atlaskit/atlassian-navigation";
+import { AtlassianNavigation } from "@atlaskit/atlassian-navigation";
+import Button from "@atlaskit/button";
 import { observer } from "mobx-react-lite";
 import { useCallback } from "react";
 import { VscSignIn } from "react-icons/vsc";
 import { Link, useLocation, useNavigate } from "react-router-dom";
 import { SessionStore } from "../../../store/local/SessionStore";
-import { toDocs, toLoginScreen } from "../../routes/routes";
+import { toDocs, toLoginScreen, toRegisterScreen } from "../../routes/routes";
 import { Logo } from "./Logo";
 import styles from "./Navigation.module.css";
 import { ProfilePopup } from "./ProfilePopup";
@@ -29,15 +27,10 @@ export const Navigation = observer((props: { sessionStore: SessionStore }) => {
   const navigate = useNavigate();
   const location = useLocation();
 
+  const isLoggedIn = sessionStore.isLoggedIn;
   const renderProfile = useCallback(() => {
-    return (
-      <>
-        {sessionStore.isLoggedIn && (
-          <ProfilePopup sessionStore={sessionStore} />
-        )}
-      </>
-    );
-  }, [sessionStore]);
+    return <>{isLoggedIn && <ProfilePopup sessionStore={sessionStore} />}</>;
+  }, [isLoggedIn, sessionStore]);
 
   return (
     <AtlassianNavigation
@@ -68,19 +61,35 @@ export const Navigation = observer((props: { sessionStore: SessionStore }) => {
       renderSignIn={observer(() => (
         <>
           {!sessionStore.isLoggedIn && (
-            <PrimaryButton
-              onClick={() =>
-                navigate(toLoginScreen(), { state: { from: location } })
-              }
-              iconBefore={
-                <VscSignIn style={{ width: "16px", height: "16px" }} />
-              }>
-              {" "}
-              Sign in{" "}
-              {/* {typeof sessionStore.user === "string"
+            <>
+              <Button
+                style={{ borderRadius: "8px" }}
+                appearance="subtle"
+                onClick={() =>
+                  navigate(toLoginScreen(), { state: { from: location } })
+                }>
+                {" "}
+                Log in
+                {/* {typeof sessionStore.user === "string"
                 ? sessionStore.user
                 : sessionStore.user.type} */}
-            </PrimaryButton>
+              </Button>
+              <Button
+                style={{ borderRadius: "8px" }}
+                appearance="primary"
+                onClick={() =>
+                  navigate(toRegisterScreen(), { state: { from: location } })
+                }
+                iconBefore={
+                  <VscSignIn style={{ width: "16px", height: "16px" }} />
+                }>
+                {" "}
+                Sign up for free{" "}
+                {/* {typeof sessionStore.user === "string"
+                ? sessionStore.user
+                : sessionStore.user.type} */}
+              </Button>
+            </>
           )}
         </>
       ))}
diff --git a/packages/editor/src/app/main/components/documentMenu/ForkAlert.tsx b/packages/editor/src/app/main/components/documentMenu/ForkAlert.tsx
@@ -4,7 +4,7 @@ import { Link, useLocation, useNavigate } from "react-router-dom";
 import { DocumentResource } from "../../../../store/DocumentResource";
 
 import { SessionStore } from "../../../../store/local/SessionStore";
-import { toLoginScreen } from "../../../routes/routes";
+import { toRegisterScreen } from "../../../routes/routes";
 import styles from "./ForkAlert.module.css";
 
 export const ForkAlert = observer(
@@ -39,8 +39,8 @@ export const ForkAlert = observer(
         <span>save a copy</span>
       </a>
     ) : (
-      <Link to={toLoginScreen()} state={{ from: location }}>
-        <span>sign in to save a copy</span>
+      <Link to={toRegisterScreen()} state={{ from: location }}>
+        <span>sign up to save a copy</span>
       </Link>
     );
 
diff --git a/packages/editor/src/app/supabase-auth/SupabaseSessionStore.ts b/packages/editor/src/app/supabase-auth/SupabaseSessionStore.ts
@@ -153,11 +153,10 @@ export class SupabaseSessionStore extends SessionStore {
       }
     }
 
-    // TODO: first check if username is available?
-
+    // create workspace
     const workspaceId = this.getIdentifierForNewDocument();
     {
-      // TODO: use syncmanager
+      // TODO: use syncmanager?
       const ydoc = new Y.Doc();
       const ret = new BaseResource(ydoc, workspaceId);
       ret.create("!project");
@@ -167,10 +166,10 @@ export class SupabaseSessionStore extends SessionStore {
       remote.dispose();
     }
 
-    // TODO: manage aliases
+    // create profile
     const profileId = this.getIdentifierForNewDocument();
     {
-      // TODO: use syncmanager
+      // TODO: use syncmanager?
       const ydoc = new Y.Doc();
       const ret = new BaseResource(ydoc, profileId);
       ret.create("!profile");
@@ -258,6 +257,7 @@ export class SupabaseSessionStore extends SessionStore {
     } else {
       runInAction(() => {
         setDefaultShorthandResolver(new DefaultShorthandResolver()); // hacky
+        this.userId = undefined;
         this.user = {
           type: "guest-user",
           supabase: this.supabase,
diff --git a/packages/editor/src/store/DocConnection.ts b/packages/editor/src/store/DocConnection.ts
@@ -62,8 +62,14 @@ export class DocConnection extends lifecycle.Disposable {
         this.manager?.dispose();
         let newManager: SyncManager | undefined = undefined;
 
-        if (sessionStore.user && sessionStore.documentCoordinator) {
+        if (this.sessionStore.user && this.sessionStore.documentCoordinator) {
+          // This is hacky, we should clear cache and probably dispose all docconnections when the sessionstore changes
           newManager = SyncManager.load(identifier, this.sessionStore);
+          const cacheKey = DocConnection.getCacheKey(
+            this.sessionStore,
+            identifier
+          );
+          cache.set(cacheKey, this);
         }
         runInAction(() => {
           this.manager = newManager;
@@ -292,6 +298,7 @@ export class DocConnection extends lifecycle.Disposable {
     const cacheKey = DocConnection.getCacheKey(sessionStore, identifier);
     let connection = cache.get(cacheKey);
     if (!connection) {
+      // console.log("DocConnection load debug", cacheKey);
       const syncManager = SyncManager.load(identifier, sessionStore);
 
       connection = new DocConnection(identifier, syncManager, sessionStore);
diff --git a/packages/editor/src/store/yjs-sync/DocumentCoordinator.ts b/packages/editor/src/store/yjs-sync/DocumentCoordinator.ts
@@ -133,6 +133,7 @@ export class DocumentCoordinator extends lifecycle.Disposable {
       throw new Error("not initialized");
     }
 
+    // console.log("Coordinator load debug", idStr, this.userId);
     if (this.loadedDocuments.has(idStr)) {
       // we expect loadDocument only to be called once per document
       throw new Error("loadDocument: document already loaded");
diff --git a/packages/editor/src/store/yjs-sync/IDBHelper.ts b/packages/editor/src/store/yjs-sync/IDBHelper.ts
diff --git a/packages/editor/src/store/yjs-sync/remote/TypeCellRemote.ts b/packages/editor/src/store/yjs-sync/remote/TypeCellRemote.ts
@@ -123,7 +123,7 @@ export class TypeCellRemote extends Remote {
       id: uuid.generateUuid(),
       created_at: date,
       updated_at: date,
-      data: "\\x" + hash.toHexString(data),
+      data: "\\x" + hash.toHexString(data.buffer),
       nano_id: this.identifier.documentId,
       public_access_level: this.identifier.documentId.endsWith("/.inbox")
         ? "write"
diff --git a/packages/editor/tests/end-to-end/login/login.spec.ts b/packages/editor/tests/end-to-end/login/login.spec.ts
@@ -1,12 +1,6 @@
 import { DEFAULT_PROVIDER } from "../setup/config";
 import { expect, test } from "../setup/fixtures";
 
-test("Sign in button exists", async ({ page }) => {
-  await page.goto("/");
-  const button = page.locator("header button");
-  await expect(button).toHaveText("Sign in");
-});
-
 if (DEFAULT_PROVIDER === "supabase") {
   test("Sign in by email (supabase)", async ({
     page,
@@ -20,7 +14,7 @@ if (DEFAULT_PROVIDER === "supabase") {
     aliceContext = aliceContext;
 
     await page.goto("/");
-    const button = page.locator("button", { hasText: "Sign in" });
+    const button = page.locator("button", { hasText: "Log in" });
 
     await button.click();
 
@@ -61,7 +55,7 @@ if (DEFAULT_PROVIDER === "supabase") {
     aliceContext = aliceContext;
 
     await page.goto("/");
-    const button = page.locator("button", { hasText: "Sign in" });
+    const button = page.locator("button", { hasText: "Log in" });
 
     await button.click();
 
diff --git a/packages/editor/tests/end-to-end/setup/userFixtures.ts b/packages/editor/tests/end-to-end/setup/userFixtures.ts
@@ -91,7 +91,7 @@ export const testWithUsers = base.extend<
   aliceUser: [
     async ({}, use, workerInfo) => {
       const aliceUser: TestUser = {
-        username: "alice-" + SESSION_ID + "-" + workerInfo.workerIndex,
+        username: "alice_" + SESSION_ID + "_" + workerInfo.workerIndex,
         password: "myPw123ABC@#$",
       };
 
@@ -102,7 +102,7 @@ export const testWithUsers = base.extend<
   bobUser: [
     async ({}, use, workerInfo) => {
       const bobUser: TestUser = {
-        username: "bob-" + SESSION_ID + "-" + workerInfo.workerIndex,
+        username: "bob_" + SESSION_ID + "_" + workerInfo.workerIndex,
         password: "myB0bPw123ABC@#$",
       };
 
diff --git a/packages/server/supabase/migrations/20230324133839_initial.sql b/packages/server/supabase/migrations/20230324133839_initial.sql
@@ -83,7 +83,7 @@ BEGIN
       SELECT MIN(check_document_access(uid, parent_id)) FROM document_relations r WHERE child_id = doc_id
     );
 END;
-$$ LANGUAGE plpgsql;
+$$ LANGUAGE plpgsql SECURITY DEFINER;
 
 create policy "Enable insert for authenticated users only"
 on "public"."documents"
@@ -152,8 +152,4 @@ for select
 to public
 using (true);
 
-
-
-
--- TODO: prevent select * using https://stackoverflow.com/questions/74283527/postgresql-remove-ability-to-query-every-row-in-a-table
--- TODO: validate formats of nanoids and usernames
+-- TODO: prevent select * using https://stackoverflow.com/questions/74283527/postgresql-remove-ability-to-query-every-row-in-a-table
diff --git a/packages/server/supabase/migrations/20230711133839_domains.sql b/packages/server/supabase/migrations/20230711133839_domains.sql
@@ -0,0 +1,80 @@
+
+CREATE DOMAIN document_nano_id_domain AS TEXT
+CHECK(
+   VALUE ~ '^d[0-9A-Za-z]{5,}(/\.inbox)?$'
+);
+
+CREATE DOMAIN name_domain AS TEXT
+CHECK (
+  VALUE NOT ILIKE '%typecell%' AND 
+  VALUE NOT ILIKE '%admin%' AND 
+  LENGTH(VALUE) BETWEEN 3 AND 20 AND 
+  VALUE ~ '^[A-Za-z][A-Za-z0-9_]*$' AND -- ensures the username only contains alphanumeric characters and underscores, and starts with a letter
+  VALUE ~ '^([^_]*(_[^_]*)*)$' -- ensures the username does not start with, end with, or contain consecutive underscores.
+);
+
+ALTER TABLE  "public"."documents"
+ALTER COLUMN "nano_id" TYPE document_nano_id_domain;
+
+ALTER TABLE  "public"."workspaces"
+ALTER COLUMN "name" TYPE name_domain,
+ALTER COLUMN "document_nano_id" TYPE document_nano_id_domain;
+
+-- make case insensitive
+DROP INDEX IF EXISTS workspaces_name_key;
+CREATE UNIQUE INDEX workspaces_name_key ON public.workspaces USING btree (lower(name));
+
+-- more RLS policies
+alter table "public"."document_permissions" enable row level security;
+alter table "public"."document_relations" enable row level security;
+
+create policy "Enable INSERT document_relations for users with write access only"
+on "public"."document_relations"
+for INSERT
+to authenticated
+with check (((SELECT public_access_level FROM documents WHERE id = parent_id) >= 'write') OR ((SELECT user_id FROM documents WHERE id = parent_id) = auth.uid()) OR (check_document_access(auth.uid(), parent_id) >= 'write'));
+
+create policy "Enable SELECT document_relations for users with write access only"
+on "public"."document_relations"
+for SELECT
+to authenticated
+using (((SELECT public_access_level FROM documents WHERE id = parent_id) >= 'write') OR ((SELECT user_id FROM documents WHERE id = parent_id) = auth.uid()) OR (check_document_access(auth.uid(), parent_id) >= 'write'));
+
+create policy "Enable UPDATE document_relations for users with write access only"
+on "public"."document_relations"
+for UPDATE
+to authenticated
+using (((SELECT public_access_level FROM documents WHERE id = parent_id) >= 'write') OR ((SELECT user_id FROM documents WHERE id = parent_id) = auth.uid()) OR (check_document_access(auth.uid(), parent_id) >= 'write'));
+
+create policy "Enable DELETE document_relations for users with write access only"
+on "public"."document_relations"
+for DELETE
+to authenticated
+using (((SELECT public_access_level FROM documents WHERE id = parent_id) >= 'write') OR ((SELECT user_id FROM documents WHERE id = parent_id) = auth.uid()) OR (check_document_access(auth.uid(), parent_id) >= 'write'));
+
+CREATE POLICY document_permissions_insert_policy
+ON document_permissions
+FOR INSERT
+TO authenticated
+WITH CHECK ((SELECT user_id FROM documents WHERE id = document_id) = auth.uid());
+
+CREATE POLICY document_permissions_select_policy
+ON document_permissions
+FOR SELECT
+TO authenticated
+USING ((SELECT user_id FROM documents WHERE id = document_id) = auth.uid());
+
+CREATE POLICY document_permissions_update_policy
+ON document_permissions
+FOR UPDATE
+TO authenticated
+USING ((SELECT user_id FROM documents WHERE id = document_id) = auth.uid());
+
+CREATE POLICY document_permissions_delete_policy
+ON document_permissions
+FOR DELETE
+TO authenticated
+USING ((SELECT user_id FROM documents WHERE id = document_id) = auth.uid());
+
+
+ALTER FUNCTION check_document_access SECURITY DEFINER;
diff --git a/packages/server/supabase/tests/database/inserts.test.sql b/packages/server/supabase/tests/database/inserts.test.sql
diff --git a/packages/shared-test/src/randomUser.ts b/packages/shared-test/src/randomUser.ts

Original file line number	Diff line number	Diff line change
`@@ -133,6 +133,7 @@ export class DocumentCoordinator extends lifecycle.Disposable {`
`133`	`133`	`throw new Error("not initialized");`
`134`	`134`	`}`
`135`	`135`
	`136`	`+ // console.log("Coordinator load debug", idStr, this.userId);`
`136`	`137`	`if (this.loadedDocuments.has(idStr)) {`
`137`	`138`	`// we expect loadDocument only to be called once per document`
`138`	`139`	`throw new Error("loadDocument: document already loaded");`