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

Do not trigger panic when there is nothing in the session on lock.Middleware #233

Open
raven-chen opened this issue May 17, 2019 · 1 comment

Comments

@raven-chen
Copy link

When GET /auth/login. the request goes into middleware first. so it is always panic since PostAuth is not yet get called. thus nothing is in the session.

I have read this #192. To handle this I have to wrap the lock.Middleware like this

func maybeCheckLockStatus(ab *authboss.Authboss) func(h http.Handler) http.Handler {
	return func(h http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			if user := (*r).Context().Value(authboss.CTXKeyUser); user != nil {
				lock.Middleware(ab)(h)
			}

			h.ServeHTTP(w, r)
		})
	}
}

I don't see the reason why we need this rather than skip the locking feature

There's no reason that a route that is not a protected resource should ever run many middlewares

Do you conditionally load middlewares in your projects?

And the last, the error notice of this is quite vague. it just a stack of error message with GET /auth/login -> 500 Internal Server Error (panic: user not found). It took me hours to figure out what was wrong.

@aarondl
Copy link
Member

aarondl commented May 20, 2019

Any route that wants to deal with users and locking users out (ie. authenticated routes only) should be the ones that have this middleware applied. In my projects using authboss I have middleware sets that routes get added to, if it's an unauthenticated route, lock/loadclientstate/remember all of these middlewares are excluded.

I'd be happy if someone were to add some additional context to the panic, since it's one you should never technically see in production unless things have been configured incorrectly we can add a bit of debugging text around it asking why this middleware is being called before state is loaded or what have you.

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