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

Passing Configuration to Middleware #23

Closed
elithrar opened this issue May 15, 2014 · 5 comments
Closed

Passing Configuration to Middleware #23

elithrar opened this issue May 15, 2014 · 5 comments

Comments

@elithrar
Copy link
Contributor

I'm looking to contribute some middleware—specifically CSRF protection (which I am comfortable in writing out) and possibly some BasicAuth and NoCache middleware.

The CSRF middleware of course needs a session store to store the "real" token to compare with the masked token from form data and/or the request header. I'll wrap gorilla/sessions and look to allowing a package user to provide their own sessions.Store, else it defaults to a sessions.CookieStore.

Is there a preferred/idiomatic way to provide an exported configuration struct, initialise it and pass it to the middleware handler?

My current approach would be something like this:

type CSRFOptions struct {
      Store *sessions.Store
      Options *sessions.Options
      CookieName string
}

func NewCSRFOptions(store *sessions.Store, opts *sessions.Options, name string) *CSRFOptions {
     c := &CSRFOptions{
         ...
     }
     return &c
}

func (c *CSRFOptions) CSRF(c web.C, h http.Handler) http.Handler {
    ...
}

...

store := NewCSRFOptions(myStore, myOpts, "_csrftoken")

...

goji.Use(store.CSRF)

Do you have a preference for an alternate approach and/or is this compatible with Goji's middleware interfaces? Perhaps a CSRFDefaults() *CSRFOptions function that makes it easier to get started with?

@dre1080
Copy link

dre1080 commented May 15, 2014

Hey @elithrar,
Wouldn't be using nosurf be much easier for CSRF?

@zenazn
Copy link
Owner

zenazn commented May 15, 2014

The way I did this last time is something like this:

type Params struct {
    foo int
}

type myMiddleware struct {
    h http.Handler
    c *web.C
    p Params
}

func (m myMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    // ...
    m.h.ServeHTTP(w, r)
    // ...
}

func MyMiddleware(p Params) func(*web.C, http.Handler) http.Handler {
    return func(c *web.C, h http.Handler) {
        return myMiddleware{h, c, p}
    }
}

// You can store this in a variable if you want to reference it again
goji.Use(MyMiddleware(Params{
    foo: 4,
}))

The way you're doing it would work as well. I haven't given much thought to which I prefer, but I think the way I wrote above probably will result in one fewer helper struct.

As for wrapping Gorilla, I've found that Gorilla does some... questionable things with global maps, which tends to result in unnecessary lock contention and (more importantly) the tendency to leak memory unless you're careful. I haven't looked very hard at nosurf, but it should be trivial to drop in to Goji and probably doesn't do quite as much spooky action at a distance as Gorilla.

@elithrar
Copy link
Contributor Author

@zenazn Thanks for the example code. Extending/wrapping http.Handler is actually something I didn't think of at first, and it does look a bit cleaner. I'm definitely aware of the need to call context.ClearHandler(myHandler) to avoid any leaks from gorilla (I know that's a gotcha when not using gorilla/mux alongside), but beyond that I plan to use goji's own request context to pass the token to the wrapped handler and avoid Gorilla's there. nosurf still uses a global context map anyway so I'm not sure it provides any distinct benefits on that front.

@dre1080 @zenazn I like nosurf, but you're also entirely tied to a cookie-based session store. Being able to use server-side sessions (particularly if you are already) is a nice win for security, and you also avoid the 4K limit on session size that browsers enforce on cookies. I'm also not quite sure nosurf's cookie contents are encrypted (as gorilla/sessions allows) from looking through the source, although if you're dealing with any POST data over vanilla HTTP that's your second problem ;)

I'm happy to take advice otherwise, but I think there's some value in providing some Goji-centric anti CSRF middleware.

@zenazn
Copy link
Owner

zenazn commented May 15, 2014

RE: nosurf's global map: oh dear.

Anyways, you seem to know what you're doing so I'll stop bothering you with my unfounded opinions :)

@elithrar
Copy link
Contributor Author

elithrar commented Jun 7, 2014

Closing this specific one out - but am still working on the CSRF & sessions middleware when I can find some spare time.

@elithrar elithrar closed this as completed Jun 7, 2014
This issue was closed.
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

No branches or pull requests

3 participants