Remove ?token= deletion-token path (#143)
The deletion token was accepted via the ?token= query parameter on both
DELETE /api/pastes/{id} and /redeem, and round-tripped through the paste
URL after creation. URL-carried bearer secrets leak into reverse-proxy
access logs and browser history.
- API: deletion tokens are now accepted only via the Authorization header
(Bearer/Token/bare); query params are ignored on both endpoints
- Web create flow: token moves to the browser via a short-lived tok_<id>
HttpOnly cookie instead of the redirect URL; the paste view reads it
from the cookie, never from ?token=
- Web view: the delete button calls redeem() which takes the token from
sessionStorage and sends it as an Authorization header
- Tests: correct token in query must be rejected (403/400); header path
still deletes/redeems; extraction unit cases updated
Fixes #143
This commit is contained in:
@@ -1,9 +1,10 @@
|
||||
package api
|
||||
|
||||
// Regression tests for #63: DELETE /api/pastes/{id} must require the
|
||||
// deletion token (Authorization header or ?token= query param, constant-time
|
||||
// compare). Without a token, or with a wrong token, the paste must survive
|
||||
// and the response must be 403.
|
||||
// Regression tests for #63 and #143: DELETE /api/pastes/{id} must require the
|
||||
// deletion token in the Authorization header (constant-time compare). The
|
||||
// ?token= query parameter is NOT accepted (#143): URL-carried tokens leak
|
||||
// into access logs and browser history. Without a token, or with a wrong
|
||||
// token, the paste must survive and the response must be 403.
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
@@ -68,10 +69,14 @@ func TestDeleteWithWrongTokenForbidden(t *testing.T) {
|
||||
h := s.routes()
|
||||
id, _ := createTestPaste(t, h)
|
||||
|
||||
// query param
|
||||
rec := doReq(t, h, "DELETE", "/api/pastes/"+id+"?token=wrong-token", "", "")
|
||||
// query param: even the CORRECT token must be rejected now (#143)
|
||||
id2, tok2 := createTestPaste(t, h)
|
||||
rec := doReq(t, h, "DELETE", "/api/pastes/"+id2+"?token="+tok2, "", "")
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("delete with wrong token (query): got %d want 403", rec.Code)
|
||||
t.Fatalf("delete with correct token in query: got %d want 403 (#143)", rec.Code)
|
||||
}
|
||||
if !pasteExists(t, h, id2) {
|
||||
t.Fatal("paste was deleted via ?token= query param (#143 regression)")
|
||||
}
|
||||
// header
|
||||
req := httptest.NewRequest("DELETE", "/api/pastes/"+id, nil)
|
||||
@@ -103,14 +108,43 @@ func TestDeleteWithCorrectToken(t *testing.T) {
|
||||
t.Fatal("paste still exists after authorized delete")
|
||||
}
|
||||
|
||||
// via query param
|
||||
// query param: even with the correct token the delete must fail (#143)
|
||||
id, tok = createTestPaste(t, h)
|
||||
rec = doReq(t, h, "DELETE", "/api/pastes/"+id+"?token="+tok, "", "")
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("delete with correct token (query): got %d want 403 (#143)", rec.Code)
|
||||
}
|
||||
if !pasteExists(t, h, id) {
|
||||
t.Fatal("paste was deleted via ?token= query param (#143 regression)")
|
||||
}
|
||||
}
|
||||
|
||||
// #143: the deletion token must be accepted via the Authorization header on
|
||||
// the redeem (hard delete) endpoint too.
|
||||
func TestRedeemWithCorrectTokenHeader(t *testing.T) {
|
||||
s := testServer(t)
|
||||
h := s.routes()
|
||||
id, tok := createTestPaste(t, h)
|
||||
|
||||
req := httptest.NewRequest("DELETE", "/api/pastes/"+id+"/redeem", nil)
|
||||
req.Header.Set("Authorization", "Bearer "+tok)
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("delete with correct token (query): got %d want 200", rec.Code)
|
||||
t.Fatalf("redeem with correct token (header): got %d want 200: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if pasteExists(t, h, id) {
|
||||
t.Fatal("paste still exists after authorized delete (query)")
|
||||
t.Fatal("paste still exists after authorized redeem")
|
||||
}
|
||||
|
||||
// query param must NOT work on redeem either
|
||||
id, tok = createTestPaste(t, h)
|
||||
rec = doReq(t, h, "DELETE", "/api/pastes/"+id+"/redeem?token="+tok, "", "")
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("redeem via ?token= query: got %d want 400 (#143)", rec.Code)
|
||||
}
|
||||
if !pasteExists(t, h, id) {
|
||||
t.Fatal("paste was hard-deleted via ?token= query param (#143 regression)")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -161,8 +195,8 @@ func TestDeletionAuthorizationExtract(t *testing.T) {
|
||||
{"bearer tok", "", "tok"},
|
||||
{"Token tok", "", "tok"},
|
||||
{"tok", "", "tok"},
|
||||
{"", "?token=q", "q"},
|
||||
{"Bearer hdr", "?token=q", "hdr"}, // header wins
|
||||
{"", "?token=q", ""}, // #143: query tokens are never accepted
|
||||
{"Bearer hdr", "?token=q", "hdr"}, // header only
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := deletionAuthorization(mk(c.hdr, c.q)); got != c.want {
|
||||
|
||||
Reference in New Issue
Block a user