From f65d9f6bb6a9d11caffe2f7bafde6b0b1172d96a Mon Sep 17 00:00:00 2001 From: Aaron L Date: Tue, 21 Feb 2017 15:04:30 -0800 Subject: [PATCH] Fix errors package - Fix many compilation errors --- auth/auth.go | 11 +-- auth/auth_test.go | 3 +- authboss.go | 138 ++++++++++++++++++---------------- authboss_test.go | 18 +++-- callbacks.go | 14 ++-- callbacks_test.go | 34 +++++---- client_storer.go | 16 ++-- config.go | 11 ++- confirm/confirm.go | 11 +-- confirm/confirm_test.go | 3 +- errors_test.go | 3 +- internal/mocks/mocks.go | 3 +- internal/response/response.go | 5 +- lock/lock.go | 7 +- mailer.go | 10 +-- mailer_test.go | 7 +- mocks_test.go | 54 ++++++++++--- module_test.go | 4 +- oauth2/oauth2.go | 15 ++-- recover/recover.go | 13 ++-- register/register.go | 9 ++- remember/remember.go | 16 ++-- router_test.go | 27 +++---- rules.go | 3 +- storer.go | 9 ++- validation_test.go | 3 +- 26 files changed, 253 insertions(+), 194 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 29e7bdb..cef61e8 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -2,13 +2,14 @@ package auth import ( - "errors" "fmt" "net/http" - "golang.org/x/crypto/bcrypt" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/response" + "golang.org/x/crypto/bcrypt" ) const ( @@ -33,15 +34,15 @@ func (a *Auth) Initialize(ab *authboss.Authboss) (err error) { a.Authboss = ab if a.Storer == nil && a.StoreMaker == nil { - return errors.New("auth: Need a Storer") + return errors.New("need a storer") } if len(a.XSRFName) == 0 { - return errors.New("auth: XSRFName must be set") + return errors.New("xsrfName must be set") } if a.XSRFMaker == nil { - return errors.New("auth: XSRFMaker must be defined") + return errors.New("xsrfMaker must be defined") } a.templates, err = response.LoadTemplates(a.Authboss, a.Layout, a.ViewsPath, tplLogin) diff --git a/auth/auth_test.go b/auth/auth_test.go index decc2a1..e6396c8 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -2,7 +2,6 @@ package auth import ( "bytes" - "errors" "html/template" "io/ioutil" "net/http" @@ -10,6 +9,8 @@ import ( "strings" "testing" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/mocks" ) diff --git a/authboss.go b/authboss.go index d54858b..64e6455 100644 --- a/authboss.go +++ b/authboss.go @@ -7,14 +7,10 @@ races without having to think about how to store passwords or remember tokens. package authboss import ( - "database/sql" - "errors" "fmt" "net/http" - "reflect" - "strings" - "golang.org/x/crypto/bcrypt" + "github.com/pkg/errors" ) // Authboss contains a configuration and other details for running. @@ -45,9 +41,9 @@ func (a *Authboss) Init(modulesToLoad ...string) error { } for _, name := range modulesToLoad { - fmt.Fprintf(a.LogWriter, "%-10s Loading\n", "["+name+"]") + fmt.Fprintf(a.LogWriter, "%-10s loading\n", "["+name+"]") if err := a.loadModule(name); err != nil { - return fmt.Errorf("[%s] Error Initializing: %v", name, err) + return errors.Wrapf(err, "[%s] error initializing", name) } } @@ -56,55 +52,61 @@ func (a *Authboss) Init(modulesToLoad ...string) error { // CurrentUser retrieves the current user from the session and the database. func (a *Authboss) CurrentUser(w http.ResponseWriter, r *http.Request) (interface{}, error) { - return a.currentUser(a.InitContext(w, r), w, r) + return nil, errors.New("TODO") } func (a *Authboss) currentUser(w http.ResponseWriter, r *http.Request) (interface{}, error) { - _, err := a.Callbacks.FireBefore(EventGetUserSession, ctx) - if err != nil { - return nil, err - } + /* + _, err := a.Callbacks.FireBefore(EventGetUserSession, ctx) + if err != nil { + return nil, err + } - key, ok := ctx.SessionStorer.Get(SessionKey) - if !ok { - return nil, nil - } + key, ok := ctx.SessionStorer.Get(SessionKey) + if !ok { + return nil, nil + } - _, err = a.Callbacks.FireBefore(EventGetUser, ctx) - if err != nil { - return nil, err - } + _, err = a.Callbacks.FireBefore(EventGetUser, ctx) + if err != nil { + return nil, err + } - var user interface{} + var user interface{} - if index := strings.IndexByte(key, ';'); index > 0 { - user, err = a.OAuth2Storer.GetOAuth(key[:index], key[index+1:]) - } else { - user, err = a.Storer.Get(key) - } + if index := strings.IndexByte(key, ';'); index > 0 { + user, err = a.OAuth2Storer.GetOAuth(key[:index], key[index+1:]) + } else { + user, err = a.Storer.Get(key) + } - if err != nil { - return nil, err - } + if err != nil { + return nil, err + } - ctx.User = Unbind(user) + ctx.User = Unbind(user) - err = a.Callbacks.FireAfter(EventGetUser, ctx) - if err != nil { - return nil, err - } + err = a.Callbacks.FireAfter(EventGetUser, ctx) + if err != nil { + return nil, err + } - return user, err + return user, err + */ + return nil, errors.New("not implemented") } // CurrentUserP retrieves the current user but panics if it's not available for // any reason. func (a *Authboss) CurrentUserP(w http.ResponseWriter, r *http.Request) interface{} { - i, err := a.CurrentUser(w, r) - if err != nil { - panic(err.Error()) - } - return i + /* + i, err := a.CurrentUser(w, r) + if err != nil { + panic(err.Error()) + } + return i + */ + panic("TODO") } /* @@ -130,37 +132,41 @@ or from the cleanup routines. func (a *Authboss) UpdatePassword(w http.ResponseWriter, r *http.Request, ptPassword string, user interface{}, updater func() error) error { - updatePwd := len(ptPassword) > 0 + /* + updatePwd := len(ptPassword) > 0 - if updatePwd { - pass, err := bcrypt.GenerateFromPassword([]byte(ptPassword), a.BCryptCost) - if err != nil { + if updatePwd { + pass, err := bcrypt.GenerateFromPassword([]byte(ptPassword), a.BCryptCost) + if err != nil { + return err + } + + val := reflect.ValueOf(user).Elem() + field := val.FieldByName("Password") + if !field.CanSet() { + return errors.New("authboss: updatePassword called without a modifyable user struct") + } + fieldPtr := field.Addr() + + if scanner, ok := fieldPtr.Interface().(sql.Scanner); ok { + if err := scanner.Scan(string(pass)); err != nil { + return err + } + } else { + field.SetString(string(pass)) + } + } + + if err := updater(); err != nil { return err } - val := reflect.ValueOf(user).Elem() - field := val.FieldByName("Password") - if !field.CanSet() { - return errors.New("authboss: UpdatePassword called without a modifyable user struct") + if !updatePwd { + return nil } - fieldPtr := field.Addr() - if scanner, ok := fieldPtr.Interface().(sql.Scanner); ok { - if err := scanner.Scan(string(pass)); err != nil { - return err - } - } else { - field.SetString(string(pass)) - } - } + return a.Callbacks.FireAfter(EventPasswordReset, a.InitContext(w, r)) + */ - if err := updater(); err != nil { - return err - } - - if !updatePwd { - return nil - } - - return a.Callbacks.FireAfter(EventPasswordReset, a.InitContext(w, r)) + return errors.New("TODO") } diff --git a/authboss_test.go b/authboss_test.go index 00251a6..b61da1e 100644 --- a/authboss_test.go +++ b/authboss_test.go @@ -1,12 +1,14 @@ package authboss import ( + "context" "database/sql" - "errors" "io/ioutil" "net/http" "net/http/httptest" "testing" + + "github.com/pkg/errors" ) func TestAuthBossInit(t *testing.T) { @@ -25,7 +27,7 @@ func TestAuthBossCurrentUser(t *testing.T) { ab := New() ab.LogWriter = ioutil.Discard - ab.Storer = mockStorer{"joe": Attributes{"email": "john@john.com", "password": "lies"}} + ab.StoreLoader = mockStoreLoader{"joe": mockUser{Email: "john@john.com", Password: "lies"}} ab.SessionStoreMaker = func(_ http.ResponseWriter, _ *http.Request) ClientStorer { return mockClientStore{SessionKey: "joe"} } @@ -53,7 +55,7 @@ func TestAuthBossCurrentUserCallbacks(t *testing.T) { ab := New() ab.LogWriter = ioutil.Discard - ab.Storer = mockStorer{"joe": Attributes{"email": "john@john.com", "password": "lies"}} + ab.StoreLoader = mockStoreLoader{"joe": mockUser{Email: "john@john.com", Password: "lies"}} ab.SessionStoreMaker = func(_ http.ResponseWriter, _ *http.Request) ClientStorer { return mockClientStore{SessionKey: "joe"} } @@ -72,21 +74,21 @@ func TestAuthBossCurrentUserCallbacks(t *testing.T) { beforeGetUser := errors.New("beforeGetUser") beforeGetUserSession := errors.New("beforeGetUserSession") - ab.Callbacks.After(EventGetUser, func(*Context) error { + ab.Callbacks.After(EventGetUser, func(context.Context) error { return afterGetUser }) if _, err := ab.CurrentUser(rec, req); err != afterGetUser { t.Error("Want:", afterGetUser, "Got:", err) } - ab.Callbacks.Before(EventGetUser, func(*Context) (Interrupt, error) { + ab.Callbacks.Before(EventGetUser, func(context.Context) (Interrupt, error) { return InterruptNone, beforeGetUser }) if _, err := ab.CurrentUser(rec, req); err != beforeGetUser { t.Error("Want:", beforeGetUser, "Got:", err) } - ab.Callbacks.Before(EventGetUserSession, func(*Context) (Interrupt, error) { + ab.Callbacks.Before(EventGetUserSession, func(context.Context) (Interrupt, error) { return InterruptNone, beforeGetUserSession }) if _, err := ab.CurrentUser(rec, req); err != beforeGetUserSession { @@ -108,7 +110,7 @@ func TestAuthbossUpdatePassword(t *testing.T) { } called := false - ab.Callbacks.After(EventPasswordReset, func(ctx *Context) error { + ab.Callbacks.After(EventPasswordReset, func(ctx context.Context) error { called = true return nil }) @@ -172,7 +174,7 @@ func TestAuthbossUpdatePasswordFail(t *testing.T) { Password string }{} - anErr := errors.New("AnError") + 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) diff --git a/callbacks.go b/callbacks.go index 233b0b4..6de99d5 100644 --- a/callbacks.go +++ b/callbacks.go @@ -3,6 +3,7 @@ package authboss import ( "context" "fmt" + "io/ioutil" "reflect" "runtime" ) @@ -45,10 +46,9 @@ const ( InterruptSessionExpired ) -// Before callbacks can interrupt the flow by returning a bool. This is used to stop -// the callback chain and the original handler from continuing execution. -// The execution should also stopped if there is an error (and therefore if error is set -// the bool is automatically considered set). +// Before callbacks can interrupt the flow by returning an interrupt value. +// This is used to stop the callback chain and the original handler from +// continuing execution. The execution should also stopped if there is an error. type Before func(context.Context) (Interrupt, error) // After is a request callback that happens after the event. @@ -95,7 +95,8 @@ func (c *Callbacks) FireBefore(e Event, ctx context.Context) (interrupt Interrup for _, fn := range callbacks { interrupt, err = fn(ctx) if err != nil { - fmt.Fprintf(ctx.LogWriter, "Callback error (%s): %v\n", runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name(), err) + // TODO(aarondl): logwriter fail + fmt.Fprintf(ioutil.Discard, "Callback error (%s): %v\n", runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name(), err) return InterruptNone, err } if interrupt != InterruptNone { @@ -112,7 +113,8 @@ func (c *Callbacks) FireAfter(e Event, ctx context.Context) (err error) { callbacks := c.after[e] for _, fn := range callbacks { if err = fn(ctx); err != nil { - fmt.Fprintf(ctx.LogWriter, "Callback error (%s): %v\n", runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name(), err) + // TODO(aarondl): logwriter fail + fmt.Fprintf(ioutil.Discard, "Callback error (%s): %v\n", runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name(), err) return err } } diff --git a/callbacks_test.go b/callbacks_test.go index 84fddfa..9141be5 100644 --- a/callbacks_test.go +++ b/callbacks_test.go @@ -2,9 +2,11 @@ package authboss import ( "bytes" - "errors" + "context" "strings" "testing" + + "github.com/pkg/errors" ) func TestCallbacks(t *testing.T) { @@ -14,11 +16,11 @@ func TestCallbacks(t *testing.T) { afterCalled := false beforeCalled := false - ab.Callbacks.Before(EventRegister, func(ctx *Context) (Interrupt, error) { + ab.Callbacks.Before(EventRegister, func(ctx context.Context) (Interrupt, error) { beforeCalled = true return InterruptNone, nil }) - ab.Callbacks.After(EventRegister, func(ctx *Context) error { + ab.Callbacks.After(EventRegister, func(ctx context.Context) error { afterCalled = true return nil }) @@ -27,7 +29,7 @@ func TestCallbacks(t *testing.T) { t.Error("Neither should be called.") } - interrupt, err := ab.Callbacks.FireBefore(EventRegister, ab.NewContext()) + interrupt, err := ab.Callbacks.FireBefore(EventRegister, context.TODO()) if err != nil { t.Error("Unexpected error:", err) } @@ -42,7 +44,7 @@ func TestCallbacks(t *testing.T) { t.Error("Expected after not to be called.") } - ab.Callbacks.FireAfter(EventRegister, ab.NewContext()) + ab.Callbacks.FireAfter(EventRegister, context.TODO()) if !afterCalled { t.Error("Expected after to be called.") } @@ -55,16 +57,16 @@ func TestCallbacksInterrupt(t *testing.T) { before1 := false before2 := false - ab.Callbacks.Before(EventRegister, func(ctx *Context) (Interrupt, error) { + ab.Callbacks.Before(EventRegister, func(ctx context.Context) (Interrupt, error) { before1 = true return InterruptAccountLocked, nil }) - ab.Callbacks.Before(EventRegister, func(ctx *Context) (Interrupt, error) { + ab.Callbacks.Before(EventRegister, func(ctx context.Context) (Interrupt, error) { before2 = true return InterruptNone, nil }) - interrupt, err := ab.Callbacks.FireBefore(EventRegister, ab.NewContext()) + interrupt, err := ab.Callbacks.FireBefore(EventRegister, context.TODO()) if err != nil { t.Error(err) } @@ -89,18 +91,18 @@ func TestCallbacksBeforeErrors(t *testing.T) { before1 := false before2 := false - errValue := errors.New("Problem occured") + errValue := errors.New("problem occured") - ab.Callbacks.Before(EventRegister, func(ctx *Context) (Interrupt, error) { + ab.Callbacks.Before(EventRegister, func(ctx context.Context) (Interrupt, error) { before1 = true return InterruptNone, errValue }) - ab.Callbacks.Before(EventRegister, func(ctx *Context) (Interrupt, error) { + ab.Callbacks.Before(EventRegister, func(ctx context.Context) (Interrupt, error) { before2 = true return InterruptNone, nil }) - interrupt, err := ab.Callbacks.FireBefore(EventRegister, ab.NewContext()) + interrupt, err := ab.Callbacks.FireBefore(EventRegister, context.TODO()) if err != errValue { t.Error("Expected an error to come back.") } @@ -129,18 +131,18 @@ func TestCallbacksAfterErrors(t *testing.T) { after1 := false after2 := false - errValue := errors.New("Problem occured") + errValue := errors.New("problem occured") - ab.Callbacks.After(EventRegister, func(ctx *Context) error { + ab.Callbacks.After(EventRegister, func(ctx context.Context) error { after1 = true return errValue }) - ab.Callbacks.After(EventRegister, func(ctx *Context) error { + ab.Callbacks.After(EventRegister, func(ctx context.Context) error { after2 = true return nil }) - err := ab.Callbacks.FireAfter(EventRegister, ab.NewContext()) + err := ab.Callbacks.FireAfter(EventRegister, context.TODO()) if err != errValue { t.Error("Expected an error to come back.") } diff --git a/client_storer.go b/client_storer.go index 907485e..6c14136 100644 --- a/client_storer.go +++ b/client_storer.go @@ -25,6 +25,15 @@ const ( FlashErrorKey = "flash_error" ) +// ClientStoreMaker is used to create a cookie storer from an http request. Keep in mind +// security considerations for your implementation, Secure, HTTP-Only, etc flags. +// +// There's two major uses for this. To create session storage, and remember me +// cookies. +type ClientStoreMaker interface { + Make(http.ResponseWriter, *http.Request) ClientStorer +} + // ClientStorer should be able to store values on the clients machine. Cookie and // Session storers are built with this interface. type ClientStorer interface { @@ -53,13 +62,6 @@ func (c clientStoreWrapper) GetErr(key string) (string, error) { return str, nil } -// ClientStoreMaker is used to create a cookie storer from an http request. Keep in mind -// security considerations for your implementation, Secure, HTTP-Only, etc flags. -// -// There's two major uses for this. To create session based client storers -// and session storers. -type ClientStoreMaker func(http.ResponseWriter, *http.Request) ClientStorer - // FlashSuccess returns FlashSuccessKey from the session and removes it. func (a *Authboss) FlashSuccess(w http.ResponseWriter, r *http.Request) string { storer := a.SessionStoreMaker(w, r) diff --git a/config.go b/config.go index e2bd094..e684581 100644 --- a/config.go +++ b/config.go @@ -2,7 +2,6 @@ package authboss import ( "context" - "html/template" "io" "io/ioutil" "net/http" @@ -26,7 +25,11 @@ type Config struct { // authboss.StoreEmail, authboss.StoreUsername (StoreEmail is default) PrimaryID string + // ViewLoader loads the templates for the application. ViewLoader RenderLoader + // MailViewLoader loads the templates for mail. If this is nil, it will + // fall back to using the Renderer created from the ViewLoader instead. + MailViewLoader RenderLoader // LayoutDataMaker is a function that can provide authboss with the layout's // template data. It will be merged with the data being provided for the current // view in order to render the templates. @@ -92,7 +95,7 @@ type Config struct { XSRFMaker XSRF // Storer is the interface through which Authboss accesses the web apps database. - StoreLoader Storer + StoreLoader StoreLoader // CookieStoreMaker must be defined to provide an interface capapable of storing cookies // for the given response, and reading them from the request. @@ -120,10 +123,6 @@ func (c *Config) Defaults() { c.PrimaryID = StoreEmail - c.Layout = template.Must(template.New("").Parse(`{{template "authboss" .}}`)) - c.LayoutHTMLEmail = template.Must(template.New("").Parse(`{{template "authboss" .}}`)) - c.LayoutTextEmail = template.Must(template.New("").Parse(`{{template "authboss" .}}`)) - c.AuthLoginOKPath = "/" c.AuthLoginFailPath = "/" c.AuthLogoutOKPath = "/" diff --git a/confirm/confirm.go b/confirm/confirm.go index c6aa336..0149016 100644 --- a/confirm/confirm.go +++ b/confirm/confirm.go @@ -5,12 +5,13 @@ import ( "crypto/md5" "crypto/rand" "encoding/base64" - "errors" "fmt" "net/http" "net/url" "path" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/response" ) @@ -27,7 +28,7 @@ const ( ) var ( - errUserMissing = errors.New("confirm: After registration user must be loaded") + errUserMissing = errors.New("after registration user must be loaded") ) // ConfirmStorer must be implemented in order to satisfy the confirm module's @@ -58,7 +59,7 @@ func (c *Confirm) Initialize(ab *authboss.Authboss) (err error) { var ok bool storer, ok := c.Storer.(ConfirmStorer) if c.StoreMaker == nil && (storer == nil || !ok) { - return errors.New("confirm: Need a ConfirmStorer") + return errors.New("need a confirmStorer") } c.emailHTMLTemplates, err = response.LoadTemplates(ab, c.LayoutHTMLEmail, c.ViewsPath, tplConfirmHTML) @@ -169,7 +170,7 @@ func (c *Confirm) confirmHandler(ctx *authboss.Context, w http.ResponseWriter, r toHash, err := base64.URLEncoding.DecodeString(token) if err != nil { return authboss.ErrAndRedirect{ - Location: "/", Err: fmt.Errorf("confirm: token failed to decode %q => %v\n", token, err), + Location: "/", Err: errors.Wrapf(err, "token failed to decode %q", token), } } @@ -178,7 +179,7 @@ func (c *Confirm) confirmHandler(ctx *authboss.Context, w http.ResponseWriter, r dbTok := base64.StdEncoding.EncodeToString(sum[:]) user, err := ctx.Storer.(ConfirmStorer).ConfirmUser(dbTok) if err == authboss.ErrUserNotFound { - return authboss.ErrAndRedirect{Location: "/", Err: errors.New("confirm: token not found")} + return authboss.ErrAndRedirect{Location: "/", Err: errors.New("token not found")} } else if err != nil { return err } diff --git a/confirm/confirm_test.go b/confirm/confirm_test.go index 7f87a28..03ad60a 100644 --- a/confirm/confirm_test.go +++ b/confirm/confirm_test.go @@ -4,7 +4,6 @@ import ( "bytes" "crypto/md5" "encoding/base64" - "errors" "html/template" "net/http" "net/http/httptest" @@ -12,6 +11,8 @@ import ( "strings" "testing" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/mocks" ) diff --git a/errors_test.go b/errors_test.go index c98a635..5772d60 100644 --- a/errors_test.go +++ b/errors_test.go @@ -1,8 +1,9 @@ package authboss import ( - "errors" "testing" + + "github.com/pkg/errors" ) func TestAttributeErr(t *testing.T) { diff --git a/internal/mocks/mocks.go b/internal/mocks/mocks.go index 653e1aa..959259e 100644 --- a/internal/mocks/mocks.go +++ b/internal/mocks/mocks.go @@ -2,13 +2,14 @@ package mocks import ( - "errors" "io" "net/http" "net/url" "strings" "time" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" ) diff --git a/internal/response/response.go b/internal/response/response.go index b60ca6d..e210d4e 100644 --- a/internal/response/response.go +++ b/internal/response/response.go @@ -5,7 +5,6 @@ package response import ( "bytes" - "errors" "html/template" "io" "io/ioutil" @@ -15,12 +14,14 @@ import ( "path/filepath" "strings" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" ) var ( // ErrTemplateNotFound should be returned from Get when the view is not found - ErrTemplateNotFound = errors.New("Template not found") + ErrTemplateNotFound = errors.New("template not found") ) // Templates is a map depicting the forms a template needs wrapped within the specified layout diff --git a/lock/lock.go b/lock/lock.go index a71ea09..b9abcbd 100644 --- a/lock/lock.go +++ b/lock/lock.go @@ -2,9 +2,10 @@ package lock import ( - "errors" "time" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" ) @@ -16,7 +17,7 @@ const ( ) var ( - errUserMissing = errors.New("lock: user not loaded in BeforeAuth callback") + errUserMissing = errors.New("user not loaded in BeforeAuth callback") ) func init() { @@ -32,7 +33,7 @@ type Lock struct { func (l *Lock) Initialize(ab *authboss.Authboss) error { l.Authboss = ab if l.Storer == nil && l.StoreMaker == nil { - return errors.New("lock: Need a Storer") + return errors.New("need a storer") } // Events diff --git a/mailer.go b/mailer.go index 3daa61a..ed37da2 100644 --- a/mailer.go +++ b/mailer.go @@ -33,7 +33,7 @@ func SMTPMailer(server string, auth smtp.Auth) Mailer { if len(server) == 0 { panic("SMTP Mailer must be created with a server string.") } - random := rand.New(rand.NewSource(time.Now())) + random := rand.New(rand.NewSource(time.Now().UnixNano())) return smtpMailer{server, auth, random} } @@ -56,7 +56,7 @@ type logMailer struct { io.Writer } -func (l logMailer) Send(mail Email) error { +func (l logMailer) Send(ctx context.Context, mail Email) error { buf := &bytes.Buffer{} data := struct { @@ -84,14 +84,14 @@ type smtpMailer struct { rand *rand.Rand } -func (s smtpMailer) Send(mail Email) error { +func (s smtpMailer) Send(ctx context.Context, mail Email) error { buf := &bytes.Buffer{} data := struct { Boundary string Mail Email }{ - Boundary: boundary(), + Boundary: s.boundary(), Mail: mail, } @@ -102,7 +102,7 @@ func (s smtpMailer) Send(mail Email) error { toSend := bytes.Replace(buf.Bytes(), []byte{'\n'}, []byte{'\r', '\n'}, -1) - return smtp.SendMail(s.Server, s.Auth, data.From, data.To, toSend) + return smtp.SendMail(s.Server, s.Auth, mail.From, mail.To, toSend) } // boundary makes mime boundaries, these are largely useless strings that just diff --git a/mailer_test.go b/mailer_test.go index f802278..1165760 100644 --- a/mailer_test.go +++ b/mailer_test.go @@ -2,6 +2,7 @@ package authboss import ( "bytes" + "context" "io/ioutil" "math/rand" "strings" @@ -15,10 +16,10 @@ func TestMailer(t *testing.T) { mailServer := &bytes.Buffer{} ab.Mailer = LogMailer(mailServer) - ab.Storer = mockStorer{} + ab.StoreLoader = mockStoreLoader{} ab.LogWriter = ioutil.Discard - err := ab.SendMail(Email{ + err := ab.SendMail(context.TODO(), Email{ To: []string{"some@email.com", "a@a.com"}, ToNames: []string{"Jake", "Noname"}, From: "some@guy.com", @@ -74,7 +75,7 @@ func TestSMTPMailer(t *testing.T) { func TestBoundary(t *testing.T) { t.Parallel() - mailer := smtpMailer{nil, nil, rand.New(rand.NewSource(3))} + mailer := smtpMailer{"server", nil, rand.New(rand.NewSource(3))} if got := mailer.boundary(); got != "ntadoe" { t.Error("boundary was wrong", got) } diff --git a/mocks_test.go b/mocks_test.go index 2647251..13f6c5d 100644 --- a/mocks_test.go +++ b/mocks_test.go @@ -2,10 +2,11 @@ package authboss import ( "context" - "errors" "net/http" "net/url" "strings" + + "github.com/pkg/errors" ) type mockUser struct { @@ -20,29 +21,60 @@ type mockStoredUser struct { type mockStoreLoader map[string]mockUser -func (m mockStoredUser) PutEmail(ctx context.Context, key string, email string) error { +func (m mockStoreLoader) Load(ctx context.Context, key string) (Storer, error) { + u, ok := m[key] + if !ok { + return nil, ErrUserNotFound + } + + return mockStoredUser{ + mockUser: u, + mockStoreLoader: m, + }, nil +} + +func (m mockStoredUser) Load(ctx context.Context) error { + u, ok := m.mockStoreLoader[m.Email] + if !ok { + return ErrUserNotFound + } + + m.Email = u.Email + m.Password = u.Password + + return nil +} + +func (m mockStoredUser) Save(ctx context.Context) error { + m.mockStoreLoader[m.Email] = m.mockUser + + return nil +} + +func (m mockStoredUser) PutEmail(ctx context.Context, email string) error { m.Email = email return nil } -func (m mockStoredUser) PutUsername(ctx context.Context, key string, username string) error { +func (m mockStoredUser) PutUsername(ctx context.Context, username string) error { return errors.New("not impl") } -func (m mockStoredUser) PutPassword(ctx context.Context, key string, password string) error { +func (m mockStoredUser) PutPassword(ctx context.Context, password string) error { m.Password = password return nil } -func (m mockStorer) PutOAuth(uid, provider string, attr Attributes) error { - m[uid+provider] = attr - return nil +func (m mockStoredUser) GetEmail(ctx context.Context) (email string, err error) { + return m.Email, nil } -func (m mockStorer) GetOAuth(uid, provider string) (result interface{}, err error) { - return &mockUser{ - m[uid+provider]["email"].(string), m[uid+provider]["password"].(string), - }, nil +func (m mockStoredUser) GetUsername(ctx context.Context) (username string, err error) { + return "", errors.New("not impl") +} + +func (m mockStoredUser) GetPassword(ctx context.Context) (password string, err error) { + return m.Password, nil } type mockClientStore map[string]string diff --git a/module_test.go b/module_test.go index 85a89c5..9d0c86e 100644 --- a/module_test.go +++ b/module_test.go @@ -13,7 +13,6 @@ func init() { } type testModule struct { - s StorageOptions r RouteTable } @@ -23,14 +22,13 @@ var testMod = &testModule{ }, } -func testHandler(ctx *Context, w http.ResponseWriter, r *http.Request) error { +func testHandler(w http.ResponseWriter, r *http.Request) error { w.Header().Set("testhandler", "test") return nil } func (t *testModule) Initialize(a *Authboss) error { return nil } func (t *testModule) Routes() RouteTable { return t.r } -func (t *testModule) Storage() StorageOptions { return t.s } func TestRegister(t *testing.T) { // RegisterModule called by init() diff --git a/oauth2/oauth2.go b/oauth2/oauth2.go index 314a938..a3544e2 100644 --- a/oauth2/oauth2.go +++ b/oauth2/oauth2.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" - "errors" "fmt" "net/http" "net/url" @@ -12,13 +11,15 @@ import ( "path/filepath" "strings" - "golang.org/x/oauth2" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/response" + "golang.org/x/oauth2" ) var ( - errOAuthStateValidation = errors.New("Could not validate oauth2 state param") + errOAuthStateValidation = errors.New("could not validate oauth2 state param") ) // OAuth2 module @@ -34,7 +35,7 @@ func init() { func (o *OAuth2) Initialize(ab *authboss.Authboss) error { o.Authboss = ab if o.OAuth2Storer == nil && o.OAuth2StoreMaker == nil { - return errors.New("oauth2: need an OAuth2Storer") + return errors.New("need an oauth2Storer") } return nil } @@ -80,7 +81,7 @@ func (o *OAuth2) oauthInit(ctx *authboss.Context, w http.ResponseWriter, r *http provider := strings.ToLower(filepath.Base(r.URL.Path)) cfg, ok := o.OAuth2Providers[provider] if !ok { - return fmt.Errorf("OAuth2 provider %q not found", provider) + return errors.Errorf("OAuth2 provider %q not found", provider) } random := make([]byte, 32) @@ -156,7 +157,7 @@ func (o *OAuth2) oauthCallback(ctx *authboss.Context, w http.ResponseWriter, r * cfg, ok := o.OAuth2Providers[provider] if !ok { - return fmt.Errorf("OAuth2 provider %q not found", provider) + return errors.Errorf("oauth2 provider %q not found", provider) } // Ensure request is genuine @@ -170,7 +171,7 @@ func (o *OAuth2) oauthCallback(ctx *authboss.Context, w http.ResponseWriter, r * code := r.FormValue("code") token, err := exchanger(cfg.OAuth2Config, o.Config.ContextProvider(r), code) if err != nil { - return fmt.Errorf("Could not validate oauth2 code: %v", err) + return errors.Wrap(err, "could not validate oauth2 code") } user, err := cfg.Callback(o.Config.ContextProvider(r), *cfg.OAuth2Config, token) diff --git a/recover/recover.go b/recover/recover.go index 8c4c0a7..b613886 100644 --- a/recover/recover.go +++ b/recover/recover.go @@ -5,16 +5,17 @@ import ( "crypto/md5" "crypto/rand" "encoding/base64" - "errors" "fmt" "net/http" "net/url" "path" "time" - "golang.org/x/crypto/bcrypt" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/response" + "golang.org/x/crypto/bcrypt" ) // Storage constants @@ -73,18 +74,18 @@ func (r *Recover) Initialize(ab *authboss.Authboss) (err error) { if r.Storer != nil { if _, ok := r.Storer.(RecoverStorer); !ok { - return errors.New("recover: RecoverStorer required for recover functionality") + return errors.New("recoverStorer required for recover functionality") } } else if r.StoreMaker == nil { - return errors.New("recover: Need a RecoverStorer") + return errors.New("need a RecoverStorer") } if len(r.XSRFName) == 0 { - return errors.New("auth: XSRFName must be set") + return errors.New("xsrfName must be set") } if r.XSRFMaker == nil { - return errors.New("auth: XSRFMaker must be defined") + return errors.New("xsrfMaker must be defined") } r.templates, err = response.LoadTemplates(r.Authboss, r.Layout, r.ViewsPath, tplRecover, tplRecoverComplete) diff --git a/register/register.go b/register/register.go index c2f7ad6..b961e18 100644 --- a/register/register.go +++ b/register/register.go @@ -2,12 +2,13 @@ package register import ( - "errors" "net/http" - "golang.org/x/crypto/bcrypt" + "github.com/pkg/errors" + "github.com/go-authboss/authboss" "github.com/go-authboss/authboss/internal/response" + "golang.org/x/crypto/bcrypt" ) const ( @@ -39,10 +40,10 @@ func (r *Register) Initialize(ab *authboss.Authboss) (err error) { if r.Storer != nil { if _, ok := r.Storer.(RegisterStorer); !ok { - return errors.New("register: RegisterStorer required for register functionality") + return errors.New("registerStorer required for register functionality") } } else if r.StoreMaker == nil { - return errors.New("register: Need a RegisterStorer") + return errors.New("need a registerStorer") } if r.templates, err = response.LoadTemplates(r.Authboss, r.Layout, r.ViewsPath, tplRegister); err != nil { diff --git a/remember/remember.go b/remember/remember.go index 033f0e9..027b5e0 100644 --- a/remember/remember.go +++ b/remember/remember.go @@ -7,8 +7,8 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" - "errors" - "fmt" + + "github.com/pkg/errors" "github.com/go-authboss/authboss" ) @@ -18,7 +18,7 @@ const ( ) var ( - errUserMissing = errors.New("remember: User not loaded in callback") + errUserMissing = errors.New("user not loaded in callback") ) // RememberStorer must be implemented in order to satisfy the remember module's @@ -54,11 +54,11 @@ func (r *Remember) Initialize(ab *authboss.Authboss) error { if r.Storer != nil || r.OAuth2Storer != nil { if _, ok := r.Storer.(RememberStorer); !ok { if _, ok := r.OAuth2Storer.(RememberStorer); !ok { - return errors.New("remember: RememberStorer required for remember functionality") + return errors.New("rememberStorer required for remember functionality") } } } else if r.StoreMaker == nil && r.OAuth2StoreMaker == nil { - return errors.New("remember: Need a RememberStorer") + return errors.New("need a rememberStorer") } r.Callbacks.Before(authboss.EventGetUserSession, r.auth) @@ -97,7 +97,7 @@ func (r *Remember) afterAuth(ctx *authboss.Context) error { } if _, err := r.new(ctx.CookieStorer, key); err != nil { - return fmt.Errorf("remember: Failed to create remember token: %v", err) + return errors.Wrapf(err, "failed to create remember token") } return nil @@ -138,7 +138,7 @@ func (r *Remember) afterOAuth(ctx *authboss.Context) error { } if _, err := r.new(ctx.CookieStorer, uid+";"+provider); err != nil { - return fmt.Errorf("remember: Failed to create remember token: %v", err) + return errors.Wrap(err, "failed to create remember token") } return nil @@ -220,7 +220,7 @@ func (r *Remember) auth(ctx *authboss.Context) (authboss.Interrupt, error) { index := bytes.IndexByte(token, ';') if index < 0 { - return authboss.InterruptNone, errors.New("remember: Invalid remember token") + return authboss.InterruptNone, errors.New("invalid remember token") } // Get the key. diff --git a/router_test.go b/router_test.go index 367c429..e184fbb 100644 --- a/router_test.go +++ b/router_test.go @@ -2,12 +2,14 @@ package authboss import ( "bytes" - "errors" + "context" "io/ioutil" "net/http" "net/http/httptest" "strings" "testing" + + "github.com/pkg/errors" ) const testRouterModName = "testrouter" @@ -22,7 +24,6 @@ type testRouterModule struct { func (t testRouterModule) Initialize(ab *Authboss) error { return nil } func (t testRouterModule) Routes() RouteTable { return t.routes } -func (t testRouterModule) Storage() StorageOptions { return nil } func testRouterSetup() (*Authboss, http.Handler, *bytes.Buffer) { ab := New() @@ -53,7 +54,7 @@ func testRouterCallbackSetup(path string, h HandlerFunc) (w *httptest.ResponseRe func TestRouter(t *testing.T) { called := false - w, r := testRouterCallbackSetup("/called", func(ctx *Context, w http.ResponseWriter, r *http.Request) error { + w, r := testRouterCallbackSetup("/called", func(http.ResponseWriter, *http.Request) error { called = true return nil }) @@ -94,7 +95,7 @@ func TestRouter_NotFound(t *testing.T) { func TestRouter_BadRequest(t *testing.T) { err := ClientDataErr{"what"} w, r := testRouterCallbackSetup("/badrequest", - func(ctx *Context, w http.ResponseWriter, r *http.Request) error { + func(http.ResponseWriter, *http.Request) error { return err }, ) @@ -133,7 +134,7 @@ func TestRouter_BadRequest(t *testing.T) { func TestRouter_Error(t *testing.T) { err := errors.New("error") w, r := testRouterCallbackSetup("/error", - func(ctx *Context, w http.ResponseWriter, r *http.Request) error { + func(http.ResponseWriter, *http.Request) error { return err }, ) @@ -178,7 +179,7 @@ func TestRouter_Redirect(t *testing.T) { } w, r := testRouterCallbackSetup("/error", - func(ctx *Context, w http.ResponseWriter, r *http.Request) error { + func(http.ResponseWriter, *http.Request) error { return err }, ) @@ -237,17 +238,17 @@ func TestRouter_redirectIfLoggedIn(t *testing.T) { {"/recover", false, false, false}, } - storer := mockStorer{"john@john.com": Attributes{ - StoreEmail: "john@john.com", - StorePassword: "password", + storer := mockStoreLoader{"john@john.com": mockUser{ + Email: "john@john.com", + Password: "password", }} ab := New() - ab.Storer = storer + ab.StoreLoader = storer for i, test := range tests { session := mockClientStore{} cookies := mockClientStore{} - ctx := ab.NewContext() + ctx := context.TODO() ctx.SessionStorer = session ctx.CookieStorer = cookies @@ -285,7 +286,7 @@ func TestRouter_redirectIfLoggedInError(t *testing.T) { session := mockClientStore{SessionKey: "john"} cookies := mockClientStore{} - ctx := ab.NewContext() + ctx := context.TODO() ctx.SessionStorer = session ctx.CookieStorer = cookies @@ -316,7 +317,7 @@ func TestRouter_redirectIfLoggedInUserNotFound(t *testing.T) { session := mockClientStore{SessionKey: "john"} cookies := mockClientStore{} - ctx := ab.NewContext() + ctx := context.TODO() ctx.SessionStorer = session ctx.CookieStorer = cookies diff --git a/rules.go b/rules.go index 618b72d..1af51c4 100644 --- a/rules.go +++ b/rules.go @@ -1,10 +1,11 @@ package authboss import ( - "errors" "fmt" "regexp" "unicode" + + "github.com/pkg/errors" ) var blankRegex = regexp.MustCompile(`^\s*$`) diff --git a/storer.go b/storer.go index 411eb47..48570d1 100644 --- a/storer.go +++ b/storer.go @@ -3,9 +3,10 @@ package authboss import ( "bytes" "context" - "errors" "reflect" "time" + + "github.com/pkg/errors" ) // Data store constants for attribute names. @@ -26,11 +27,11 @@ const ( var ( // ErrUserNotFound should be returned from Get when the record is not found. - ErrUserNotFound = errors.New("User not found") + ErrUserNotFound = errors.New("user not found") // ErrTokenNotFound should be returned from UseToken when the record is not found. - ErrTokenNotFound = errors.New("Token not found") + ErrTokenNotFound = errors.New("token not found") // ErrUserFound should be returned from Create when the primaryID of the record is found. - ErrUserFound = errors.New("User found") + ErrUserFound = errors.New("user found") ) // StoreLoader represents the data store that's capable of loading users diff --git a/validation_test.go b/validation_test.go index e5c0e8c..c1726ed 100644 --- a/validation_test.go +++ b/validation_test.go @@ -1,8 +1,9 @@ package authboss import ( - "errors" "testing" + + "github.com/pkg/errors" ) func TestErrorList_Error(t *testing.T) {