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

@@ -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)
-}

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")
-	}
-}

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
 }

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)
+}

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")
+	}
+}