Finish TODOs

authboss.go

@@ -6,7 +6,12 @@ races without having to think about how to store passwords or remember tokens.
 */
 package authboss
 
-import "github.com/pkg/errors"
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	"golang.org/x/crypto/bcrypt"
+)
 
 // Authboss contains a configuration and other details for running.
 type Authboss struct {
@@ -43,52 +48,30 @@ func (a *Authboss) Init(modulesToLoad ...string) error {
 	return nil
 }
 
-/*
-TODO(aarondl): Fixup
-
-UpdatePassword should be called to recalculate hashes and do any cleanup
-that should occur on password resets. Updater should return an error if the
-update to the user failed (for reasons say like validation, duplicate
-primary key, etc...). In that case the cleanup will not be performed.
-
-The w and r parameters are for establishing session and cookie storers.
-
-The ptPassword parameter is the new password to update to. updater is called
-regardless if this is empty or not, but if it is empty, it will not set a new
-password before calling updater.
-
-The user parameter is the user struct which will have it's
-Password string/sql.NullString value set to the new bcrypted password. Therefore
-it must be passed in as a pointer with the Password field exported or an error
-will be returned.
-
-The error returned is returned either from the updater if that produced an error
-or from the cleanup routines.
-func (a *Authboss) UpdatePassword(w http.ResponseWriter, r *http.Request,
-	ptPassword string, user Storer, updater func() error) error {
-
-	/*updatePwd := len(ptPassword) > 0
-
-	if updatePwd {
-		pass, err := bcrypt.GenerateFromPassword([]byte(ptPassword), a.BCryptCost)
-		if err != nil {
-			return err
-		}
-
-		user.PutPassword(r.Context(),
-	}
-
-	if err := updater(); err != nil {
+// UpdatePassword updates the password field of a user using the same semantics
+// that register/auth do to create and verify passwords. It saves this using the storer.
+//
+// In addition to that, it also invalidates any remember me tokens, if the storer supports
+// that kind of operation.
+//
+// If it's also desirable to log the user out, use: authboss.DelKnown(Session|Cookie)
+func (a *Authboss) UpdatePassword(ctx context.Context, user AuthableUser, newPassword string) error {
+	pass, err := bcrypt.GenerateFromPassword([]byte(newPassword), a.Config.Modules.BCryptCost)
+	if err != nil {
 		return err
 	}
 
-	if !updatePwd {
+	user.PutPassword(string(pass))
+
+	storer := a.Config.Storage.Server
+	if err := storer.Save(ctx, user); err != nil {
+		return err
+	}
+
+	rmStorer, ok := storer.(RememberingServerStorer)
+	if !ok {
 		return nil
 	}
 
-	return a.Events.FireAfter(EventPasswordReset, r.Context())
-	// TODO(aarondl): Fix
-	return errors.New("not implemented")
+	return rmStorer.DelRememberTokens(user.GetPID())
 }
-
-*/

authboss_test.go

@@ -1,6 +1,7 @@
 package authboss
 
 import (
+	"context"
 	"testing"
 )
 
@@ -15,88 +16,19 @@ func TestAuthBossInit(t *testing.T) {
 }
 
 func TestAuthbossUpdatePassword(t *testing.T) {
-	t.Skip("TODO(aarondl): Implement")
-	/*
-		t.Parallel()
+	t.Parallel()
 
-		ab := New()
-		session := mockClientStore{}
-		cookies := mockClientStore{}
-		ab.SessionStoreMaker = newMockClientStoreMaker(session)
-		ab.CookieStoreMaker = newMockClientStoreMaker(cookies)
+	user := &mockUser{}
+	storer := newMockServerStorer()
 
-		called := false
-		ab.Events.After(EventPasswordReset, func(ctx context.Context) error {
-			called = true
-			return nil
-		})
+	ab := New()
+	ab.Config.Storage.Server = storer
 
-		user1 := struct {
-			Password string
-		}{}
-		user2 := struct {
-			Password sql.NullString
-		}{}
+	if err := ab.UpdatePassword(context.Background(), user, "hello world"); err != nil {
+		t.Error(err)
+	}
 
-		r, _ := http.NewRequest("GET", "http://localhost", nil)
-
-		called = false
-		err := ab.UpdatePassword(nil, r, "newpassword", &user1, func() error { return nil })
-		if err != nil {
-			t.Error(err)
-		}
-
-		if len(user1.Password) == 0 {
-			t.Error("Password not updated")
-		}
-		if !called {
-			t.Error("Events should have been called.")
-		}
-
-		called = false
-		err = ab.UpdatePassword(nil, r, "newpassword", &user2, func() error { return nil })
-		if err != nil {
-			t.Error(err)
-		}
-
-		if !user2.Password.Valid || len(user2.Password.String) == 0 {
-			t.Error("Password not updated")
-		}
-		if !called {
-			t.Error("Events should have been called.")
-		}
-
-		called = false
-		oldPassword := user1.Password
-		err = ab.UpdatePassword(nil, r, "", &user1, func() error { return nil })
-		if err != nil {
-			t.Error(err)
-		}
-
-		if user1.Password != oldPassword {
-			t.Error("Password not updated")
-		}
-		if called {
-			t.Error("Events should not have been called")
-		}
-	*/
-}
-
-func TestAuthbossUpdatePasswordFail(t *testing.T) {
-	t.Skip("TODO(aarondl): Implement")
-	/*
-		t.Parallel()
-
-		ab := New()
-
-		user1 := struct {
-			Password string
-		}{}
-
-		anErr := errors.New("anError")
-		err := ab.UpdatePassword(nil, nil, "update", &user1, func() error { return anErr })
-		if err != anErr {
-			t.Error("Expected an specific error:", err)
-		}
-	*/
+	if len(user.Password) == 0 {
+		t.Error("password was not updated")
+	}
 }

client_state.go

@@ -221,6 +221,20 @@ func (c *ClientStateResponseWriter) putClientState() error {
 	return nil
 }
 
+// DelKnownSession deletes all known session variables, effectively
+// logging a user out.
+func DelKnownSession(w http.ResponseWriter) {
+	DelSession(w, SessionKey)
+	DelSession(w, SessionLastAction)
+	DelSession(w, SessionHalfAuthKey)
+}
+
+// DelKnownCookie deletes all known cookie variables, which can be used
+// to delete remember me pieces.
+func DelKnownCookie(w http.ResponseWriter) {
+	DelCookie(w, CookieRemember)
+}
+
 // PutSession puts a value into the session
 func PutSession(w http.ResponseWriter, key, val string) {
 	putState(w, CTXKeySessionState, key, val)

confirm/confirm.go

@@ -229,7 +229,6 @@ func (c *Confirm) Get(w http.ResponseWriter, r *http.Request) error {
 //
 // Panics if the user was not able to be loaded in order to allow a panic handler to show
 // a nice error page, also panics if it failed to redirect for whatever reason.
-// TODO(aarondl): Document this middleware better
 func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

context_test.go

@@ -19,8 +19,10 @@ func loadClientStateP(ab *Authboss, w http.ResponseWriter, r *http.Request) *htt
 func testSetupContext() (*Authboss, *http.Request) {
 	ab := New()
 	ab.Storage.SessionState = newMockClientStateRW(SessionKey, "george-pid")
-	ab.Storage.Server = mockServerStorer{
-		"george-pid": mockUser{Email: "george-pid", Password: "unreadable"},
+	ab.Storage.Server = &mockServerStorer{
+		Users: map[string]*mockUser{
+			"george-pid": &mockUser{Email: "george-pid", Password: "unreadable"},
+		},
 	}
 	r := httptest.NewRequest("GET", "/", nil)
 	w := ab.NewResponse(httptest.NewRecorder())
@@ -29,9 +31,9 @@ func testSetupContext() (*Authboss, *http.Request) {
 	return ab, r
 }
 
-func testSetupContextCached() (*Authboss, mockUser, *http.Request) {
+func testSetupContextCached() (*Authboss, *mockUser, *http.Request) {
 	ab := New()
-	wantUser := mockUser{Email: "george-pid", Password: "unreadable"}
+	wantUser := &mockUser{Email: "george-pid", Password: "unreadable"}
 	req := httptest.NewRequest("GET", "/", nil)
 	ctx := context.WithValue(req.Context(), CTXKeyPID, "george-pid")
 	ctx = context.WithValue(ctx, CTXKeyUser, wantUser)
@@ -43,7 +45,7 @@ func testSetupContextCached() (*Authboss, mockUser, *http.Request) {
 func testSetupContextPanic() *Authboss {
 	ab := New()
 	ab.Storage.SessionState = newMockClientStateRW(SessionKey, "george-pid")
-	ab.Storage.Server = mockServerStorer{}
+	ab.Storage.Server = &mockServerStorer{}
 
 	return ab
 }
@@ -201,8 +203,8 @@ func TestLoadCurrentUser(t *testing.T) {
 		t.Error("got:", got)
 	}
 
-	want := user.(mockUser)
-	got := r.Context().Value(CTXKeyUser).(mockUser)
+	want := user.(*mockUser)
+	got := r.Context().Value(CTXKeyUser).(*mockUser)
 	if got != want {
 		t.Errorf("users mismatched:\nwant: %#v\ngot: %#v", want, got)
 	}
@@ -218,7 +220,7 @@ func TestLoadCurrentUserContext(t *testing.T) {
 		t.Error(err)
 	}
 
-	got := user.(mockUser)
+	got := user.(*mockUser)
 	if got != wantUser {
 		t.Errorf("users mismatched:\nwant: %#v\ngot: %#v", wantUser, got)
 	}

defaults/log_mailer_test.go

@@ -16,7 +16,7 @@ func TestMailer(t *testing.T) {
 	mailServer := &bytes.Buffer{}
 	mailer := NewLogMailer(mailServer)
 
-	err := mailer.Send(context.TODO(), authboss.Email{
+	err := mailer.Send(context.Background(), authboss.Email{
 		To:       []string{"some@email.com", "a@a.com"},
 		ToNames:  []string{"Jake", "Noname"},
 		From:     "some@guy.com",

internal/mocks/mocks.go

@@ -192,12 +192,21 @@ func (s *ServerStorer) DelRememberTokens(key string) error {
 
 // UseRememberToken if it exists, deleting it in the process
 func (s *ServerStorer) UseRememberToken(givenKey, token string) (err error) {
-	if arr, ok := s.RMTokens[givenKey]; ok {
-		for _, tok := range arr {
-			if tok == token {
+	arr, ok := s.RMTokens[givenKey]
+	if !ok {
+		return authboss.ErrTokenNotFound
+	}
+
+	for i, tok := range arr {
+		if tok == token {
+			if len(arr) == 1 {
 				delete(s.RMTokens, givenKey)
 				return nil
 			}
+
+			arr[i] = arr[len(arr)-1]
+			s.RMTokens[givenKey] = arr[:len(arr)-2]
+			return nil
 		}
 	}
 

lock/lock.go

@@ -36,7 +36,7 @@ func (l *Lock) Init(ab *authboss.Authboss) error {
 	l.Authboss = ab
 
 	l.Events.Before(authboss.EventAuth, l.BeforeAuth)
-	l.Events.Before(authboss.EventOAuth, l.BeforeAuth)
+	l.Events.Before(authboss.EventOAuth2, l.BeforeAuth)
 	l.Events.After(authboss.EventAuth, l.AfterAuthSuccess)
 	l.Events.After(authboss.EventAuthFail, l.AfterAuthFail)
 
@@ -160,7 +160,6 @@ func (l *Lock) Unlock(ctx context.Context, key string) error {
 //
 // Panics if the user was not able to be loaded in order to allow a panic handler to show
 // a nice error page, also panics if it failed to redirect for whatever reason.
-// TODO(aarondl): Document this middleware better
 func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

logout/logout.go

@@ -38,21 +38,15 @@ func (l *Logout) Init(ab *authboss.Authboss) error {
 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(r)
-	if err != nil {
-		return err
+	if err == nil && user != nil {
+		logger.Infof("user %s logged out", user.GetPID())
+	} else {
+		logger.Info("user (unknown) logged out")
 	}
 
-	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)
-	}
+	authboss.DelKnownSession(w)
+	authboss.DelKnownCookie(w)
 
 	ro := authboss.RedirectOptions{
 		Code:         http.StatusTemporaryRedirect,

mocks_test.go

@@ -15,10 +15,20 @@ type mockUser struct {
 	Password string
 }
 
-type mockServerStorer map[string]mockUser
+func newMockServerStorer() *mockServerStorer {
+	return &mockServerStorer{
+		Users:  make(map[string]*mockUser),
+		Tokens: make(map[string][]string),
+	}
+}
 
-func (m mockServerStorer) Load(ctx context.Context, key string) (User, error) {
-	u, ok := m[key]
+type mockServerStorer struct {
+	Users  map[string]*mockUser
+	Tokens map[string][]string
+}
+
+func (m *mockServerStorer) Load(ctx context.Context, key string) (User, error) {
+	u, ok := m.Users[key]
 	if !ok {
 		return nil, ErrUserNotFound
 	}
@@ -26,26 +36,53 @@ func (m mockServerStorer) Load(ctx context.Context, key string) (User, error) {
 	return u, nil
 }
 
-func (m mockServerStorer) Save(ctx context.Context, user User) error {
-	pid := user.GetPID()
-	m[pid] = user.(mockUser)
+func (m *mockServerStorer) Save(ctx context.Context, user User) error {
+	u := user.(*mockUser)
+	m.Users[u.Email] = u
 
 	return nil
 }
 
-func (m mockUser) PutPID(email string) {
+func (m *mockServerStorer) AddRememberToken(pid, token string) error {
+	m.Tokens[pid] = append(m.Tokens[pid], token)
+	return nil
+}
+
+func (m *mockServerStorer) DelRememberTokens(pid string) error {
+	delete(m.Tokens, pid)
+	return nil
+}
+
+func (m *mockServerStorer) UseRememberToken(pid, token string) error {
+	arr, ok := m.Tokens[pid]
+	if !ok {
+		return ErrTokenNotFound
+	}
+
+	for i, tok := range arr {
+		if tok == token {
+			arr[i] = arr[len(arr)-1]
+			m.Tokens[pid] = arr[:len(arr)-2]
+			return nil
+		}
+	}
+
+	return ErrTokenNotFound
+}
+
+func (m *mockUser) PutPID(email string) {
 	m.Email = email
 }
 
-func (m mockUser) PutPassword(password string) {
+func (m *mockUser) PutPassword(password string) {
 	m.Password = password
 }
 
-func (m mockUser) GetPID() (email string) {
+func (m *mockUser) GetPID() (email string) {
 	return m.Email
 }
 
-func (m mockUser) GetPassword() (password string) {
+func (m *mockUser) GetPassword() (password string) {
 	return m.Password
 }
 

remember/remember.go

@@ -35,7 +35,7 @@ func (r *Remember) Init(ab *authboss.Authboss) error {
 	r.Authboss = ab
 
 	r.Events.After(authboss.EventAuth, r.RememberAfterAuth)
-	r.Events.After(authboss.EventOAuth, r.RememberAfterAuth)
+	r.Events.After(authboss.EventOAuth2, r.RememberAfterAuth)
 	r.Events.After(authboss.EventPasswordReset, r.AfterPasswordReset)
 
 	return nil
@@ -66,50 +66,6 @@ func (r *Remember) RememberAfterAuth(w http.ResponseWriter, req *http.Request, h
 	return false, nil
 }
 
-/*
-// TODO(aarondl): Either discard or make this useful later after oauth2
-// afterOAuth is called after oauth authentication is successful.
-// Has to pander to horrible state variable packing to figure out if we want
-// to be remembered.
-func (r *Remember) afterOAuth(ctx *authboss.Context) error {
-	sessValues, ok := ctx.SessionStorer.Get(authboss.SessionOAuth2Params)
-	if !ok {
-		return nil
-	}
-
-	var values map[string]string
-	if err := json.Unmarshal([]byte(sessValues), &values); err != nil {
-		return err
-	}
-
-	val, ok := values[authboss.CookieRemember]
-	should := ok && val == "true"
-
-	if !should {
-		return nil
-	}
-
-	if ctx.User == nil {
-		return errUserMissing
-	}
-
-	uid, err := ctx.User.StringErr(authboss.StoreOAuth2Provider)
-	if err != nil {
-		return err
-	}
-	provider, err := ctx.User.StringErr(authboss.StoreOAuth2Provider)
-	if err != nil {
-		return err
-	}
-
-	if _, err := r.new(ctx.CookieStorer, uid+";"+provider); err != nil {
-		return errors.Wrap(err, "failed to create remember token")
-	}
-
-	return nil
-}
-*/
-
 // Middleware automatically authenticates users if they have remember me tokens
 // If the user has been loaded already, it returns early
 func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {