Use random token for claiming scratches (#945)

* Use random token for claiming scratches

* Fix migration dep

* Fix claim_token serialization & address PR comments

* Improve error handling

* Update backend tests

* reformat

* Update another test

* Minor update to ClaimableScratchSerializer

backend/coreapp/migrations/0050_scratch_claim_token.py

@@ -0,0 +1,20 @@
+# Generated by Django 5.0 on 2024-01-17 00:43
+
+import coreapp.models.scratch
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("coreapp", "0049_alter_assembly_source_asm"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="scratch",
+            name="claim_token",
+            field=models.CharField(
+                default=coreapp.models.scratch.gen_claim_token, max_length=64, null=True
+            ),
+        ),
+    ]

backend/coreapp/models/scratch.py

@@ -20,6 +20,10 @@ def gen_scratch_id() -> str:
     return ret
 
 
+def gen_claim_token() -> str:
+    return get_random_string(length=32)
+
+
 class Asm(models.Model):
     hash = models.CharField(max_length=64, primary_key=True)
     data = models.TextField()
@@ -106,6 +110,7 @@ class Scratch(models.Model):
     libraries = LibrariesField(default=list)
     parent = models.ForeignKey("self", null=True, blank=True, on_delete=models.SET_NULL)
     owner = models.ForeignKey(Profile, null=True, blank=True, on_delete=models.SET_NULL)
+    claim_token = models.CharField(max_length=64, null=True, default=gen_claim_token)
 
     class Meta:
         ordering = ["-creation_time"]

backend/coreapp/serializers.py

@@ -230,7 +230,10 @@ class ScratchSerializer(serializers.ModelSerializer[Scratch]):
 
     class Meta:
         model = Scratch
-        exclude = ["target_assembly"]
+        exclude = [
+            "claim_token",
+            "target_assembly",
+        ]
         read_only_fields = [
             "parent",
             "owner",
@@ -295,6 +298,16 @@ class Meta:
         ]
 
 
+# On initial creation, include the "claim_token" field.
+class ClaimableScratchSerializer(ScratchSerializer):
+    claim_token = serializers.CharField(read_only=True)
+
+    class Meta(ScratchSerializer.Meta):
+        exclude = [
+            field for field in ScratchSerializer.Meta.exclude if field != "claim_token"
+        ]
+
+
 class ProjectSerializer(JSONFormSerializer, serializers.ModelSerializer[Project]):
     slug = serializers.SlugField()
 

backend/coreapp/tests/test_scratch.py

@@ -141,7 +141,12 @@ def test_update_scratch_score(self) -> None:
         self.assertGreater(scratch.score, 0)
 
         # Obtain ownership of the scratch
-        response = self.client.post(reverse("scratch-claim", kwargs={"pk": slug}))
+        response = self.client.post(
+            reverse("scratch-claim", kwargs={"pk": slug}),
+            {"token": scratch.claim_token},
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue(response.json()["success"])
 
         # Update the scratch's code and compiler output
         scratch_patch = {
@@ -261,6 +266,7 @@ def test_fork_scratch(self) -> None:
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
+        new_claim_token = response.json()["claim_token"]
         new_slug = response.json()["slug"]
 
         scratch = Scratch.objects.get(slug=slug)
@@ -272,6 +278,10 @@ def test_fork_scratch(self) -> None:
         # Make sure the name carried over to the fork
         self.assertEqual(scratch.name, fork.name)
 
+        # Ensure the new scratch has a (unique) claim token
+        self.assertIsNotNone(new_claim_token)
+        self.assertIsNot(new_claim_token, scratch.claim_token)
+
 
 class ScratchDetailTests(BaseTestCase):
     def test_404_head(self) -> None:
@@ -330,20 +340,27 @@ def test_double_claim(self) -> None:
         Create a scratch anonymously, claim it, then verify that claiming it again doesn't work.
         """
         scratch = self.create_nop_scratch()
-
         self.assertIsNone(scratch.owner)
 
-        response = self.client.post(f"/api/scratch/{scratch.slug}/claim")
+        scratch.claim_token = "1234"
+        scratch.save()
+
+        response = self.client.post(
+            f"/api/scratch/{scratch.slug}/claim", {"token": "1234"}
+        )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertTrue(response.json()["success"])
 
-        response = self.client.post(f"/api/scratch/{scratch.slug}/claim")
+        response = self.client.post(
+            f"/api/scratch/{scratch.slug}/claim", {"token": "1234"}
+        )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertFalse(response.json()["success"])
 
         updated_scratch = Scratch.objects.first()
         assert updated_scratch is not None
         self.assertIsNotNone(updated_scratch.owner)
+        self.assertIsNone(updated_scratch.claim_token)
 
     def test_family(self) -> None:
         root = self.create_nop_scratch()

backend/coreapp/tests/test_user.py

@@ -192,11 +192,14 @@ def test_own_scratch(self) -> None:
             },
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        claim_token = response.json()["claim_token"]
         slug = response.json()["slug"]
 
         self.test_github_login()
 
-        response = self.client.post(f"/api/scratch/{slug}/claim")
+        response = self.client.post(
+            f"/api/scratch/{slug}/claim", {"token": claim_token}
+        )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertTrue(response.json()["success"])
 
@@ -226,11 +229,14 @@ def test_cant_delete_scratch(self) -> None:
             },
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.json())
+        claim_token = response.json()["claim_token"]
         slug = response.json()["slug"]
 
         self.test_github_login()
 
-        response = self.client.post(f"/api/scratch/{slug}/claim")
+        response = self.client.post(
+            f"/api/scratch/{slug}/claim", {"token": claim_token}
+        )
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
         self.assertTrue(response.json()["success"])
 

backend/coreapp/views/scratch.py

@@ -31,6 +31,7 @@
 from ..models.scratch import Asm, Assembly, Scratch
 from ..platforms import Platform
 from ..serializers import (
+    ClaimableScratchSerializer,
     ScratchCreateSerializer,
     ScratchSerializer,
     TerseScratchSerializer,
@@ -318,7 +319,7 @@ def create(self, request: Any, *args: Any, **kwargs: Any) -> Response:
         scratch = create_scratch(request.data)
 
         return Response(
-            ScratchSerializer(scratch, context={"request": request}).data,
+            ClaimableScratchSerializer(scratch, context={"request": request}).data,
             status=status.HTTP_201_CREATED,
         )
 
@@ -426,15 +427,20 @@ def decompile(self, request: Request, pk: str) -> Response:
     @action(detail=True, methods=["POST"])
     def claim(self, request: Request, pk: str) -> Response:
         scratch: Scratch = self.get_object()
+        token = request.data.get("token")
 
         if not scratch.is_claimable():
             return Response({"success": False})
 
+        if scratch.claim_token and scratch.claim_token != token:
+            return Response({"success": False})
+
         profile = request.profile
 
         logger.debug(f"Granting ownership of scratch {scratch} to {profile}")
 
         scratch.owner = profile
+        scratch.claim_token = None
         scratch.save()
 
         return Response({"success": True})
@@ -466,7 +472,7 @@ def fork(self, request: Request, pk: str) -> Response:
         compile_scratch_update_score(new_scratch)
 
         return Response(
-            ScratchSerializer(new_scratch, context={"request": request}).data,
+            ClaimableScratchSerializer(new_scratch, context={"request": request}).data,
             status=status.HTTP_201_CREATED,
         )
 

frontend/src/app/(navfooter)/new/NewScratchForm.tsx

@@ -193,7 +193,7 @@ export default function NewScratchForm({ serverCompilers }: {
 
     const submit = async () => {
         try {
-            const scratch: api.Scratch = await api.post("/scratch", {
+            const scratch: api.ClaimableScratch = await api.post("/scratch", {
                 target_asm: asm,
                 context: context || "",
                 platform,

frontend/src/app/scratch/[slug]/claim/page.tsx

@@ -0,0 +1,46 @@
+"use client"
+
+import { useEffect, useRef, useState } from "react"
+
+import { useRouter } from "next/navigation"
+
+import LoadingSkeleton from "@/app/scratch/[slug]/loading"
+import { post } from "@/lib/api/request"
+
+export default function Page({ params, searchParams }: {
+    params: { slug: string }
+    searchParams: { token: string }
+}) {
+    const router = useRouter()
+
+    // The POST request must happen on the client so
+    // that the Django session cookie is present.
+    const effectRan = useRef(false)
+    const [error, setError] = useState(null)
+    useEffect(() => {
+        if (!effectRan.current) {
+            post(`/scratch/${params.slug}/claim`, { token: searchParams.token })
+                .then(data => {
+                    if (data.success) {
+                        router.replace(`/scratch/${params.slug}`)
+                    } else {
+                        throw new Error("Unable to claim scratch")
+                    }
+                })
+                .catch(err => {
+                    console.error("Failed to claim scratch", err)
+                    setError(err)
+                })
+        }
+
+        return () => {
+            effectRan.current = true
+        }
+    }, [params.slug, router, searchParams.token])
+
+    if (error) {
+        // Rely on error boundary to catch and display error
+        throw error
+    }
+    return <LoadingSkeleton/>
+}

frontend/src/components/Scratch/hooks/useFuzzySaveCallback.ts

@@ -3,7 +3,6 @@ import { useCallback } from "react"
 import * as api from "@/lib/api"
 
 export enum FuzzySaveAction {
-    CLAIM,
     SAVE,
     FORK,
     NONE,
@@ -18,9 +17,7 @@ export default function useFuzzySaveCallback(
     const userIsYou = api.useUserIsYou()
 
     let action = FuzzySaveAction.NONE
-    if (!scratch.owner) {
-        action = FuzzySaveAction.CLAIM
-    } else if (userIsYou(scratch.owner)) {
+    if (userIsYou(scratch.owner)) {
         action = FuzzySaveAction.SAVE
     } else {
         action = FuzzySaveAction.FORK
@@ -30,9 +27,6 @@ export default function useFuzzySaveCallback(
         action,
         useCallback(async () => {
             switch (action) {
-            case FuzzySaveAction.CLAIM:
-                await api.claimScratch(scratch)
-                // fallthrough
             case FuzzySaveAction.SAVE:
                 setScratch(await saveScratch())
                 break
@@ -42,6 +36,6 @@ export default function useFuzzySaveCallback(
             case FuzzySaveAction.NONE:
                 break
             }
-        }, [action, forkScratch, saveScratch, scratch, setScratch]),
+        }, [action, forkScratch, saveScratch, setScratch]),
     ]
 }

frontend/src/lib/api.ts

@@ -6,7 +6,7 @@ import useSWR, { Revalidator, RevalidatorOptions, mutate } from "swr"
 import { useDebouncedCallback } from "use-debounce"
 
 import { ResponseError, get, post, patch, delete_ } from "./api/request"
-import { AnonymousUser, User, Scratch, TerseScratch, Compilation, Page, Compiler, LibraryVersions, Platform, Project, ProjectMember, Preset } from "./api/types"
+import { AnonymousUser, User, Scratch, TerseScratch, Compilation, Page, Compiler, LibraryVersions, Platform, Project, ProjectMember, Preset, ClaimableScratch } from "./api/types"
 import { projectUrl, scratchUrl } from "./api/urls"
 import { ignoreNextWarnBeforeUnload } from "./hooks"
 
@@ -100,13 +100,16 @@ export function useSaveScratch(localScratch: Scratch): () => Promise<Scratch> {
     return saveScratch
 }
 
-export async function claimScratch(scratch: Scratch): Promise<void> {
-    const { success } = await post(`${scratchUrl(scratch)}/claim`, {})
+export async function claimScratch(scratch: ClaimableScratch): Promise<void> {
+    const { success } = await post(`${scratchUrl(scratch)}/claim`, {
+        token: scratch.claim_token,
+    })
     const user = await get("/user")
 
     if (!success)
         throw new Error("Scratch cannot be claimed")
 
+    delete scratch.claim_token
     await mutate(scratchUrl(scratch), {
         ...scratch,
         owner: user,

frontend/src/lib/api/types.ts

@@ -54,6 +54,10 @@ export interface Scratch extends TerseScratch {
     diff_label: string
 }
 
+export interface ClaimableScratch extends Scratch {
+    claim_token: string
+}
+
 export interface Project {
     slug: string
     creation_time: string