diff --git a/authboss.go b/authboss.go index 49247cf..8c0bf27 100644 --- a/authboss.go +++ b/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()) } - -*/ diff --git a/authboss_test.go b/authboss_test.go index 9683488..0378fb3 100644 --- a/authboss_test.go +++ b/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") + } } diff --git a/client_state.go b/client_state.go index 05043a1..f3c4a4d 100644 --- a/client_state.go +++ b/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) diff --git a/confirm/confirm.go b/confirm/confirm.go index 26ccf72..02e4894 100644 --- a/confirm/confirm.go +++ b/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) { diff --git a/context_test.go b/context_test.go index 7c529a4..d269d92 100644 --- a/context_test.go +++ b/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) } diff --git a/defaults/log_mailer_test.go b/defaults/log_mailer_test.go index 72c06de..473a68e 100644 --- a/defaults/log_mailer_test.go +++ b/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", diff --git a/internal/mocks/mocks.go b/internal/mocks/mocks.go index aff508d..3f771bc 100644 --- a/internal/mocks/mocks.go +++ b/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 } } diff --git a/lock/lock.go b/lock/lock.go index d4f2769..e5f75db 100644 --- a/lock/lock.go +++ b/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) { diff --git a/logout/logout.go b/logout/logout.go index 6c00444..ee7aaca 100644 --- a/logout/logout.go +++ b/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, diff --git a/mocks_test.go b/mocks_test.go index 7136480..0905acd 100644 --- a/mocks_test.go +++ b/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 } diff --git a/remember/remember.go b/remember/remember.go index fe44a09..052923e 100644 --- a/remember/remember.go +++ b/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 {