Empty the add target form on Cancel; encoding failures stay a 500
The form's reason and values now come from the targetForm component, loaded from the section's data attributes after a refusal and emptied by Cancel, which also resets the form. Each type's fields used to be recreated with the refused values written into the markup, so they came back after Cancel. The browser test checks this after a refusal. A target configuration that cannot be encoded is again a logged 500 with the generic error page, on the add and the edit path; only refusals of submitted values come back on the form. The README paragraph on the browser test is re-wrapped at 80 columns and names Cancel at the type step. Model: opus-5-5
This commit is contained in:
@@ -1326,18 +1326,20 @@ markup. The CSP build runs no expressions, so every Alpine directive in
|
|||||||
A browser test in `internal/server` loads the webhook page and the event log
|
A browser test in `internal/server` loads the webhook page and the event log
|
||||||
under the real policy and checks that: the add entrypoint form stays hidden
|
under the real policy and checks that: the add entrypoint form stays hidden
|
||||||
until Add is clicked; for every target type, the targets section's Add shows
|
until Add is clicked; for every target type, the targets section's Add shows
|
||||||
only a choice of type and Next, Next shows only that type's fields (no url field
|
only a choice of type with Next and Cancel, Next shows only that type's fields
|
||||||
for `database` or `log`), Cancel closes the form, and saving adds the target;
|
(no url field for `database` or `log`), Cancel at either step closes the form,
|
||||||
a refused target comes back with its form open, the values entered and the
|
and saving adds the target; a refused target comes back with its form open, the
|
||||||
reason; the Copy button beside an entrypoint URL reads "Copied" once clicked; an
|
values entered and the reason, and after Cancel the next Add starts with an
|
||||||
event expands and collapses, and so do a delivery's attempts inside it; and at
|
empty form and no reason; the Copy button beside an entrypoint URL reads
|
||||||
phone width the menu button opens and closes the mobile menu. It also fails if the browser reports a console warning or error,
|
"Copied" once clicked; an event expands and collapses, and so do a delivery's
|
||||||
an uncaught exception, or anything the policy refused. `make check` and the
|
attempts inside it; and at phone width the menu button opens and closes the
|
||||||
image build lint it but do not run it, and `make test` leaves it out (its file
|
mobile menu. It also fails if the browser reports a console warning or error, an
|
||||||
is built only with the `browser` build tag). Run it with `make test-browser`
|
uncaught exception, or anything the policy refused. `make check` and the image
|
||||||
after changing `templates/` or `static/js/`: that builds `Dockerfile.browser`,
|
build lint it but do not run it, and `make test` leaves it out (its file is
|
||||||
which runs the test in a digest-pinned headless browser image, so the host
|
built only with the `browser` build tag). Run it with `make test-browser` after
|
||||||
needs no browser.
|
changing `templates/` or `static/js/`: that builds `Dockerfile.browser`, which
|
||||||
|
runs the test in a digest-pinned headless browser image, so the host needs no
|
||||||
|
browser.
|
||||||
|
|
||||||
The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
|
The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
|
||||||
byte as the npm registry publishes it. It is a dependency, not this repo's build
|
byte as the npm registry publishes it. It is a dependency, not this repo's build
|
||||||
|
|||||||
@@ -139,7 +139,7 @@ func (s *Handlers) RenderTemplateForTest(
|
|||||||
func (s *Handlers) BuildSlackTargetConfigForTest(
|
func (s *Handlers) BuildSlackTargetConfigForTest(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
targetURL string,
|
targetURL string,
|
||||||
) (string, string) {
|
) (string, string, error) {
|
||||||
return s.buildSlackTargetConfig(ctx, targetURL)
|
return s.buildSlackTargetConfig(ctx, targetURL)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -149,7 +149,7 @@ func (s *Handlers) BuildSlackTargetConfigForTest(
|
|||||||
func (s *Handlers) BuildHTTPTargetConfigForTest(
|
func (s *Handlers) BuildHTTPTargetConfigForTest(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
targetURL, headers, timeout string,
|
targetURL, headers, timeout string,
|
||||||
) (string, string) {
|
) (string, string, error) {
|
||||||
return s.buildHTTPTargetConfig(ctx, targetFormInput{
|
return s.buildHTTPTargetConfig(ctx, targetFormInput{
|
||||||
URL: targetURL,
|
URL: targetURL,
|
||||||
Headers: headers,
|
Headers: headers,
|
||||||
@@ -160,6 +160,8 @@ func (s *Handlers) BuildHTTPTargetConfigForTest(
|
|||||||
// BuildDatabaseTargetConfigForTest exposes
|
// BuildDatabaseTargetConfigForTest exposes
|
||||||
// buildDatabaseTargetConfig for use in the handlers_test
|
// buildDatabaseTargetConfig for use in the handlers_test
|
||||||
// package.
|
// package.
|
||||||
func BuildDatabaseTargetConfigForTest(expiry string) (string, string) {
|
func BuildDatabaseTargetConfigForTest(
|
||||||
|
expiry string,
|
||||||
|
) (string, string, error) {
|
||||||
return buildDatabaseTargetConfig(expiry)
|
return buildDatabaseTargetConfig(expiry)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -314,10 +314,11 @@ func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
|
|||||||
|
|
||||||
t.Cleanup(app.RequireStop)
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
cfg, errMsg := h.BuildSlackTargetConfigForTest(
|
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
|
||||||
t.Context(), "http://93.184.216.34/services/T00/B00/xxx",
|
t.Context(), "http://93.184.216.34/services/T00/B00/xxx",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Empty(t, errMsg)
|
assert.Empty(t, errMsg)
|
||||||
assert.Contains(t, cfg, "webhookUrl")
|
assert.Contains(t, cfg, "webhookUrl")
|
||||||
}
|
}
|
||||||
@@ -332,10 +333,11 @@ func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
|
|||||||
|
|
||||||
t.Cleanup(app.RequireStop)
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
cfg, errMsg := h.BuildSlackTargetConfigForTest(
|
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
|
||||||
t.Context(), "http://169.254.169.254/latest/meta-data/",
|
t.Context(), "http://169.254.169.254/latest/meta-data/",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Contains(t, errMsg, "Invalid target URL")
|
assert.Contains(t, errMsg, "Invalid target URL")
|
||||||
assert.Empty(t, cfg)
|
assert.Empty(t, cfg)
|
||||||
}
|
}
|
||||||
@@ -435,17 +437,20 @@ func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
// Empty expiry: the keep-forever default, empty config.
|
// Empty expiry: the keep-forever default, empty config.
|
||||||
cfg, errMsg := handlers.BuildDatabaseTargetConfigForTest("")
|
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest("")
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Empty(t, errMsg)
|
assert.Empty(t, errMsg)
|
||||||
assert.Empty(t, cfg)
|
assert.Empty(t, cfg)
|
||||||
|
|
||||||
// Explicit never is stored as config.
|
// Explicit never is stored as config.
|
||||||
cfg, errMsg = handlers.BuildDatabaseTargetConfigForTest("never")
|
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("never")
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Empty(t, errMsg)
|
assert.Empty(t, errMsg)
|
||||||
assert.JSONEq(t, `{"expiry":"never"}`, cfg)
|
assert.JSONEq(t, `{"expiry":"never"}`, cfg)
|
||||||
|
|
||||||
// A positive duration is stored as config.
|
// A positive duration is stored as config.
|
||||||
cfg, errMsg = handlers.BuildDatabaseTargetConfigForTest("720h")
|
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("720h")
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Empty(t, errMsg)
|
assert.Empty(t, errMsg)
|
||||||
assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
|
assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
|
||||||
}
|
}
|
||||||
@@ -456,8 +461,9 @@ func TestBuildDatabaseTargetConfig_RejectsBadExpiry(
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
for _, bad := range []string{"nonsense", "7d", "-5h"} {
|
for _, bad := range []string{"nonsense", "7d", "-5h"} {
|
||||||
cfg, errMsg := handlers.BuildDatabaseTargetConfigForTest(bad)
|
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest(bad)
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
assert.Contains(
|
assert.Contains(
|
||||||
t, errMsg, "Invalid archive expiry",
|
t, errMsg, "Invalid archive expiry",
|
||||||
"expiry %q should be refused", bad,
|
"expiry %q should be refused", bad,
|
||||||
|
|||||||
@@ -1457,14 +1457,20 @@ func (h *Handlers) processTargetCreate(
|
|||||||
) {
|
) {
|
||||||
in := targetFormInputFrom(r)
|
in := targetFormInputFrom(r)
|
||||||
|
|
||||||
target, errMsg := h.newTarget(r.Context(), webhook.ID, in)
|
target, errMsg, err := h.newTarget(r.Context(), webhook.ID, in)
|
||||||
|
if err != nil {
|
||||||
|
h.serverError(w, r, "failed to encode target config", err)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if errMsg != "" {
|
if errMsg != "" {
|
||||||
h.renderSourceDetail(w, r, webhook, in, errMsg)
|
h.renderSourceDetail(w, r, webhook, in, errMsg)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
err := h.db.DB().Create(target).Error
|
err = h.db.DB().Create(target).Error
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.serverError(w, r, "failed to create target", err)
|
h.serverError(w, r, "failed to create target", err)
|
||||||
|
|
||||||
@@ -1479,24 +1485,26 @@ func (h *Handlers) processTargetCreate(
|
|||||||
|
|
||||||
// newTarget validates a new target for a webhook and returns the row
|
// newTarget validates a new target for a webhook and returns the row
|
||||||
// to create, or, when it refuses the target, the message the form
|
// to create, or, when it refuses the target, the message the form
|
||||||
// shows. Every form that creates a target goes through here, so they
|
// shows. An error is the server's fault, not a refusal: the accepted
|
||||||
// all accept and refuse the same things.
|
// configuration could not be encoded. Every form that creates a
|
||||||
|
// target goes through here, so they all accept and refuse the same
|
||||||
|
// things.
|
||||||
func (h *Handlers) newTarget(
|
func (h *Handlers) newTarget(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
webhookID string,
|
webhookID string,
|
||||||
in targetFormInput,
|
in targetFormInput,
|
||||||
) (*database.Target, string) {
|
) (*database.Target, string, error) {
|
||||||
if in.Name == "" {
|
if in.Name == "" {
|
||||||
return nil, "Name is required"
|
return nil, "Name is required", nil
|
||||||
}
|
}
|
||||||
|
|
||||||
if !isValidTargetType(in.Type) {
|
if !isValidTargetType(in.Type) {
|
||||||
return nil, "Invalid target type"
|
return nil, "Invalid target type", nil
|
||||||
}
|
}
|
||||||
|
|
||||||
configJSON, errMsg := h.buildTargetConfig(ctx, in.Type, in)
|
configJSON, errMsg, err := h.buildTargetConfig(ctx, in.Type, in)
|
||||||
if errMsg != "" {
|
if err != nil || errMsg != "" {
|
||||||
return nil, errMsg
|
return nil, errMsg, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// A new target has no stored retry count, so an absent field
|
// A new target has no stored retry count, so an absent field
|
||||||
@@ -1505,7 +1513,7 @@ func (h *Handlers) newTarget(
|
|||||||
// that default.
|
// that default.
|
||||||
maxRetries, err := parseMaxRetries(in.MaxRetries, 0)
|
maxRetries, err := parseMaxRetries(in.MaxRetries, 0)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, "Invalid max retries: " + retriesErrorMessage(err)
|
return nil, "Invalid max retries: " + retriesErrorMessage(err), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return &database.Target{
|
return &database.Target{
|
||||||
@@ -1515,7 +1523,7 @@ func (h *Handlers) newTarget(
|
|||||||
Active: true,
|
Active: true,
|
||||||
Config: configJSON,
|
Config: configJSON,
|
||||||
MaxRetries: maxRetries,
|
MaxRetries: maxRetries,
|
||||||
}, ""
|
}, "", nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// isValidTargetType checks whether the target type is supported.
|
// isValidTargetType checks whether the target type is supported.
|
||||||
@@ -1599,13 +1607,15 @@ func targetFormInputFrom(r *http.Request) targetFormInput {
|
|||||||
|
|
||||||
// buildTargetConfig builds the JSON config string for a target from
|
// buildTargetConfig builds the JSON config string for a target from
|
||||||
// the submitted form values, or returns the message the form shows
|
// the submitted form values, or returns the message the form shows
|
||||||
// for a value it refuses. Which fields of in apply depends on the
|
// for a value it refuses. An error is the server's fault, not a
|
||||||
// target type; a type without a URL ignores any URL submitted.
|
// refusal: the accepted configuration could not be encoded. Which
|
||||||
|
// fields of in apply depends on the target type; a type without a URL
|
||||||
|
// ignores any URL submitted.
|
||||||
func (h *Handlers) buildTargetConfig(
|
func (h *Handlers) buildTargetConfig(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
targetType database.TargetType,
|
targetType database.TargetType,
|
||||||
in targetFormInput,
|
in targetFormInput,
|
||||||
) (string, string) {
|
) (string, string, error) {
|
||||||
switch targetType {
|
switch targetType {
|
||||||
case database.TargetTypeHTTP:
|
case database.TargetTypeHTTP:
|
||||||
return h.buildHTTPTargetConfig(ctx, in)
|
return h.buildHTTPTargetConfig(ctx, in)
|
||||||
@@ -1614,9 +1624,9 @@ func (h *Handlers) buildTargetConfig(
|
|||||||
case database.TargetTypeDatabase:
|
case database.TargetTypeDatabase:
|
||||||
return buildDatabaseTargetConfig(in.Expiry)
|
return buildDatabaseTargetConfig(in.Expiry)
|
||||||
case database.TargetTypeLog:
|
case database.TargetTypeLog:
|
||||||
return "", ""
|
return "", "", nil
|
||||||
default:
|
default:
|
||||||
return "", "Invalid target type"
|
return "", "Invalid target type", nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1626,29 +1636,31 @@ func (h *Handlers) buildTargetConfig(
|
|||||||
func (h *Handlers) buildHTTPTargetConfig(
|
func (h *Handlers) buildHTTPTargetConfig(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
in targetFormInput,
|
in targetFormInput,
|
||||||
) (string, string) {
|
) (string, string, error) {
|
||||||
errMsg := h.validateTargetURL(
|
errMsg := h.validateTargetURL(
|
||||||
ctx, in.URL, "URL is required for HTTP targets",
|
ctx, in.URL, "URL is required for HTTP targets",
|
||||||
)
|
)
|
||||||
if errMsg != "" {
|
if errMsg != "" {
|
||||||
return "", errMsg
|
return "", errMsg, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
headers, err := delivery.ParseTargetHeaders(in.Headers)
|
headers, err := delivery.ParseTargetHeaders(in.Headers)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "Invalid headers: " + err.Error()
|
return "", fmt.Sprintf("Invalid headers: %v", err), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
timeout, err := delivery.ParseTargetTimeout(in.Timeout)
|
timeout, err := delivery.ParseTargetTimeout(in.Timeout)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "Invalid timeout: " + err.Error()
|
return "", fmt.Sprintf("Invalid timeout: %v", err), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return marshalTargetConfig(delivery.HTTPTargetConfig{
|
configJSON, err := marshalTargetConfig(delivery.HTTPTargetConfig{
|
||||||
URL: in.URL,
|
URL: in.URL,
|
||||||
Headers: headers,
|
Headers: headers,
|
||||||
Timeout: timeout,
|
Timeout: timeout,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
return configJSON, "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
// buildSlackTargetConfig builds config JSON for a Slack target,
|
// buildSlackTargetConfig builds config JSON for a Slack target,
|
||||||
@@ -1656,18 +1668,20 @@ func (h *Handlers) buildHTTPTargetConfig(
|
|||||||
func (h *Handlers) buildSlackTargetConfig(
|
func (h *Handlers) buildSlackTargetConfig(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
targetURL string,
|
targetURL string,
|
||||||
) (string, string) {
|
) (string, string, error) {
|
||||||
errMsg := h.validateTargetURL(
|
errMsg := h.validateTargetURL(
|
||||||
ctx, targetURL,
|
ctx, targetURL,
|
||||||
"Webhook URL is required for Slack targets",
|
"Webhook URL is required for Slack targets",
|
||||||
)
|
)
|
||||||
if errMsg != "" {
|
if errMsg != "" {
|
||||||
return "", errMsg
|
return "", errMsg, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return marshalTargetConfig(delivery.SlackTargetConfig{
|
configJSON, err := marshalTargetConfig(delivery.SlackTargetConfig{
|
||||||
WebhookURL: targetURL,
|
WebhookURL: targetURL,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
return configJSON, "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
// validateTargetURL refuses an empty or SSRF-blocked destination,
|
// validateTargetURL refuses an empty or SSRF-blocked destination,
|
||||||
@@ -1718,16 +1732,14 @@ func (h *Handlers) validateTargetURL(
|
|||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// marshalTargetConfig serialises a target configuration for storage,
|
// marshalTargetConfig serialises a target configuration for storage.
|
||||||
// or returns the message the form shows if it cannot.
|
func marshalTargetConfig(cfg any) (string, error) {
|
||||||
func marshalTargetConfig(cfg any) (string, string) {
|
|
||||||
configBytes, err := json.Marshal(cfg)
|
configBytes, err := json.Marshal(cfg)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "Could not encode the target configuration: " +
|
return "", err
|
||||||
err.Error()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return string(configBytes), ""
|
return string(configBytes), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// buildDatabaseTargetConfig builds config JSON for a database
|
// buildDatabaseTargetConfig builds config JSON for a database
|
||||||
@@ -1735,18 +1747,20 @@ func marshalTargetConfig(cfg any) (string, string) {
|
|||||||
// creation time, so an unparseable value is refused instead of
|
// creation time, so an unparseable value is refused instead of
|
||||||
// failing every subsequent delivery. An empty expiry yields an
|
// failing every subsequent delivery. An empty expiry yields an
|
||||||
// empty config (the keep-forever default).
|
// empty config (the keep-forever default).
|
||||||
func buildDatabaseTargetConfig(expiry string) (string, string) {
|
func buildDatabaseTargetConfig(expiry string) (string, string, error) {
|
||||||
expiry = strings.TrimSpace(expiry)
|
expiry = strings.TrimSpace(expiry)
|
||||||
if expiry == "" {
|
if expiry == "" {
|
||||||
return "", ""
|
return "", "", nil
|
||||||
}
|
}
|
||||||
|
|
||||||
err := delivery.ValidateArchiveExpiry(expiry)
|
err := delivery.ValidateArchiveExpiry(expiry)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "Invalid archive expiry: " + err.Error()
|
return "", fmt.Sprintf("Invalid archive expiry: %v", err), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return marshalTargetConfig(map[string]any{"expiry": expiry})
|
configJSON, err := marshalTargetConfig(map[string]any{"expiry": expiry})
|
||||||
|
|
||||||
|
return configJSON, "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
// HandleEntrypointDelete handles deleting an entrypoint.
|
// HandleEntrypointDelete handles deleting an entrypoint.
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"html"
|
"html"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
@@ -131,9 +132,18 @@ func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
|
|||||||
)
|
)
|
||||||
assert.Contains(t, page, html.EscapeString(tc.reason))
|
assert.Contains(t, page, html.EscapeString(tc.reason))
|
||||||
|
|
||||||
|
// Each value comes back in a data attribute of the targets
|
||||||
|
// section named after its field (max_retries as
|
||||||
|
// data-max-retries), except url, which comes back in
|
||||||
|
// data-destination; templates/source_detail.html says why.
|
||||||
for field := range typed {
|
for field := range typed {
|
||||||
|
attr := "data-" + strings.ReplaceAll(field, "_", "-")
|
||||||
|
if field == "url" {
|
||||||
|
attr = "data-destination"
|
||||||
|
}
|
||||||
|
|
||||||
assert.Contains(
|
assert.Contains(
|
||||||
t, page, `name="`+field+`" value="`+
|
t, page, attr+`="`+
|
||||||
html.EscapeString(typed.Get(field))+`"`,
|
html.EscapeString(typed.Get(field))+`"`,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -120,16 +120,20 @@ func (h *Handlers) applyTargetEdit(
|
|||||||
) {
|
) {
|
||||||
name := r.PostFormValue("name")
|
name := r.PostFormValue("name")
|
||||||
if name == "" {
|
if name == "" {
|
||||||
http.Error(
|
http.Error(w, "Name is required", http.StatusBadRequest)
|
||||||
w, "Name is required", http.StatusBadRequest,
|
|
||||||
)
|
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
configJSON, errMsg := h.buildTargetConfig(
|
configJSON, errMsg, err := h.buildTargetConfig(
|
||||||
r.Context(), target.Type, targetFormInputFrom(r),
|
r.Context(), target.Type, targetFormInputFrom(r),
|
||||||
)
|
)
|
||||||
|
if err != nil {
|
||||||
|
h.serverError(w, r, "failed to encode target config", err)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if errMsg != "" {
|
if errMsg != "" {
|
||||||
http.Error(w, errMsg, http.StatusBadRequest)
|
http.Error(w, errMsg, http.StatusBadRequest)
|
||||||
|
|
||||||
@@ -162,7 +166,7 @@ func (h *Handlers) applyTargetEdit(
|
|||||||
// A new name renames the archive file before it is saved (see
|
// A new name renames the archive file before it is saved (see
|
||||||
// delivery.Engine.Rename). If either step fails, it goes back to
|
// delivery.Engine.Rename). If either step fails, it goes back to
|
||||||
// the name that is still stored.
|
// the name that is still stored.
|
||||||
err := h.renameTargetArchive(target, webhook.Name, oldName, name)
|
err = h.renameTargetArchive(target, webhook.Name, oldName, name)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
err = h.db.DB().Save(target).Error
|
err = h.db.DB().Save(target).Error
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -387,13 +387,16 @@ func chooseTargetType(ctx context.Context, t *testing.T, targetType string) {
|
|||||||
|
|
||||||
// checkRefusedTarget submits an http target the server refuses, a
|
// checkRefusedTarget submits an http target the server refuses, a
|
||||||
// loopback destination, and checks that the page comes back with the
|
// loopback destination, and checks that the page comes back with the
|
||||||
// form open on the http fields, the values entered and the reason.
|
// form open on the http fields, the values entered and the reason, and
|
||||||
|
// that after Cancel the next Add starts with an empty form and no
|
||||||
|
// reason.
|
||||||
func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
|
func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
const (
|
const (
|
||||||
refusedURL = "http://127.0.0.1/hook"
|
refusedURL = "http://127.0.0.1/hook"
|
||||||
urlField = `form[action$="/targets"] input[name="url"]`
|
urlField = `form[action$="/targets"] input[name="url"]`
|
||||||
|
reason = `//div[@class="alert-error"]`
|
||||||
)
|
)
|
||||||
|
|
||||||
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
|
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
|
||||||
@@ -407,7 +410,7 @@ func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
|
|||||||
|
|
||||||
click(ctx, t, saveButton)
|
click(ctx, t, saveButton)
|
||||||
|
|
||||||
assert.True(t, shown(ctx, `//div[@class="alert-error"]`),
|
assert.True(t, shown(ctx, reason),
|
||||||
"a refused target does not show the reason")
|
"a refused target does not show the reason")
|
||||||
|
|
||||||
var name, typed string
|
var name, typed string
|
||||||
@@ -426,6 +429,21 @@ func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
|
|||||||
"a refused target does not come back with the form open")
|
"a refused target does not come back with the form open")
|
||||||
assert.True(t, hidden(ctx, typeSelect),
|
assert.True(t, hidden(ctx, typeSelect),
|
||||||
"a refused target comes back on the type choice")
|
"a refused target comes back on the type choice")
|
||||||
|
|
||||||
|
click(ctx, t, cancelFields)
|
||||||
|
chooseTargetType(ctx, t, "http")
|
||||||
|
|
||||||
|
assert.True(t, hidden(ctx, reason),
|
||||||
|
"after Cancel, the next Add still shows the reason")
|
||||||
|
|
||||||
|
require.NoError(t, chromedp.Run(
|
||||||
|
ctx,
|
||||||
|
chromedp.Value(targetName, &name, chromedp.ByQuery),
|
||||||
|
chromedp.Value(urlField, &typed, chromedp.ByQuery),
|
||||||
|
))
|
||||||
|
|
||||||
|
assert.Empty(t, name, "after Cancel, the next Add keeps the name entered")
|
||||||
|
assert.Empty(t, typed, "after Cancel, the next Add keeps the url entered")
|
||||||
}
|
}
|
||||||
|
|
||||||
// checkCopy loads a webhook page and checks that the Copy control beside
|
// checkCopy loads a webhook page and checks that the Copy control beside
|
||||||
|
|||||||
+33
-3
@@ -91,14 +91,36 @@ document.addEventListener("alpine:init", function () {
|
|||||||
// choosing a type, then filling in that type's fields. targetType
|
// choosing a type, then filling in that type's fields. targetType
|
||||||
// is empty until Next takes it from the type select.
|
// is empty until Next takes it from the type select.
|
||||||
//
|
//
|
||||||
// A refused submission comes back with the type it was submitted
|
// The reason and the fields' values come from the properties below
|
||||||
// with in the section's data-type, and starts on that type's fields.
|
// rather than from the markup, because each type's fields are made
|
||||||
|
// afresh from the markup whenever that type is chosen. A refused
|
||||||
|
// submission comes back with its type, reason and values in the
|
||||||
|
// section's data attributes, and starts on that type's fields with
|
||||||
|
// them. Cancel empties these properties and resets the form, which
|
||||||
|
// holds whatever was typed, so the next Add starts with an empty
|
||||||
|
// form and no reason.
|
||||||
window.Alpine.data("targetForm", function () {
|
window.Alpine.data("targetForm", function () {
|
||||||
return {
|
return {
|
||||||
choosing: false,
|
choosing: false,
|
||||||
targetType: "",
|
targetType: "",
|
||||||
|
reason: "",
|
||||||
|
name: "",
|
||||||
|
url: "",
|
||||||
|
headers: "",
|
||||||
|
timeout: "",
|
||||||
|
maxRetries: "",
|
||||||
|
expiry: "",
|
||||||
init() {
|
init() {
|
||||||
this.targetType = this.$root.dataset.type;
|
const refused = this.$root.dataset;
|
||||||
|
|
||||||
|
this.targetType = refused.type;
|
||||||
|
this.reason = refused.reason;
|
||||||
|
this.name = refused.name;
|
||||||
|
this.url = refused.destination;
|
||||||
|
this.headers = refused.headers;
|
||||||
|
this.timeout = refused.timeout;
|
||||||
|
this.maxRetries = refused.maxRetries;
|
||||||
|
this.expiry = refused.expiry;
|
||||||
},
|
},
|
||||||
add() {
|
add() {
|
||||||
this.choosing = true;
|
this.choosing = true;
|
||||||
@@ -110,6 +132,14 @@ document.addEventListener("alpine:init", function () {
|
|||||||
cancel() {
|
cancel() {
|
||||||
this.choosing = false;
|
this.choosing = false;
|
||||||
this.targetType = "";
|
this.targetType = "";
|
||||||
|
this.reason = "";
|
||||||
|
this.name = "";
|
||||||
|
this.url = "";
|
||||||
|
this.headers = "";
|
||||||
|
this.timeout = "";
|
||||||
|
this.maxRetries = "";
|
||||||
|
this.expiry = "";
|
||||||
|
this.$refs.form.reset();
|
||||||
},
|
},
|
||||||
get filling() {
|
get filling() {
|
||||||
return this.targetType !== "";
|
return this.targetType !== "";
|
||||||
|
|||||||
@@ -90,8 +90,20 @@
|
|||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<!-- Targets -->
|
<!-- Targets. The data attributes carry a refused add target
|
||||||
<div class="card" x-data="targetForm" data-type="{{.TargetForm.Type}}">
|
submission's type, reason and values back to the form. The
|
||||||
|
URL is data-destination, not data-url: html/template treats
|
||||||
|
an attribute named like a URL as a link and would rewrite
|
||||||
|
a refused ftp: or javascript: value. -->
|
||||||
|
<div class="card" x-data="targetForm"
|
||||||
|
data-type="{{.TargetForm.Type}}"
|
||||||
|
data-reason="{{.TargetError}}"
|
||||||
|
data-name="{{.TargetForm.Name}}"
|
||||||
|
data-destination="{{.TargetForm.URL}}"
|
||||||
|
data-headers="{{.TargetForm.Headers}}"
|
||||||
|
data-timeout="{{.TargetForm.Timeout}}"
|
||||||
|
data-max-retries="{{.TargetForm.MaxRetries}}"
|
||||||
|
data-expiry="{{.TargetForm.Expiry}}">
|
||||||
<div class="p-4 border-b border-gray-200 flex justify-between items-center">
|
<div class="p-4 border-b border-gray-200 flex justify-between items-center">
|
||||||
<h2 class="text-lg font-medium text-gray-900">Targets</h2>
|
<h2 class="text-lg font-medium text-gray-900">Targets</h2>
|
||||||
<button type="button" @click="add" x-show="closed" class="btn-small">
|
<button type="button" @click="add" x-show="closed" class="btn-small">
|
||||||
@@ -106,8 +118,9 @@
|
|||||||
it with the chosen type's fields. Each type's fields,
|
it with the chosen type's fields. Each type's fields,
|
||||||
and the hidden type field submitted with them, exist
|
and the hidden type field submitted with them, exist
|
||||||
only while that type is chosen. A refused submission
|
only while that type is chosen. A refused submission
|
||||||
comes back open on its type, with the values entered. -->
|
comes back open on its type, with the values entered;
|
||||||
<form method="POST" action="/hook/{{.Webhook.ID}}/targets">
|
Cancel empties the form. -->
|
||||||
|
<form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-ref="form">
|
||||||
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
|
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
|
||||||
<div x-show="choosing" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 flex flex-wrap gap-2">
|
<div x-show="choosing" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 flex flex-wrap gap-2">
|
||||||
<select x-ref="type" aria-label="Target type" class="input text-sm w-40">
|
<select x-ref="type" aria-label="Target type" class="input text-sm w-40">
|
||||||
@@ -120,26 +133,24 @@
|
|||||||
<button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
|
<button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
|
||||||
</div>
|
</div>
|
||||||
<div x-show="filling" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 space-y-3">
|
<div x-show="filling" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 space-y-3">
|
||||||
{{if .TargetError}}
|
<div x-show="reason" x-text="reason" class="alert-error"></div>
|
||||||
<div class="alert-error">{{.TargetError}}</div>
|
<input type="text" name="name" :value="name" placeholder="Target name" required class="input text-sm">
|
||||||
{{end}}
|
|
||||||
<input type="text" name="name" value="{{.TargetForm.Name}}" placeholder="Target name" required class="input text-sm">
|
|
||||||
<template x-if="isHttp">
|
<template x-if="isHttp">
|
||||||
<div class="space-y-3">
|
<div class="space-y-3">
|
||||||
<input type="hidden" name="type" value="http">
|
<input type="hidden" name="type" value="http">
|
||||||
<input type="url" name="url" value="{{.TargetForm.URL}}" placeholder="https://example.com/webhook" class="input text-sm">
|
<input type="url" name="url" :value="url" placeholder="https://example.com/webhook" class="input text-sm">
|
||||||
<div>
|
<div>
|
||||||
<textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." class="input text-sm">{{.TargetForm.Headers}}</textarea>
|
<textarea name="headers" rows="3" :value="headers" placeholder="Authorization: Bearer ..." class="input text-sm"></textarea>
|
||||||
<p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
|
<p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
|
||||||
</div>
|
</div>
|
||||||
<div class="flex gap-2 items-center">
|
<div class="flex gap-2 items-center">
|
||||||
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
|
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
|
||||||
<input type="number" name="timeout" value="{{.TargetForm.Timeout}}" min="0" max="300" class="input text-sm w-24">
|
<input type="number" name="timeout" :value="timeout" min="0" max="300" class="input text-sm w-24">
|
||||||
</div>
|
</div>
|
||||||
<div>
|
<div>
|
||||||
<div class="flex gap-2 items-center">
|
<div class="flex gap-2 items-center">
|
||||||
<label class="text-sm text-gray-700">Max retries:</label>
|
<label class="text-sm text-gray-700">Max retries:</label>
|
||||||
<input type="number" name="max_retries" value="{{.TargetForm.MaxRetries}}" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
||||||
</div>
|
</div>
|
||||||
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
|
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
|
||||||
</div>
|
</div>
|
||||||
@@ -149,13 +160,13 @@
|
|||||||
<div class="space-y-3">
|
<div class="space-y-3">
|
||||||
<input type="hidden" name="type" value="slack">
|
<input type="hidden" name="type" value="slack">
|
||||||
<div>
|
<div>
|
||||||
<input type="url" name="url" value="{{.TargetForm.URL}}" placeholder="https://hooks.slack.com/services/..." class="input text-sm">
|
<input type="url" name="url" :value="url" placeholder="https://hooks.slack.com/services/..." class="input text-sm">
|
||||||
<p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
|
<p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
|
||||||
</div>
|
</div>
|
||||||
<div>
|
<div>
|
||||||
<div class="flex gap-2 items-center">
|
<div class="flex gap-2 items-center">
|
||||||
<label class="text-sm text-gray-700">Max retries:</label>
|
<label class="text-sm text-gray-700">Max retries:</label>
|
||||||
<input type="number" name="max_retries" value="{{.TargetForm.MaxRetries}}" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
||||||
</div>
|
</div>
|
||||||
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
|
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
|
||||||
</div>
|
</div>
|
||||||
@@ -164,7 +175,7 @@
|
|||||||
<template x-if="isDatabase">
|
<template x-if="isDatabase">
|
||||||
<div>
|
<div>
|
||||||
<input type="hidden" name="type" value="database">
|
<input type="hidden" name="type" value="database">
|
||||||
<input type="text" name="expiry" value="{{.TargetForm.Expiry}}" placeholder="never" class="input text-sm">
|
<input type="text" name="expiry" :value="expiry" placeholder="never" class="input text-sm">
|
||||||
<p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
|
<p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
|
||||||
</div>
|
</div>
|
||||||
</template>
|
</template>
|
||||||
|
|||||||
Reference in New Issue
Block a user