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

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

Closed
frederikhors opened this issue Jan 11, 2019 · 1 comment
Closed

Comments

@frederikhors
Copy link
Contributor

frederikhors commented Jan 11, 2019

Issue opened for the creation of a wiki page that summarizes the doubts and problems for newbies (#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
Copy link
Member

aarondl commented Jan 12, 2019

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.

@aarondl aarondl closed this as completed Jan 12, 2019
@frederikhors frederikhors changed the title ab.CurrentUser() vs Ab.LoadCurrentUser(). Save DB queries. What is the right one to use? ab.CurrentUser() vs Ab.LoadCurrentUser(). What is the right one to use? Jan 12, 2019
@frederikhors frederikhors changed the title ab.CurrentUser() vs Ab.LoadCurrentUser(). What is the right one to use? CurrentUser() vs LoadCurrentUser(). What is the right one to use? Jan 12, 2019
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

2 participants