Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e73ebe2f2f |
+32
-2
@@ -13,6 +13,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
"slices"
|
"slices"
|
||||||
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/coreos/go-oidc/v3/oidc"
|
"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}
|
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 {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("auth: OIDC discovery: %w", err)
|
return nil, discoveryError(err, int(recorder.status.Load()))
|
||||||
}
|
}
|
||||||
endpoint := provider.Endpoint()
|
endpoint := provider.Endpoint()
|
||||||
var discovered struct {
|
var discovered struct {
|
||||||
@@ -159,6 +163,32 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
|
|||||||
}, nil
|
}, 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
|
// checkEndpoint applies secureurl.Check to the provider URL raw, naming the
|
||||||
// endpoint in the error.
|
// endpoint in the error.
|
||||||
func checkEndpoint(name, raw string) error {
|
func checkEndpoint(name, raw string) error {
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import (
|
|||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"maps"
|
"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) {
|
func TestNewFailsOnIssuerMismatch(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
p := newFakeProvider(t)
|
p := newFakeProvider(t)
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ import (
|
|||||||
// GET /api/v1/me.
|
// GET /api/v1/me.
|
||||||
const CSRFHeader = "X-MealQuest-CSRF"
|
const CSRFHeader = "X-MealQuest-CSRF"
|
||||||
|
|
||||||
// requireCSRF passes GET, HEAD and OPTIONS requests to next; every other
|
// requireCSRF passes GET requests to next; every other
|
||||||
// request must carry CSRFHeader equal to its session's CSRF token or get 403
|
// request must carry CSRFHeader equal to its session's CSRF token or get 403
|
||||||
// csrf. It runs behind requireSession, which puts the session in the context.
|
// csrf. It runs behind requireSession, which puts the session in the context.
|
||||||
func requireCSRF(logger *slog.Logger) func(http.Handler) http.Handler {
|
func requireCSRF(logger *slog.Logger) func(http.Handler) http.Handler {
|
||||||
@@ -21,7 +21,7 @@ func requireCSRF(logger *slog.Logger) func(http.Handler) http.Handler {
|
|||||||
return func(next http.Handler) http.Handler {
|
return func(next http.Handler) http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
switch r.Method {
|
switch r.Method {
|
||||||
case http.MethodGet, http.MethodHead, http.MethodOptions:
|
case http.MethodGet:
|
||||||
next.ServeHTTP(w, r)
|
next.ServeHTTP(w, r)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -38,10 +38,9 @@ func TestCSRFThroughRouter(t *testing.T) {
|
|||||||
{"POST with right header", http.MethodPost, "/api/v1/me", []string{testSession.CSRFToken}, http.StatusMethodNotAllowed, methodNotAllowedJSON},
|
{"POST with right header", http.MethodPost, "/api/v1/me", []string{testSession.CSRFToken}, http.StatusMethodNotAllowed, methodNotAllowedJSON},
|
||||||
{"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusNotFound, notFoundJSON},
|
{"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusNotFound, notFoundJSON},
|
||||||
{"GET without header", http.MethodGet, "/api/v1/me", nil, http.StatusOK, ""},
|
{"GET without header", http.MethodGet, "/api/v1/me", nil, http.StatusOK, ""},
|
||||||
// HEAD and OPTIONS pass the check and reach the router, which
|
// Only GET is exempt; HEAD and OPTIONS need the header too.
|
||||||
// answers neither for /api/v1/me.
|
{"HEAD without header", http.MethodHead, "/api/v1/me", nil, http.StatusForbidden, csrfJSON},
|
||||||
{"HEAD without header", http.MethodHead, "/api/v1/me", nil, http.StatusMethodNotAllowed, ""},
|
{"OPTIONS without header", http.MethodOptions, "/api/v1/me", nil, http.StatusForbidden, csrfJSON},
|
||||||
{"OPTIONS without header", http.MethodOptions, "/api/v1/me", nil, http.StatusMethodNotAllowed, methodNotAllowedJSON},
|
|
||||||
} {
|
} {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
@@ -89,6 +88,8 @@ func TestRequireCSRF(t *testing.T) {
|
|||||||
{"right header reaches handler", http.MethodPost, &testSession, testSession.CSRFToken, http.StatusNoContent},
|
{"right header reaches handler", http.MethodPost, &testSession, testSession.CSRFToken, http.StatusNoContent},
|
||||||
{"wrong header", http.MethodPut, &testSession, "nope", http.StatusForbidden},
|
{"wrong header", http.MethodPut, &testSession, "nope", http.StatusForbidden},
|
||||||
{"GET needs no header", http.MethodGet, &testSession, "", http.StatusNoContent},
|
{"GET needs no header", http.MethodGet, &testSession, "", http.StatusNoContent},
|
||||||
|
{"HEAD needs the header", http.MethodHead, &testSession, "", http.StatusForbidden},
|
||||||
|
{"OPTIONS needs the header", http.MethodOptions, &testSession, "", http.StatusForbidden},
|
||||||
{"no session in context", http.MethodPost, nil, "", http.StatusInternalServerError},
|
{"no session in context", http.MethodPost, nil, "", http.StatusInternalServerError},
|
||||||
{"session without token", http.MethodPost, &auth.Session{Subject: "user-123"}, "", http.StatusInternalServerError},
|
{"session without token", http.MethodPost, &auth.Session{Subject: "user-123"}, "", http.StatusInternalServerError},
|
||||||
} {
|
} {
|
||||||
|
|||||||
@@ -27,8 +27,7 @@ type RouteMounter interface {
|
|||||||
// only from a peer in trusted (see resolveOrigin). version is reported by
|
// only from a peer in trusted (see resolveOrigin). version is reported by
|
||||||
// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and
|
// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and
|
||||||
// /media; assets is the web build served for every other GET. /health, /auth
|
// /media; assets is the web build served for every other GET. /health, /auth
|
||||||
// and the web build are public. Requests to /api/v1 other than GET, HEAD and
|
// and the web build are public. Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader.
|
||||||
// OPTIONS also need the session's CSRF token in CSRFHeader.
|
|
||||||
func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
|
func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
|
||||||
notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found")
|
notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found")
|
||||||
methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed")
|
methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed")
|
||||||
|
|||||||
Reference in New Issue
Block a user