From ef172b3b3771aef1044fb7f377e7c1cc90aa676e Mon Sep 17 00:00:00 2001 From: Aaron L Date: Wed, 7 Mar 2018 11:41:14 -0800 Subject: [PATCH] Extract logout to it's own module - This may seems silly but the functionality is shared between oauth2 and auth with no changes so it makes it nicer not to have an oauth2/logout route like before --- auth/auth.go | 37 -------------- auth/auth_test.go | 55 -------------------- config.go | 15 +++--- logout/logout.go | 62 +++++++++++++++++++++++ logout/logout_test.go | 113 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 99 deletions(-) create mode 100644 logout/logout.go create mode 100644 logout/logout_test.go diff --git a/auth/auth.go b/auth/auth.go index 2d4db98..5a4d24d 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -7,7 +7,6 @@ import ( "golang.org/x/crypto/bcrypt" - "github.com/pkg/errors" "github.com/volatiletech/authboss" ) @@ -33,21 +32,8 @@ func (a *Auth) Init(ab *authboss.Authboss) (err error) { return err } - var logoutRouteMethod func(string, http.Handler) - switch a.Authboss.Config.Modules.AuthLogoutMethod { - case "GET": - logoutRouteMethod = a.Authboss.Config.Core.Router.Get - case "POST": - logoutRouteMethod = a.Authboss.Config.Core.Router.Post - case "DELETE": - logoutRouteMethod = a.Authboss.Config.Core.Router.Delete - default: - return errors.Errorf("auth wants to register a logout route but was given an invalid method: %s", a.Authboss.Config.Modules.AuthLogoutMethod) - } - a.Authboss.Config.Core.Router.Get("/login", a.Authboss.Core.ErrorHandler.Wrap(a.LoginGet)) a.Authboss.Config.Core.Router.Post("/login", a.Authboss.Core.ErrorHandler.Wrap(a.LoginPost)) - logoutRouteMethod("/logout", a.Authboss.Core.ErrorHandler.Wrap(a.Logout)) return nil } @@ -125,26 +111,3 @@ func (a *Auth) LoginPost(w http.ResponseWriter, r *http.Request) error { } return a.Authboss.Core.Redirector.Redirect(w, r, ro) } - -// Logout a user -func (a *Auth) Logout(w http.ResponseWriter, r *http.Request) error { - logger := a.RequestLogger(r) - user, err := a.CurrentUser(w, r) - if err != nil { - return err - } - - logger.Infof("user %s logged out", user.GetPID()) - - authboss.DelSession(w, authboss.SessionKey) - authboss.DelSession(w, authboss.SessionLastAction) - authboss.DelSession(w, authboss.SessionHalfAuthKey) - authboss.DelCookie(w, authboss.CookieRemember) - - ro := authboss.RedirectOptions{ - Code: http.StatusTemporaryRedirect, - RedirectPath: a.Authboss.Paths.AuthLogoutOK, - Success: "You have been logged out", - } - return a.Authboss.Core.Redirector.Redirect(w, r, ro) -} diff --git a/auth/auth_test.go b/auth/auth_test.go index 4d23adf..78cab58 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -36,9 +36,6 @@ func TestAuthInit(t *testing.T) { if err := router.HasPosts("/login"); err != nil { t.Error(err) } - if err := router.HasDeletes("/logout"); err != nil { - t.Error(err) - } } func TestAuthGet(t *testing.T) { @@ -82,7 +79,6 @@ func testSetup() *testHarness { harness.storer = mocks.NewServerStorer() harness.ab.Paths.AuthLoginOK = "/login/ok" - harness.ab.Paths.AuthLogoutOK = "/logout/ok" harness.ab.Config.Core.BodyReader = harness.bodyReader harness.ab.Config.Core.Logger = mocks.Logger{} @@ -352,54 +348,3 @@ func TestAuthPostUserNotFound(t *testing.T) { t.Error("after should not have been called") } } - -func TestAuthLogout(t *testing.T) { - t.Parallel() - - h := testSetup() - h.storer.Users["test@test.com"] = &mocks.User{ - Email: "test@test.com", - Password: "$2a$10$IlfnqVyDZ6c1L.kaA/q3bu1nkAC6KukNUsizvlzay1pZPXnX2C9Ji", // hello world - } - - h.session.ClientValues[authboss.SessionKey] = "test@test.com" - h.session.ClientValues[authboss.SessionHalfAuthKey] = "true" - - cookies := mocks.NewClientRW() - cookies.ClientValues[authboss.CookieRemember] = "token" - h.ab.Config.Storage.CookieState = cookies - - r := mocks.Request("POST") - resp := httptest.NewRecorder() - w := h.ab.NewResponse(resp, r) - - var err error - r, err = h.ab.LoadClientState(w, r) - if err != nil { - t.Error(err) - } - - if err := h.auth.Logout(w, r); err != nil { - t.Error(err) - } - - if resp.Code != http.StatusTemporaryRedirect { - t.Error("response code wrong:", resp.Code) - } - if h.redirector.Options.RedirectPath != "/logout/ok" { - t.Error("redirect path was wrong:", h.redirector.Options.RedirectPath) - } - - if _, ok := h.session.ClientValues[authboss.SessionKey]; ok { - t.Error("want session key gone") - } - if _, ok := h.session.ClientValues[authboss.SessionHalfAuthKey]; ok { - t.Error("want session half auth key gone") - } - if _, ok := h.session.ClientValues[authboss.SessionLastAction]; ok { - t.Error("want session last action") - } - if _, ok := cookies.ClientValues[authboss.CookieRemember]; ok { - t.Error("want remember me cookies gone") - } -} diff --git a/config.go b/config.go index f1602df..2361dcf 100644 --- a/config.go +++ b/config.go @@ -14,8 +14,6 @@ type Config struct { // AuthLoginOK is the redirect path after a successful authentication. AuthLoginOK string - // AuthLogoutOK is the redirect path after a log out. - AuthLogoutOK string // ConfirmOK once a user has confirmed their account, where should they go ConfirmOK string @@ -26,6 +24,9 @@ type Config struct { // LockNotOK is a path to go to when the user fails LockNotOK string + // LogoutOK is the redirect path after a log out. + LogoutOK string + // RecoverOK is the redirect path after a successful recovery of a password. RecoverOK string @@ -40,9 +41,6 @@ type Config struct { // BCryptCost is the cost of the bcrypt password hashing function. BCryptCost int - // AuthLogoutMethod is the method the logout route should use (default should be DELETE) - AuthLogoutMethod string - // ExpireAfter controls the time an account is idle before being logged out // by the ExpireMiddleware. ExpireAfter time.Duration @@ -54,6 +52,9 @@ type Config struct { // LockDuration is how long an account is locked for. LockDuration time.Duration + // LogoutMethod is the method the logout route should use (default should be DELETE) + LogoutMethod string + // RegisterPreserveFields are fields used with registration that are to be rendered when // post fails in a normal way (for example validation errors), they will be passed // back in the data of the response under the key DataPreserve which will be a map[string]string. @@ -142,17 +143,17 @@ func (c *Config) Defaults() { c.Paths.Mount = "/auth" c.Paths.RootURL = "http://localhost:8080" c.Paths.AuthLoginOK = "/" - c.Paths.AuthLogoutOK = "/" c.Paths.ConfirmOK = "/" c.Paths.ConfirmNotOK = "/" + c.Paths.LogoutOK = "/" c.Paths.RecoverOK = "/" c.Paths.RegisterOK = "/" c.Modules.BCryptCost = bcrypt.DefaultCost - c.Modules.AuthLogoutMethod = "DELETE" c.Modules.ExpireAfter = 60 * time.Minute c.Modules.LockAfter = 3 c.Modules.LockWindow = 5 * time.Minute c.Modules.LockDuration = 5 * time.Hour + c.Modules.LogoutMethod = "DELETE" c.Modules.RecoverTokenDuration = time.Duration(24) * time.Hour } diff --git a/logout/logout.go b/logout/logout.go new file mode 100644 index 0000000..74ecc68 --- /dev/null +++ b/logout/logout.go @@ -0,0 +1,62 @@ +package logout + +import ( + "net/http" + + "github.com/pkg/errors" + "github.com/volatiletech/authboss" +) + +// Logout module +type Logout struct { + *authboss.Authboss +} + +// Init the module +func (l *Logout) Init(ab *authboss.Authboss) error { + l.Authboss = ab + + var logoutRouteMethod func(string, http.Handler) + switch l.Authboss.Config.Modules.LogoutMethod { + case "GET": + logoutRouteMethod = l.Authboss.Config.Core.Router.Get + case "POST": + logoutRouteMethod = l.Authboss.Config.Core.Router.Post + case "DELETE": + logoutRouteMethod = l.Authboss.Config.Core.Router.Delete + default: + return errors.Errorf("logout wants to register a logout route but was given an invalid method: %s", l.Authboss.Config.Modules.LogoutMethod) + } + + logoutRouteMethod("/logout", l.Authboss.Core.ErrorHandler.Wrap(l.Logout)) + + return nil +} + +// Logout the user +func (l *Logout) Logout(w http.ResponseWriter, r *http.Request) error { + logger := l.RequestLogger(r) + + // TODO(aarondl): Evaluate this log messages usefulness, there's no other reason + // to pull the user out of the context here. + user, err := l.CurrentUser(w, r) + if err != nil { + return err + } + + logger.Infof("user %s logged out", user.GetPID()) + + authboss.DelSession(w, authboss.SessionKey) + authboss.DelSession(w, authboss.SessionLastAction) + authboss.DelSession(w, authboss.SessionHalfAuthKey) + if l.Authboss.Config.Storage.CookieState != nil { + authboss.DelCookie(w, authboss.CookieRemember) + } + + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + RedirectPath: l.Authboss.Paths.LogoutOK, + Success: "You have been logged out", + } + return l.Authboss.Core.Redirector.Redirect(w, r, ro) +} diff --git a/logout/logout_test.go b/logout/logout_test.go new file mode 100644 index 0000000..348a668 --- /dev/null +++ b/logout/logout_test.go @@ -0,0 +1,113 @@ +package logout + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/volatiletech/authboss" + "github.com/volatiletech/authboss/internal/mocks" +) + +func TestLogout(t *testing.T) { + t.Parallel() + + ab := authboss.New() + + router := &mocks.Router{} + errHandler := &mocks.ErrorHandler{} + ab.Config.Core.Router = router + ab.Config.Core.ErrorHandler = errHandler + + l := &Logout{} + if err := l.Init(ab); err != nil { + t.Fatal(err) + } + + if err := router.HasDeletes("/logout"); err != nil { + t.Error(err) + } +} + +type testHarness struct { + logout *Logout + ab *authboss.Authboss + + redirector *mocks.Redirector + session *mocks.ClientStateRW + cookies *mocks.ClientStateRW + storer *mocks.ServerStorer +} + +func testSetup() *testHarness { + harness := &testHarness{} + + harness.ab = authboss.New() + harness.redirector = &mocks.Redirector{} + harness.session = mocks.NewClientRW() + harness.cookies = mocks.NewClientRW() + harness.storer = mocks.NewServerStorer() + + harness.ab.Paths.LogoutOK = "/logout/ok" + + harness.ab.Config.Core.Logger = mocks.Logger{} + harness.ab.Config.Core.Redirector = harness.redirector + harness.ab.Config.Storage.SessionState = harness.session + harness.ab.Config.Storage.CookieState = harness.cookies + harness.ab.Config.Storage.Server = harness.storer + + harness.logout = &Logout{harness.ab} + + return harness +} + +func TestLogoutLogout(t *testing.T) { + t.Parallel() + + h := testSetup() + + h.session.ClientValues[authboss.SessionKey] = "test@test.com" + h.session.ClientValues[authboss.SessionHalfAuthKey] = "true" + h.session.ClientValues[authboss.SessionLastAction] = time.Now().UTC().Format(time.RFC3339) + h.cookies.ClientValues[authboss.CookieRemember] = "token" + + r := mocks.Request("POST") + resp := httptest.NewRecorder() + w := h.ab.NewResponse(resp, r) + + // This enables the logging portion, which is debatable-y not useful in a log out method + user := &mocks.User{Email: "test@test.com"} + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + + var err error + r, err = h.ab.LoadClientState(w, r) + if err != nil { + t.Error(err) + } + + if err := h.logout.Logout(w, r); err != nil { + t.Fatal(err) + } + + if resp.Code != http.StatusTemporaryRedirect { + t.Error("response code wrong:", resp.Code) + } + if h.redirector.Options.RedirectPath != "/logout/ok" { + t.Error("redirect path was wrong:", h.redirector.Options.RedirectPath) + } + + if _, ok := h.session.ClientValues[authboss.SessionKey]; ok { + t.Error("want session key gone") + } + if _, ok := h.session.ClientValues[authboss.SessionHalfAuthKey]; ok { + t.Error("want session half auth key gone") + } + if _, ok := h.session.ClientValues[authboss.SessionLastAction]; ok { + t.Error("want session last action") + } + if _, ok := h.cookies.ClientValues[authboss.CookieRemember]; ok { + t.Error("want remember me cookies gone") + } +}