Add delete token UI for info page

2026-09-13 00:32:51 +02:00
parent ca34f1506d
commit 74bfdbfd8a
9 changed files with 293 additions and 25 deletions
@@ -27,12 +27,23 @@ func (s *Server) handleDelete(w http.ResponseWriter, r *http.Request) {
 	}

 	presented := s.deleteCredentials(w, r)
+	from := r.PostFormValue("from")
+
 	if len(presented) == 0 {
-		s.fail(w, r, http.StatusUnauthorized, "A delete token or an owning token is required.")
+		s.refuse(w, r, m, from, http.StatusUnauthorized,
+			"A delete token or an owning token is required.")
 		return
 	}
 	if !s.authorised(m, presented) {
-		s.fail(w, r, http.StatusForbidden, "That token cannot delete this file.")
+		// Only failures are throttled, so a correct token is never delayed.
+		// The info page is publicly shareable and now carries a credential
+		// field, which is reason enough not to let it be hammered freely.
+		if !s.deleteLimiter.allow(clientIP(r, s.cfg), s.now()) {
+			s.refuse(w, r, m, from, http.StatusTooManyRequests,
+				"Too many failed attempts; try again shortly.")
+			return
+		}
+		s.refuse(w, r, m, from, http.StatusForbidden, "That delete token is not correct.")
 		return
 	}

@@ -50,7 +61,7 @@ func (s *Server) handleDelete(w http.ResponseWriter, r *http.Request) {
 	// Deleting from the administration table goes back to it. The destination
 	// is built from configuration, never from the request, so this cannot be
 	// turned into an open redirect.
-	if r.PostFormValue("from") == "admin" {
+	if from == "admin" {
 		http.Redirect(w, r, s.cfg.BasePath+"admin", http.StatusSeeOther)
 		return
 	}
@@ -61,6 +72,17 @@ func (s *Server) handleDelete(w http.ResponseWriter, r *http.Request) {
 	})
 }

+// refuse reports a rejected deletion. A failed attempt from the file's own page
+// lands back on that page with the reason, rather than on a generic error page
+// that has thrown away what the reader typed.
+func (s *Server) refuse(w http.ResponseWriter, r *http.Request, m *store.Meta, from string, status int, msg string) {
+	if from == "info" && !wantsJSON(r) {
+		s.renderInfo(w, r, m, status, msg)
+		return
+	}
+	s.fail(w, r, status, msg)
+}
+
 // deleteCredentials collects every secret the request carries.
 //
 // Three can legitimately arrive at once — the object's delete token in the
@@ -124,6 +124,11 @@ type objectPage struct {
 	URL         string
 	InfoURL     string
 	DeleteToken string
+
+	// CanDelete is set when the viewer's own token already authorises removing
+	// this file, so they are offered a button instead of a token field.
+	CanDelete bool
+	Error     string
 }

 func (s *Server) handleInfo(w http.ResponseWriter, r *http.Request) {
@@ -137,11 +142,21 @@ func (s *Server) handleInfo(w http.ResponseWriter, r *http.Request) {
 		s.fail(w, r, http.StatusNotFound, "No such file.")
 		return
 	}
-	s.render(w, http.StatusOK, "info.html", objectPage{
-		page:    s.page(r, m.Filename, false),
-		Meta:    m,
-		Size:    config.FormatSize(m.Size),
-		Expires: describeExpiry(m.Expires, s.now()),
+	s.renderInfo(w, r, m, http.StatusOK, "")
+}
+
+// renderInfo draws the file's page, optionally with an error from a failed
+// delete attempt, so a mistyped token lands back on the form rather than on a
+// dead end.
+func (s *Server) renderInfo(w http.ResponseWriter, r *http.Request, m *store.Meta, status int, errMsg string) {
+	s.render(w, status, "info.html", objectPage{
+		page:      s.page(r, m.Filename, true),
+		Meta:      m,
+		Size:      config.FormatSize(m.Size),
+		Expires:   describeExpiry(m.Expires, s.now()),
+		URL:       s.objectURL(r, m.ID),
+		CanDelete: s.mayDelete(m, credential(r)),
+		Error:     errMsg,
 	})
 }

@@ -24,10 +24,11 @@ type Server struct {
 	tokens *auth.File
 	log    *slog.Logger

-	pages   map[string]*template.Template
-	handler http.Handler
-	limiter *limiter
-	slots   chan struct{} // bounds uploads in flight
+	pages         map[string]*template.Template
+	handler       http.Handler
+	limiter       *limiter
+	deleteLimiter *limiter      // consumed only by failed deletions
+	slots         chan struct{} // bounds uploads in flight

 	now func() time.Time // swappable in tests
 }
@@ -38,14 +39,15 @@ func New(cfg *config.Config, st *store.Store, tokens *auth.File, log *slog.Logge
 		return nil, err
 	}
 	s := &Server{
-		cfg:     cfg,
-		store:   st,
-		tokens:  tokens,
-		log:     log,
-		pages:   pages,
-		limiter: newLimiter(cfg.UploadRate, cfg.UploadBurst),
-		slots:   make(chan struct{}, cfg.MaxConcurrent),
-		now:     time.Now,
+		cfg:           cfg,
+		store:         st,
+		tokens:        tokens,
+		log:           log,
+		pages:         pages,
+		limiter:       newLimiter(cfg.UploadRate, cfg.UploadBurst),
+		deleteLimiter: newLimiter(120, 20),
+		slots:         make(chan struct{}, cfg.MaxConcurrent),
+		now:           time.Now,
 	}
 	s.handler = s.routes()
 	return s, nil
@@ -1273,3 +1273,153 @@ func TestFormFieldsOverrideTheHeaders(t *testing.T) {
 		t.Errorf("id = %q, want the form field to win", res.ID)
 	}
 }
+
+// --- deleting from the info page -----------------------------------------
+
+func (h *harness) postForm(t *testing.T, path string, form url.Values, cookie *http.Cookie) *http.Response {
+	t.Helper()
+	req, _ := http.NewRequest("POST", h.ts.URL+path, strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Accept", "text/html")
+	if cookie != nil {
+		req.AddCookie(cookie)
+	}
+	resp, err := h.ts.Client().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return resp
+}
+
+// The delete token is shown once and then has to be usable somewhere. The info
+// page is the link an uploader would have kept, so the form lives there.
+func TestInfoPageAcceptsTheDeleteToken(t *testing.T) {
+	h := newHarness(t, nil)
+	res := decode[uploadResult](t, h.upload(t, []byte("x"), nil))
+
+	info := h.get(t, "/i/"+res.ID, "")
+	raw, _ := io.ReadAll(info.Body)
+	info.Body.Close()
+	page := string(raw)
+
+	if !strings.Contains(page, "Remove this file") {
+		t.Error("the info page offers no way to use a delete token")
+	}
+	if !strings.Contains(page, `name="token"`) {
+		t.Error("the info page has no field for the delete token")
+	}
+	if strings.Contains(page, res.DeleteToken) {
+		t.Fatal("the info page leaks the delete token to anyone holding the link")
+	}
+
+	resp := h.postForm(t, "/api/d/"+res.ID+"/delete",
+		url.Values{"from": {"info"}, "token": {res.DeleteToken}}, nil)
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("deleting with the right token => %s", resp.Status)
+	}
+	if _, err := h.store.Get(res.ID, h.now); err == nil {
+		t.Error("the file was not deleted")
+	}
+}
+
+// A mistyped token must land back on the file's page with the reason, not on a
+// generic error page that has thrown the form away.
+func TestWrongDeleteTokenReturnsToTheInfoPage(t *testing.T) {
+	h := newHarness(t, nil)
+	res := decode[uploadResult](t, h.upload(t, []byte("x"), map[string]string{
+		"Content-Disposition": `attachment; filename="keepme.bin"`}))
+
+	resp := h.postForm(t, "/api/d/"+res.ID+"/delete",
+		url.Values{"from": {"info"}, "token": {"wrong"}}, nil)
+	raw, _ := io.ReadAll(resp.Body)
+	resp.Body.Close()
+	page := string(raw)
+
+	if resp.StatusCode != http.StatusForbidden {
+		t.Errorf("status = %s, want 403", resp.Status)
+	}
+	if !strings.Contains(page, "keepme.bin") {
+		t.Error("the response is not the file's own page")
+	}
+	if !strings.Contains(page, "not correct") {
+		t.Error("the page does not say what went wrong")
+	}
+	if !strings.Contains(page, "<details open>") {
+		t.Error("the delete section is collapsed, hiding the error")
+	}
+	if _, err := h.store.Get(res.ID, h.now); err != nil {
+		t.Error("the file was deleted despite a wrong token")
+	}
+}
+
+// Someone whose own token already authorises removal gets a button, not a
+// field asking for a token they do not have.
+func TestInfoPageOffersADirectButtonToAnOwner(t *testing.T) {
+	h := newHarness(t, nil)
+	res := decode[uploadResult](t, h.upload(t, []byte("x"), map[string]string{
+		"Authorization": "Bearer " + h.token}))
+
+	for _, c := range []struct {
+		who       string
+		token     string
+		expectBtn bool
+	}{
+		{"the owner", h.token, true},
+		{"an admin", h.admin, true},
+		{"a stranger", "", false},
+	} {
+		req, _ := http.NewRequest("GET", h.ts.URL+"/i/"+res.ID, nil)
+		if c.token != "" {
+			req.AddCookie(&http.Cookie{Name: tokenCookie, Value: c.token})
+		}
+		resp, err := h.ts.Client().Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		raw, _ := io.ReadAll(resp.Body)
+		resp.Body.Close()
+		got := strings.Contains(string(raw), "Your token can remove this file")
+		if got != c.expectBtn {
+			t.Errorf("direct delete button shown to %s = %v, want %v", c.who, got, c.expectBtn)
+		}
+	}
+
+	// And that button actually works with no token field at all.
+	resp := h.postForm(t, "/api/d/"+res.ID+"/delete",
+		url.Values{"from": {"info"}}, &http.Cookie{Name: tokenCookie, Value: h.token})
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("owner delete => %s", resp.Status)
+	}
+}
+
+// Guessing is throttled, but only the guessing: a correct token is never
+// delayed by someone else's failed attempts.
+func TestFailedDeletesAreThrottledAndSuccessIsNot(t *testing.T) {
+	h := newHarness(t, nil)
+	h.deleteLimiter = newLimiter(1, 3)
+
+	res := decode[uploadResult](t, h.upload(t, []byte("x"), nil))
+
+	var last *http.Response
+	for range 5 {
+		if last != nil {
+			last.Body.Close()
+		}
+		last = h.postForm(t, "/api/d/"+res.ID+"/delete",
+			url.Values{"from": {"info"}, "token": {"guess"}}, nil)
+	}
+	if last.StatusCode != http.StatusTooManyRequests {
+		t.Fatalf("repeated guesses => %s, want 429", last.Status)
+	}
+	last.Body.Close()
+
+	// The real token still works, having consumed nothing from the bucket.
+	resp := h.postForm(t, "/api/d/"+res.ID+"/delete",
+		url.Values{"from": {"info"}, "token": {res.DeleteToken}}, nil)
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("the correct token was throttled: %s", resp.Status)
+	}
+}