disallow accidental updates to existing links

Prior to #177, our XSRF tokens were bound to link IDs, with a special
`.new` value used for newly created links. So if a user tried to create
a link that already existed, the XSRF check would fail. After #177, this
now silently allows the user to overwrite the existing link without any
indication that this happened.

This change adds a hidden `update` param to the details edit form that
must be present when updating an existing link.

Updates #177

Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d
Signed-off-by: Will Norris <will@tailscale.com>

Author: willnorris

golink.go

@@ -881,6 +881,13 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Prevent accidental overwrites of existing links.
+	// If the link already exists, make sure this request is an intentional update.
+	if link != nil && r.FormValue("update") == "" {
+		http.Error(w, "link already exists", http.StatusForbidden)
+		return
+	}
+
 	if !canEditLink(r.Context(), link, cu) {
 		http.Error(w, fmt.Sprintf("cannot update link owned by %q", link.Owner), http.StatusForbidden)
 		return

golink_test.go

@@ -230,6 +230,7 @@ func TestServeSave(t *testing.T) {
 		name              string
 		short             string
 		long              string
+		update            bool
 		allowUnknownUsers bool
 		currentUser       func(*http.Request) (user, error)
 		wantStatus        int
@@ -252,6 +253,19 @@ func TestServeSave(t *testing.T) {
 			long:       "http://who/",
 			wantStatus: http.StatusOK,
 		},
+		{
+			name:       "disallow accidental updates",
+			short:      "who",
+			long:       "http://who2/",
+			wantStatus: http.StatusForbidden,
+		},
+		{
+			name:       "allow intentional updates",
+			short:      "who",
+			long:       "http://who/",
+			update:     true,
+			wantStatus: http.StatusOK,
+		},
 		{
 			name:        "disallow editing another's link",
 			short:       "who",
@@ -263,13 +277,15 @@ func TestServeSave(t *testing.T) {
 			name:        "allow editing link owned by tagged-devices",
 			short:       "link-owned-by-tagged-devices",
 			long:        "/after",
+			update:      true,
 			currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com"}, nil },
 			wantStatus:  http.StatusOK,
 		},
 		{
 			name:        "admins can edit any link",
 			short:       "who",
 			long:        "http://who/",
+			update:      true,
 			currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com", isAdmin: true}, nil },
 			wantStatus:  http.StatusOK,
 		},
@@ -304,10 +320,15 @@ func TestServeSave(t *testing.T) {
 			*allowUnknownUsers = tt.allowUnknownUsers
 			t.Cleanup(func() { *allowUnknownUsers = oldAllowUnknownUsers })
 
-			r := httptest.NewRequest("POST", "/", strings.NewReader(url.Values{
+			v := url.Values{
 				"short": {tt.short},
 				"long":  {tt.long},
-			}.Encode()))
+			}
+			if tt.update {
+				v.Set("update", "1")
+			}
+
+			r := httptest.NewRequest("POST", "/", strings.NewReader(v.Encode()))
 			r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 			w := httptest.NewRecorder()
 			serveSave(w, r)

tmpl/detail.html

@@ -26,6 +26,7 @@ <h2 class="text-xl font-bold pb-2">Link Details</h2>
         <dd>{{.Link.LastEdit.Format "Jan _2, 2006 3:04pm MST"}}</dd>
       </dl>
 
+      <input type="hidden" name="update" value="1">
       <button type=submit class="py-2 px-4 my-4 rounded-md bg-blue-500 border-blue-500 text-white hover:bg-blue-600 hover:border-blue-600">Update</button>
     </form>
 

tmpl/help.html

@@ -139,5 +139,8 @@ <h2 id="api">Application Programming Interface (API)</h2>
 {{`{"Short":"cs","Long":"https://cs.github.com/","Created":"2022-06-03T22:15:29.993978392Z","LastEdit":"2022-06-03T22:15:29.993978392Z","Owner":"amelie@example.com"}`}}
 </pre>
 
+<p>
+To update an existing link, also include `-d update=1`.
+
 </article>
 {{ end }}