Sanitize OIDC discovery errors and require CSRF on HEAD and OPTIONS
F03-01: go-oidc embeds the provider's response body in its error for a non-200 discovery response, which cmd/server logged at startup. Discovery now runs through a status-recording transport and a non-200 response is reported as "provider answered HTTP <status>" only; transport errors keep their cause. A test with a sentinel body proves it reaches neither the error nor the log. F03-02: the CSRF middleware exempts only GET, so HEAD and OPTIONS under /api/v1 without X-MealQuest-CSRF get 403 csrf, matching CONTEXT.MD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+32
-2
@@ -13,6 +13,7 @@ import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"slices"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/coreos/go-oidc/v3/oidc"
|
||||
@@ -107,9 +108,12 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
|
||||
}
|
||||
|
||||
client := &http.Client{Timeout: providerTimeout, CheckRedirect: checkRedirect}
|
||||
provider, err := oidc.NewProvider(oidc.ClientContext(ctx, client), cfg.Issuer)
|
||||
discovery := *client
|
||||
recorder := &statusRecorder{next: http.DefaultTransport}
|
||||
discovery.Transport = recorder
|
||||
provider, err := oidc.NewProvider(oidc.ClientContext(ctx, &discovery), cfg.Issuer)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("auth: OIDC discovery: %w", err)
|
||||
return nil, discoveryError(err, int(recorder.status.Load()))
|
||||
}
|
||||
endpoint := provider.Endpoint()
|
||||
var discovered struct {
|
||||
@@ -159,6 +163,32 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
|
||||
}, nil
|
||||
}
|
||||
|
||||
// statusRecorder records the status of the last response it carries.
|
||||
type statusRecorder struct {
|
||||
next http.RoundTripper
|
||||
status atomic.Int32
|
||||
}
|
||||
|
||||
func (s *statusRecorder) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
resp, err := s.next.RoundTrip(req)
|
||||
if err == nil {
|
||||
s.status.Store(int32(resp.StatusCode))
|
||||
}
|
||||
return resp, err
|
||||
}
|
||||
|
||||
// discoveryError reports a failed discovery. go-oidc puts the provider's
|
||||
// response body in its error for a non-200 discovery response, so that
|
||||
// error is replaced by the status alone. Transport errors carry no response
|
||||
// content and keep their cause.
|
||||
func discoveryError(err error, status int) error {
|
||||
var urlErr *url.Error
|
||||
if !errors.As(err, &urlErr) && status != 0 && status != http.StatusOK {
|
||||
return fmt.Errorf("auth: OIDC discovery: provider answered HTTP %d", status)
|
||||
}
|
||||
return fmt.Errorf("auth: OIDC discovery: %w", err)
|
||||
}
|
||||
|
||||
// checkEndpoint applies secureurl.Check to the provider URL raw, naming the
|
||||
// endpoint in the error.
|
||||
func checkEndpoint(name, raw string) error {
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"maps"
|
||||
@@ -789,6 +790,41 @@ func TestNewFailsWhenDiscoveryFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewDiscoveryErrorOmitsProviderBody(t *testing.T) {
|
||||
t.Parallel()
|
||||
const sentinel = "sentinel-discovery-body-7f3a"
|
||||
for _, status := range []int{http.StatusNotFound, http.StatusInternalServerError} {
|
||||
t.Run(http.StatusText(status), func(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(status)
|
||||
io.WriteString(w, sentinel)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
logs := &syncBuffer{}
|
||||
_, err := New(context.Background(), Config{
|
||||
Issuer: srv.URL,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: testClientSecret,
|
||||
PublicURL: testPublicURL,
|
||||
SecretKey: []byte("k"),
|
||||
AllowedGroups: testAllowedGroups,
|
||||
}, &memorySessions{}, slog.New(slog.NewTextHandler(logs, nil)))
|
||||
if err == nil {
|
||||
t.Fatal("New succeeded without a discovery document")
|
||||
}
|
||||
// cmd/server logs the returned error at startup.
|
||||
slog.New(slog.NewTextHandler(logs, nil)).Error("startup failed", "err", err)
|
||||
if strings.Contains(logs.String(), sentinel) {
|
||||
t.Errorf("logs contain the provider response body: %s", logs)
|
||||
}
|
||||
if want := fmt.Sprintf("auth: OIDC discovery: provider answered HTTP %d", status); err.Error() != want {
|
||||
t.Errorf("error = %q, want %q", err, want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewFailsOnIssuerMismatch(t *testing.T) {
|
||||
t.Parallel()
|
||||
p := newFakeProvider(t)
|
||||
|
||||
Reference in New Issue
Block a user