volatiletech / authboss

The boss of http auth.
MIT License
3.85k stars 207 forks source link

CurrentUser() vs LoadCurrentUser(). What is the right one to use? #220

Closed frederikhors closed 5 years ago

frederikhors commented 5 years ago

Issue opened for the creation of a wiki page that summarizes the doubts and problems for newbies (https://github.com/volatiletech/authboss/issues/210).

I'm using https://github.com/justinas/nosurf for CSRF (like in authboss-sample after all).

I'm using the below code to create the "X-CSRF-Token" to send to the javascript app.

func CSRFCookie(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                // if user, _ := Ab.CurrentUser(r); user != nil { // <- the question is about this line
        if userInter, err := Ab.LoadCurrentUser(&r); userInter != nil && err == nil {
            cookie := &http.Cookie{
                Name:     "X-CSRF-Token",
                Value:    nosurf.Token(r),
            }
            http.SetCookie(w, cookie)
        }
        handler.ServeHTTP(w, r)
    })
}

And this is the main():

func main() {
        setupAuthboss(ab)
    r := chi.NewRouter()
        r.Use(middleware.RequestID, middleware.RealIP, middleware.Logger, middleware.Recoverer)
        r.Use(nosurfing, ab.LoadClientStateMiddleware, remember.Middleware(ab), CSRFCookie)

    r.Group(func(r chi.Router) {
            r.Use(authboss.Middleware2(ab, 0, 1))
        r.Get("/", func(w http.ResponseWriter, r *http.Request) {
            w.Write([]byte("welcome"))
        })
        })

    r.Group(func(r chi.Router) {
        r.Use(auth.DataInjector)
        r.Use(authboss.ModuleListMiddleware(ab))
        r.Mount(AUTH_URL, http.StripPrefix(AUTH_URL, ab.Config.Core.Router))
    })
    http.ListenAndServe(":3000", r)
}

As you can read I'm using

if userInter, err := Ab.LoadCurrentUser(&r); userInter != nil && err == nil {

to query for the User and not

if user, _ := ab.CurrentUser(r); user != nil {

as you suggest in Readme because ab.CurrentUser(r) is making a query, the other one not.

My newbie question: WHY?

What is the right thing to do?

aarondl commented 5 years ago

Both make a query. LoadCurrentUser calls CurrentUser under the hood anyway. The difference is the parameters. One takes a **http.Request that's a pointer to a pointer meaning that the original pointer that you're passing in can be modified. This allows it to replace the http.Request with one that has the user loaded into it's request.Context. Which isn't possible with CurrentUser.

It depends on your needs which one you're going to call. The advantage being that LoadCurrentUser caches it in the request. But if you're not a middleware you're the endpoint handler for example then there's no point to calling LoadCurrentUser, just use CurrentUser.