webhooker/internal/handlers/target_retries_test.go

package handlers_test

import (
	"net/http"
	"net/url"
	"strconv"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"sneak.berlin/go/webhooker/internal/database"
	"sneak.berlin/go/webhooker/internal/handlers"
)

// retriesTargetURL is the destination the retry-validation targets
// point at. It is a literal public address rather than a hostname so
// the SSRF check resolves nothing and a sandbox without DNS cannot
// make these cases pass or fail for the wrong reason.
const retriesTargetURL = "https://93.184.216.34/hooks/retries"

const (
	// wayAboveCeiling is the typo'd-extra-zero case from the report.
	wayAboveCeiling = "999999999"

	// notANumber is the plainest garbage an operator can type, and
	// the value the report submitted on the edit form.
	notANumber = "abc"

	// workingRetries is the retry count a seeded target is already
	// delivering with, which a rejected submission must not disturb.
	workingRetries = 2
)

// aboveCeiling is the smallest rejected whole number.
func aboveCeiling() string {
	return strconv.Itoa(handlers.MaxTargetRetriesForTest + 1)
}

// overCeilingRetries is whole-number input past the limit, which is
// rejected with the limit named.
func overCeilingRetries() []string {
	return []string{aboveCeiling(), wayAboveCeiling}
}

// unparseableRetries is input an operator can type into the field
// that is not a retry count. Each must be REJECTED: silently reading
// any of them as 0 turns a store-and-forward proxy into
// fire-and-forget without saying so.
//
// The twenty-digit case is here because it parses as digits but
// overflows int, which is the one failure the field's own min/max
// attributes cannot describe.
func unparseableRetries() []string {
	return []string{
		notANumber,
		"2.7",
		"-5",
		"12345678901234567890",
		"1e3",
	}
}

// createRetriesForm is a complete, otherwise-valid HTTP target
// creation, so the only thing any case below varies is max_retries.
func createRetriesForm(retries string) url.Values {
	form := url.Values{}
	form.Set("name", "retries-target")
	form.Set("type", string(database.TargetTypeHTTP))
	form.Set("url", retriesTargetURL)

	if retries != absentField {
		form.Set("max_retries", retries)
	}

	return form
}

// absentField marks a field the form does not submit at all, which is
// the case that legitimately takes a default and must stay distinct
// from a field submitted with garbage in it.
const absentField = "\x00absent"

// absentRetries is every way of saying "the operator did not set
// this", each of which takes the default rather than a 400. Blank and
// whitespace-only count as absent here because they do in the timeout
// and retention controls on the same forms; a rule the fields do not
// share would be its own surprise.
func absentRetries() []string {
	return []string{absentField, "", "  "}
}

// createWithRetries posts the target create form for a fresh webhook
// and returns the webhook and the response.
func createWithRetries(
	t *testing.T,
	env *sourceTestEnv,
	retries string,
) (database.Webhook, int, string) {
	t.Helper()

	webhook := seedWebhookWithRetention(t, env.db, 30)

	w := serveTarget(
		env, http.MethodPost,
		"/hook/"+webhook.ID+"/targets",
		createRetriesForm(retries),
	)

	return webhook, w.Code, w.Body.String()
}

// TestTargetCreate_RetriesAboveCeilingRejected proves the create form
// enforces a ceiling at all, and that the 400 names it — a rejection
// that does not say what the limit is leaves the operator guessing.
func TestTargetCreate_RetriesAboveCeilingRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)
	ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)

	for _, retries := range overCeilingRetries() {
		webhook, code, body := createWithRetries(t, env, retries)

		assert.Equal(t, http.StatusBadRequest, code,
			"max_retries=%s should be rejected", retries)
		assert.Contains(t, body, ceiling,
			"the rejection for %s should name the ceiling",
			retries)
		assert.Empty(t,
			targetsForWebhook(t, env.db, webhook.ID),
			"no target should be created for %s", retries)
	}
}

// TestTargetCreate_UnparseableRetriesRejected is the core of the
// defect: each of these was accepted with HTTP 200 and stored as 0.
func TestTargetCreate_UnparseableRetriesRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, retries := range unparseableRetries() {
		webhook, code, body := createWithRetries(t, env, retries)

		assert.Equal(t, http.StatusBadRequest, code,
			"max_retries=%q should be rejected, not coerced",
			retries)
		assert.Contains(t, body, "whole number",
			"the rejection for %q should say why", retries)
		assert.Empty(t,
			targetsForWebhook(t, env.db, webhook.ID),
			"no target should be created for %q", retries)
	}
}

// TestTargetCreate_ValidRetriesStored covers the accepting half,
// including the ceiling itself: a bound that rejects its own limit
// would make the advertised maximum unreachable.
func TestTargetCreate_ValidRetriesStored(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, want := range []int{0, 3, handlers.MaxTargetRetriesForTest} {
		webhook, code, body := createWithRetries(
			t, env, strconv.Itoa(want),
		)

		require.Equal(t, http.StatusSeeOther, code, body)

		targets := targetsForWebhook(t, env.db, webhook.ID)
		require.Len(t, targets, 1)
		assert.Equal(t, want, targets[0].MaxRetries)
	}
}

// TestTargetCreate_AbsentRetriesTakesDefault keeps the two cases
// distinct. An omitted field is not an operator asking for something
// invalid, so it still gets the fire-and-forget default rather than a
// 400 — otherwise the fix above would make the form unusable.
func TestTargetCreate_AbsentRetriesTakesDefault(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, retries := range absentRetries() {
		webhook, code, body := createWithRetries(t, env, retries)

		require.Equal(t, http.StatusSeeOther, code, body)

		targets := targetsForWebhook(t, env.db, webhook.ID)
		require.Len(t, targets, 1)
		assert.Equal(t, 0, targets[0].MaxRetries,
			"an absent max_retries should take the default")
	}
}

// seedRetriesTarget creates an HTTP target already delivering with
// workingRetries retries, through the real create handler.
func seedRetriesTarget(
	t *testing.T,
	env *sourceTestEnv,
) (database.Webhook, database.Target) {
	t.Helper()

	webhook, code, body := createWithRetries(
		t, env, strconv.Itoa(workingRetries),
	)
	require.Equal(t, http.StatusSeeOther, code, body)

	targets := targetsForWebhook(t, env.db, webhook.ID)
	require.Len(t, targets, 1)
	require.Equal(t, workingRetries, targets[0].MaxRetries)

	return webhook, targets[0]
}

// editRetriesForm is a complete edit submission that changes the
// target's name as well, so a rejected submission can be shown to
// have written nothing at all rather than merely to have left
// max_retries alone.
func editRetriesForm(retries string) url.Values {
	form := url.Values{}
	form.Set("name", "renamed-by-edit")
	form.Set("url", retriesTargetURL)

	if retries != absentField {
		form.Set("max_retries", retries)
	}

	return form
}

// assertEditRejectedAndUnchanged submits an edit expected to fail and
// checks both halves of the requirement: the 400 explains itself, and
// the target it was submitted against is untouched.
func assertEditRejectedAndUnchanged(
	t *testing.T,
	env *sourceTestEnv,
	retries, wantReason string,
) {
	t.Helper()

	webhook, target := seedRetriesTarget(t, env)

	w := submitTargetEdit(
		env, webhook.ID, target.ID, editRetriesForm(retries),
	)

	assert.Equal(t, http.StatusBadRequest, w.Code,
		"max_retries=%q should be rejected on edit", retries)
	assert.Contains(t, w.Body.String(), wantReason,
		"the rejection for %q should say why", retries)

	stored := storedTarget(t, env, target.ID)
	assert.Equal(t, workingRetries, stored.MaxRetries,
		"a rejected edit must not destroy the working retry "+
			"count with %q", retries)
	assert.Equal(t, "retries-target", stored.Name,
		"a rejected edit must write nothing at all")
}

// TestTargetEdit_UnparseableRetriesRejected is the damaging half of
// the defect. A target delivering with two retries, re-saved with a
// typo in the field, returned 200 and was left with retries disabled
// and nothing said.
func TestTargetEdit_UnparseableRetriesRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, retries := range unparseableRetries() {
		assertEditRejectedAndUnchanged(
			t, env, retries, "whole number",
		)
	}
}

// TestTargetEdit_RetriesAboveCeilingRejected proves the ceiling
// applies to the edit path too, naming itself, so the two paths
// cannot disagree about what is storable.
func TestTargetEdit_RetriesAboveCeilingRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)
	ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)

	for _, retries := range overCeilingRetries() {
		assertEditRejectedAndUnchanged(t, env, retries, ceiling)
	}
}

// TestTargetEdit_ValidRetriesStored covers the accepting half of the
// edit path, so the ceiling cannot be enforced by simply refusing
// every submission.
func TestTargetEdit_ValidRetriesStored(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, want := range []int{0, 9, handlers.MaxTargetRetriesForTest} {
		webhook, target := seedRetriesTarget(t, env)

		w := submitTargetEdit(
			env, webhook.ID, target.ID,
			editRetriesForm(strconv.Itoa(want)),
		)
		require.Equal(t,
			http.StatusSeeOther, w.Code, w.Body.String(),
		)

		assert.Equal(t, want,
			storedTarget(t, env, target.ID).MaxRetries)
	}
}

// TestTargetEdit_AbsentRetriesLeavesStoredValue is the edit path's
// absent-versus-invalid case. Retries are only offered by the forms
// for types that retry, so a submission without the field must leave
// the stored count alone rather than be rejected or zeroed.
func TestTargetEdit_AbsentRetriesLeavesStoredValue(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, retries := range absentRetries() {
		webhook, target := seedRetriesTarget(t, env)

		w := submitTargetEdit(
			env, webhook.ID, target.ID,
			editRetriesForm(retries),
		)
		require.Equal(t,
			http.StatusSeeOther, w.Code, w.Body.String(),
		)

		assert.Equal(t, workingRetries,
			storedTarget(t, env, target.ID).MaxRetries,
			"an absent max_retries must leave the stored "+
				"count alone (%q)", retries)
	}
}

// TestTargetRetries_CreateAndEditAgreeOnEveryCase proves the two
// paths cannot disagree, which is what let the create form and the
// edit form drift apart in the first place. Every input is submitted
// to both and the accept/reject verdicts are compared.
func TestTargetRetries_CreateAndEditAgreeOnEveryCase(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	accepted := []string{
		"0", "1",
		strconv.Itoa(handlers.MaxTargetRetriesForTest),
	}
	overCeiling := overCeilingRetries()
	unparseable := unparseableRetries()

	cases := make(
		[]string, 0,
		len(accepted)+len(overCeiling)+len(unparseable),
	)
	cases = append(cases, accepted...)
	cases = append(cases, overCeiling...)
	cases = append(cases, unparseable...)

	for _, retries := range cases {
		_, createCode, _ := createWithRetries(t, env, retries)

		webhook, target := seedRetriesTarget(t, env)
		editCode := submitTargetEdit(
			env, webhook.ID, target.ID,
			editRetriesForm(retries),
		).Code

		assert.Equal(t,
			createCode == http.StatusBadRequest,
			editCode == http.StatusBadRequest,
			"create and edit must agree on max_retries=%q "+
				"(create %d, edit %d)",
			retries, createCode, editCode,
		)
	}
}

// TestPageOrFirst_CoercesRatherThanRejects pins the one place a
// non-numeric form value legitimately falls back. A page number says
// where to send the browser after an action that has already
// happened, so it is not configuration and rejecting it would report
// a failure that did not occur.
func TestPageOrFirst_CoercesRatherThanRejects(t *testing.T) {
	t.Parallel()

	for _, s := range []string{"", "abc", "0", "-1", "2.7", " "} {
		assert.Equal(t, 1, handlers.PageOrFirstForTest(s),
			"%q should fall back to the first page", s)
	}

	assert.Equal(t, 4, handlers.PageOrFirstForTest("4"))
	assert.Equal(t, 4, handlers.PageOrFirstForTest(" 4 "))
}