Fix four deployability blockers found by QA (closes #189, closes #190, closes #191, closes #192)
Some checks failed
Check / check (pull_request) Failing after 13s
Some checks failed
Check / check (pull_request) Failing after 13s
- CSRF over plain HTTP (#189): gorilla/csrf assumed https for its same-origin check, so setup and every POST returned 403 over plain HTTP. Gate csrf.PlaintextHTTPRequest on a new UPAAS_PLAINTEXT_HTTP config value; the default keeps https, correct for a TLS-terminating reverse proxy. The README plain-HTTP recipe now sets it. - git image never pulled (#190): ensureImage pulls alpine/git (pinned digest unchanged) when absent, before the clone container is created. - port-mapping 500 (#192): the ports delete form used {{ .CSRFField }} inside {{range .Ports}}, where the dot is a *models.Port; use {{ $.CSRFField }} like the labels and volumes blocks. - env-var 403 (#191): the editor read the CSRF token from $el (the submitting form, which has none) instead of $root, sending an empty token; read from $root. Model: opus-4-8
This commit is contained in:
67
internal/middleware/csrf_test.go
Normal file
67
internal/middleware/csrf_test.go
Normal file
@@ -0,0 +1,67 @@
|
||||
package middleware //nolint:testpackage // tests internal CSRF behavior
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"sneak.berlin/go/upaas/internal/config"
|
||||
)
|
||||
|
||||
//nolint:gosec // test credentials
|
||||
func newCSRFTestMiddleware(plaintextHTTP bool) *Middleware {
|
||||
return &Middleware{
|
||||
log: slog.Default(),
|
||||
params: &Params{
|
||||
Config: &config.Config{
|
||||
SessionSecret: "test-secret-32-bytes-long-enough",
|
||||
PlaintextHTTP: plaintextHTTP,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// postWithPlainHTTPOrigin drives a tokenless POST carrying a plain-HTTP Origin
|
||||
// through the CSRF middleware and returns the "Forbidden - <reason>" body.
|
||||
// gorilla/csrf checks the Origin before the token, so the reason reveals which
|
||||
// check rejected the request.
|
||||
func postWithPlainHTTPOrigin(t *testing.T, plaintextHTTP bool) string {
|
||||
t.Helper()
|
||||
|
||||
m := newCSRFTestMiddleware(plaintextHTTP)
|
||||
handler := m.CSRF()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodPost, "http://example.com/setup", nil)
|
||||
req.Header.Set("Origin", "http://example.com")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
assert.Equal(t, http.StatusForbidden, rec.Code)
|
||||
|
||||
return rec.Body.String()
|
||||
}
|
||||
|
||||
// Without PlaintextHTTP the origin check assumes https and rejects a browser's
|
||||
// http:// Origin, which is what broke setup over plain HTTP.
|
||||
func TestCSRF_PlaintextDisabled_RejectsPlainHTTPOrigin(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Contains(t, postWithPlainHTTPOrigin(t, false), "origin invalid")
|
||||
}
|
||||
|
||||
// With PlaintextHTTP the origin check uses http, so a matching http:// Origin
|
||||
// passes it and the request only fails later for the missing token.
|
||||
func TestCSRF_PlaintextEnabled_AllowsPlainHTTPOrigin(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
body := postWithPlainHTTPOrigin(t, true)
|
||||
assert.NotContains(t, body, "origin invalid")
|
||||
assert.Contains(t, body, "CSRF token not found")
|
||||
}
|
||||
@@ -255,12 +255,34 @@ func (m *Middleware) SessionAuth() func(http.Handler) http.Handler {
|
||||
}
|
||||
|
||||
// CSRF returns CSRF protection middleware using gorilla/csrf.
|
||||
//
|
||||
// gorilla/csrf assumes the request scheme is https for its same-origin check
|
||||
// unless the request is marked plaintext. A TLS-terminating reverse proxy
|
||||
// (the default deployment) presents https to the browser, so the default is
|
||||
// correct there. When µPaaS is reached over plain HTTP — directly, or behind a
|
||||
// proxy that does not terminate TLS — set UPAAS_PLAINTEXT_HTTP so the origin
|
||||
// check compares against http:// and setup over plain HTTP works.
|
||||
func (m *Middleware) CSRF() func(http.Handler) http.Handler {
|
||||
return csrf.Protect(
|
||||
protect := csrf.Protect(
|
||||
[]byte(m.params.Config.SessionSecret),
|
||||
csrf.Secure(false), // Allow HTTP for development; reverse proxy handles TLS
|
||||
csrf.Secure(false), // cookie Secure flag; TLS is terminated upstream
|
||||
csrf.Path("/"),
|
||||
)
|
||||
|
||||
if !m.params.Config.PlaintextHTTP {
|
||||
return protect
|
||||
}
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
protected := protect(next)
|
||||
|
||||
return http.HandlerFunc(func(
|
||||
writer http.ResponseWriter,
|
||||
request *http.Request,
|
||||
) {
|
||||
protected.ServeHTTP(writer, csrf.PlaintextHTTPRequest(request))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// loginRateLimit configures the login rate limiter.
|
||||
|
||||
Reference in New Issue
Block a user