allow deletion of unowned links

It should be possible to delete links owned by non-existing users
(tagged-devices or deleted) in the same ways as it's possible
to edit these links.

Signed-off-by: Marco Hofstetter <[email protected]>

Author: mhofstetter

golink.go

@@ -605,8 +605,8 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if link.Owner != login {
-		http.Error(w, "cannot delete link owned by another user", http.StatusForbidden)
+	if err := checkLinkOwnership(r.Context(), link, login); err != nil {
+		http.Error(w, fmt.Sprintf("cannot delete link: %v", err), http.StatusForbidden)
 		return
 	}
 
@@ -653,17 +653,9 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 
-	if link != nil && link.Owner != "" && link.Owner != login {
-		exists, err := userExists(r.Context(), link.Owner)
-		if err != nil {
-			log.Printf("looking up tailnet user %q: %v", link.Owner, err)
-		}
-		// Don't allow taking over links if the owner account still exists
-		// or if we're unsure because an error occurred.
-		if exists || err != nil {
-			http.Error(w, "not your link; owned by "+link.Owner, http.StatusForbidden)
-			return
-		}
+	if err := checkLinkOwnership(r.Context(), link, login); err != nil {
+		http.Error(w, fmt.Sprintf("cannot update link: %v", err), http.StatusForbidden)
+		return
 	}
 
 	// allow transferring ownership to valid users. If empty, set owner to current user.
@@ -705,6 +697,23 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func checkLinkOwnership(ctx context.Context, link *Link, login string) error {
+	if link == nil || link.Owner == "" {
+		return nil
+	}
+
+	linkOwnerExists, err := userExists(ctx, link.Owner)
+	if err != nil {
+		log.Printf("looking up tailnet user %q: %v", link.Owner, err)
+	}
+	// Don't allow deleting or updating links if the owner account still exists
+	// or if we're unsure because an error occurred.
+	if (linkOwnerExists && link.Owner != login) || err != nil {
+		return fmt.Errorf("link owned by user %q", link.Owner)
+	}
+	return nil
+}
+
 // serveExport prints a snapshot of the link database. Links are JSON encoded
 // and printed one per line. This format is used to restore link snapshots on
 // startup.

golink_test.go

@@ -124,6 +124,8 @@ func TestServeSave(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	db.Save(&Link{Short: "link-owned-by-tagged-devices", Long: "/before", Owner: "tagged-devices"})
+
 	tests := []struct {
 		name              string
 		short             string
@@ -157,6 +159,13 @@ func TestServeSave(t *testing.T) {
 			currentUser: func(*http.Request) (string, error) { return "[email protected]", nil },
 			wantStatus:  http.StatusForbidden,
 		},
+		{
+			name:        "allow editing link owned by tagged-devices",
+			short:       "link-owned-by-tagged-devices",
+			long:        "/after",
+			currentUser: func(*http.Request) (string, error) { return "[email protected]", nil },
+			wantStatus:  http.StatusOK,
+		},
 		{
 			name:        "disallow unknown users",
 			short:       "who2",
@@ -211,6 +220,7 @@ func TestServeDelete(t *testing.T) {
 	}
 	db.Save(&Link{Short: "a", Owner: "[email protected]"})
 	db.Save(&Link{Short: "foo", Owner: "[email protected]"})
+	db.Save(&Link{Short: "link-owned-by-tagged-devices", Long: "/before", Owner: "tagged-devices"})
 
 	xsrf := func(short string) string {
 		return xsrftoken.Generate(xsrfKey, "[email protected]", short)
@@ -238,6 +248,12 @@ func TestServeDelete(t *testing.T) {
 			short:      "a",
 			wantStatus: http.StatusForbidden,
 		},
+		{
+			name:       "allow deleting link owned by tagged-devices",
+			short:      "link-owned-by-tagged-devices",
+			xsrf:       xsrf("link-owned-by-tagged-devices"),
+			wantStatus: http.StatusOK,
+		},
 		{
 			name:       "invalid xsrf",
 			short:      "foo",