Proposal: Autogenerate Security Scheme checkers

Question

Proposal: Autogenerate Security Scheme checkers

diamondburned opened this issue 3 years ago · 3 comments

Problem

goapi-gen currently does not autogenerate anything for security. Instead, the user is supposed to use a long-winded way of adding a middleware which also doesn't support dependency injection via context.Contexts.

The current method also requires the user to manually verify authorization information by reading the *http.Request instance within the authentication callback.

Below is an example of such code for BearerAuth:

func (h handler) authenticate(ctx context.Context, in *openapi3filter.AuthenticationInput) error {
	switch in.SecuritySchemeName {
	case "BearerAuth":
		auth := in.RequestValidationInput.Request.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			return fmt.Errorf("invalid BearerAuth header: missing prefix")
		}

		auth = strings.TrimPrefix(auth, "Bearer ")

		s, err := h.server.AuthorizerServer().Authorize(ctx, auth)
		if err != nil {
			return err
		}

		// Checking is done, but s cannot be passed down!
		return nil
	default:
		return fmt.Errorf("unsupported auth scheme %q", in.SecuritySchemeName)
	}
}

This code is then used like so:

	validator := middleware.OapiRequestValidatorWithOptions(nil, &middleware.Options{
		Options: openapi3filter.Options{
			AuthenticationFunc: handler.authenticate,
		},
	})

This abstraction by itself isn't any easier than just using a regular middleware, except for the fact that it's still not possible to inject any value into later handlers. For comparison, such a middleware would look something like this:

func (h handler) mustAuthorize(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		token := r.Header.Get("Authorization")
		if !strings.HasPrefix(token, "Bearer ") {
			h.writeError(w, 401, fmt.Errorf("invalid BearerAuth header: missing prefix"))
			return
		}

		token = strings.TrimPrefix(token, "Bearer ")

		s, err := h.server.AuthorizerServer().Authorize(r.Context(), token)
		if err != nil {
			h.writeError(w, 401, err)
			return
		}

		ctx := context.WithValue(r.Context(), sessionKey, s)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

Clearly, goapi-gen isn't generating much at all when it comes to security.

Proposal

A possible way to improve this situation would be to make goapi-gen generate middlewares that inject the related security information for the user to check from a middleware.

For example, given the following JSON (with all routes and schemas omitted):

{
  "security": [
    {
      "BearerAuth": [],
    }
  ],  
  "components": {
    "securitySchemes": {
      "BearerAuth": {
        "type": "http",
        "scheme": "bearer"
      }
    }
  }
}

goapi-gen can generate code that handles checking for a valid security scheme and validating the Authorization: Bearer header. It can then generate the above callbacks, which can be tucked inside the autogenerated ServerOptions.

For the above example, that would look like this:

// UnauthorizedError exists so the user can
type UnauthorizedError struct {
	error
	SecuritySchemer SecuritySchemer
}

type ctxKey uint8

const (
	_ ctxKey = iota
	securityCtxKey
)

// SecuritySchemer describes any of the security methods.
type SecuritySchemer interface {
	SecurityScheme() *openapi3.SecurityScheme
	SecuritySchemeName() string
}

// SecuritySchemeFromContext returns a SecuritySchemer for the current endpoint,
// if any. The user should type-switch on the returned result.
func SecuritySchemeFromContext(ctx context.Context) SecuritySchemer {
	ss, _ := ctx.Value(securityCtxKey).(SecuritySchemer)
	return ss
}

// BearerAuthScheme is a SecuritySchemer type for BearerAuth.
type BearerAuthScheme struct {
	// Bearer contains the token from the Authorization header.
	Bearer string
}

// I'm not sure what goes in here.
func (s *BearerAuthScheme) SecurityScheme() *openapi3.SecurityScheme { return nil }

// SecuritySchemeName implements SecuritySchemer.
func (s *BearerAuthScheme) SecuritySchemeName() string { "BearerAuth" }

func handleBearerAuth(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		scheme := &BearerAuthScheme{}

		auth := r.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			err := fmt.Errorf("invalid format for Authorization header: missing prefix")
			siw.ErrorHandlerFunc(w, r, &UnauthorizedError{err, scheme})
			return
		}

		auth = strings.TrimPrefix(auth, "Bearer")

		ctx = context.WithValue(ctx, securityCtxKey, scheme)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

type ServerOptions struct {
	BaseURL            string
	BaseRouter         chi.Router
	Middlewares        map[string]func(http.Handler) http.Handler
	SecurityMiddleware func(next http.Handler) http.Handler
	ErrorHandlerFunc   func(w http.ResponseWriter, r *http.Request, err error)
}

// Handler creates http.Handler with routing matching OpenAPI spec.
func Handler(si ServerInterface, opts ...ServerOption) http.Handler {
	options := &ServerOptions{
		BaseURL:     "/",
		BaseRouter:  chi.NewRouter(),
		Middlewares: make(map[string]func(http.Handler) http.Handler),
		ErrorHandlerFunc: func(w http.ResponseWriter, r *http.Request, err error) {
			http.Error(w, err.Error(), http.StatusBadRequest)
		},
	}

	for _, f := range opts {
		f(options)
	}

	r := options.BaseRouter
	wrapper := ServerInterfaceWrapper{
		Handler:          si,
		Middlewares:      options.Middlewares,
		ErrorHandlerFunc: options.ErrorHandlerFunc,
	}

	r.Route(options.BaseURL, func(r chi.Router) {
		r.Post("/login", wrapper.Login)
		r.Post("/register", wrapper.Register)

		r.Group(func(r chi.Router) {
			r.Use(handleBearerAuth)
			r.Use(options.SecurityMiddleware)

			r.Get("/posts", wrapper.NextPosts)
			r.Get("/posts/liked", wrapper.GetLikedPosts)
			r.Delete("/posts/{id}", wrapper.DeletePosts)
			r.Get("/users/self", wrapper.GetSelf)
			r.Get("/users/{id}", wrapper.GetUser)
		})
	})

	return r
}

The user can then inject the middleware like so:

openapi.WithSecurityMiddleware(func(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		ctx := r.Context()

		switch ss := openapi.SecuritySchemeFromContext(ctx).(type) {
		case *openapi.BearerAuthScheme:
			s, err := h.server.AuthorizerServer().Authorize(ctx, ss.Bearer)
			if err != nil {
				// TODO: consider a better and more expressive way.
				h.writeError(w, 401, err)
				return
			}

			ctx = context.WithValue(ctx, sessionKey, s)
		}

		next.ServeHTTP(w, r.WithContext(ctx))
	})
})

A few important notes:

There should be a better way of writing errors from middlewares than the user calling their own writeError callback.
r.Group is required here because the user's SecurityMiddleware must be called after our security middleware handlers (handleBearerAuth in this case) are called. Because of this, the user cannot arbitrarily add a middleware.
Not filling out SecurityMiddleware is a runtime error, not a compile-time error.
The user must type-assert the error in their ErrorHandlerFunc for the server to possibly respond with a correct status code.
- This can possibly be improved by making handleBearerAuth not call ErrorHandlerFunc and instead write its own code, like so:

func handleBearerAuth(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		scheme := &BearerAuthScheme{}

		auth := r.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			http.Error(w, "invalid format for Authorization header: missing prefix", 401)
			return
		}

		auth = strings.TrimPrefix(auth, "Bearer")

		ctx = context.WithValue(ctx, securityCtxKey, scheme)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

A very important note regarding the above snippet: there is no way for the user to change how the error is written if they feed the backend an invalid header. That is, unfortunately, on par with what OapiRequestValidatorWithOptions is currently doing, which is a terrible idea!

func OapiRequestValidatorWithOptions(swagger *openapi3.T, options *Options) func(next http.Handler) http.Handler {
	router, err := gorillamux.NewRouter(swagger)
	if err != nil {
		panic(err)
	}

	return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			// validate request
			if statusCode, err := validateRequest(r, router, options); err != nil {
				http.Error(w, err.Error(), statusCode) // !!!
				return
			}

			// serve
			next.ServeHTTP(w, r)
		})
	}

}

Alternatives

I've considered alternatives to injecting into context.Context, including having a callback that the user implements specifically for authenticating. Something like this:

type SecurityHandlers struct {
	BearerAuth func(next http.Handler) http.Handler
	BearerAuth func(ctx context.Context, token string) (context.Context, error)
}

This API, while slightly more declarative (in that it doesn't require pulling an instance from the blackbox that is context.Context), it has other annoying flaws: the first BearerAuth does no parsing at all, while the second BearerAuth has a generally very weird method signature.

Answer 1 · 2022-03-18T06:56:48.000Z

cc @zacharyburkett

Answer 2 · 2022-03-19T13:04:56.000Z

We could inject security related middlewares as middlewares required on function calls by codegen.

Such that, having

{
  "security": [
    {
      "BearerAuth": [],
    }
  ]
}

Would be similar to

x-go-middlewares: ["bearer-auth", existing-middlewares...]

In essence, those could be simply set by the user directly in the middleware section, just like how 99% of the people using the stdlib does authorization and validation.

Answer 3 · 2022-03-19T19:52:41.000Z

We could inject security related middlewares as middlewares required on function calls by codegen.

I think this issue is effectively suggesting that:

		r.Group(func(r chi.Router) {
			r.Use(handleBearerAuth)           // !!!
			r.Use(options.SecurityMiddleware) // !!!

There's a slight difference between this and just letting the user inject the middleware manually: the idea is for goapi-gen to also generate OAuth library code that abstracts checks for the users wherever possible. In this case, goapi-gen can generate a code that validates that the header is a valid Bearer header.