From 738e2aabcc2b6fcc37612144467b7b06f2741011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Aaron?= Date: Tue, 9 Jun 2026 10:03:15 +0200 Subject: [PATCH 1/2] fix: stop reporting expected client errors to Sentry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OAuth handlers reported every authorize/token rejection to Sentry — malformed requests, unknown clients, redirect-URI mismatches, disallowed scopes and aborted connections. These are 4xx-class, caller-driven outcomes, not server faults, and made up the large majority of this project's Sentry volume. Decide per call site whether an error is worth reporting: - Authorize/token handlers report only errors that aren't a known go-oauth2 protocol error with a 4xx status (so 5xx and unrecognised errors, e.g. a DB failure, are still captured). - preRedirectErrorHandler no longer captures; the error is reported once downstream in AuthorizationHandler instead of twice. - checkRedirectUriDomain and AuthorizeScopeHandler return the standard OAuth errors (invalid_request / invalid_scope) and log the detail, instead of bespoke errors that bypass classification. - FetchClientHandler reports only genuine DB failures, not ErrRecordNotFound for unknown client ids. - InternalErrorHandler skips context.Canceled (client aborted). --- internal/clients/clients.go | 7 +++++-- internal/gateway/server.go | 8 ++++++++ internal/tokens/oauth.go | 13 ++++++++----- internal/tokens/tokens.go | 34 +++++++++++++++++++++++++--------- 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/internal/clients/clients.go b/internal/clients/clients.go index 39ce87e..62a6111 100644 --- a/internal/clients/clients.go +++ b/internal/clients/clients.go @@ -3,6 +3,7 @@ package clients import ( "context" "encoding/json" + "errors" "net/http" "net/url" "strings" @@ -211,10 +212,12 @@ func (service *Service) FetchClientHandler(w http.ResponseWriter, r *http.Reques id := mux.Vars(r)["clientId"] result, err := service.cs.GetClient(id) if err != nil { - sentry.CaptureException(err) status := http.StatusInternalServerError - if err == gorm.ErrRecordNotFound { + if errors.Is(err, gorm.ErrRecordNotFound) { + // Unknown client_id — a client error, not a server fault. status = http.StatusNotFound + } else { + sentry.CaptureException(err) } http.Error(w, err.Error(), status) return diff --git a/internal/gateway/server.go b/internal/gateway/server.go index b56419c..470c9dc 100644 --- a/internal/gateway/server.go +++ b/internal/gateway/server.go @@ -34,6 +34,8 @@ type OriginServer struct { } func (origin *OriginServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + //check authorization token := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") @@ -43,6 +45,12 @@ func (origin *OriginServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { } tokenInfo, err := origin.CheckTokenFunc(r.Context(), token) + + // Client has aborted the request + if ctx.Err() == context.Canceled { + return + } + if err != nil { if status, found := errorResponses[err.Error()]; found { writeErrorResponse(w, err.Error(), status) diff --git a/internal/tokens/oauth.go b/internal/tokens/oauth.go index 78a1257..4d25ce1 100644 --- a/internal/tokens/oauth.go +++ b/internal/tokens/oauth.go @@ -1,17 +1,17 @@ package tokens import ( - "fmt" "net/http" "net/url" "strconv" "strings" "time" - "github.com/getsentry/sentry-go" "github.com/go-oauth2/oauth2/v4" + oauthErrors "github.com/go-oauth2/oauth2/v4/errors" "github.com/go-oauth2/oauth2/v4/manage" "github.com/go-oauth2/oauth2/v4/server" + "github.com/sirupsen/logrus" ) func initOauthServer(conf Config, cs oauth2.ClientStore, ts oauth2.TokenStore) (srv *server.Server, err error) { @@ -99,9 +99,12 @@ func checkRedirectUriDomain(baseURI, redirectURI string) error { clientHost := parsedClientUri.Host redirectHost := parsedRedirect.Host if parsedClientUri.Scheme != parsedRedirect.Scheme || !isUriValid(clientHost, redirectHost) { - err = fmt.Errorf("wrong redirect uri, provided: [ scheme: %s, host: %s ], expected: [ scheme: %s, host: %s ]", parsedRedirect.Scheme, parsedRedirect.Host, parsedClientUri.Scheme, parsedClientUri.Host) - sentry.CaptureException(err) - return err + // Caller supplied a redirect URI that doesn't match the registered one — a + // client misconfiguration, not a server fault. Log the detail and return a + // standard OAuth error so it isn't reported to Sentry downstream. + logrus.WithField("provided", redirectURI).WithField("expected", baseURI). + Warn("rejected request with mismatched redirect uri") + return oauthErrors.ErrInvalidRequest } return nil } diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go index 6847bf5..09f6624 100644 --- a/internal/tokens/tokens.go +++ b/internal/tokens/tokens.go @@ -1,7 +1,9 @@ package tokens import ( + "context" "encoding/json" + "errors" "fmt" "net/http" "net/http/httputil" @@ -89,7 +91,12 @@ func NewService(cs oauth2.ClientStore, ts oauth2.TokenStore, scopes map[string]s func (svc *service) AuthorizationHandler(w http.ResponseWriter, r *http.Request) { err := svc.OauthServer.HandleAuthorizeRequest(w, r) if err != nil { - sentry.CaptureException(err) + // Known OAuth protocol errors with a 4xx status code are client mistakes, + // not server faults. Report everything else (5xx OAuth errors and any + // unrecognised error, e.g. a DB failure). + if code, ok := oauthErrors.StatusCodes[err]; !ok || code >= 500 { + sentry.CaptureException(err) + } http.Error(w, err.Error(), http.StatusBadRequest) } } @@ -103,7 +110,9 @@ func (svc *service) TokenHandler(w http.ResponseWriter, r *http.Request) { logrus.WithField("token_request", fmt.Sprintf("%q", string(dump))). WithError(err). Error("error validating token request") - sentry.CaptureException(err) + if code, ok := oauthErrors.StatusCodes[err]; !ok || code >= 500 { + sentry.CaptureException(err) + } svc.tokenError(w, err) return } @@ -194,8 +203,12 @@ func (svc *service) token(w http.ResponseWriter, data map[string]interface{}, he } func (svc *service) InternalErrorHandler(err error) (re *oauthErrors.Response) { - //workaround to not show "sql: no rows in result set" to user - sentry.CaptureException(err) + // This handler only receives errors the OAuth library couldn't classify, i.e. + // genuine server faults — except for context.Canceled, which just means the + // client aborted the request, so don't report that to Sentry. + if !errors.Is(err, context.Canceled) { + sentry.CaptureException(err) + } description := oauthErrors.Descriptions[err] statusCode := oauthErrors.StatusCodes[err] if description != "" && statusCode != 0 { @@ -219,21 +232,24 @@ func (svc *service) InternalErrorHandler(err error) (re *oauthErrors.Response) { } func preRedirectErrorHandler(w http.ResponseWriter, r *server.AuthorizeRequest, err error) error { + // The error is returned up to AuthorizationHandler, which decides whether to + // report it to Sentry, so only log it here to avoid a duplicate capture. logrus.WithField("Authorize request", r).Error(err) - sentry.CaptureException(err) return err } func (svc *service) AuthorizeScopeHandler(w http.ResponseWriter, r *http.Request) (scope string, err error) { requestedScope := r.FormValue("scope") if requestedScope == "" { - return "", fmt.Errorf("empty scope is not allowed") + logrus.Warn("rejected request with empty scope") + return "", oauthErrors.ErrInvalidScope } for _, scope := range strings.Split(requestedScope, " ") { if _, found := svc.scopes[scope]; !found { - err = fmt.Errorf("scope not allowed: %s", scope) - sentry.CaptureException(err) - return "", err + // Caller requested an unknown scope — a client error, not a server + // fault. Return the standard OAuth error so it isn't reported to Sentry. + logrus.WithField("scope", scope).Warn("rejected request with disallowed scope") + return "", oauthErrors.ErrInvalidScope } } return requestedScope, nil From b204c53a95f7687409c9628889663e8f827bb30a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Aaron?= Date: Tue, 9 Jun 2026 22:17:13 +0200 Subject: [PATCH 2/2] fix: log only redirect uri scheme/host, not full URIs Per review: the mismatch warning logged the full client-supplied redirect_uri and the registered base URI, which can leak path/query/ userinfo and adds high-cardinality fields on a hot bad-request path. Log only the parsed scheme and host, matching what the previous error message exposed. --- internal/tokens/oauth.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/tokens/oauth.go b/internal/tokens/oauth.go index 4d25ce1..6ec29e3 100644 --- a/internal/tokens/oauth.go +++ b/internal/tokens/oauth.go @@ -100,10 +100,15 @@ func checkRedirectUriDomain(baseURI, redirectURI string) error { redirectHost := parsedRedirect.Host if parsedClientUri.Scheme != parsedRedirect.Scheme || !isUriValid(clientHost, redirectHost) { // Caller supplied a redirect URI that doesn't match the registered one — a - // client misconfiguration, not a server fault. Log the detail and return a - // standard OAuth error so it isn't reported to Sentry downstream. - logrus.WithField("provided", redirectURI).WithField("expected", baseURI). - Warn("rejected request with mismatched redirect uri") + // client misconfiguration, not a server fault. Log only scheme/host (not the + // full client-supplied URIs) and return a standard OAuth error so it isn't + // reported to Sentry downstream. + logrus.WithFields(logrus.Fields{ + "provided_scheme": parsedRedirect.Scheme, + "provided_host": redirectHost, + "expected_scheme": parsedClientUri.Scheme, + "expected_host": clientHost, + }).Warn("rejected request with mismatched redirect uri") return oauthErrors.ErrInvalidRequest } return nil