Test metrics auth, CORS preflight, login logging and metrics (closes #79)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
New tests only. The basic auth in front of /metrics answers 401 with a challenge without credentials or with a wrong username or password, and lets the configured ones through. A CORS preflight request gets the same Access-Control-Allow-Origin as a GET. A POST / whose form carries the signing key leaves the key out of the log line. The metrics middleware records a request it served, and the router records nothing while no metrics username is set. The pinned basicauth-go already compares the password in constant time with crypto/subtle, so no code changes. Model: opus-5-5
This commit is contained in:
@@ -29,6 +29,16 @@ P2: security: referer blacklist
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-04 the metrics basic auth, CORS preflight, request logging and
|
||||||
|
metrics recording have tests (closes #79): the basic auth in front of
|
||||||
|
`/metrics` answers 401 with a challenge without credentials or with a wrong
|
||||||
|
username or password and lets the configured ones through; a preflight
|
||||||
|
request gets `*` for any origin when `access_control_allow_origin` is `*` and
|
||||||
|
no `Access-Control-Allow-Origin` from another origin than the configured
|
||||||
|
one; a `POST /` carrying the signing key leaves no trace of it in the log
|
||||||
|
line; the metrics middleware records a request it served, and the router
|
||||||
|
records nothing while no metrics username is set. Tests only; the basic auth
|
||||||
|
library already compares the password in constant time.
|
||||||
- 2026-10-04 routes, encrypted URLs and config file documented (closes #75):
|
- 2026-10-04 routes, encrypted URLs and config file documented (closes #75):
|
||||||
"Routes" in `README.md` lists every route with its method, purpose, what it
|
"Routes" in `README.md` lists every route with its method, purpose, what it
|
||||||
needs and the status codes it answers with, and says `q` and `fit` are part
|
needs and the status codes it answers with, and says `q` and `fit` are part
|
||||||
|
|||||||
@@ -1,11 +1,16 @@
|
|||||||
package middleware
|
package middleware
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||||
|
|
||||||
"sneak.berlin/go/pixa/internal/config"
|
"sneak.berlin/go/pixa/internal/config"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -56,6 +61,202 @@ func TestCORSAnswersWithConfiguredOrigin(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestCORSAnswersPreflightWithConfiguredOrigin checks that the CORS
|
||||||
|
// middleware answers a preflight request, which the CORS library handles
|
||||||
|
// apart from other requests, the same way: "*" lets any origin read
|
||||||
|
// responses and a single origin lets only that origin read them.
|
||||||
|
func TestCORSAnswersPreflightWithConfiguredOrigin(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const appOrigin = "https://app.example.com"
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
configured string
|
||||||
|
requestOrigin string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{"*", "https://any.example.com", "*"},
|
||||||
|
{appOrigin, appOrigin, appOrigin},
|
||||||
|
{appOrigin, "https://other.example.com", ""},
|
||||||
|
}
|
||||||
|
|
||||||
|
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
})
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
mw := &Middleware{
|
||||||
|
log: slog.Default(),
|
||||||
|
config: &config.Config{AccessControlAllowOrigin: tc.configured},
|
||||||
|
}
|
||||||
|
|
||||||
|
handler := mw.CORS()(testHandler)
|
||||||
|
|
||||||
|
// An OPTIONS request naming the method it asks about is the
|
||||||
|
// preflight a browser sends before some cross-origin requests.
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodOptions, "/v1/image/example.com/a.jpg/1x1.png", nil)
|
||||||
|
req.Header.Set("Origin", tc.requestOrigin)
|
||||||
|
req.Header.Set("Access-Control-Request-Method", http.MethodGet)
|
||||||
|
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
|
||||||
|
handler.ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
got := rec.Header().Get("Access-Control-Allow-Origin")
|
||||||
|
if got != tc.want {
|
||||||
|
t.Errorf("configured %q, preflight from %q: "+
|
||||||
|
"Access-Control-Allow-Origin = %q, want %q",
|
||||||
|
tc.configured, tc.requestOrigin, got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMetricsAuthRequiresConfiguredCredentials checks that the basic auth
|
||||||
|
// in front of /metrics answers 401 with a challenge to a request without
|
||||||
|
// credentials or with a wrong username or password, and lets a request with
|
||||||
|
// the configured username and password through.
|
||||||
|
func TestMetricsAuthRequiresConfiguredCredentials(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const (
|
||||||
|
username = "metricsuser"
|
||||||
|
password = "metricspass"
|
||||||
|
challenge = `Basic realm="metrics"`
|
||||||
|
)
|
||||||
|
|
||||||
|
// An empty username stands for a request sent without credentials.
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
username string
|
||||||
|
password string
|
||||||
|
wantReached bool
|
||||||
|
}{
|
||||||
|
{"no credentials", "", "", false},
|
||||||
|
{"wrong username", "someone", password, false},
|
||||||
|
{"wrong password", username, "wrongpass", false},
|
||||||
|
{"configured credentials", username, password, true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
mw := &Middleware{
|
||||||
|
log: slog.Default(),
|
||||||
|
config: &config.Config{
|
||||||
|
MetricsUsername: username,
|
||||||
|
MetricsPassword: password,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
reached := false
|
||||||
|
handler := mw.MetricsAuth()(http.HandlerFunc(
|
||||||
|
func(http.ResponseWriter, *http.Request) {
|
||||||
|
reached = true
|
||||||
|
}))
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodGet, "/metrics", nil)
|
||||||
|
|
||||||
|
if tc.username != "" {
|
||||||
|
req.SetBasicAuth(tc.username, tc.password)
|
||||||
|
}
|
||||||
|
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
|
||||||
|
handler.ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
if reached != tc.wantReached {
|
||||||
|
t.Fatalf("request reached /metrics = %v, want %v",
|
||||||
|
reached, tc.wantReached)
|
||||||
|
}
|
||||||
|
|
||||||
|
if tc.wantReached {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if rec.Code != http.StatusUnauthorized {
|
||||||
|
t.Errorf("status = %d, want %d",
|
||||||
|
rec.Code, http.StatusUnauthorized)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := rec.Header().Get("WWW-Authenticate"); got != challenge {
|
||||||
|
t.Errorf("WWW-Authenticate = %q, want %q", got, challenge)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMetricsRecordsServedRequest checks that the metrics middleware
|
||||||
|
// records a request it served, so /metrics reports it. It is the only test
|
||||||
|
// in this package that sets up the metrics middleware, which registers with
|
||||||
|
// the process-wide Prometheus registry and can do so only once.
|
||||||
|
func TestMetricsRecordsServedRequest(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// The line /metrics shows once one GET /test has been served.
|
||||||
|
const want = `http_request_duration_seconds_count{` +
|
||||||
|
`code="200",handler="/test",method="GET",service=""} 1`
|
||||||
|
|
||||||
|
mw := &Middleware{log: slog.Default(), config: &config.Config{}}
|
||||||
|
|
||||||
|
handler := mw.Metrics()(http.HandlerFunc(
|
||||||
|
func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
}))
|
||||||
|
|
||||||
|
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodGet, "/test", nil))
|
||||||
|
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
promhttp.Handler().ServeHTTP(rec, httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodGet, "/metrics", nil))
|
||||||
|
|
||||||
|
if !strings.Contains(rec.Body.String(), want) {
|
||||||
|
t.Errorf("/metrics does not report the GET /test served; "+
|
||||||
|
"want the line %q in:\n%s", want, rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
|
||||||
|
// whose form carries the signing key, leaves no trace of the key in the
|
||||||
|
// request's log line.
|
||||||
|
func TestLoggingLeavesOutSubmittedSigningKey(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const signingKey = "test-signing-key-0123456789abcdef"
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
mw := newTestMiddleware(t, &buf)
|
||||||
|
|
||||||
|
// The handler reads the key from the form, as the login handler does.
|
||||||
|
handler := mw.Logging()(http.HandlerFunc(
|
||||||
|
func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if got := r.FormValue("key"); got != signingKey {
|
||||||
|
t.Errorf("key in form = %q, want %q", got, signingKey)
|
||||||
|
}
|
||||||
|
|
||||||
|
w.WriteHeader(http.StatusSeeOther)
|
||||||
|
}))
|
||||||
|
|
||||||
|
form := url.Values{"key": {signingKey}}
|
||||||
|
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/",
|
||||||
|
strings.NewReader(form.Encode()))
|
||||||
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||||
|
|
||||||
|
handler.ServeHTTP(httptest.NewRecorder(), req)
|
||||||
|
|
||||||
|
if !strings.Contains(buf.String(), `"method":"POST"`) {
|
||||||
|
t.Fatalf("no log line for the request; got %q", buf.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.Contains(buf.String(), signingKey) {
|
||||||
|
t.Errorf("log output contains the signing key; got %q", buf.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSecurityHeaders(t *testing.T) {
|
func TestSecurityHeaders(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,32 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestNoMetricsRecordedWithoutMetricsUsername checks that with no metrics
|
||||||
|
// username set the router records nothing about the requests it serves.
|
||||||
|
// /metrics is not served then, so the process-wide Prometheus registry is
|
||||||
|
// read directly. TestMaintenanceModeKeepsOtherRoutes records into the same
|
||||||
|
// registry, but never a GET /robots.txt.
|
||||||
|
func TestNoMetricsRecordedWithoutMetricsUsername(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
s := newTestServer(t)
|
||||||
|
s.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodGet, "/robots.txt", nil))
|
||||||
|
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
promhttp.Handler().ServeHTTP(rec, httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodGet, "/metrics", nil))
|
||||||
|
|
||||||
|
if strings.Contains(rec.Body.String(), `handler="/robots.txt"`) {
|
||||||
|
t.Errorf("with no metrics username GET /robots.txt was recorded:\n%s",
|
||||||
|
rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user