volatiletech / authboss

The boss of http auth.
MIT License
3.79k stars 204 forks source link

Doesn't work with gin-gonic #327

Closed camaeel closed 2 years ago

camaeel commented 2 years ago

When using with gin-gonic with authboss I get exception:

ResponseWriter must be a ClientStateResponseWriter or UnderlyingResponseWriter in (see: authboss.LoadClientStateMiddleware): *gin.responseWriter
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/client_state.go:174 (0x7cde7d)
    MustClientStateResponseWriter: panic(fmt.Sprintf("ResponseWriter must be a ClientStateResponseWriter or UnderlyingResponseWriter in (see: authboss.LoadClientStateMiddleware): %T", w))
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/client_state.go:303 (0x7ce2a4)
    setState: csrw := MustClientStateResponseWriter(w)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/client_state.go:295 (0x7d3a1b)
    putState: setState(w, CTXKey, ClientStateEventPut, key, val)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/client_state.go:266 (0x7d39eb)
    PutSession: putState(w, CTXKeySessionState, key, val)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/defaults/responder.go:134 (0x7d39ea)
    Redirector.redirectNonAPI: authboss.PutSession(w, authboss.FlashErrorKey, ro.Failure)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/defaults/responder.go:74 (0x7d338a)
    (*Redirector).Redirect: return redirectFunction(w, req, ro)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/authboss.go:190 (0x7cd335)
    MountedMiddleware2.func1.1.1: if err := ab.Config.Core.Redirector.Redirect(w, r, ro); err != nil {
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/authboss.go:203 (0x7cce81)
    MountedMiddleware2.func1.1: fail(w, r)
/opt/go/src/net/http/server.go:2046 (0x61ceee)
    HandlerFunc.ServeHTTP: f(w, r)
/home/kamil/go-libs/pkg/mod/github.com/gwatts/gin-adapter@v0.0.0-20170508204228-c44433c485ad/adapter.go:49 (0x7c8307)
    New.func1.1: h.ServeHTTP(c.Writer, c.Request.WithContext(ctx))
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/context.go:108 (0x7bf4b5)
    (*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gwatts/gin-adapter@v0.0.0-20170508204228-c44433c485ad/adapter.go:76 (0x7c84c4)
    (*connectHandler).ServeHTTP: state.ctx.Next()
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/authboss@v2.2.0+incompatible/client_state.go:121 (0x7cd701)
    (*Authboss).LoadClientStateMiddleware.func1: h.ServeHTTP(writer, request)
/opt/go/src/net/http/server.go:2046 (0x61ceee)
    HandlerFunc.ServeHTTP: f(w, r)
/home/kamil/go-libs/pkg/mod/github.com/gwatts/gin-adapter@v0.0.0-20170508204228-c44433c485ad/adapter.go:49 (0x7c8307)
    New.func1.1: h.ServeHTTP(c.Writer, c.Request.WithContext(ctx))
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/context.go:108 (0x7bf4b5)
    (*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/recovery.go:48 (0x7c3485)
    RecoveryWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/context.go:108 (0x7bf4b5)
    (*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/recovery.go:48 (0x7c3485)
    RecoveryWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/context.go:108 (0x7bf4b5)
    (*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/logger.go:84 (0x7c26b8)
    LoggerWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/context.go:108 (0x7bf4b5)
    (*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/gin.go:361 (0x7c1a38)
    (*Engine).handleHTTPRequest: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/gin@v1.3.0/gin.go:326 (0x7c16ad)
    (*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/opt/go/src/net/http/server.go:2878 (0x61fb1a)
    serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/opt/go/src/net/http/server.go:1929 (0x61c247)
    (*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/opt/go/src/runtime/asm_amd64.s:1581 (0x464720)
    goexit: BYTE    $0x90   // NOP

Reply steps:

  1. clone repository: https://github.com/jesusvazquez/authboss-gin-sample
  2. Add

        // authorization required
    rAdmin := router.Group("/admin")
    
    rAdmin.Use(adapter.Wrap(authboss.Middleware2(ab, authboss.RequireNone, authboss.RespondRedirect)))
    rAdmin.GET("/", status)

    in main() after router.GET("/status", status)

  3. Build & run
  4. Execute curl localhost:3000/admin/ -> an exception is thrown
aarondl commented 2 years ago

If you're using a framework like gin-gonic which takes over the response writer you need to ensure proper ordering of middlewares. I don't generally help sort out custom frameworks issues though. This library works fine with the stdlib http package and many other packages like chi and httprouter.

oliverdain commented 6 months ago

I just ran into this and did some debugging. If you're using the example from https://github.com/jesusvazquez/authboss-gin-sample that's using github.com/gwatts/gin-adapter to convert regular middleware into Gin middleware.

If you step through the flow in a debugger you see that authboss.LoadClientStateMiddleware calls h.ServeHTTP at the end of that method. If you step into that you'll find yourself in the github.com/gwatts/gin-adapter code, specifically here:

func (h *connectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    state := r.Context().Value(h).(*middlewareCtx)
    defer func(r *http.Request) { state.ctx.Request = r }(state.ctx.Request)
    defer func(w gin.ResponseWriter) { state.ctx.Writer = w }(state.ctx.Writer)
    state.ctx.Request = r
    state.ctx.Writer = swap(state.ctx.Writer, w)
    state.childCalled = true
    state.ctx.Next()
}

Note that swaps out the response writer via the swap call which returns a swappedResponseWriter which contains the original writer but hidden as a member. The defer should swap it back when this returns but authboss tries to use the writer before this method returns so you get the panic.

To make this work we'd need an alternative form of adapter but I'm not sure how that could work. authboss creates a new response writer derived from the http.ResponseWriter but we can't just construct a Gin one from that because it's missing some of the data in gin's response writer (number of bytes written so far and status code to send). I might be missing something but I think this just won't work with gin.

oliverdain commented 6 months ago

Oh! It looks like maybe somebody found a workaround. This is a update to github.com/gwatts/gin-adapter that has their swappedResponseWriter implement the UnderlyingResponseWriter interface: https://github.com/bscottm/authboss-worked/blob/6b1cea2dd7df0788790fbf6d48d4d25591aa5413/abossworked/adapter/ginWrap.go