From 0936891734e7f92b7ad724caea27715f2e3e7b5c Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Tue, 21 Oct 2025 14:06:46 +0200 Subject: [PATCH] auth: ensure semantic errors are bubbled up to user properly --- auth/middleware.go | 70 +++++++++++++++++++++++++++------------------- errors/errors.go | 18 ++++++------ 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/auth/middleware.go b/auth/middleware.go index 74d44aa48d0d914e9d78bff0429f659caf803675..08372d6c8000696f0bbd1afcac7002e18f1c6d52 100644 --- a/auth/middleware.go +++ b/auth/middleware.go @@ -100,8 +100,7 @@ func (authctx *AuthContext) Access(scope, kind string) error { } } -func authError(w http.ResponseWriter, reason string, code int) { - gqlerr := gqlerror.Errorf("Authentication error: %s", reason) +func authError(w http.ResponseWriter, gqlerr *gqlerror.Error, code int) { b, err := json.Marshal(struct { Errors []*gqlerror.Error `json:"errors"` }{ @@ -115,7 +114,7 @@ func authError(w http.ResponseWriter, reason string, code int) { w.Write(b) } -func authForUsername(ctx context.Context, username string) (*AuthContext, error) { +func authForUsername(ctx context.Context, username string) (*AuthContext, *gqlerror.Error) { var auth AuthContext if err := LookupUser(ctx, username, &auth); err != nil { return nil, err @@ -124,7 +123,7 @@ func authForUsername(ctx context.Context, username string) (*AuthContext, error) } // NOTE: This only works for meta.sr.ht (should we move it?) -func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, error) { +func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, *gqlerror.Error) { var auth AuthContext if err := database.WithTx(ctx, &sql.TxOptions{ Isolation: 0, @@ -172,11 +171,13 @@ func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, e } return nil }); err != nil { - return nil, err + gqlerr := err.(*gqlerror.Error) + return nil, gqlerr } if auth.UserType == USER_TYPE_SUSPENDED { - return nil, fmt.Errorf( + return nil, errors.Errorf( + errors.AccessDenied, "Account suspended with the following notice: %s\nContact support", *auth.SuspensionNotice) } @@ -193,7 +194,8 @@ func cookieAuth(cookie *http.Cookie, w http.ResponseWriter, r *http.Request, next http.Handler) { payload := crypto.DecryptWithoutExpiration([]byte(cookie.Value)) if payload == nil { - authError(w, "Invalid authentication cookie", http.StatusForbidden) + err := errors.Errorf(errors.Unauthorized, "Invalid authentication cookie") + authError(w, err, http.StatusForbidden) return } @@ -204,12 +206,13 @@ func cookieAuth(cookie *http.Cookie, w http.ResponseWriter, auth, err := authForUsername(r.Context(), authCookie.Name) if err != nil { - authError(w, err.Error(), http.StatusForbidden) + authError(w, err, http.StatusForbidden) return } if auth.UserType == USER_TYPE_SUSPENDED { - authError(w, fmt.Sprintf( + authError(w, errors.Errorf( + errors.AccessDenied, "Account suspended with the following notice: %s\nContact support", *auth.SuspensionNotice), http.StatusForbidden) @@ -247,13 +250,13 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h panic(fmt.Errorf("Unable to parse remote address")) } if !config.IsInternalIP(ip) { - authError(w, fmt.Sprintf("Invalid source IP %s for internal auth", ip), http.StatusUnauthorized) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid source IP %s for internal auth", ip), http.StatusUnauthorized) return } payload = crypto.DecryptWithExpiration(payload, 30*time.Second) if payload == nil { - authError(w, "Invalid Authorization header (encryption error)", http.StatusForbidden) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header (encryption error)"), http.StatusForbidden) return } @@ -263,18 +266,21 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h } if internalAuth.ClientID == "" || internalAuth.NodeID == "" { - authError(w, "Invalid Authorization header (missing Client ID or Node ID)", http.StatusForbidden) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header (missing Client ID or Node ID)"), http.StatusForbidden) } - var auth *AuthContext + var ( + auth *AuthContext + gqlerr *gqlerror.Error + ) if internalAuth.OAuthClientUUID != "" { - auth, err = authForOAuthClient(r.Context(), internalAuth.OAuthClientUUID) - if err == nil { + auth, gqlerr = authForOAuthClient(r.Context(), internalAuth.OAuthClientUUID) + if gqlerr == nil { auth.AuthMethod = AUTH_INTERNAL } } else if internalAuth.Name != "" { - auth, err = authForUsername(r.Context(), internalAuth.Name) - if err == nil { + auth, gqlerr = authForUsername(r.Context(), internalAuth.Name) + if gqlerr == nil { auth.AuthMethod = AUTH_INTERNAL } } else { @@ -283,8 +289,8 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h auth = &AuthContext{} auth.AuthMethod = AUTH_ANON_INTERNAL } - if err != nil { - authError(w, err.Error(), http.StatusForbidden) + if gqlerr != nil { + authError(w, gqlerr, http.StatusForbidden) return } @@ -400,8 +406,8 @@ func FetchMetaProfile(ctx context.Context, username string, user *AuthContext) e }) } -func LookupUser(ctx context.Context, username string, user *AuthContext) error { - return database.WithTx(ctx, &sql.TxOptions{ +func LookupUser(ctx context.Context, username string, user *AuthContext) *gqlerror.Error { + err := database.WithTx(ctx, &sql.TxOptions{ Isolation: 0, ReadOnly: true, }, func(tx *sql.Tx) error { @@ -463,6 +469,11 @@ func LookupUser(ctx context.Context, username string, user *AuthContext) error { } return nil }) + if err != nil { + gqlerr := err.(*gqlerror.Error) + return gqlerr + } + return nil } // Returns true if this token or client ID has been revoked (and therefore @@ -504,7 +515,7 @@ func OAuth2(token string, hash [64]byte, w http.ResponseWriter, bt := DecodeBearerToken(token) if bt == nil { - authError(w, `Invalid or expired OAuth 2.0 bearer token`, http.StatusForbidden) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid or expired OAuth 2.0 bearer token"), http.StatusForbidden) return } @@ -534,15 +545,16 @@ func OAuth2(token string, hash [64]byte, w http.ResponseWriter, wg.Wait() if res != 2 { if tempErr != 0 { - authError(w, "Temporary error; try again later", http.StatusInternalServerError) + authError(w, errors.Errorf(errors.InternalError, "Temporary error; try again later"), http.StatusInternalServerError) } else { - authError(w, "Invalid or expired OAuth 2.0 bearer token", http.StatusForbidden) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid or expired OAuth 2.0 bearer token"), http.StatusForbidden) } return } if auth.UserType == USER_TYPE_SUSPENDED { - authError(w, fmt.Sprintf( + authError(w, errors.Errorf( + errors.AccessDenied, "Account suspended with the following notice: %s\nContact support", *auth.SuspensionNotice), http.StatusForbidden) return @@ -605,13 +617,13 @@ func Middleware(conf ini.File, apiconf string) func(http.Handler) http.Handler { auth := r.Header.Get("Authorization") if auth == "" { w.Header().Set("WWW-Authenticate", "Bearer") - authError(w, `Authorization header is required. Expected 'Authorization: Bearer [token]'`, http.StatusUnauthorized) + authError(w, errors.Errorf(errors.Unauthorized, "Authorization header is required. Expected 'Authorization: Bearer [token]'"), http.StatusUnauthorized) return } z := strings.SplitN(auth, " ", 2) if len(z) != 2 { - authError(w, "Invalid Authorization header", http.StatusBadRequest) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header"), http.StatusBadRequest) return } @@ -625,14 +637,14 @@ func Middleware(conf ini.File, apiconf string) func(http.Handler) http.Handler { OAuth2(bearer, hash, w, r, next) return } - authError(w, "Invalid OAuth bearer token", http.StatusBadRequest) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid OAuth bearer token"), http.StatusBadRequest) return case "internal": payload := []byte(z[1]) internalAuth(payload, w, r, next) return default: - authError(w, "Invalid Authorization header", http.StatusBadRequest) + authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header"), http.StatusBadRequest) return } }) diff --git a/errors/errors.go b/errors/errors.go index 7f8eef32dd3c1ac62636370f281edd9c969c8c07..05a450f44ccd672e40fd0d0900c85fb8f9c83b7b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -32,16 +32,18 @@ func Field(err *gqlerror.Error, field string) *gqlerror.Error { // Error codes as string constants var ( - AccessDenied ErrorCode = "ERR_ACCESS_DENIED" - NotFound ErrorCode = "ERR_NOT_FOUND" - Unsupported ErrorCode = "ERR_UNSUPPORTED" - Unauthorized ErrorCode = "ERR_UNAUTHORIZED" + AccessDenied ErrorCode = "ERR_ACCESS_DENIED" + NotFound ErrorCode = "ERR_NOT_FOUND" + Unsupported ErrorCode = "ERR_UNSUPPORTED" + Unauthorized ErrorCode = "ERR_UNAUTHORIZED" + InternalError ErrorCode = "ERR_INTERNAL" ) // Error codes as Go errors var ( - ErrAccessDenied = New(AccessDenied, "Access denied") - ErrNotFound = New(NotFound, "Resource not found") - ErrUnsupported = New(Unsupported, "Not supported") - ErrUnauthorized = New(Unauthorized, "Unauthorized") + ErrAccessDenied = New(AccessDenied, "Access denied") + ErrNotFound = New(NotFound, "Resource not found") + ErrUnsupported = New(Unsupported, "Not supported") + ErrUnauthorized = New(Unauthorized, "Unauthorized") + ErrInternalError = New(InternalError, "Internal server error") )