danielgtaylor / huma

Huma REST/HTTP API Framework for Golang with OpenAPI 3.1
https://huma.rocks/
MIT License
1.99k stars 146 forks source link

Bug in middleware or expected? #605

Open spa5k opened 6 days ago

spa5k commented 6 days ago

I've been trying to make a session middleware for huma, based on SCS, at https://github.com/spa5k/huma-scs, but the problem I'm having is that it's not considering the cookies/anything that is set after the next(ctx) is called


// LoadAndSave provides middleware which automatically loads and saves session
// data for the current request, and communicates the session token to and from
// the client in a cookie.
func (s *SessionManager) LoadAndSave(ctx huma.Context, next func(huma.Context)) {
    ctx.SetHeader("Vary", "Cookie")
    var token string
    cookie, err := huma.ReadCookie(ctx, s.Cookie.Name)

    if err == nil {
        token = cookie.Value
    }

    newCtx, err := s.Load(ctx.Context(), token)
    if err != nil {
        s.ErrorFunc(ctx, err)
        return
    }

    ctx = huma.WithContext(ctx, newCtx)

    next(ctx)

    s.commitAndWriteSessionCookie(ctx)
}

I need a way to still make these headers available. If I put them above the next, they work, but I need them below for the logic to work.

It passes the test cases tho, the values come correctly for some reason, that is why it feels like a bug

spa5k commented 6 days ago

In here, both cache-control and the Set-Cookie header done in s.commitAndWriteSessionCookie, you can check the full code in the repo above

spa5k commented 6 days ago

Sample setup and routes to test the middleware -

humaConfig := huma.DefaultConfig("ReviewHQ API", "1")
api := humachi.New(r, humaConfig)
sessionManager := scs.New()
sessionManager.Lifetime = 1 * 365 * 24 * time.Hour
sessionManager.Cookie.Persist = true

api.UseMiddleware(sessionManager.LoadAndSave)

func sessionTestRoutes(api huma.API, sessionManager *scs.SessionManager) {
    huma.Register(api, huma.Operation{
        Method: http.MethodPut,
        Path:   "/put",
    }, func(ctx context.Context, input *struct{}) (*struct {
        Status  int          `json:"status"`
    }, error,
    ) {
        sessionManager.Put(ctx, "foo", "bar")
        fmt.Println("writing session cookie")
        return &struct {
            Status  int          `json:"status"`
        }{Status: http.StatusOK}, nil
    })

    huma.Register(api, huma.Operation{
        Method: http.MethodGet,
        Path:   "/get",
    }, func(ctx context.Context, input *struct {
        Session string `cookie:"session"`
    }) (*struct {
        Status int    `json:"status"`
        Body   string `json:"body"`
    }, error,
    ) {
        v := sessionManager.Get(ctx, "foo")
        if v == nil {
            return nil, huma.NewError(http.StatusInternalServerError, "foo does not exist in session")
        }

        return &struct {
            Status int    `json:"status"`
            Body   string `json:"body"`
        }{Status: http.StatusOK, Body: v.(string)}, nil
    })
}
danielgtaylor commented 1 day ago

@spa5k I'm not familiar with scs, but assuming you mean https://github.com/alexedwards/scs

the problem I'm having is that it's not considering the cookies/anything that is set after the next(ctx) is called

I think this would be the case with anything that tries to write headers (including cookies) to the response after data has already been written for the body. Even in the SCS example they have the session manager's Put(...) called within the handler itself and before any body is sent.

I think this is expected behavior, but I also understand what you are trying to do. You may have to do something like a defer in the handler which ensures the cookie is written before the body is serialized. You could make this generic by wrapping huma.Register with your own code that adds the defer, then using that function instead of huma.Register. Hope that helps!