Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Middlewares per handler in chi-server #259

Merged
merged 3 commits into from Jan 5, 2021

Conversation

maclav3
Copy link
Contributor

@maclav3 maclav3 commented Nov 30, 2020

1. Add HandlerMiddlewares to ServerInterfaceWrapper

What changed

A HandlerMiddlewares []middlewareFunc field was added to ServerInterfaceWrapper, storing a slice of middleware functions.

type middlewareFunc func(http.HandlerFunc) http.HandlerFunc follows the basic middleware convention used within chi.

The middlewares are applied from 0 to n before the wrapped handler:

        var handler = func(w http.ResponseWriter, r *http.Request) {
		siw.Handler.DeletePet(w, r, id)
	}

	for _, middleware := range siw.HandlerMiddlewares {
		handler = middleware(handler)
	}

	handler(w, r.WithContext(ctx))

The stack receives a *http.Request with any context values set for the handler.

Rationale

It was previously possible to add middlewares to the auto-generated chi server, but it was attained through passing a custom chi.Router to HandlerFromMux or HandlerFromMuxWithBaseURL.

Before, the context values were added to ctx after all chi.Middlewares had already fired, between the auto-generated wrapper and the wrapped handler:

// DeletePet operation middleware
func (siw *ServerInterfaceWrapper) DeletePet(w http.ResponseWriter, r *http.Request) {
	ctx := r.Context()

	var err error

	// ------------- Path parameter "id" -------------
	var id int64

	err = runtime.BindStyledParameter("simple", false, "id", chi.URLParam(r, "id"), &id)
	if err != nil {
		http.Error(w, fmt.Sprintf("Invalid format for parameter id: %s", err), http.StatusBadRequest)
		return
	}

	ctx = context.WithValue(ctx, "bearer.Scopes", []string{"pets:delete"})

	siw.Handler.DeletePet(w, r.WithContext(ctx), id)
}

note: I added a pets:delete expected scope to the operation to make my point clearer

Now, the enriched context will be passed through the stack of middlewares, enabling a permissions middleware that reads the expected scopes from ctx values dictated by the OpenApi definition, and compares them to the scopes that came with the request:

// DeletePet operation middleware
func (siw *ServerInterfaceWrapper) DeletePet(w http.ResponseWriter, r *http.Request) {
	ctx := r.Context()

	var err error

	// ------------- Path parameter "id" -------------
	var id int64

	err = runtime.BindStyledParameter("simple", false, "id", chi.URLParam(r, "id"), &id)
	if err != nil {
		http.Error(w, fmt.Sprintf("Invalid format for parameter id: %s", err), http.StatusBadRequest)
		return
	}

	ctx = context.WithValue(ctx, "bearer.Scopes", []string{"pets:delete"})

	var handler = func(w http.ResponseWriter, r *http.Request) {
		siw.Handler.DeletePet(w, r, id)
	}

	for _, middleware := range siw.HandlerMiddlewares {
		handler = middleware(handler)
	}

	handler(w, r.WithContext(ctx))
}

The permissions middleware can read the bearer.Scopes context value to find out what scopes are expected for the call and compare with what's presented for the request.
The Openapi def becomes the only source of truth for expected security scopes for every operation, and there's no need to duplicate them in non-auto-generated code.

Because of the middleware pattern, it may pass to lower handler in the stack or cut it short, writing a 401 or 403 status.

2. Add a HandlerWithOptions auto-generated constructor for the chi Server handler.

What changed

A new constructor HandlerWithOptions(si ServerInterface, options ChiServerOptions) http.Handler was added that takes an options struct:

type ChiServerOptions struct {
    BaseURL string
    BaseRouter chi.Router
    Middlewares []middlewareFunc
}

Unfortunately, there are 3 different constructors introduced previously, so this is starting to pollute the scope.
However, this newest constructor is extensible, so no new ones will be necessary.

All the previous constructors (Handler, HandlerFromMux, HandlerFromMuxWithBaseURL) have been expressed in terms of the new one, to avoid duplication and to retain previous functionality.

Zero values of ChiServerOptions behave in a sane manner.

Rationale

I wanted to retain symmetry with the previous approaches that there's a designated Handler* function to create a new handler, so that ServerInterfaceWrapper doesn't have to be enriched with Middlewares manually.

This will improve adding new features in the future, because ChiServerOptions is extensible without breaking the API.

@deepmap-marcinr deepmap-marcinr merged commit 411db58 into deepmap:master Jan 5, 2021
@deepmap-marcinr
Copy link
Contributor

Thank you.

Snaipe added a commit to Snaipe/oapi-codegen that referenced this pull request May 11, 2021
This is essentially the same thing as deepmap#259, but for echo.

I could not find a way to access the OAuth scope of the route from the normal
middlewares, as the value gets set by ServerInterfaceWrapper. With this change,
it's now possible to add a middleware to check that the scope is valid prior
to running the actual handler.

This commit introduces a new Middlewares field to ServerInterfaceWrapper,
which is a slice of echo.MiddlewareFunc, that gets executed in order
after the route parameters get decoded. It also adds a new registration
function, RegisterHandlersWithOptions, to allow users to specify these
middlewares.
Snaipe added a commit to Snaipe/oapi-codegen that referenced this pull request Sep 10, 2021
This is essentially the same thing as deepmap#259, but for echo.

This commit introduces a new Middlewares field to ServerInterfaceWrapper,
which is a slice of echo.MiddlewareFunc, that gets registered as per-handler
middlewares. It also adds a new registration function, RegisterHandlersWithOptions,
to allow users to specify these middlewares.
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
* Allow for server middlewares in chi server

* Export MiddlewareFunc

* Fix unexported field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants