webhooker/internal/handlers/target_edit_test.go

package handlers_test

import (
	"context"
	"encoding/json"
	"net/http"
	"net/http/httptest"
	"net/url"
	"strings"
	"testing"

	"github.com/go-chi/chi"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"gorm.io/gorm/clause"
	"sneak.berlin/go/webhooker/internal/database"
	"sneak.berlin/go/webhooker/internal/delivery"
)

// The destinations the target edit tests configure. Both are literal
// public addresses rather than hostnames so the SSRF check resolves
// nothing: with a hostname, a sandbox without DNS would reject the
// URL for the wrong reason and a test asserting rejection would pass
// even with the guard removed.
const (
	editOriginalURL = "https://93.184.216.34/hooks/original"
	editReplacedURL = "https://93.184.216.34/hooks/replaced"
	// editBlockedURL resolves to loopback, which the SSRF guard
	// refuses. It is what proves the guard runs on the edit path.
	editBlockedURL = "http://127.0.0.1/hooks/internal"
)

// editAuthHeader carries a bearer credential, the case the headers
// field exists for.
const (
	editBearerSecret = "QQEDITSECRETQQ"
	editAuthHeader   = "Authorization: Bearer " + editBearerSecret
)

// targetRouter mounts the target create and edit routes on a chi
// router so the handlers see the URL parameters they read.
func targetRouter(env *sourceTestEnv) *chi.Mux {
	router := chi.NewRouter()
	router.Post(
		"/source/{sourceID}/targets",
		env.handlers.HandleTargetCreate(),
	)
	router.Get(
		"/source/{sourceID}/targets/{targetID}/edit",
		env.handlers.HandleTargetEdit(),
	)
	router.Post(
		"/source/{sourceID}/targets/{targetID}/edit",
		env.handlers.HandleTargetEditSubmit(),
	)

	return router
}

// serveTarget drives one request through the target routes as the
// authenticated test user.
func serveTarget(
	env *sourceTestEnv,
	method, path string,
	form url.Values,
) *httptest.ResponseRecorder {
	body := ""
	if form != nil {
		body = form.Encode()
	}

	req := httptest.NewRequestWithContext(
		context.Background(), method, path,
		strings.NewReader(body),
	)
	if form != nil {
		req.Header.Set(
			"Content-Type",
			"application/x-www-form-urlencoded",
		)
	}

	for _, c := range env.cookies {
		req.AddCookie(c)
	}

	w := httptest.NewRecorder()
	targetRouter(env).ServeHTTP(w, req)

	return w
}

// seedHTTPTarget creates a webhook and an HTTP target on it through
// the real create handler, so every case starts from a target the
// production path produced rather than a hand-written row.
//
// Standing the fx app up is what a handler test mostly costs, and
// internal/handlers is already the slowest package in the suite, so
// the tests below share one env per test function and give each case
// its own webhook rather than its own app.
func seedHTTPTarget(
	t *testing.T,
	env *sourceTestEnv,
	headers, timeout string,
) (database.Webhook, database.Target) {
	t.Helper()

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

	form := url.Values{}
	form.Set("name", "original-name")
	form.Set("type", string(database.TargetTypeHTTP))
	form.Set("url", editOriginalURL)
	form.Set("headers", headers)
	form.Set("timeout", timeout)
	form.Set("max_retries", "3")

	w := serveTarget(
		env, http.MethodPost,
		"/source/"+webhook.ID+"/targets", form,
	)
	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())

	targets := targetsForWebhook(t, env.db, webhook.ID)
	require.Len(t, targets, 1)

	return webhook, targets[0]
}

// storedTarget reloads a target row.
func storedTarget(
	t *testing.T,
	env *sourceTestEnv,
	targetID string,
) database.Target {
	t.Helper()

	var target database.Target

	require.NoError(
		t,
		env.db.DB().Where("id = ?", targetID).
			First(&target).Error,
	)

	return target
}

// storedHTTPConfig reloads a target and parses its stored HTTP
// configuration.
func storedHTTPConfig(
	t *testing.T,
	env *sourceTestEnv,
	targetID string,
) delivery.HTTPTargetConfig {
	t.Helper()

	var cfg delivery.HTTPTargetConfig

	require.NoError(
		t,
		json.Unmarshal(
			[]byte(storedTarget(t, env, targetID).Config), &cfg,
		),
	)

	return cfg
}

// editForm is the fully populated edit submission for an HTTP
// target.
func editForm(targetURL, headers, timeout string) url.Values {
	form := url.Values{}
	form.Set("name", "edited-name")
	form.Set("url", targetURL)
	form.Set("headers", headers)
	form.Set("timeout", timeout)
	form.Set("max_retries", "5")

	return form
}

// submitTargetEdit posts the edit form for a target.
func submitTargetEdit(
	env *sourceTestEnv,
	webhookID, targetID string,
	form url.Values,
) *httptest.ResponseRecorder {
	return serveTarget(
		env, http.MethodPost,
		"/source/"+webhookID+"/targets/"+targetID+"/edit",
		form,
	)
}

// TestHandleTargetCreate_Configuration covers the half of the gap
// that is not about editing at all: HTTPTargetConfig has carried
// Headers and Timeout, and the delivery path has honoured them, but
// the create form wrote {"url":...} and nothing else, so a
// destination needing an Authorization header could not be
// configured through the UI at all.
func TestHandleTargetCreate_Configuration(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	t.Run("stores headers and timeout", func(t *testing.T) {
		t.Parallel()
		assertCreateStoresHeadersAndTimeout(t, env)
	})

	t.Run("without them keeps a url-only config", func(t *testing.T) {
		t.Parallel()
		assertCreateKeepsURLOnlyConfig(t, env)
	})
}

func assertCreateStoresHeadersAndTimeout(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	_, target := seedHTTPTarget(
		t, env, editAuthHeader+"\nX-Tenant: acme\n", "12",
	)

	cfg := storedHTTPConfig(t, env, target.ID)

	assert.Equal(t, editOriginalURL, cfg.URL)
	assert.Equal(t, 12, cfg.Timeout)
	assert.Equal(
		t,
		map[string]string{
			"Authorization": "Bearer " + editBearerSecret,
			"X-Tenant":      "acme",
		},
		cfg.Headers,
	)
}

// Without the new fields the stored shape must be the same
// {"url":...} the create form wrote before they existed, so no
// existing target's configuration is rewritten by this change.
func assertCreateKeepsURLOnlyConfig(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	_, target := seedHTTPTarget(t, env, "", "")

	assert.JSONEq(
		t, `{"url":"`+editOriginalURL+`"}`, target.Config,
	)
}

// TestHandleTargetEditSubmit_Saves is the round trip the issue asks
// for: create a target, edit it, and confirm the stored config
// changed.
func TestHandleTargetEditSubmit_Saves(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	t.Run("changes the destination URL", func(t *testing.T) {
		t.Parallel()
		assertEditChangesDestination(t, env)
	})

	t.Run("round trips headers and timeout", func(t *testing.T) {
		t.Parallel()
		assertEditRoundTripsHeadersAndTimeout(t, env)
	})

	t.Run("clearing them removes them", func(t *testing.T) {
		t.Parallel()
		assertEditClearingRemovesThem(t, env)
	})

	t.Run("absent max_retries is not zeroed", func(t *testing.T) {
		t.Parallel()
		assertEditKeepsAbsentMaxRetries(t, env)
	})
}

func assertEditChangesDestination(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "")

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

	assert.Equal(
		t,
		editReplacedURL,
		storedHTTPConfig(t, env, target.ID).URL,
	)

	reloaded := storedTarget(t, env, target.ID)
	assert.Equal(t, "edited-name", reloaded.Name)
	assert.Equal(t, 5, reloaded.MaxRetries)
	assert.Equal(
		t, database.TargetTypeHTTP, reloaded.Type,
		"the edit form must not change a target's type",
	)
}

// The two previously unreachable fields must survive create,
// pre-fill and save.
func assertEditRoundTripsHeadersAndTimeout(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")

	w := submitTargetEdit(
		env, webhook.ID, target.ID,
		editForm(
			editOriginalURL,
			"Authorization: Bearer rotated\nX-Trace: on",
			"21",
		),
	)
	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())

	cfg := storedHTTPConfig(t, env, target.ID)

	assert.Equal(t, 21, cfg.Timeout)
	assert.Equal(
		t,
		map[string]string{
			"Authorization": "Bearer rotated",
			"X-Trace":       "on",
		},
		cfg.Headers,
	)
}

// The direction a naive "only set what was submitted" implementation
// gets wrong: an emptied field must remove the stored value, not
// leave the previous one in place.
func assertEditClearingRemovesThem(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")

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

	cfg := storedHTTPConfig(t, env, target.ID)

	assert.Empty(t, cfg.Headers)
	assert.Zero(t, cfg.Timeout)
}

// Retries are offered only by the forms for target types that retry.
// An absent field means the form does not edit retries, not that
// they should be turned off.
func assertEditKeepsAbsentMaxRetries(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "")
	require.Equal(t, 3, target.MaxRetries)

	form := editForm(editOriginalURL, "", "")
	form.Del("max_retries")

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

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

// TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked covers the
// deliberate exception to the masking rule. The operator cannot
// correct a value they cannot see, so this page — and only this page
// — renders the destination and the header values in full.
func TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked(
	t *testing.T,
) {
	t.Parallel()

	env := setupSourceTest(t)

	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")

	w := serveTarget(
		env, http.MethodGet,
		"/source/"+webhook.ID+"/targets/"+target.ID+"/edit",
		nil,
	)
	require.Equal(t, http.StatusOK, w.Code)

	page := w.Body.String()

	assert.Contains(t, page, editOriginalURL)
	assert.Contains(t, page, "Bearer "+editBearerSecret)
	assert.Contains(t, page, `value="7"`)
	assert.Contains(t, page, "original-name")
}

// TestHandleTargetEditSubmit_Rejects covers every submission that
// must not reach storage.
//
// The SSRF case is the most important assertion on this change: the
// edited destination goes through the same guard the create path
// uses. An edit that stored an unvalidated URL would reopen a closed
// hole, since a target could then be created public and edited to
// point at loopback.
//
// The header and timeout cases keep input that could not be
// delivered as written out of storage: a stored value that provably
// never reaches the wire reports a configuration that did not take
// effect.
func TestHandleTargetEditSubmit_Rejects(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	t.Run("an SSRF-blocked destination", func(t *testing.T) {
		t.Parallel()
		assertEditRejectsBlockedDestination(t, env)
	})

	t.Run("a query-string destination", func(t *testing.T) {
		t.Parallel()
		assertEditIgnoresQueryString(t, env)
	})

	headerCases := map[string]string{
		"no colon":        "Authorization Bearer token",
		"empty name":      ": value",
		"invalid name":    "X Bad Name: value",
		"reserved header": "User-Agent: curl/8",
		"duplicate name":  "X-A: one\nx-a: two",
	}

	for name, headers := range headerCases {
		t.Run("headers: "+name, func(t *testing.T) {
			t.Parallel()
			assertEditRejectsHeaders(t, env, headers)
		})
	}

	timeoutCases := map[string]string{
		"not a number": "soon",
		"negative":     "-1",
		"over ceiling": "100000",
	}

	for name, timeout := range timeoutCases {
		t.Run("timeout: "+name, func(t *testing.T) {
			t.Parallel()
			assertEditRejectsTimeout(t, env, timeout)
		})
	}
}

func assertEditRejectsBlockedDestination(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "")

	w := submitTargetEdit(
		env, webhook.ID, target.ID,
		editForm(editBlockedURL, "", ""),
	)

	assert.Equal(t, http.StatusBadRequest, w.Code)
	assert.Contains(t, w.Body.String(), "Invalid target URL")
	assert.Equal(
		t, editOriginalURL,
		storedHTTPConfig(t, env, target.ID).URL,
		"a rejected edit must leave the stored config alone",
	)
}

// The ingress rule the create path already follows applies to the
// edit path too: reading a field with FormValue would let the request
// line carry the credential, and the request line is what logs,
// proxies and Referer headers record.
func assertEditIgnoresQueryString(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "")

	form := url.Values{}
	form.Set("name", "edited-name")

	w := serveTarget(
		env, http.MethodPost,
		"/source/"+webhook.ID+"/targets/"+target.ID+
			"/edit?url="+url.QueryEscape(editReplacedURL)+
			"&headers="+url.QueryEscape(editAuthHeader),
		form,
	)

	assert.Equal(t, http.StatusBadRequest, w.Code)

	cfg := storedHTTPConfig(t, env, target.ID)
	assert.Equal(t, editOriginalURL, cfg.URL)
	assert.Empty(t, cfg.Headers)
}

func assertEditRejectsHeaders(
	t *testing.T, env *sourceTestEnv, headers string,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "")

	w := submitTargetEdit(
		env, webhook.ID, target.ID,
		editForm(editOriginalURL, headers, ""),
	)

	assert.Equal(t, http.StatusBadRequest, w.Code)
	assert.Contains(t, w.Body.String(), "Invalid headers")
	assert.Empty(
		t, storedHTTPConfig(t, env, target.ID).Headers,
		"a rejected header must not be stored",
	)
}

func assertEditRejectsTimeout(
	t *testing.T, env *sourceTestEnv, timeout string,
) {
	t.Helper()

	webhook, target := seedHTTPTarget(t, env, "", "9")

	w := submitTargetEdit(
		env, webhook.ID, target.ID,
		editForm(editOriginalURL, "", timeout),
	)

	assert.Equal(t, http.StatusBadRequest, w.Code)
	assert.Contains(t, w.Body.String(), "Invalid timeout")
	assert.Equal(
		t, 9, storedHTTPConfig(t, env, target.ID).Timeout,
		"a rejected timeout must leave the stored one alone",
	)
}

// TestHandleTargetEdit_Scoping keeps the edit routes scoped the way
// the delete and toggle routes are: ownership is decided by the
// webhook, and the target is then scoped to it.
func TestHandleTargetEdit_Scoping(t *testing.T) {
	t.Parallel()

	env := setupSourceTest(t)

	t.Run("a target of another webhook", func(t *testing.T) {
		t.Parallel()
		assertTargetOfAnotherWebhook404s(t, env)
	})

	t.Run("a webhook of another user", func(t *testing.T) {
		t.Parallel()
		assertWebhookOfAnotherUser404s(t, env)
	})
}

// A target id from elsewhere must not become editable by pairing it
// with a webhook the user does own.
func assertTargetOfAnotherWebhook404s(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	mine := seedWebhookWithRetention(t, env.db, 30)
	_, target := seedHTTPTarget(t, env, "", "")

	get := serveTarget(
		env, http.MethodGet,
		"/source/"+mine.ID+"/targets/"+target.ID+"/edit", nil,
	)
	assert.Equal(t, http.StatusNotFound, get.Code)

	post := submitTargetEdit(
		env, mine.ID, target.ID,
		editForm(editReplacedURL, "", ""),
	)
	assert.Equal(t, http.StatusNotFound, post.Code)

	assert.Equal(
		t, editOriginalURL,
		storedHTTPConfig(t, env, target.ID).URL,
	)
}

func assertWebhookOfAnotherUser404s(
	t *testing.T, env *sourceTestEnv,
) {
	t.Helper()

	other := &database.Webhook{
		UserID:        "some-other-user",
		Name:          "not mine",
		RetentionDays: 30,
	}
	require.NoError(
		t,
		env.db.DB().Omit(clause.Associations).Create(other).Error,
	)

	target := seedConfiguredTarget(
		t, env.db, other.ID, database.TargetTypeHTTP,
		`{"url":"`+editOriginalURL+`"}`,
	)

	w := serveTarget(
		env, http.MethodGet,
		"/source/"+other.ID+"/targets/"+target.ID+"/edit", nil,
	)

	assert.Equal(t, http.StatusNotFound, w.Code)
}