From 982025bbc3d0a2c3f7064636563550f9d7aa6bb0 Mon Sep 17 00:00:00 2001 From: Aaron L Date: Tue, 27 Feb 2018 07:14:30 -0800 Subject: [PATCH] Finish implementing and testing confirm - Rejig tests to remember to test the smtp mailer --- config.go | 6 + confirm/confirm.go | 291 +++++++++++++++----------- confirm/confirm_test.go | 383 ++++++++++++++++++++++++++++++++++- context.go | 4 +- defaults/error_handler.go | 2 +- defaults/log_mailer_test.go | 17 -- defaults/smtp_mailer_test.go | 24 +++ internal/mocks/mocks.go | 29 +++ response.go | 43 ++-- storage.go | 2 +- user.go | 2 + values.go | 37 +++- 12 files changed, 667 insertions(+), 173 deletions(-) create mode 100644 defaults/smtp_mailer_test.go diff --git a/config.go b/config.go index 97e0ffd..5db69a3 100644 --- a/config.go +++ b/config.go @@ -17,6 +17,12 @@ type Config struct { // 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 + // ConfirmNotOK is used by the middleware, when a user is still supposed to + // confirm their account, this is where they should be redirected to. + ConfirmNotOK string + // RecoverOK is the redirect path after a successful recovery of a password. RecoverOK string diff --git a/confirm/confirm.go b/confirm/confirm.go index 40d63df..3a15ced 100644 --- a/confirm/confirm.go +++ b/confirm/confirm.go @@ -3,8 +3,8 @@ package confirm import ( "context" - "crypto/md5" "crypto/rand" + "crypto/sha512" "encoding/base64" "fmt" "net/http" @@ -14,45 +14,25 @@ import ( "github.com/pkg/errors" "github.com/volatiletech/authboss" - "github.com/volatiletech/authboss/internal/response" ) -// Storer and FormValue constants const ( - StoreConfirmToken = "confirm_token" - StoreConfirmed = "confirmed" + // PageConfirm is only really used for the BodyReader + PageConfirm = "confirm" + // EmailConfirmHTML is the name of the html template for e-mails + EmailConfirmHTML = "confirm_html" + // EmailConfirmTxt is the name of the text template for e-mails + EmailConfirmTxt = "confirm_txt" + + // FormValueConfirm is the name of the form value for FormValueConfirm = "cnf" - tplConfirmHTML = "confirm_email.html.tpl" - tplConfirmText = "confirm_email.txt.tpl" + // DataConfirmURL is the name of the e-mail template variable + // that gives the url to send to the user for confirmation. + DataConfirmURL = "url" ) -var ( - errUserMissing = errors.New("after registration user must be loaded") -) - -// ConfirmStoreLoader allows lookup of users by different parameters. -type ConfirmStoreLoader interface { - authboss.StoreLoader - - // ConfirmUser looks up a user by a confirm token. See confirm module for - // attribute names. If the token is not found in the data store, - // simply return nil, ErrUserNotFound. - LoadByConfirmToken(confirmToken string) (ConfirmStorer, error) -} - -// ConfirmStorer defines attributes for the confirm module. -type ConfirmStorer interface { - authboss.Storer - - PutConfirmed(ctx context.Context, confirmed bool) error - PutConfirmToken(ctx context.Context, token string) error - - GetConfirmed(ctx context.Context) (confirmed bool, err error) - GetConfirmToken(ctx context.Context) (token string, err error) -} - func init() { authboss.RegisterModule("confirm", &Confirm{}) } @@ -62,134 +42,215 @@ type Confirm struct { *authboss.Authboss } -// Initialize the module -func (c *Confirm) Initialize(ab *authboss.Authboss) (err error) { +// Init module +func (c *Confirm) Init(ab *authboss.Authboss) (err error) { c.Authboss = ab - c.Events.After(authboss.EventGetUser, func(ctx context.Context) error { - _, err := c.beforeGet(ctx) + if err = c.Authboss.Config.Core.MailRenderer.Load(EmailConfirmHTML, EmailConfirmTxt); err != nil { return err - }) - c.Events.Before(authboss.EventAuth, c.beforeGet) - c.Events.After(authboss.EventRegister, c.afterRegister) + } + + c.Authboss.Config.Core.Router.Get("/confirm", c.Authboss.Config.Core.ErrorHandler.Wrap(c.Get)) + + c.Events.Before(authboss.EventAuth, c.PreventAuth) + c.Events.After(authboss.EventRegister, c.StartConfirmationWeb) return nil } -// Routes for the module -func (c *Confirm) Routes() authboss.RouteTable { - return authboss.RouteTable{ - "/confirm": c.confirmHandler, +// PreventAuth stops the EventAuth from succeeding when a user is not confirmed +// This relies on the fact that the context holds the user at this point in time +// loaded by the auth module (or something else). +func (c *Confirm) PreventAuth(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) { + logger := c.Authboss.RequestLogger(r) + + user, err := c.Authboss.CurrentUser(w, r) + if err != nil { + return false, err } + + cuser := authboss.MustBeConfirmable(user) + if cuser.GetConfirmed() { + logger.Infof("user %s was confirmed, allowing auth", user.GetPID()) + return false, nil + } + + logger.Infof("user %s was not confirmed, preventing auth", user.GetPID()) + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + RedirectPath: c.Authboss.Config.Paths.ConfirmNotOK, + Failure: "Your account has not been confirmed, please check your e-mail.", + } + return true, c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) } -// Templates returns the list of templates required by this module -func (c *Confirm) Templates() []string { - return []string{tplConfirmHTML, tplConfirmText} +// StartConfirmationWeb hijacks a request and forces a user to be confirmed first +// it's assumed that the current user is loaded into the request context. +func (c *Confirm) StartConfirmationWeb(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) { + user, err := c.Authboss.CurrentUser(w, r) + if err != nil { + return false, err + } + + cuser := authboss.MustBeConfirmable(user) + if err = c.StartConfirmation(r.Context(), cuser, true); err != nil { + return false, err + } + + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + RedirectPath: c.Authboss.Config.Paths.ConfirmNotOK, + Success: "Please verify your account, an e-mail has been sent to you.", + } + return true, c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) } -func (c *Confirm) beforeGet(ctx context.Context) (authboss.Interrupt, error) { - if confirmed, err := ctx.User.BoolErr(StoreConfirmed); err != nil { - return authboss.InterruptNone, err - } else if !confirmed { - return authboss.InterruptAccountNotConfirmed, nil - } +// StartConfirmation begins confirmation on a user by setting them to require confirmation +// via a created token, and optionally sending them an e-mail. +func (c *Confirm) StartConfirmation(ctx context.Context, user authboss.ConfirmableUser, sendEmail bool) error { + logger := c.Authboss.Logger(ctx) - return authboss.InterruptNone, nil -} - -// AfterRegister ensures the account is not activated. -func (c *Confirm) afterRegister(ctx context.Context) error { - if ctx.User == nil { - return errUserMissing - } - - token := make([]byte, 32) - if _, err := rand.Read(token); err != nil { - return err - } - sum := md5.Sum(token) - - ctx.User[StoreConfirmToken] = base64.StdEncoding.EncodeToString(sum[:]) - - if err := ctx.SaveUser(); err != nil { - return err - } - - email, err := ctx.User.StringErr(authboss.StoreEmail) + hash, token, err := GenerateToken() if err != nil { return err } - goConfirmEmail(c, ctx, email, base64.URLEncoding.EncodeToString(token)) + user.PutConfirmed(false) + user.PutConfirmToken(hash) + + logger.Infof("generated new confirm token for user: %s", user.GetPID()) + if err := c.Authboss.Config.Storage.Server.Save(ctx, user); err != nil { + return errors.Wrap(err, "failed to save user during StartConfirmation, user data may be in weird state") + } + + goConfirmEmail(c, ctx, user.GetEmail(), token) return nil } +// This is here so it can be mocked out by a test var goConfirmEmail = func(c *Confirm, ctx context.Context, to, token string) { - if ctx.MailMaker != nil { - c.confirmEmail(ctx, to, token) - } else { - go c.confirmEmail(ctx, to, token) - } + go c.SendConfirmEmail(ctx, to, token) } -// confirmEmail sends a confirmation e-mail. -func (c *Confirm) confirmEmail(ctx context.Context, to, token string) { - p := path.Join(c.MountPath, "confirm") - url := fmt.Sprintf("%s%s?%s=%s", c.RootURL, p, url.QueryEscape(FormValueConfirm), url.QueryEscape(token)) +// SendConfirmEmail sends a confirmation e-mail to a user +func (c *Confirm) SendConfirmEmail(ctx context.Context, to, token string) { + logger := c.Authboss.Logger(ctx) + + p := path.Join(c.Config.Paths.Mount, "confirm") + url := fmt.Sprintf("%s%s?%s=%s", c.Paths.RootURL, p, url.QueryEscape(FormValueConfirm), url.QueryEscape(token)) email := authboss.Email{ To: []string{to}, - From: c.EmailFrom, - Subject: c.EmailSubjectPrefix + "Confirm New Account", + From: c.Config.Mail.From, + Subject: c.Config.Mail.SubjectPrefix + "Confirm New Account", } - err := response.Email(ctx.Mailer, email, c.emailHTMLTemplates, tplConfirmHTML, c.emailTextTemplates, tplConfirmText, url) - if err != nil { - fmt.Fprintf(ctx.LogWriter, "confirm: Failed to send e-mail: %v", err) + logger.Infof("sending confirm e-mail to: %s", to) + + ro := authboss.EmailResponseOptions{ + Data: authboss.NewHTMLData(DataConfirmURL, url), + HTMLTemplate: EmailConfirmHTML, + TextTemplate: EmailConfirmTxt, + } + if err := c.Authboss.Email(ctx, email, ro); err != nil { + logger.Errorf("failed to send confirm e-mail to %s: %+v", to, err) } } -func (c *Confirm) confirmHandler(w http.ResponseWriter, r *http.Request) error { - token := r.FormValue(FormValueConfirm) - if len(token) == 0 { - return authboss.ClientDataErr{Name: FormValueConfirm} - } +// Get is a request that confirms a user with a valid token +func (c *Confirm) Get(w http.ResponseWriter, r *http.Request) error { + logger := c.RequestLogger(r) - toHash, err := base64.URLEncoding.DecodeString(token) + validator, err := c.Authboss.Config.Core.BodyReader.Read(PageConfirm, r) if err != nil { - return authboss.ErrAndRedirect{ - Location: "/", Err: errors.Wrapf(err, "token failed to decode %q", token), - } + return err } - sum := md5.Sum(toHash) + if errs := validator.Validate(); errs != nil { + logger.Infof("validation failed in Confirm.Get, this typically means a bad token: %+v", errs) + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + Failure: "Invalid confirm token.", + RedirectPath: c.Authboss.Config.Paths.ConfirmNotOK, + } + return c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) + } - dbTok := base64.StdEncoding.EncodeToString(sum[:]) - user, err := ctx.Storer.(ConfirmStorer).ConfirmUser(dbTok) + values := authboss.MustHaveConfirmValues(validator) + + toHash, err := base64.URLEncoding.DecodeString(values.GetToken()) + if err != nil { + logger.Infof("error decoding token in Confirm.Get, this typically means a bad token: %s %+v", values.GetToken(), err) + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + Failure: "Invalid confirm token.", + RedirectPath: c.Authboss.Config.Paths.ConfirmNotOK, + } + return c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) + } + + sum := sha512.Sum512(toHash) + token := base64.StdEncoding.EncodeToString(sum[:]) + + storer := authboss.EnsureCanConfirm(c.Authboss.Config.Storage.Server) + user, err := storer.LoadByToken(r.Context(), token) if err == authboss.ErrUserNotFound { - return authboss.ErrAndRedirect{Location: "/", Err: errors.New("token not found")} + logger.Infof("confirm token was not found in database: %s", token) + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + Failure: "Invalid confirm token.", + RedirectPath: c.Authboss.Config.Paths.ConfirmNotOK, + } + return c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) } else if err != nil { return err } - ctx.User = authboss.Unbind(user) + user.PutConfirmToken("") + user.PutConfirmed(true) - ctx.User[StoreConfirmToken] = "" - ctx.User[StoreConfirmed] = true - - key, err := ctx.User.StringErr(c.PrimaryID) - if err != nil { + logger.Infof("user %s confirmed their account", user.GetPID()) + if err = c.Authboss.Config.Storage.Server.Save(r.Context(), user); err != nil { return err } - if err := ctx.SaveUser(); err != nil { - return err + ro := authboss.RedirectOptions{ + Code: http.StatusTemporaryRedirect, + Success: "You have successfully confirmed your account.", + RedirectPath: c.Authboss.Config.Paths.ConfirmOK, } - - ctx.SessionStorer.Put(authboss.SessionKey, key) - response.Redirect(ctx, w, r, c.RegisterOKPath, "You have successfully confirmed your account.", "", true) - - return nil + return c.Authboss.Config.Core.Redirector.Redirect(w, r, ro) +} + +// Middleware ensures that a user is confirmed, or else it will intercept the request +// and send them to the confirm page, this will load the user if he's not been loaded +// yet from the session. +// +// 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, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user := ab.LoadCurrentUserP(w, &r) + + cu := authboss.MustBeConfirmable(user) + if cu.GetConfirmed() { + next.ServeHTTP(w, r) + return + } + + logger := ab.RequestLogger(r) + logger.Infof("user %s prevented from accessing %s: not confirmed", user.GetPID(), r.URL.Path) + }) +} + +// GenerateToken creates a random token that will be used to confirm the user. +func GenerateToken() (hash string, token string, err error) { + tok := make([]byte, 32) + if _, err := rand.Read(tok); err != nil { + return "", "", err + } + sum := sha512.Sum512(tok) + return base64.StdEncoding.EncodeToString(sum[:]), base64.URLEncoding.EncodeToString(tok[:]), nil } diff --git a/confirm/confirm_test.go b/confirm/confirm_test.go index 1ab99e2..9f73e67 100644 --- a/confirm/confirm_test.go +++ b/confirm/confirm_test.go @@ -2,21 +2,391 @@ package confirm import ( "bytes" - "crypto/md5" + "context" + "crypto/sha512" "encoding/base64" - "html/template" + "errors" "net/http" "net/http/httptest" - "reflect" - "strings" "testing" - "github.com/pkg/errors" - "github.com/volatiletech/authboss" "github.com/volatiletech/authboss/internal/mocks" ) +func TestInit(t *testing.T) { + t.Parallel() + + ab := authboss.New() + + router := &mocks.Router{} + renderer := &mocks.Renderer{} + errHandler := &mocks.ErrorHandler{} + ab.Config.Core.Router = router + ab.Config.Core.MailRenderer = renderer + ab.Config.Core.ErrorHandler = errHandler + + c := &Confirm{} + if err := c.Init(ab); err != nil { + t.Fatal(err) + } + + if err := renderer.HasLoadedViews(EmailConfirmHTML, EmailConfirmTxt); err != nil { + t.Error(err) + } + + if err := router.HasGets("/confirm"); err != nil { + t.Error(err) + } +} + +type testHarness struct { + confirm *Confirm + ab *authboss.Authboss + + bodyReader *mocks.BodyReader + mailer *mocks.Emailer + redirector *mocks.Redirector + renderer *mocks.Renderer + responder *mocks.Responder + session *mocks.ClientStateRW + storer *mocks.ServerStorer +} + +func testSetup() *testHarness { + harness := &testHarness{} + + harness.ab = authboss.New() + harness.bodyReader = &mocks.BodyReader{} + harness.mailer = &mocks.Emailer{} + harness.redirector = &mocks.Redirector{} + harness.renderer = &mocks.Renderer{} + harness.responder = &mocks.Responder{} + harness.session = mocks.NewClientRW() + harness.storer = mocks.NewServerStorer() + + harness.ab.Paths.ConfirmOK = "/confirm/ok" + harness.ab.Paths.ConfirmNotOK = "/confirm/not/ok" + + harness.ab.Config.Core.BodyReader = harness.bodyReader + harness.ab.Config.Core.Logger = mocks.Logger{} + harness.ab.Config.Core.Mailer = harness.mailer + harness.ab.Config.Core.Redirector = harness.redirector + harness.ab.Config.Core.MailRenderer = harness.renderer + harness.ab.Config.Core.Responder = harness.responder + harness.ab.Config.Storage.SessionState = harness.session + harness.ab.Config.Storage.Server = harness.storer + + harness.confirm = &Confirm{harness.ab} + + return harness +} + +func TestPreventAuthAllow(t *testing.T) { + t.Parallel() + + harness := testSetup() + + user := &mocks.User{ + Confirmed: true, + } + + r := mocks.Request("GET") + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + w := httptest.NewRecorder() + + handled, err := harness.confirm.PreventAuth(w, r, false) + if err != nil { + t.Error(err) + } + + if handled { + t.Error("it should not have been handled") + } +} + +func TestPreventDisallow(t *testing.T) { + t.Parallel() + + harness := testSetup() + + user := &mocks.User{ + Confirmed: false, + } + + r := mocks.Request("GET") + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + w := httptest.NewRecorder() + + handled, err := harness.confirm.PreventAuth(w, r, false) + if err != nil { + t.Error(err) + } + + if !handled { + t.Error("it should have been handled") + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("redirect did not occur") + } + + if p := harness.redirector.Options.RedirectPath; p != "/confirm/not/ok" { + t.Error("redirect path was wrong:", p) + } +} + +func TestStartConfirmationWeb(t *testing.T) { + // no t.Parallel(), global var mangling + + oldConfirm := goConfirmEmail + goConfirmEmail = func(c *Confirm, ctx context.Context, to, token string) { + c.SendConfirmEmail(ctx, to, token) + } + + defer func() { + goConfirmEmail = oldConfirm + }() + + harness := testSetup() + + user := &mocks.User{Email: "test@test.com"} + harness.storer.Users["test@test.com"] = user + + r := mocks.Request("GET") + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + w := httptest.NewRecorder() + + handled, err := harness.confirm.StartConfirmationWeb(w, r, false) + if err != nil { + t.Error(err) + } + + if !handled { + t.Error("it should always be handled") + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("redirect did not occur") + } + + if p := harness.redirector.Options.RedirectPath; p != "/confirm/not/ok" { + t.Error("redirect path was wrong:", p) + } + + if to := harness.mailer.Email.To[0]; to != "test@test.com" { + t.Error("mailer sent e-mail to wrong person:", to) + } +} + +func TestGetSuccess(t *testing.T) { + t.Parallel() + + harness := testSetup() + + hash, token, err := GenerateToken() + if err != nil { + t.Fatal(err) + } + + user := &mocks.User{Email: "test@test.com", Confirmed: false, ConfirmToken: hash} + harness.storer.Users["test@test.com"] = user + harness.bodyReader.Return = mocks.Values{ + Token: token, + } + + r := mocks.Request("GET") + w := httptest.NewRecorder() + + if err := harness.confirm.Get(w, r); err != nil { + t.Error(err) + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("expected a redirect, got:", w.Code) + } + if p := harness.redirector.Options.RedirectPath; p != harness.ab.Paths.ConfirmOK { + t.Error("redir path was wrong:", p) + } + + if len(user.ConfirmToken) != 0 { + t.Error("the confirm token should have been erased") + } + if !user.Confirmed { + t.Error("the user should have been confirmed") + } +} + +func TestGetValidationFailure(t *testing.T) { + t.Parallel() + + harness := testSetup() + + harness.bodyReader.Return = mocks.Values{ + Errors: []error{errors.New("fail")}, + } + + r := mocks.Request("GET") + w := httptest.NewRecorder() + + if err := harness.confirm.Get(w, r); err != nil { + t.Error(err) + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("expected a redirect, got:", w.Code) + } + if p := harness.redirector.Options.RedirectPath; p != harness.ab.Paths.ConfirmNotOK { + t.Error("redir path was wrong:", p) + } + if reason := harness.redirector.Options.Failure; reason != "Invalid confirm token." { + t.Error("reason for failure was wrong:", reason) + } +} + +func TestGetBase64DecodeFailure(t *testing.T) { + t.Parallel() + + harness := testSetup() + + harness.bodyReader.Return = mocks.Values{ + Token: "5", + } + + r := mocks.Request("GET") + w := httptest.NewRecorder() + + if err := harness.confirm.Get(w, r); err != nil { + t.Error(err) + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("expected a redirect, got:", w.Code) + } + if p := harness.redirector.Options.RedirectPath; p != harness.ab.Paths.ConfirmNotOK { + t.Error("redir path was wrong:", p) + } + if reason := harness.redirector.Options.Failure; reason != "Invalid confirm token." { + t.Error("reason for failure was wrong:", reason) + } +} + +func TestGetUserNotFoundFailure(t *testing.T) { + t.Parallel() + + harness := testSetup() + + _, token, err := GenerateToken() + if err != nil { + t.Fatal(err) + } + + harness.bodyReader.Return = mocks.Values{ + Token: token, + } + + r := mocks.Request("GET") + w := httptest.NewRecorder() + + if err := harness.confirm.Get(w, r); err != nil { + t.Error(err) + } + + if w.Code != http.StatusTemporaryRedirect { + t.Error("expected a redirect, got:", w.Code) + } + if p := harness.redirector.Options.RedirectPath; p != harness.ab.Paths.ConfirmNotOK { + t.Error("redir path was wrong:", p) + } + if reason := harness.redirector.Options.Failure; reason != "Invalid confirm token." { + t.Error("reason for failure was wrong:", reason) + } +} + +func TestMiddlewareAllow(t *testing.T) { + t.Parallel() + + ab := authboss.New() + called := false + server := Middleware(ab, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + })) + + user := &mocks.User{ + Confirmed: true, + } + + r := mocks.Request("GET") + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + w := httptest.NewRecorder() + + server.ServeHTTP(w, r) + + if !called { + t.Error("The user should have been allowed through") + } +} + +func TestMiddlewareDisallow(t *testing.T) { + t.Parallel() + + ab := authboss.New() + ab.Config.Core.Logger = mocks.Logger{} + called := false + server := Middleware(ab, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + })) + + user := &mocks.User{ + Confirmed: false, + } + + r := mocks.Request("GET") + r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, user)) + w := httptest.NewRecorder() + + server.ServeHTTP(w, r) + + if called { + t.Error("The user should not have been allowed through") + } +} + +func TestGenerateToken(t *testing.T) { + t.Parallel() + + hash, token, err := GenerateToken() + if err != nil { + t.Error(err) + } + + // base64 length: n = 64; 4*(64/3) = 85.3; round to nearest 4: 88 + if len(hash) != 88 { + t.Errorf("string length was wrong (%d): %s", len(hash), hash) + } + + // base64 length: n = 32; 4*(32/3) = 42.6; round to nearest 4: 44 + if len(token) != 44 { + t.Errorf("string length was wrong (%d): %s", len(token), token) + } + + rawToken, err := base64.URLEncoding.DecodeString(token) + if err != nil { + t.Error(err) + } + + rawHash, err := base64.StdEncoding.DecodeString(hash) + if err != nil { + t.Error(err) + } + + checkHash := sha512.Sum512(rawToken) + if 0 != bytes.Compare(checkHash[:], rawHash) { + t.Error("expected hashes to match") + } +} + +/* func setup() *Confirm { ab := authboss.New() ab.Storage.Server = mocks.NewMockStorer() @@ -240,3 +610,4 @@ func TestConfirm_Confirm(t *testing.T) { t.Error("Should have left a nice message.") } } +*/ diff --git a/context.go b/context.go index ac1424b..e7adc6b 100644 --- a/context.go +++ b/context.go @@ -49,6 +49,8 @@ func (a *Authboss) CurrentUserIDP(w http.ResponseWriter, r *http.Request) string } // CurrentUser retrieves the current user from the session and the database. +// Before the user is loaded from the database the context key is checked. +// If the session doesn't have the user ID ErrUserNotFound will be returned. func (a *Authboss) CurrentUser(w http.ResponseWriter, r *http.Request) (User, error) { if user := r.Context().Value(CTXKeyUser); user != nil { return user.(User), nil @@ -58,7 +60,7 @@ func (a *Authboss) CurrentUser(w http.ResponseWriter, r *http.Request) (User, er if err != nil { return nil, err } else if len(pid) == 0 { - return nil, nil + return nil, ErrUserNotFound } return a.currentUser(r.Context(), pid) diff --git a/defaults/error_handler.go b/defaults/error_handler.go index 0530534..7b90915 100644 --- a/defaults/error_handler.go +++ b/defaults/error_handler.go @@ -41,5 +41,5 @@ func (e errorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - e.LogWriter.Error(fmt.Sprintf("error at %s: %+v", r.URL.String(), err)) + e.LogWriter.Error(fmt.Sprintf("request error from (%s) %s: %+v", r.RemoteAddr, r.URL.String(), err)) } diff --git a/defaults/log_mailer_test.go b/defaults/log_mailer_test.go index 975a42a..72c06de 100644 --- a/defaults/log_mailer_test.go +++ b/defaults/log_mailer_test.go @@ -56,23 +56,6 @@ func TestMailer(t *testing.T) { } } -func TestSMTPMailer(t *testing.T) { - t.Parallel() - - _ = NewSMTPMailer("server", nil) - - recovered := false - defer func() { - recovered = recover() != nil - }() - - NewSMTPMailer("", nil) - - if !recovered { - t.Error("Should have panicd.") - } -} - func TestBoundary(t *testing.T) { t.Parallel() diff --git a/defaults/smtp_mailer_test.go b/defaults/smtp_mailer_test.go new file mode 100644 index 0000000..70d0e6f --- /dev/null +++ b/defaults/smtp_mailer_test.go @@ -0,0 +1,24 @@ +package defaults + +import "testing" + +func TestSMTPMailer(t *testing.T) { + t.Skip("must implement test against real smtp servers here") +} + +func TestSMTPMailerPanic(t *testing.T) { + t.Parallel() + + _ = NewSMTPMailer("server", nil) + + recovered := false + defer func() { + recovered = recover() != nil + }() + + NewSMTPMailer("", nil) + + if !recovered { + t.Error("Should have panicked.") + } +} diff --git a/internal/mocks/mocks.go b/internal/mocks/mocks.go index 6551be0..6c5c1bb 100644 --- a/internal/mocks/mocks.go +++ b/internal/mocks/mocks.go @@ -33,6 +33,7 @@ type User struct { } func (m User) GetPID() string { return m.Email } +func (m User) GetEmail() string { return m.Email } func (m User) GetUsername() string { return m.Username } func (m User) GetPassword() string { return m.Password } func (m User) GetRecoverToken() string { return m.RecoverToken } @@ -114,6 +115,17 @@ func (s *ServerStorer) Save(ctx context.Context, user authboss.User) error { return nil } +// LoadByToken finds a user by his token +func (s *ServerStorer) LoadByToken(ctx context.Context, token string) (authboss.ConfirmableUser, error) { + for _, v := range s.Users { + if v.ConfirmToken == token { + return v, nil + } + } + + return nil, authboss.ErrUserNotFound +} + /* // TODO(aarondl): What is this? // AddToken for remember me @@ -428,6 +440,17 @@ func (r *Redirector) Redirect(w http.ResponseWriter, req *http.Request, ro authb return nil } +// Emailer that holds the options it was given +type Emailer struct { + Email authboss.Email +} + +// Send an e-mail +func (e *Emailer) Send(ctx context.Context, email authboss.Email) error { + e.Email = email + return nil +} + // BodyReader reads the body of a request and returns some values type BodyReader struct { Return authboss.Validator @@ -442,6 +465,7 @@ func (b BodyReader) Read(page string, r *http.Request) (authboss.Validator, erro type Values struct { PID string Password string + Token string Errors []error } @@ -456,6 +480,11 @@ func (v Values) GetPassword() string { return v.Password } +// GetToken from values +func (v Values) GetToken() string { + return v.Token +} + // Validate the values func (v Values) Validate() []error { return v.Errors diff --git a/response.go b/response.go index 77c97eb..21ea764 100644 --- a/response.go +++ b/response.go @@ -1,11 +1,31 @@ package authboss import ( + "context" "net/http" "github.com/pkg/errors" ) +// HTTPResponder knows how to respond to an HTTP request +// Must consider: +// - Flash messages +// - XSRF handling (template data) +// - Assembling template data from various sources +// +// Authboss controller methods (like the one called in response to POST /auth/login) +// will call this method to write a response to the user. +type HTTPResponder interface { + Respond(w http.ResponseWriter, r *http.Request, code int, templateName string, data HTMLData) error +} + +// HTTPRedirector redirects http requests to a different url (must handle both json and html) +// When an authboss controller wants to redirect a user to a different path, it will use +// this interface. +type HTTPRedirector interface { + Redirect(w http.ResponseWriter, r *http.Request, ro RedirectOptions) error +} + // RedirectOptions packages up all the pieces a module needs to write out a // response. type RedirectOptions struct { @@ -36,29 +56,8 @@ type EmailResponseOptions struct { TextTemplate string } -// HTTPResponder knows how to respond to an HTTP request -// Must consider: -// - Flash messages -// - XSRF handling (template data) -// - Assembling template data from various sources -// -// Authboss controller methods (like the one called in response to POST /auth/login) -// will call this method to write a response to the user. -type HTTPResponder interface { - Respond(w http.ResponseWriter, r *http.Request, code int, templateName string, data HTMLData) error -} - -// HTTPRedirector redirects http requests to a different url (must handle both json and html) -// When an authboss controller wants to redirect a user to a different path, it will use -// this interface. -type HTTPRedirector interface { - Redirect(w http.ResponseWriter, r *http.Request, ro RedirectOptions) error -} - // Email renders the e-mail templates and sends it using the mailer. -func (a *Authboss) Email(w http.ResponseWriter, r *http.Request, email Email, ro EmailResponseOptions) error { - ctx := r.Context() - +func (a *Authboss) Email(ctx context.Context, email Email, ro EmailResponseOptions) error { if len(ro.HTMLTemplate) != 0 { htmlBody, _, err := a.Core.MailRenderer.Render(ctx, ro.HTMLTemplate, ro.Data) if err != nil { diff --git a/storage.go b/storage.go index d35fd41..36a99c8 100644 --- a/storage.go +++ b/storage.go @@ -64,7 +64,7 @@ type CreatingServerStorer interface { // ConfirmingServerStorer can find a user by a confirm token type ConfirmingServerStorer interface { - Load(ctx context.Context, token string) (User, error) + LoadByToken(ctx context.Context, token string) (ConfirmableUser, error) } // EnsureCanCreate makes sure the server storer supports create operations diff --git a/user.go b/user.go index 2a38f23..704b070 100644 --- a/user.go +++ b/user.go @@ -30,9 +30,11 @@ type ConfirmableUser interface { GetConfirmed() (confirmed bool) GetConfirmToken() (token string) + GetEmail() (email string) PutConfirmed(confirmed bool) PutConfirmToken(token string) + PutEmail(email string) } // ArbitraryUser allows arbitrary data from the web form through. You should diff --git a/values.go b/values.go index 23b9e79..0c1f831 100644 --- a/values.go +++ b/values.go @@ -31,16 +31,6 @@ type UserValuer interface { GetPassword() string } -// MustHaveUserValues upgrades a validatable set of values -// to ones specific to the user. -func MustHaveUserValues(v Validator) UserValuer { - if u, ok := v.(UserValuer); ok { - return u - } - - panic(fmt.Sprintf("bodyreader returned a type that could not be upgraded to UserValuer: %T", v)) -} - // ArbitraryValuer provides the "rest" of the fields // that aren't strictly needed for anything in particular, // address, secondary e-mail, etc. @@ -58,3 +48,30 @@ type ArbitraryValuer interface { GetValues() map[string]string } + +// ConfirmValuer allows us to pull out the token from the request +type ConfirmValuer interface { + Validator + + GetToken() string +} + +// MustHaveUserValues upgrades a validatable set of values +// to ones specific to an authenticating user. +func MustHaveUserValues(v Validator) UserValuer { + if u, ok := v.(UserValuer); ok { + return u + } + + panic(fmt.Sprintf("bodyreader returned a type that could not be upgraded to UserValuer: %T", v)) +} + +// MustHaveConfirmValues upgrades a validatable set of values +// to ones specific to a user that needs to be confirmed. +func MustHaveConfirmValues(v Validator) ConfirmValuer { + if u, ok := v.(ConfirmValuer); ok { + return u + } + + panic(fmt.Sprintf("bodyreader returned a type that could not be upgraded to ConfirmValuer: %T", v)) +}