Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions packages/api/internal/db/apikeys.go

This file was deleted.

73 changes: 4 additions & 69 deletions packages/api/internal/utils/error.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,17 @@
package utils

import (
"errors"
"fmt"
"net/http"
"strings"

"github.com/getkin/kin-openapi/openapi3"
"github.com/getkin/kin-openapi/openapi3filter"
"github.com/gin-gonic/gin"
"go.opentelemetry.io/otel/attribute"

"github.com/e2b-dev/infra/packages/api/internal/auth"
"github.com/e2b-dev/infra/packages/api/internal/db"
"github.com/e2b-dev/infra/packages/auth/pkg/auth"
"github.com/e2b-dev/infra/packages/shared/pkg/telemetry"
)

const (
securityErrPrefix = "error in openapi3filter.SecurityRequirementsError: security requirements failed: "
forbiddenErrPrefix = "team forbidden: "
blockedErrPrefix = "team blocked: "
)

func ErrorHandler(c *gin.Context, message string, statusCode int) {
var errMsg error

Expand Down Expand Up @@ -49,7 +39,7 @@ func ErrorHandler(c *gin.Context, message string, statusCode int) {
c.Error(errMsg)

// Handle forbidden errors
if after, ok := strings.CutPrefix(message, forbiddenErrPrefix); ok {
if after, ok := strings.CutPrefix(message, auth.ForbiddenErrPrefix); ok {
c.AbortWithStatusJSON(
http.StatusForbidden,
gin.H{
Expand All @@ -62,7 +52,7 @@ func ErrorHandler(c *gin.Context, message string, statusCode int) {
}

// Handle blocked errors
if after, ok := strings.CutPrefix(message, blockedErrPrefix); ok {
if after, ok := strings.CutPrefix(message, auth.BlockedErrPrefix); ok {
c.AbortWithStatusJSON(
http.StatusForbidden,
gin.H{
Expand All @@ -75,7 +65,7 @@ func ErrorHandler(c *gin.Context, message string, statusCode int) {
}

// Handle security requirements errors from the openapi3filter
if after, ok := strings.CutPrefix(message, securityErrPrefix); ok {
if after, ok := strings.CutPrefix(message, auth.SecurityErrPrefix); ok {
// Keep the original status code as it can be also timeout (read body timeout) error code.
// The securityErrPrefix is added for all errors going through the processCustomErrors function.
c.AbortWithStatusJSON(
Expand All @@ -91,58 +81,3 @@ func ErrorHandler(c *gin.Context, message string, statusCode int) {

c.AbortWithStatusJSON(statusCode, gin.H{"code": statusCode, "message": fmt.Errorf("validation error: %s", message).Error()})
}

// MultiErrorHandler handles wrapped SecurityRequirementsError, so there are no multiple errors returned to the user.
func MultiErrorHandler(me openapi3.MultiError) error {
if len(me) == 0 {
return nil
}
err := me[0]

// Recreate logic from oapi-codegen/gin-middleware to handle the error
// Source: https://github.com/oapi-codegen/gin-middleware/blob/main/oapi_validate.go
switch e := err.(type) { //nolint:errorlint // we copied this and don't want it to change
case *openapi3filter.RequestError:
// We've got a bad request
// Split up the verbose error by lines and return the first one
// openapi errors seem to be multi-line with a decent message on the first
errorLines := strings.Split(e.Error(), "\n")

return fmt.Errorf("error in openapi3filter.RequestError: %s", errorLines[0])
case *openapi3filter.SecurityRequirementsError:
return processCustomErrors(e) // custom implementation
default:
// This should never happen today, but if our upstream code changes,
// we don't want to crash the server, so handle the unexpected error.
return fmt.Errorf("error validating request: %w", err)
}
}

func processCustomErrors(e *openapi3filter.SecurityRequirementsError) error {
// Return only one security requirement error (there may be multiple securitySchemes)
unwrapped := e.Errors
err := unwrapped[0]

var teamForbidden *db.TeamForbiddenError
var teamBlocked *db.TeamBlockedError
// Return only the first non-missing authorization header error (if possible)
for _, errW := range unwrapped {
if errors.Is(errW, auth.ErrNoAuthHeader) {
continue
}

if errors.As(errW, &teamForbidden) {
return fmt.Errorf("%s%s", forbiddenErrPrefix, err.Error())
}

if errors.As(errW, &teamBlocked) {
return fmt.Errorf("%s%s", blockedErrPrefix, err.Error())
}

err = errW

break
}

return fmt.Errorf("%s%s", securityErrPrefix, err.Error())
}
2 changes: 1 addition & 1 deletion packages/api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func NewGinServer(ctx context.Context, config cfg.Config, tel *telemetry.Client,
statusCode := max(c.Writer.Status(), fallbackStatusCode)
utils.ErrorHandler(c, message, statusCode)
},
MultiErrorHandler: utils.MultiErrorHandler,
MultiErrorHandler: auth.MultiErrorHandler,
Options: openapi3filter.Options{
AuthenticationFunc: AuthenticationFunc,
// Handle multiple errors as MultiError type
Expand Down
71 changes: 71 additions & 0 deletions packages/auth/pkg/auth/multi_error_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package auth

import (
"errors"
"fmt"
"strings"

"github.com/getkin/kin-openapi/openapi3"
"github.com/getkin/kin-openapi/openapi3filter"
)

const (
SecurityErrPrefix = "error in openapi3filter.SecurityRequirementsError: security requirements failed: "
ForbiddenErrPrefix = "team forbidden: "
BlockedErrPrefix = "team blocked: "
)

// MultiErrorHandler handles wrapped SecurityRequirementsError, so there are no multiple errors returned to the user.
func MultiErrorHandler(me openapi3.MultiError) error {
if len(me) == 0 {
return nil
}
err := me[0]

// Recreate logic from oapi-codegen/gin-middleware to handle the error
// Source: https://github.com/oapi-codegen/gin-middleware/blob/main/oapi_validate.go
switch e := err.(type) { //nolint:errorlint // we copied this and don't want it to change
case *openapi3filter.RequestError:
// We've got a bad request
// Split up the verbose error by lines and return the first one
// openapi errors seem to be multi-line with a decent message on the first
errorLines := strings.Split(e.Error(), "\n")

return fmt.Errorf("error in openapi3filter.RequestError: %s", errorLines[0])
case *openapi3filter.SecurityRequirementsError:
return processCustomErrors(e) // custom implementation
default:
// This should never happen today, but if our upstream code changes,
// we don't want to crash the server, so handle the unexpected error.
return fmt.Errorf("error validating request: %w", err)
}
}

func processCustomErrors(e *openapi3filter.SecurityRequirementsError) error {
// Return only one security requirement error (there may be multiple securitySchemes)
unwrapped := e.Errors
err := unwrapped[0]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential panic: unwrapped[0] is accessed without a bounds check. This risk already existed in packages/api, but it is now also reachable from dashboard-api, which previously used a simple join handler that never hit this code path. If a SecurityRequirementsError is ever produced with an empty Errors slice this will panic.


var teamForbidden *TeamForbiddenError
var teamBlocked *TeamBlockedError
// Return only the first non-missing authorization header error (if possible)
for _, errW := range unwrapped {
if errors.Is(errW, ErrNoAuthHeader) {
continue
}

if errors.As(errW, &teamForbidden) {
return fmt.Errorf("%s%s", ForbiddenErrPrefix, err.Error())
}

if errors.As(errW, &teamBlocked) {
return fmt.Errorf("%s%s", BlockedErrPrefix, err.Error())
}

err = errW

break
}

return fmt.Errorf("%s%s", SecurityErrPrefix, err.Error())
}
11 changes: 1 addition & 10 deletions packages/dashboard-api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import (
"os"
"os/signal"
"strconv"
"strings"
"sync"
"sync/atomic"
"syscall"
"time"

"github.com/getkin/kin-openapi/openapi3"
"github.com/getkin/kin-openapi/openapi3filter"
"github.com/gin-contrib/cors"
"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -214,14 +212,7 @@ func run() int {
"message": message,
})
},
MultiErrorHandler: func(me openapi3.MultiError) error {
msgs := make([]string, 0, len(me))
for _, e := range me {
msgs = append(msgs, e.Error())
}

return fmt.Errorf("%s", strings.Join(msgs, "; "))
},
MultiErrorHandler: sharedauth.MultiErrorHandler,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral regression: the previous handler joined all errors from openapi3.MultiError; this replacement only processes me[0] and returns errors prefixed with SecurityErrPrefix for security failures. Unlike packages/api, the dashboard-api error handler does not strip these prefixes. Auth/security validation failures would now expose the raw internal error string in API responses.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral regression: the previous handler joined all errors from openapi3.MultiError; this replacement only processes me[0] and returns errors prefixed with SecurityErrPrefix for security failures.

Unlike packages/api, the dashboard-api error handler does not strip these prefixes (there is no equivalent to the api ErrorHandler that calls strings.CutPrefix on auth.SecurityErrPrefix). Auth/security validation failures would now expose the raw internal string "error in openapi3filter.SecurityRequirementsError: security requirements failed: ..." in API responses.

Options: openapi3filter.Options{
AuthenticationFunc: authenticationFunc,
},
Expand Down
Loading