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:
@@ -20,10 +20,12 @@ func (a *apiServer) burnViewerWindow() int {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// handleRedeemDeletion lets a holder of the deletion token hard-delete immediately.
|
// handleRedeemDeletion lets a holder of the deletion token hard-delete immediately.
|
||||||
// DELETE /api/pastes/{id}/redeem?token=...
|
// DELETE /api/pastes/{id}/redeem with the token in the Authorization header
|
||||||
|
// (#143: the ?token= query path was removed so the secret stays out of
|
||||||
|
// access logs and browser history).
|
||||||
func (a *apiServer) handleRedeemDeletion(w http.ResponseWriter, r *http.Request) {
|
func (a *apiServer) handleRedeemDeletion(w http.ResponseWriter, r *http.Request) {
|
||||||
id := chi.URLParam(r, "id")
|
id := chi.URLParam(r, "id")
|
||||||
token := r.URL.Query().Get("token")
|
token := deletionAuthorization(r)
|
||||||
if token == "" {
|
if token == "" {
|
||||||
writeErr(w, 400, "token required")
|
writeErr(w, 400, "token required")
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -53,8 +53,9 @@ func TestDeletionTokenRedeem(t *testing.T) {
|
|||||||
t.Fatal("no deletion token in create response")
|
t.Fatal("no deletion token in create response")
|
||||||
}
|
}
|
||||||
|
|
||||||
// wrong token
|
// wrong token (#143: token goes in the Authorization header, not the URL)
|
||||||
req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem?token=wrong", nil)
|
req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem", nil)
|
||||||
|
req.Header.Set("Authorization", "Bearer wrong")
|
||||||
rec = httptest.NewRecorder()
|
rec = httptest.NewRecorder()
|
||||||
h.ServeHTTP(rec, req)
|
h.ServeHTTP(rec, req)
|
||||||
if rec.Code != 403 {
|
if rec.Code != 403 {
|
||||||
@@ -62,7 +63,8 @@ func TestDeletionTokenRedeem(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// right token: hard delete
|
// right token: hard delete
|
||||||
req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem?token="+created.DeletionToken, nil)
|
req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem", nil)
|
||||||
|
req.Header.Set("Authorization", "Bearer "+created.DeletionToken)
|
||||||
rec = httptest.NewRecorder()
|
rec = httptest.NewRecorder()
|
||||||
h.ServeHTTP(rec, req)
|
h.ServeHTTP(rec, req)
|
||||||
if rec.Code != 200 {
|
if rec.Code != 200 {
|
||||||
|
|||||||
@@ -1,9 +1,10 @@
|
|||||||
package api
|
package api
|
||||||
|
|
||||||
// Regression tests for #63: DELETE /api/pastes/{id} must require the
|
// Regression tests for #63 and #143: DELETE /api/pastes/{id} must require the
|
||||||
// deletion token (Authorization header or ?token= query param, constant-time
|
// deletion token in the Authorization header (constant-time compare). The
|
||||||
// compare). Without a token, or with a wrong token, the paste must survive
|
// ?token= query parameter is NOT accepted (#143): URL-carried tokens leak
|
||||||
// and the response must be 403.
|
// 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 (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
@@ -68,10 +69,14 @@ func TestDeleteWithWrongTokenForbidden(t *testing.T) {
|
|||||||
h := s.routes()
|
h := s.routes()
|
||||||
id, _ := createTestPaste(t, h)
|
id, _ := createTestPaste(t, h)
|
||||||
|
|
||||||
// query param
|
// query param: even the CORRECT token must be rejected now (#143)
|
||||||
rec := doReq(t, h, "DELETE", "/api/pastes/"+id+"?token=wrong-token", "", "")
|
id2, tok2 := createTestPaste(t, h)
|
||||||
|
rec := doReq(t, h, "DELETE", "/api/pastes/"+id2+"?token="+tok2, "", "")
|
||||||
if rec.Code != http.StatusForbidden {
|
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
|
// header
|
||||||
req := httptest.NewRequest("DELETE", "/api/pastes/"+id, nil)
|
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")
|
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)
|
id, tok = createTestPaste(t, h)
|
||||||
rec = doReq(t, h, "DELETE", "/api/pastes/"+id+"?token="+tok, "", "")
|
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 {
|
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) {
|
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"},
|
{"bearer tok", "", "tok"},
|
||||||
{"Token tok", "", "tok"},
|
{"Token tok", "", "tok"},
|
||||||
{"tok", "", "tok"},
|
{"tok", "", "tok"},
|
||||||
{"", "?token=q", "q"},
|
{"", "?token=q", ""}, // #143: query tokens are never accepted
|
||||||
{"Bearer hdr", "?token=q", "hdr"}, // header wins
|
{"Bearer hdr", "?token=q", "hdr"}, // header only
|
||||||
}
|
}
|
||||||
for _, c := range cases {
|
for _, c := range cases {
|
||||||
if got := deletionAuthorization(mk(c.hdr, c.q)); got != c.want {
|
if got := deletionAuthorization(mk(c.hdr, c.q)); got != c.want {
|
||||||
|
|||||||
+10
-7
@@ -311,9 +311,10 @@ func (a *apiServer) handleDeletePaste(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
// #63: deletion requires authorization. Either the deletion token issued
|
// #63: deletion requires authorization. Either the deletion token issued
|
||||||
// at create time (Authorization header or ?token= query param, matching
|
// at create time (Authorization header; #143 removed the ?token= query
|
||||||
// the create response's "deletion_token" field), or the creator browser
|
// path so the bearer secret never lands in access logs or history), or
|
||||||
// itself (client-sent vwr cookie matching the paste's viewer, #37).
|
// the creator browser itself (client-sent vwr cookie matching the
|
||||||
|
// paste's viewer, #37).
|
||||||
if !a.deletionAuthorized(r, row) {
|
if !a.deletionAuthorized(r, row) {
|
||||||
writeErr(w, 403, "deletion token required")
|
writeErr(w, 403, "deletion token required")
|
||||||
return
|
return
|
||||||
@@ -325,9 +326,11 @@ func (a *apiServer) handleDeletePaste(w http.ResponseWriter, r *http.Request) {
|
|||||||
writeJSON(w, 200, map[string]string{"status": "soft-deleted"})
|
writeJSON(w, 200, map[string]string{"status": "soft-deleted"})
|
||||||
}
|
}
|
||||||
|
|
||||||
// deletionAuthorization extracts the deletion token from the request: the
|
// deletionAuthorization extracts the deletion token from the request:
|
||||||
// Authorization header ("Bearer <t>", "Token <t>", or a bare token) or the
|
// the Authorization header ("Bearer <t>", "Token <t>", or a bare token).
|
||||||
// token query parameter. Returns "" when absent.
|
// The ?token= query parameter is deliberately NOT accepted (#143): URL
|
||||||
|
// query strings end up in proxy access logs and browser history. Returns
|
||||||
|
// "" when absent.
|
||||||
func deletionAuthorization(r *http.Request) string {
|
func deletionAuthorization(r *http.Request) string {
|
||||||
if h := r.Header.Get("Authorization"); h != "" {
|
if h := r.Header.Get("Authorization"); h != "" {
|
||||||
for _, prefix := range []string{"Bearer ", "Token "} {
|
for _, prefix := range []string{"Bearer ", "Token "} {
|
||||||
@@ -337,7 +340,7 @@ func deletionAuthorization(r *http.Request) string {
|
|||||||
}
|
}
|
||||||
return strings.TrimSpace(h)
|
return strings.TrimSpace(h)
|
||||||
}
|
}
|
||||||
return r.URL.Query().Get("token")
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// deletionAuthorized reports whether the request may soft-delete the paste:
|
// deletionAuthorized reports whether the request may soft-delete the paste:
|
||||||
|
|||||||
@@ -291,7 +291,9 @@ function finishCreate(data) {
|
|||||||
setTimeout(() => { copyBtn.classList.remove('ok'); copyBtn.textContent = '⧉'; }, 2000);
|
setTimeout(() => { copyBtn.classList.remove('ok'); copyBtn.textContent = '⧉'; }, 2000);
|
||||||
} catch(e) { toast('Copy failed', 'error'); }
|
} catch(e) { toast('Copy failed', 'error'); }
|
||||||
});
|
});
|
||||||
const dest = '/' + data.id + '?created=1&token=' + encodeURIComponent(data.deletion_token || '');
|
// token carried via sessionStorage, never in the URL (#143)
|
||||||
|
const dest = '/' + data.id + '?created=1';
|
||||||
|
try { sessionStorage.setItem('deletion_token_' + data.id, data.deletion_token || ''); } catch(e) {}
|
||||||
// password-protected: unlock now with the password we already have (#26)
|
// password-protected: unlock now with the password we already have (#26)
|
||||||
if ($('haspw').checked && data.id) {
|
if ($('haspw').checked && data.id) {
|
||||||
const fd = new FormData();
|
const fd = new FormData();
|
||||||
|
|||||||
@@ -9,7 +9,7 @@
|
|||||||
<button type="button" class="iconbtn wrap-toggle" title="Toggle line wrap" aria-pressed="false">wrap</button>
|
<button type="button" class="iconbtn wrap-toggle" title="Toggle line wrap" aria-pressed="false">wrap</button>
|
||||||
<a class="iconbtn" href="/raw/{{.ID}}">raw</a>
|
<a class="iconbtn" href="/raw/{{.ID}}">raw</a>
|
||||||
<a class="iconbtn" href="#" id="copy-btn" onclick="copyContent(this); return false;">copy</a>
|
<a class="iconbtn" href="#" id="copy-btn" onclick="copyContent(this); return false;">copy</a>
|
||||||
{{if .DeletionToken}}<a class="iconbtn danger" href="#" onclick="redeem('{{.DeletionToken}}'); return false;">delete</a>{{end}}
|
{{if .DeletionToken}}<a class="iconbtn danger" href="#" onclick="redeem(); return false;">delete</a>{{end}}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="float">
|
<div class="float">
|
||||||
@@ -89,9 +89,12 @@ function copyContent(btn) {
|
|||||||
toast('Copied', 'success');
|
toast('Copied', 'success');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
function redeem(token) {
|
function redeem() {
|
||||||
if (!confirm('Hard delete this paste immediately?')) return;
|
if (!confirm('Hard delete this paste immediately?')) return;
|
||||||
fetch('/api/pastes/{{.ID}}/redeem?token=' + encodeURIComponent(token), {method: 'DELETE'})
|
let tok = '';
|
||||||
|
try { tok = sessionStorage.getItem('deletion_token_{{.ID}}') || ''; } catch(e) {}
|
||||||
|
if (!tok) { alert('deletion token not available in this browser'); return; }
|
||||||
|
fetch('/api/pastes/{{.ID}}/redeem', {method: 'DELETE', headers: {'Authorization': 'Bearer ' + tok}})
|
||||||
.then(r => { if (r.ok) location.href = '/history'; else alert('delete failed'); });
|
.then(r => { if (r.ok) location.href = '/history'; else alert('delete failed'); });
|
||||||
}
|
}
|
||||||
</script>
|
</script>
|
||||||
|
|||||||
+7
-1
@@ -310,7 +310,13 @@ func (h *Handlers) HandlePasteView(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
justCreated := r.URL.Query().Get("created") == "1"
|
justCreated := r.URL.Query().Get("created") == "1"
|
||||||
token := r.URL.Query().Get("token")
|
// #143: the deletion token is no longer round-tripped through the URL
|
||||||
|
// (?token= leaks into access logs and history). The create flow sets a
|
||||||
|
// short-lived tok_<id> cookie; the paste view reads it once from there.
|
||||||
|
token := ""
|
||||||
|
if c, err := r.Cookie("tok_" + row.ID); err == nil {
|
||||||
|
token = c.Value
|
||||||
|
}
|
||||||
if justCreated && token != "" {
|
if justCreated && token != "" {
|
||||||
// one-time display of the deletion token via the created banner
|
// one-time display of the deletion token via the created banner
|
||||||
http.SetCookie(w, &http.Cookie{Name: "tok_" + row.ID, Value: token, Path: "/", MaxAge: 60, HttpOnly: true, SameSite: http.SameSiteLaxMode})
|
http.SetCookie(w, &http.Cookie{Name: "tok_" + row.ID, Value: token, Path: "/", MaxAge: 60, HttpOnly: true, SameSite: http.SameSiteLaxMode})
|
||||||
|
|||||||
Reference in New Issue
Block a user