From 1dc57accc672983fea715d2ccdd3ecce204490b0 Mon Sep 17 00:00:00 2001 From: Oliver Baehler Date: Thu, 2 Jul 2026 14:13:05 +0200 Subject: [PATCH 1/2] fix: hanlding panic Signed-off-by: Oliver Baehler --- .../capsule-proxy/templates/flowschema.yaml | 4 ++ go.mod | 1 - go.sum | 2 - internal/webserver/errors/panic.go | 8 ++-- internal/webserver/middleware/jwt.go | 6 +++ .../webserver/middleware/user_in_group.go | 7 ++++ internal/webserver/webserver.go | 41 ++++++++++++++++--- 7 files changed, 57 insertions(+), 12 deletions(-) diff --git a/charts/capsule-proxy/templates/flowschema.yaml b/charts/capsule-proxy/templates/flowschema.yaml index 1f7cfc61..e63579e4 100644 --- a/charts/capsule-proxy/templates/flowschema.yaml +++ b/charts/capsule-proxy/templates/flowschema.yaml @@ -40,4 +40,8 @@ spec: apiGroups: ["authorization.k8s.io"] resources: ["subjectaccessreviews"] clusterScope: true + - verbs: ["create"] + apiGroups: ["authentication.k8s.io"] + resources: ["tokenreviews"] + clusterScope: true {{- end }} diff --git a/go.mod b/go.mod index 4be56b67..2e01cf06 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.26.4 require ( github.com/go-logr/logr v1.4.3 github.com/golang-jwt/jwt/v5 v5.3.1 - github.com/gorilla/handlers v1.5.2 github.com/gorilla/mux v1.8.1 github.com/onsi/ginkgo/v2 v2.29.0 github.com/onsi/gomega v1.41.0 diff --git a/go.sum b/go.sum index 5f4e243c..f3adfcc7 100644 --- a/go.sum +++ b/go.sum @@ -134,8 +134,6 @@ github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 h1:EwtI+Al+DeppwYX2oX github.com/google/pprof v0.0.0-20260402051712-545e8a4df936/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyEE= -github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= diff --git a/internal/webserver/errors/panic.go b/internal/webserver/errors/panic.go index 20f5c117..5048d0cf 100644 --- a/internal/webserver/errors/panic.go +++ b/internal/webserver/errors/panic.go @@ -27,12 +27,11 @@ func HandleUnauthorized(w http.ResponseWriter, err error, message string) { } w.Header().Set("content-type", "application/json") + w.WriteHeader(http.StatusForbidden) //nolint:errchkjson b, _ := json.Marshal(status) _, _ = w.Write(b) - - panic(message) } func HandleError(w http.ResponseWriter, err error, message string) { @@ -42,15 +41,16 @@ func HandleError(w http.ResponseWriter, err error, message string) { Kind: "Status", APIVersion: "v1", }, + Status: metav1.StatusFailure, Message: message, Reason: metav1.StatusReasonInternalError, + Code: http.StatusInternalServerError, } w.Header().Set("content-type", "application/json") + w.WriteHeader(http.StatusInternalServerError) //nolint:errchkjson b, _ := json.Marshal(status) _, _ = w.Write(b) - - panic(message) } diff --git a/internal/webserver/middleware/jwt.go b/internal/webserver/middleware/jwt.go index 77b9c50d..9ba46abf 100644 --- a/internal/webserver/middleware/jwt.go +++ b/internal/webserver/middleware/jwt.go @@ -39,15 +39,21 @@ func CheckJWTMiddleware(client client.Writer) mux.MiddlewareFunc { } if err = client.Create(request.Context(), &tr); err != nil { errors.HandleError(writer, err, "cannot create TokenReview") + + return } if statusErr := tr.Status.Error; len(statusErr) > 0 { invalidatedToken.Insert(token) errors.HandleUnauthorized(writer, goerrors.New(statusErr), "cannot authenticate the token due to error") + + return } case invalidatedToken.Has(token): errors.HandleUnauthorized(writer, goerrors.New("token is invalid"), "cannot authenticate the token due to error") + + return } next.ServeHTTP(writer, request) diff --git a/internal/webserver/middleware/user_in_group.go b/internal/webserver/middleware/user_in_group.go index fc346d29..08585aff 100644 --- a/internal/webserver/middleware/user_in_group.go +++ b/internal/webserver/middleware/user_in_group.go @@ -15,6 +15,7 @@ import ( "github.com/projectcapsule/capsule-proxy/internal/controllers" req "github.com/projectcapsule/capsule-proxy/internal/request" + "github.com/projectcapsule/capsule-proxy/internal/webserver/errors" ) func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredUserGroups sets.Set[string], ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool, xfcc_header string, fn func(writer http.ResponseWriter, request *http.Request)) mux.MiddlewareFunc { @@ -30,6 +31,9 @@ func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, cl request, user, groups, err = req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header) if err != nil { log.Error(err, "Cannot retrieve username and group from request") + errors.HandleError(writer, err, "cannot retrieve user and group from the request") + + return } if slices.ContainsFunc(groups, func(group string) bool { @@ -53,6 +57,9 @@ func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, cl request, user, groups, err := req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header) if err != nil { log.Error(err, "Cannot retrieve username and group from request") + errors.HandleError(writer, err, "cannot retrieve user and group from the request") + + return } log.V(10).Info("request groups", "groups", groups) diff --git a/internal/webserver/webserver.go b/internal/webserver/webserver.go index a4cf12fc..f94e5c54 100644 --- a/internal/webserver/webserver.go +++ b/internal/webserver/webserver.go @@ -22,7 +22,6 @@ import ( "github.com/go-logr/logr" "github.com/golang-jwt/jwt/v5" - "github.com/gorilla/handlers" "github.com/gorilla/mux" "github.com/pkg/errors" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" @@ -167,7 +166,7 @@ func (n *kubeFilter) NeedLeaderElection() bool { //nolint:funlen func (n *kubeFilter) Start(ctx context.Context) error { r := mux.NewRouter() - r.Use(handlers.RecoveryHandler()) + r.Use(n.recoveryMiddleware) r.Path("/_healthz").Subrouter().HandleFunc("", func(writer http.ResponseWriter, _ *http.Request) { writer.WriteHeader(http.StatusOK) @@ -350,12 +349,16 @@ func (n *kubeFilter) authorizationMiddleware(next http.Handler) http.Handler { request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header) if err != nil { server.HandleError(writer, err, "cannot retrieve user and group from the request") + + return } //nolint:contextcheck proxyTenants, err := n.getTenantsForOwner(request.Context(), username, groups) if err != nil { server.HandleError(writer, err, "cannot list Tenant resources") + + return } obj, gvk, err := n.universalDecoder.Decode(body, nil, nil) @@ -548,11 +551,15 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) { request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header) if err != nil { server.HandleError(writer, err, "cannot retrieve user and group from the request") + + return } proxyTenants, err := n.getTenantsForOwner(ctx, username, groups) if err != nil { server.HandleError(writer, err, "cannot list Tenant resources") + + return } var selector labels.Selector @@ -574,19 +581,23 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) { case err != nil: var t moderrors.Error if errors.As(err, &t) { + writer.Header().Set("Content-Type", "application/json") + if t.Status().Code > 0 { writer.WriteHeader(int(t.Status().Code)) + } else { + writer.WriteHeader(http.StatusInternalServerError) } - writer.Header().Set("Content-Type", "application/json") - b, _ := json.Marshal(t.Status()) _, _ = writer.Write(b) - panic(err.Error()) + return } server.HandleError(writer, err, err.Error()) + + return case selector == nil: // if there's no selector, let it pass to the n.impersonateHandler(writer, request) @@ -597,6 +608,26 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) { } } +func (n *kubeFilter) recoveryMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + defer func() { + recovered := recover() + if recovered == nil { + return + } + + if err, ok := recovered.(error); ok && errors.Is(err, http.ErrAbortHandler) { + return + } + + n.log.Error(fmt.Errorf("%v", recovered), "panic while handling request") + http.Error(writer, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + }() + + next.ServeHTTP(writer, request) + }) +} + func (n *kubeFilter) getTenantsForOwner(ctx context.Context, username string, groups []string) (proxyTenants []*tenant.ProxyTenant, err error) { if strings.HasPrefix(username, serviceaccount.ServiceAccountUsernamePrefix) { proxyTenants, err = n.getProxyTenantsForOwnerKind(ctx, capsulerbac.ServiceAccountOwner, username) From 3e6d5d37150d116826993d1d157b24fcd760cfa0 Mon Sep 17 00:00:00 2001 From: Oliver Baehler Date: Thu, 2 Jul 2026 14:46:20 +0200 Subject: [PATCH 2/2] fix: hanlding panic Signed-off-by: Oliver Baehler --- internal/webserver/middleware/jwt.go | 2 - .../webserver/middleware/user_in_group.go | 60 +++++++++++-------- internal/webserver/webserver.go | 32 ++++++---- 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/internal/webserver/middleware/jwt.go b/internal/webserver/middleware/jwt.go index 9ba46abf..1bc8113f 100644 --- a/internal/webserver/middleware/jwt.go +++ b/internal/webserver/middleware/jwt.go @@ -44,8 +44,6 @@ func CheckJWTMiddleware(client client.Writer) mux.MiddlewareFunc { } if statusErr := tr.Status.Error; len(statusErr) > 0 { - invalidatedToken.Insert(token) - errors.HandleUnauthorized(writer, goerrors.New(statusErr), "cannot authenticate the token due to error") return diff --git a/internal/webserver/middleware/user_in_group.go b/internal/webserver/middleware/user_in_group.go index 08585aff..e72bd776 100644 --- a/internal/webserver/middleware/user_in_group.go +++ b/internal/webserver/middleware/user_in_group.go @@ -4,6 +4,7 @@ package middleware import ( + "errors" "net/http" "regexp" "slices" @@ -15,35 +16,33 @@ import ( "github.com/projectcapsule/capsule-proxy/internal/controllers" req "github.com/projectcapsule/capsule-proxy/internal/request" - "github.com/projectcapsule/capsule-proxy/internal/webserver/errors" + weberrors "github.com/projectcapsule/capsule-proxy/internal/webserver/errors" ) func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredUserGroups sets.Set[string], ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool, xfcc_header string, fn func(writer http.ResponseWriter, request *http.Request)) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { - if ignoredUserGroups.Len() > 0 { - var ( - err error - user string - groups []string - ) - - request, user, groups, err = req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header) - if err != nil { - log.Error(err, "Cannot retrieve username and group from request") - errors.HandleError(writer, err, "cannot retrieve user and group from the request") - - return - } - - if slices.ContainsFunc(groups, func(group string) bool { - return ignoredUserGroups.Has(group) - }) { - log.V(5).Info("current user belongs to ignored groups", "user", user) - fn(writer, request) - - return - } + if ignoredUserGroups.Len() == 0 { + next.ServeHTTP(writer, request) + + return + } + + request, user, groups, err := req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header) + if err != nil { + log.Error(err, "Cannot retrieve username and group from request") + handleResolveUserAndGroupsError(writer, err) + + return + } + + if slices.ContainsFunc(groups, func(group string) bool { + return ignoredUserGroups.Has(group) + }) { + log.V(5).Info("current user belongs to ignored groups", "user", user) + fn(writer, request) + + return } next.ServeHTTP(writer, request) @@ -57,7 +56,7 @@ func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, cl request, user, groups, err := req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header) if err != nil { log.Error(err, "Cannot retrieve username and group from request") - errors.HandleError(writer, err, "cannot retrieve user and group from the request") + handleResolveUserAndGroupsError(writer, err) return } @@ -83,3 +82,14 @@ func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, cl }) } } + +func handleResolveUserAndGroupsError(writer http.ResponseWriter, err error) { + var unauthorizedErr *req.ErrUnauthorized + if errors.As(err, &unauthorizedErr) { + weberrors.HandleUnauthorized(writer, err, "cannot retrieve user and group from the request") + + return + } + + weberrors.HandleError(writer, err, "cannot retrieve user and group from the request") +} diff --git a/internal/webserver/webserver.go b/internal/webserver/webserver.go index f94e5c54..6e7ad2ea 100644 --- a/internal/webserver/webserver.go +++ b/internal/webserver/webserver.go @@ -7,6 +7,7 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" "net" @@ -23,7 +24,7 @@ import ( "github.com/go-logr/logr" "github.com/golang-jwt/jwt/v5" "github.com/gorilla/mux" - "github.com/pkg/errors" + pkgerrors "github.com/pkg/errors" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" capsulerbac "github.com/projectcapsule/capsule/pkg/api/rbac" "golang.org/x/net/http/httpguts" @@ -81,7 +82,7 @@ func NewKubeFilter( reverseProxyTransport, err := opts.ReverseProxyTransport() if err != nil { - return nil, errors.Wrap(err, "cannot create transport for reverse proxy") + return nil, pkgerrors.Wrap(err, "cannot create transport for reverse proxy") } reverseProxy.Transport = reverseProxyTransport @@ -91,12 +92,12 @@ func NewKubeFilter( err = corev1.AddToScheme(scheme) if err != nil { - return nil, errors.Wrap(err, "cannot add corev1 to scheme") + return nil, pkgerrors.Wrap(err, "cannot add corev1 to scheme") } err = authorizationv1.AddToScheme(scheme) if err != nil { - return nil, errors.Wrap(err, "cannot add authorizationv1 to scheme") + return nil, pkgerrors.Wrap(err, "cannot add authorizationv1 to scheme") } codecFactory := serializer.NewCodecFactory(scheme) @@ -282,13 +283,13 @@ func (n *kubeFilter) ReadinessProbe(req *http.Request) (err error) { var r *http.Request if r, err = http.NewRequestWithContext(req.Context(), http.MethodGet, url, nil); err != nil { - return errors.Wrap(err, "cannot create request") + return pkgerrors.Wrap(err, "cannot create request") } var resp *http.Response if resp, err = clt.Do(r); err != nil { - return errors.Wrap(err, "cannot make local _healthz request") + return pkgerrors.Wrap(err, "cannot make local _healthz request") } defer func() { @@ -348,7 +349,7 @@ func (n *kubeFilter) authorizationMiddleware(next http.Handler) http.Handler { request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header) if err != nil { - server.HandleError(writer, err, "cannot retrieve user and group from the request") + n.handleResolveUserAndGroupsError(writer, err) return } @@ -550,7 +551,7 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) { sr.HandleFunc("", func(writer http.ResponseWriter, request *http.Request) { request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header) if err != nil { - server.HandleError(writer, err, "cannot retrieve user and group from the request") + n.handleResolveUserAndGroupsError(writer, err) return } @@ -617,17 +618,28 @@ func (n *kubeFilter) recoveryMiddleware(next http.Handler) http.Handler { } if err, ok := recovered.(error); ok && errors.Is(err, http.ErrAbortHandler) { - return + panic(err) } n.log.Error(fmt.Errorf("%v", recovered), "panic while handling request") - http.Error(writer, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + server.HandleError(writer, fmt.Errorf("internal server error"), "panic while handling request") }() next.ServeHTTP(writer, request) }) } +func (n *kubeFilter) handleResolveUserAndGroupsError(writer http.ResponseWriter, err error) { + var unauthorizedErr *req.ErrUnauthorized + if errors.As(err, &unauthorizedErr) { + server.HandleUnauthorized(writer, err, "cannot retrieve user and group from the request") + + return + } + + server.HandleError(writer, err, "cannot retrieve user and group from the request") +} + func (n *kubeFilter) getTenantsForOwner(ctx context.Context, username string, groups []string) (proxyTenants []*tenant.ProxyTenant, err error) { if strings.HasPrefix(username, serviceaccount.ServiceAccountUsernamePrefix) { proxyTenants, err = n.getProxyTenantsForOwnerKind(ctx, capsulerbac.ServiceAccountOwner, username)