diff --git a/bearer/status.go b/bearer/status.go new file mode 100644 index 0000000000000000000000000000000000000000..f92a6782dd3e369cefa73a311d0ccf06a0fb821b --- /dev/null +++ b/bearer/status.go @@ -0,0 +1,64 @@ +package bearer + +import ( + "errors" + "net/http" + "strconv" +) + +// StatusFor is the instance's answer to "a token was refused — what does the +// caller see?", as one table rather than one per service. +// +// The mapping itself is not subtle. What makes it worth sharing is the arm +// that is easy to get wrong and impossible to notice: ErrUnavailable is 503, +// never 401. Reading an unreachable token daemon as "revoked" tells every CI +// job on the instance that its credential is bad for as long as tokens.sr.ht +// takes to restart, and somebody spends the evening re-minting tokens that +// were never broken. bench alone had this switch written out three times — in +// its REST surface, its MCP surface and its resolver — which is three chances +// to fold the unreachable case into the invalid one. +// +// The three refusals that ARE the caller's fault share one status and one +// sentence on purpose: telling a prober "that token exists but is revoked" is +// information they have not earned. Which of them it was belongs in the log. +// +// ErrNotOurs is the one arm a service must decide before asking: it means a +// well-formed token from another issuer, almost certainly a meta.sr.ht PAT, +// and SPEC ch. 6 step 2 leaves each service to accept it (dolt) or refuse it +// (bench, cover). Handle it first; reaching here it is a refusal, so it maps +// to 401. +// +// A nil error maps to 200 so a caller can write the status unconditionally. +func StatusFor(err error) int { + switch { + case err == nil: + return http.StatusOK + case errors.Is(err, ErrForbidden): + return http.StatusForbidden + case errors.Is(err, ErrUnavailable): + return http.StatusServiceUnavailable + default: + // ErrInvalid, ErrRevoked, ErrNotOurs, and anything a future validator + // step adds: an unrecognised failure is the caller's credential, not + // the instance's health. A new sentinel that deserves 503 has to say + // so here, which is the point of the default going this way — a + // forgotten arm refuses a request rather than declaring the service + // unwell. + return http.StatusUnauthorized + } +} + +// Challenge is the WWW-Authenticate value a 401 carries: the scheme, and the +// service's own config section as the realm. +// +// RFC 9110 requires the header on a 401, and every service on the instance was +// assembling the same string from the same constant. The realm is the section +// name ("bench.sr.ht") because that is what identifies the service everywhere +// else on this instance — in the config, in the nav, in a grant. +// +// The realm is quoted per RFC 9110 §11.6.1; a quote or backslash in it would +// end the parameter early, so the value is escaped rather than trusted. In +// practice a section name contains neither. +func Challenge(realm string) string { + return "Bearer realm=" + strconv.Quote(realm) +} diff --git a/bearer/status_test.go b/bearer/status_test.go new file mode 100644 index 0000000000000000000000000000000000000000..f5adaaad42187df0475782e9b3762a539f4766a5 --- /dev/null +++ b/bearer/status_test.go @@ -0,0 +1,39 @@ +package bearer + +import ( + "errors" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStatusForKeepsTheUnreachableDaemonOutOfThe401(t *testing.T) { + // The arm the whole table exists for: a token service that cannot be + // reached must not read as a bad credential. + assert.Equal(t, http.StatusServiceUnavailable, StatusFor(ErrUnavailable)) + assert.Equal(t, http.StatusServiceUnavailable, + StatusFor(fmt.Errorf("ask tokens.sr.ht: %w", ErrUnavailable)), + "a wrapped one too — services wrap before returning") + + assert.Equal(t, http.StatusForbidden, StatusFor(ErrForbidden)) + assert.Equal(t, http.StatusForbidden, + StatusFor(fmt.Errorf("check the grant: %w", ErrForbidden))) + + // The three the caller is responsible for answer alike, on purpose. + for _, err := range []error{ErrInvalid, ErrRevoked, ErrNotOurs} { + assert.Equal(t, http.StatusUnauthorized, StatusFor(err), "%v", err) + } + + assert.Equal(t, http.StatusOK, StatusFor(nil)) + assert.Equal(t, http.StatusUnauthorized, StatusFor(errors.New("something new")), + "an unrecognised failure refuses the request rather than declaring the service unwell") +} + +func TestChallengeNamesTheServiceAndQuotesIt(t *testing.T) { + assert.Equal(t, `Bearer realm="bench.sr.ht"`, Challenge("bench.sr.ht")) + assert.Equal(t, `Bearer realm="dolt.sr.ht"`, Challenge("dolt.sr.ht")) + // A quote in the realm would end the parameter early if it were pasted in. + assert.Equal(t, `Bearer realm="a\"b"`, Challenge(`a"b`)) +} diff --git a/pages/form.go b/pages/form.go new file mode 100644 index 0000000000000000000000000000000000000000..e8ab15688fb729fd2e3b91a74eaff7f72d017af0 --- /dev/null +++ b/pages/form.go @@ -0,0 +1,44 @@ +package pages + +import ( + "errors" + "fmt" + "net/http" + "net/url" +) + +// DefaultMaxFormBytes bounds a urlencoded body. net/http's own ceiling for one +// is 10 MiB per request, which is three orders of magnitude more than any form +// on this instance sends and enough to be worth refusing on a page anyone can +// reach without logging in. +const DefaultMaxFormBytes = 1 << 16 + +// ErrInvalidForm is returned when the body could not be read as a form: it was +// malformed, or it exceeded the limit. A service maps this to 400. +var ErrInvalidForm = errors.New("pages: the form could not be read") + +// FormValues reads a urlencoded body, bounded at max (DefaultMaxFormBytes when +// max <= 0), and returns the body's values. +// +// It returns r.PostForm and never r.Form, and that is the whole reason this +// three-line function is shared rather than copied. r.Form merges the query +// string into the body's values, so a mutation could be driven entirely from a +// URL somebody was linked to — and that request is precisely the one the +// same-origin guard sees nothing wrong with, because it really did come from +// our own page. A form posts its fields in its body; anything in the query +// string of such a POST is not that form. +// +// The difference between the safe version and the hole is one character in a +// field name, in a function every service with a form writes for itself, and +// nothing at review time makes its absence visible. That is what makes it +// belong beside csrf rather than in each service. +func FormValues(w http.ResponseWriter, r *http.Request, max int64) (url.Values, error) { + if max <= 0 { + max = DefaultMaxFormBytes + } + r.Body = http.MaxBytesReader(w, r.Body, max) + if err := r.ParseForm(); err != nil { + return nil, fmt.Errorf("%w (%v)", ErrInvalidForm, err) + } + return r.PostForm, nil +} diff --git a/pages/form_test.go b/pages/form_test.go new file mode 100644 index 0000000000000000000000000000000000000000..1ea77d73ae8d938104ec549bd5176ab4e00461f4 --- /dev/null +++ b/pages/form_test.go @@ -0,0 +1,56 @@ +package pages + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func postForm(body string) *http.Request { + r := httptest.NewRequest(http.MethodPost, "/tokens?grants=%2A&revoke=all", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return r +} + +// TestFormValuesIgnoresTheQueryString is the reason this function is shared. +// A mutation must not be drivable from a URL a viewer was linked to, which the +// same-origin guard would happily let through. +func TestFormValuesIgnoresTheQueryString(t *testing.T) { + r := postForm("grants=bench%3Aupload") + + got, err := FormValues(httptest.NewRecorder(), r, 0) + require.NoError(t, err) + + assert.Equal(t, "bench:upload", got.Get("grants"), "the body's value, not the URL's") + assert.Empty(t, got.Get("revoke"), "a field present only in the query is not this form's") + // r.Form, the wrong one, would have carried it — that merge is the hole. + assert.Equal(t, "all", r.Form.Get("revoke"), "the merge is what we are avoiding") + assert.Equal(t, []string{"bench:upload", "*"}, r.Form["grants"], + "and it would have appended the URL's value to the body's") +} + +func TestFormValuesRefusesAnOversizedBody(t *testing.T) { + r := postForm("grants=" + strings.Repeat("x", 4096)) + + _, err := FormValues(httptest.NewRecorder(), r, 128) + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidForm) + + // The same body inside the limit reads fine, so the refusal is the limit + // and not the shape. + got, err := FormValues(httptest.NewRecorder(), postForm("grants=ok"), 128) + require.NoError(t, err) + assert.Equal(t, "ok", got.Get("grants")) +} + +func TestFormValuesDefaultsTheLimit(t *testing.T) { + r := postForm("grants=" + strings.Repeat("x", DefaultMaxFormBytes+1)) + + _, err := FormValues(httptest.NewRecorder(), r, 0) + require.Error(t, err, "max <= 0 means the default, not unbounded") + assert.ErrorIs(t, err, ErrInvalidForm) +}