Render admin page errors in the normal layout (closes #382)
check / check (push) Waiting to run
check / check (push) Waiting to run
Every 400, 403, 404 and 500 on an admin page now answers with an error page in the normal layout: one fixed line for the status and a link back to the webhook list, or to sign-in when nobody is signed in. The router's handler for unknown paths, the CSRF middleware's refusal and a panic in an admin page route group use the same page; each such group has its own recoverer and error reporting for that. The page always sends Cache-Control: no-store. Status codes are unchanged. The receiver, the healthcheck and /metrics keep their plain answers. If the error page fails to render, the answer is the same status in plain text; if it panics, the answer is a 500. Model: opus-5-5
This commit is contained in:
@@ -0,0 +1,222 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/getsentry/sentry-go"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
// The link back the error page offers: to the webhook list for a
|
||||
// signed-in user, to sign-in for anyone else.
|
||||
const (
|
||||
backToWebhooks = `<a href="/hooks" class="btn-secondary">` +
|
||||
`Back to webhooks</a>`
|
||||
backToSignIn = `<a href="/pages/login" class="btn-primary">` +
|
||||
`Sign in</a>`
|
||||
)
|
||||
|
||||
// assertErrorPage checks that w is the error page for status, in the
|
||||
// normal layout, offering link.
|
||||
func assertErrorPage(
|
||||
t *testing.T,
|
||||
w *httptest.ResponseRecorder,
|
||||
status int,
|
||||
link string,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
body := w.Body.String()
|
||||
|
||||
assert.Equal(t, status, w.Code)
|
||||
assert.Equal(
|
||||
t, "text/html; charset=utf-8", w.Header().Get("Content-Type"),
|
||||
)
|
||||
assert.Equal(t, "no-store", w.Header().Get("Cache-Control"))
|
||||
assert.Contains(t, body, `<nav class="app-bar"`)
|
||||
assert.Contains(
|
||||
t, body, strconv.Itoa(status)+" "+http.StatusText(status),
|
||||
)
|
||||
assert.Contains(t, body, link)
|
||||
}
|
||||
|
||||
func TestErrorPage_DeletedWebhook(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
userID, _ := env.seedUser(t, "owner", "somepassword")
|
||||
cookies := env.authCookies(t, userID, "owner")
|
||||
|
||||
wh := env.seedWebhook(t, userID)
|
||||
require.NoError(t, env.db.DB().Delete(wh).Error)
|
||||
|
||||
w := env.get("/hook/"+wh.ID, cookies)
|
||||
|
||||
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
|
||||
}
|
||||
|
||||
func TestErrorPage_DeletedTarget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
userID, _ := env.seedUser(t, "owner", "somepassword")
|
||||
cookies := env.authCookies(t, userID, "owner")
|
||||
|
||||
wh := env.seedWebhook(t, userID)
|
||||
tgt := env.seedTarget(t, wh.ID)
|
||||
require.NoError(t, env.db.DB().Delete(tgt).Error)
|
||||
|
||||
w := env.get(
|
||||
"/hook/"+wh.ID+"/targets/"+tgt.ID+"/edit", cookies,
|
||||
)
|
||||
|
||||
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
|
||||
}
|
||||
|
||||
func TestErrorPage_UnknownPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
userID, _ := env.seedUser(t, "owner", "somepassword")
|
||||
cookies := env.authCookies(t, userID, "owner")
|
||||
|
||||
assertErrorPage(
|
||||
t, env.get("/no-such-page", nil),
|
||||
http.StatusNotFound, backToSignIn,
|
||||
)
|
||||
|
||||
// Outside every route group there is no form token, so the
|
||||
// page leaves out the logout form rather than offer one that
|
||||
// would be refused.
|
||||
w := env.get("/no-such-page", cookies)
|
||||
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
|
||||
assert.NotContains(t, w.Body.String(), `action="/pages/logout"`)
|
||||
|
||||
// Inside a route group the page has a token, and logout works.
|
||||
wh := env.seedWebhook(t, userID)
|
||||
w = env.get("/hook/"+wh.ID+"/no-such-page", cookies)
|
||||
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
|
||||
assert.Contains(t, w.Body.String(), `action="/pages/logout"`)
|
||||
}
|
||||
|
||||
func TestErrorPage_BadCSRFToken(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("username", "someone")
|
||||
form.Set("password", "irrelevant")
|
||||
form.Set("csrf_token", "not-a-token")
|
||||
|
||||
assertErrorPage(
|
||||
t, env.post("/pages/login", form, nil),
|
||||
http.StatusForbidden, backToSignIn,
|
||||
)
|
||||
|
||||
userID, _ := env.seedUser(t, "owner", "somepassword")
|
||||
cookies := env.authCookies(t, userID, "owner")
|
||||
wh := env.seedWebhook(t, userID)
|
||||
|
||||
edit := url.Values{}
|
||||
edit.Set("name", "renamed")
|
||||
|
||||
assertErrorPage(
|
||||
t, env.post("/hook/"+wh.ID+"/edit", edit, cookies),
|
||||
http.StatusForbidden, backToWebhooks,
|
||||
)
|
||||
}
|
||||
|
||||
// TestErrorPage_PanicOnAdminPage sends a panicking handler in an
|
||||
// admin page route group through the real router, with error
|
||||
// tracking on: the client gets the 500 error page, and the tracker
|
||||
// still gets the panic, once. The same panic outside the admin page
|
||||
// route groups keeps the plain 500.
|
||||
func TestErrorPage_PanicOnAdminPage(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
|
||||
transport := &captureTransport{}
|
||||
|
||||
opts := server.SentryClientOptionsForTest(
|
||||
"https://public@sentry.invalid/1", "webhooker-test",
|
||||
)
|
||||
opts.Transport = transport
|
||||
|
||||
client, err := sentry.NewClient(opts)
|
||||
require.NoError(t, err)
|
||||
|
||||
serve := func(router http.Handler, path string) *httptest.ResponseRecorder {
|
||||
req := httptest.NewRequestWithContext(
|
||||
sentry.SetHubOnContext(
|
||||
context.Background(),
|
||||
sentry.NewHub(client, sentry.NewScope()),
|
||||
),
|
||||
http.MethodGet, path, nil,
|
||||
)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
return w
|
||||
}
|
||||
|
||||
w := serve(
|
||||
server.NewRouterWithPageProbeForTest(
|
||||
env.log.Get(), env.cfg, env.mw, env.hnd,
|
||||
true, panicProbeHandler,
|
||||
),
|
||||
server.PageProbePattern,
|
||||
)
|
||||
assertErrorPage(t, w, http.StatusInternalServerError, backToSignIn)
|
||||
|
||||
w = serve(
|
||||
server.NewRouterWithProbeForTest(
|
||||
env.log.Get(), env.cfg, env.mw, env.hnd,
|
||||
true, panicProbeHandler,
|
||||
),
|
||||
server.ProbePattern,
|
||||
)
|
||||
assert.Equal(t, http.StatusInternalServerError, w.Code)
|
||||
assert.Equal(t, "Internal Server Error\n", w.Body.String())
|
||||
|
||||
require.Len(t, transport.events, 2)
|
||||
|
||||
for _, event := range transport.events {
|
||||
assert.Contains(t, marshalEvent(t, event), panicProbeMarker)
|
||||
}
|
||||
}
|
||||
|
||||
// TestErrorPage_ReceiverStaysPlain pins that the error page is for
|
||||
// the web UI only: a sender posting to an entrypoint that does not
|
||||
// exist still gets the plain-text answer.
|
||||
func TestErrorPage_ReceiverStaysPlain(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// newTestEnv leaves the receiver rate limit at zero, which
|
||||
// refuses every request before it reaches the receiver.
|
||||
env := newTestEnvWithConfig(t, &config.Config{
|
||||
DataDir: t.TempDir(),
|
||||
Environment: config.EnvironmentDev,
|
||||
ReceiverRateLimit: 10,
|
||||
})
|
||||
|
||||
w := env.post(
|
||||
"/h/0b8f3c1e-7d2a-4e6b-9f15-3a9c2d4e6f70", url.Values{}, nil,
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||
assert.Equal(t, "404 page not found\n", w.Body.String())
|
||||
}
|
||||
Reference in New Issue
Block a user