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

Doesn't work with gin-gonic #327

Closed
camaeel opened this issue Sep 25, 2021 · 3 comments
Closed

Doesn't work with gin-gonic #327

camaeel opened this issue Sep 25, 2021 · 3 comments

Comments

@camaeel
Copy link

camaeel commented Sep 25, 2021

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/[email protected]+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/[email protected]+incompatible/client_state.go:303 (0x7ce2a4)
	setState: csrw := MustClientStateResponseWriter(w)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+incompatible/client_state.go:295 (0x7d3a1b)
	putState: setState(w, CTXKey, ClientStateEventPut, key, val)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+incompatible/client_state.go:266 (0x7d39eb)
	PutSession: putState(w, CTXKeySessionState, key, val)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+incompatible/defaults/responder.go:134 (0x7d39ea)
	Redirector.redirectNonAPI: authboss.PutSession(w, authboss.FlashErrorKey, ro.Failure)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+incompatible/defaults/responder.go:74 (0x7d338a)
	(*Redirector).Redirect: return redirectFunction(w, req, ro)
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+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/[email protected]+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/[email protected]/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/[email protected]/context.go:108 (0x7bf4b5)
	(*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gwatts/[email protected]/adapter.go:76 (0x7c84c4)
	(*connectHandler).ServeHTTP: state.ctx.Next()
/home/kamil/go-libs/pkg/mod/github.com/volatiletech/[email protected]+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/[email protected]/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/[email protected]/context.go:108 (0x7bf4b5)
	(*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/recovery.go:48 (0x7c3485)
	RecoveryWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/context.go:108 (0x7bf4b5)
	(*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/recovery.go:48 (0x7c3485)
	RecoveryWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/context.go:108 (0x7bf4b5)
	(*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/logger.go:84 (0x7c26b8)
	LoggerWithWriter.func1: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/context.go:108 (0x7bf4b5)
	(*Context).Next: c.handlers[c.index](c)
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:361 (0x7c1a38)
	(*Engine).handleHTTPRequest: c.Next()
/home/kamil/go-libs/pkg/mod/github.com/gin-gonic/[email protected]/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
@camaeel camaeel changed the title Support for gin-gonic Doesn't work with gin-gonic Sep 25, 2021
@aarondl
Copy link
Member

aarondl commented Sep 26, 2021

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.

@aarondl aarondl closed this as completed Sep 26, 2021
@oliverdain
Copy link

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
Copy link

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

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