webhooker/internal/handlers/target_attempts_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"
)

// attemptsTargetURL is the destination the attempts-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 attemptsTargetURL = "https://93.184.216.34/hooks/attempts"

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"

	// workingAttempts is the number of attempts a seeded target is
	// already delivering with, which a rejected submission must not
	// disturb.
	workingAttempts = 2
)

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

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

// belowOneAttempts is whole-number input under the floor of one
// attempt, which is rejected with the floor named.
func belowOneAttempts() []string {
	return []string{"0", "-5"}
}

// unparseableAttempts is input an operator can type into the field
// that is not a number of attempts. Each must be REJECTED: silently
// reading any of them as a default turns retries off on a
// store-and-forward proxy 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 unparseableAttempts() []string {
	return []string{
		notANumber,
		"2.7",
		"12345678901234567890",
		"1e3",
	}
}

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

	if attempts != absentField {
		form.Set("max_attempts", attempts)
	}

	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"

// absentAttempts 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 absentAttempts() []string {
	return []string{absentField, "", "  "}
}

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

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

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

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

// TestTargetCreate_AttemptsAboveCeilingRejected 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_AttemptsAboveCeilingRejected(t *testing.T) {
	t.Parallel()

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

	for _, attempts := range overCeilingAttempts() {
		webhook, code, body := createWithAttempts(t, env, attempts)

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

// TestTargetCreate_AttemptsBelowOneRejected proves the create form
// refuses 0 and less, naming the floor: every delivery is attempted
// at least once.
func TestTargetCreate_AttemptsBelowOneRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, attempts := range belowOneAttempts() {
		webhook, code, body := createWithAttempts(t, env, attempts)

		assert.Equal(t, http.StatusBadRequest, code,
			"max_attempts=%s should be rejected", attempts)
		assert.Contains(t, body, "at least 1",
			"the rejection for %s should name the floor",
			attempts)
		assert.Empty(t,
			targetsForWebhook(t, env.db, webhook.ID),
			"no target should be created for %s", attempts)
	}
}

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

	env := setupSourceTest(t)

	for _, attempts := range unparseableAttempts() {
		webhook, code, body := createWithAttempts(t, env, attempts)

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

// TestTargetCreate_ValidAttemptsStored covers the accepting half,
// including both bounds: a bound that rejects its own limit would
// make the advertised range unreachable.
func TestTargetCreate_ValidAttemptsStored(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, want := range []int{1, 3, handlers.MaxTargetAttemptsForTest} {
		webhook, code, body := createWithAttempts(
			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].MaxAttempts)
	}
}

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

	env := setupSourceTest(t)

	for _, attempts := range absentAttempts() {
		webhook, code, body := createWithAttempts(t, env, attempts)

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

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

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

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

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

	return webhook, targets[0]
}

// editAttemptsForm 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_attempts alone.
func editAttemptsForm(attempts string) url.Values {
	form := url.Values{}
	form.Set("name", "renamed-by-edit")
	form.Set("url", attemptsTargetURL)

	if attempts != absentField {
		form.Set("max_attempts", attempts)
	}

	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,
	attempts, wantReason string,
) {
	t.Helper()

	webhook, target := seedAttemptsTarget(t, env)

	w := submitTargetEdit(
		env, webhook.ID, target.ID, editAttemptsForm(attempts),
	)

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

	stored := storedTarget(t, env, target.ID)
	assert.Equal(t, workingAttempts, stored.MaxAttempts,
		"a rejected edit must not destroy the working number "+
			"of attempts with %q", attempts)
	assert.Equal(t, "attempts-target", stored.Name,
		"a rejected edit must write nothing at all")
}

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

	env := setupSourceTest(t)

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

// TestTargetEdit_AttemptsBelowOneRejected proves the floor applies
// to the edit path too.
func TestTargetEdit_AttemptsBelowOneRejected(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	for _, attempts := range belowOneAttempts() {
		assertEditRejectedAndUnchanged(t, env, attempts, "at least 1")
	}
}

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

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

	for _, attempts := range overCeilingAttempts() {
		assertEditRejectedAndUnchanged(t, env, attempts, ceiling)
	}
}

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

	env := setupSourceTest(t)

	for _, want := range []int{1, 9, handlers.MaxTargetAttemptsForTest} {
		webhook, target := seedAttemptsTarget(t, env)

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

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

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

	env := setupSourceTest(t)

	for _, attempts := range absentAttempts() {
		webhook, target := seedAttemptsTarget(t, env)

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

		assert.Equal(t, workingAttempts,
			storedTarget(t, env, target.ID).MaxAttempts,
			"an absent max_attempts must leave the stored "+
				"count alone (%q)", attempts)
	}
}

// TestTargetAttempts_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 TestTargetAttempts_CreateAndEditAgreeOnEveryCase(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	accepted := []string{
		"1", "2",
		strconv.Itoa(handlers.MaxTargetAttemptsForTest),
	}
	overCeiling := overCeilingAttempts()
	belowOne := belowOneAttempts()
	unparseable := unparseableAttempts()

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

	for _, attempts := range cases {
		_, createCode, _ := createWithAttempts(t, env, attempts)

		webhook, target := seedAttemptsTarget(t, env)
		editCode := submitTargetEdit(
			env, webhook.ID, target.ID,
			editAttemptsForm(attempts),
		).Code

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