From cb390566956cc7efd291cfd14bdedf78b2398d32 Mon Sep 17 00:00:00 2001 From: Kris Runzer Date: Mon, 12 Jan 2015 21:08:52 -0800 Subject: [PATCH] Cleaned up auth module and tests --- .gitignore | 53 +++---- auth/auth.go | 90 ++++++------ auth/auth_test.go | 326 +++++++++++++++++++++++++++++-------------- auth/bindata.go | 4 +- auth/mocks.go | 44 ++++++ auth/views/login.tpl | 3 - 6 files changed, 336 insertions(+), 184 deletions(-) create mode 100644 auth/mocks.go diff --git a/.gitignore b/.gitignore index 8249369..ced52f6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,27 +1,28 @@ -# Compiled Object files, Static and Dynamic libs (Shared Objects) -*.o -*.a -*.so - -# Folders -_obj -_test - -# Architecture specific extensions/prefixes -*.[568vq] -[568vq].out - -*.cgo1.go -*.cgo2.c -_cgo_defun.c -_cgo_gotypes.go -_cgo_export.* - -_testmain.go - -*.exe -*.test -*.prof - -*.iml +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof +*.out + +*.iml .idea \ No newline at end of file diff --git a/auth/auth.go b/auth/auth.go index d962f23..b4d0078 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -5,68 +5,68 @@ import ( "net/http" "path/filepath" + "code.google.com/p/go.crypto/bcrypt" + "gopkg.in/authboss.v0" "html/template" - "io/ioutil" - "bytes" "io" ) const ( methodGET = "GET" methodPOST = "POST" -) -var errAuthFailed = errors.New("invalid username and/or password") + pageLogin = "login.tpl" + + attrUsername = "Username" + attrPassword = "Password" +) func init() { a := &Auth{} authboss.RegisterModule("auth", a) } +type AuthPage struct { + Error string + Username string +} + type Auth struct { - routes authboss.RouteTable - storageOptions authboss.StorageOptions - users authboss.Storer - loginPage *bytes.Buffer - logoutRedirect string - loginSuccessRedirect string - logger io.Writer + routes authboss.RouteTable + storageOptions authboss.StorageOptions + users authboss.Storer + logoutRedirect string + loginRedirect string + logger io.Writer + templates *template.Template } func (a *Auth) Initialize(c *authboss.Config) (err error) { - var data []byte - - if data, err = ioutil.ReadFile(filepath.Join(c.ViewsPath, "login.tpl")); err != nil { - if data, err = views_login_tpl_bytes(); err != nil { + if a.templates, err = template.ParseFiles(filepath.Join(c.ViewsPath, pageLogin)); err != nil { + var loginTplBytes []byte + if loginTplBytes, err = views_login_tpl_bytes(); err != nil { return err } - } - var tpl *template.Template - if tpl, err = template.New("login.tpl").Parse(string(data)); err != nil { - return err - } else { - a.loginPage = &bytes.Buffer{} - if err = tpl.Execute(a.loginPage, nil); err != nil { + if a.templates, err = template.New(pageLogin).Parse(string(loginTplBytes)); err != nil { return err } } a.storageOptions = authboss.StorageOptions{ - "Username": authboss.String, - "Password": authboss.String, + attrUsername: authboss.String, + attrPassword: authboss.String, } a.routes = authboss.RouteTable{ - "login": a.loginHandler, - "logout": a.logoutHandler, + "login": a.loginHandlerFunc, + "logout": a.logoutHandlerFunc, } a.users = c.Storer - a.logoutRedirect = c.AuthLogoutRoute - a.loginSuccessRedirect = c.AuthLoginSuccessRoute + a.loginRedirect = c.AuthLoginSuccessRoute return nil } @@ -79,41 +79,39 @@ func (a *Auth) Storage() authboss.StorageOptions { return a.storageOptions } -func (a *Auth) loginHandler(c *authboss.Context, w http.ResponseWriter, r *http.Request) { +func (a *Auth) loginHandlerFunc(c *authboss.Context, w http.ResponseWriter, r *http.Request) { switch r.Method { case methodGET: - w.Write(a.loginPage.Bytes()) + a.templates.ExecuteTemplate(w, pageLogin, nil) case methodPOST: - if err := a.authenticate(r.PostFormValue("username"), r.PostFormValue("password")); err != nil { - //Todo : page of things + u := r.PostFormValue("username") + p := r.PostFormValue("password") + + if err := a.authenticate(u, p); err != nil { w.WriteHeader(http.StatusForbidden) + a.templates.ExecuteTemplate(w, pageLogin, AuthPage{"invalid username and/or password", u}) return } - - http.Redirect(w, r, a.loginSuccessRedirect, http.StatusFound) + http.Redirect(w, r, a.loginRedirect, http.StatusFound) default: w.WriteHeader(http.StatusMethodNotAllowed) } } func (a *Auth) authenticate(username, password string) error { - userInter, err := a.users.Get(username, nil) - if err != nil { - return errAuthFailed - } - - userAttrs := authboss.Unbind(userInter) - - if pwd, ok := userAttrs["Password"]; !ok { - return errAuthFailed - } else if pwd.Value.(string) != password { - return errAuthFailed + if userInter, err := a.users.Get(username, nil); err != nil { + return err + } else { + userAttrs := authboss.Unbind(userInter) + if err := bcrypt.CompareHashAndPassword([]byte(userAttrs[attrPassword].Value.(string)), []byte(password)); err != nil { + return errors.New("invalid password") + } } return nil } -func (a *Auth) logoutHandler(c *authboss.Context, w http.ResponseWriter, r *http.Request) { +func (a *Auth) logoutHandlerFunc(c *authboss.Context, w http.ResponseWriter, r *http.Request) { switch r.Method { case methodGET: http.Redirect(w, r, a.logoutRedirect, http.StatusFound) diff --git a/auth/auth_test.go b/auth/auth_test.go index 7b1123b..d20bbad 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -1,152 +1,264 @@ package auth import ( - "net/http" "testing" "bytes" - "reflect" + "io/ioutil" + + "net/http" + + "html/template" "net/http/httptest" + "net/url" + "strings" + "gopkg.in/authboss.v0" ) -func TestAuth_Initialize_LoadsDefaultLoginPageWhenOverrideNotSpecified(t *testing.T) { - t.Parallel() - - a := &Auth{} - if err := a.Initialize(&authboss.Config{}); err != nil { - t.Errorf("Unexpected config error: %v", err) +func getCompiledTemplate(path string, data interface{}) (b *bytes.Buffer, err error) { + var file []byte + if file, err = ioutil.ReadFile(path); err != nil { + return nil, err } - bindata, err := views_login_tpl_bytes() - if err != nil { - t.Errorf("Unexpected bindata error: %v", err) + var tpl *template.Template + if tpl, err = template.New("tpl").Parse(string(file)); err != nil { + return nil, err } - if !bytes.Equal(a.loginPage.Bytes(), bindata) { - t.Errorf("Expected '%s', got '%s'", bindata, a.loginPage.Bytes()) + b = &bytes.Buffer{} + if err = tpl.Execute(b, data); err != nil { + return nil, err } + + return b, nil } -/*func TestAuth_Initialize_LoadsSpecifiedLoginPageWhenOverrideSpecified(t *testing.T) { +func TestAuth_Storage(t *testing.T) { t.Parallel() a := &Auth{} - if err := a.Initialize(&authboss.Config{ - AuthLoginPageURI: "auth_test.go", - }); err != nil { + if err := a.Initialize(authboss.NewConfig()); err != nil { t.Errorf("Unexpected config error: %v", err) } + options := a.Storage() - file, err := ioutil.ReadFile("auth_test.go") - if err != nil { - t.Errorf("Unexpected bindata error: %v", err) + tests := []struct { + Name string + Type authboss.DataType + }{ + {"Username", authboss.String}, + {"Password", authboss.String}, } - if !bytes.Equal(a.loginPage.Bytes(), file) { - t.Errorf("Expected '%s', got '%s'", file, a.loginPage.Bytes()) - } -}*/ - -func TestAuth_Initialize_RegistersRoutes(t *testing.T) { - t.Parallel() - - a := &Auth{} - if err := a.Initialize(&authboss.Config{}); err != nil { - t.Errorf("Unexpected config error: %v", err) - } - - if handler, ok := a.routes["login"]; !ok { - t.Error("Expected route 'login' but was not found'") - } else if reflect.ValueOf(handler).Pointer() != reflect.ValueOf(a.loginHandler).Pointer() { - t.Errorf("Expcted func 'loginHandler' but was not found") - } - - if handler, ok := a.routes["logout"]; !ok { - t.Error("Expected route 'logout' but was not found'") - } else if reflect.ValueOf(handler).Pointer() != reflect.ValueOf(a.logoutHandler).Pointer() { - t.Errorf("Expcted func 'logoutHandler' but was not found") + for i, test := range tests { + if value, ok := options[test.Name]; !ok { + t.Errorf("%d> Expected key %s", i, test.Name) + continue + } else if value != test.Type { + t.Errorf("$d> Expected key %s to have value %v, got %v", i, test.Name, test.Type, value) + continue + } } } func TestAuth_Routes(t *testing.T) { t.Parallel() - routes := authboss.RouteTable{ - "a": func(_ http.ResponseWriter, _ *http.Request) {}, - "b": func(_ http.ResponseWriter, _ *http.Request) {}, - } - a := Auth{routes: routes} - - if !reflect.DeepEqual(routes, a.Routes()) { - t.Errorf("Failed to retrieve routes") - } -} - -func TestAuth_loginHandler_GET(t *testing.T) { - t.Parallel() - a := &Auth{} - if err := a.Initialize(&authboss.Config{}); err != nil { - t.Errorf("Unexpected config error: %$", err) + if err := a.Initialize(authboss.NewConfig()); err != nil { + t.Errorf("Unexpected config error: %v", err) } - - w := httptest.NewRecorder() - r, err := http.NewRequest("GET", "/login", nil) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - - bindata, err := views_login_tpl_bytes() - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - - a.loginHandler(w, r) - - if http.StatusOK != w.Code { - t.Errorf("%Expected response code %d, got %d", http.StatusOK, w.Code) - } - if !bytes.Equal(bindata, w.Body.Bytes()) { - t.Errorf("Expected body '%s', got '%s'", string(bindata), w.Body.String()) - } -} - -func TestAuth_logoutHandler_GET(t *testing.T) { - t.Parallel() + routes := a.Routes() tests := []struct { - Config *authboss.Config - RedirectPath string + Route string }{ - {&authboss.Config{}, "/"}, - {&authboss.Config{AuthLogoutRoute: "/logout"}, "/logout"}, - {&authboss.Config{MountPath: "/auth", AuthLogoutRoute: "/logout"}, "/auth/logout"}, + {"login"}, + {"logout"}, } for i, test := range tests { - a := Auth{} - if err := a.Initialize(test.Config); err != nil { - t.Errorf("%d> Unexpected config error: %v", i, err) - } - - w := httptest.NewRecorder() - r, err := http.NewRequest("GET", "/logout", nil) - if err != nil { - t.Errorf("%d> Unexpected error: %v", i, err) - } - - a.logoutHandler(w, r) - - if http.StatusTemporaryRedirect != w.Code { - t.Errorf("%d> Expected response code %d, got %d", i, http.StatusTemporaryRedirect, w.Code) - } - if test.RedirectPath != w.HeaderMap["Location"][0] { - t.Errorf("%d> Expected header Location '%s', got '%s'", 1, test.RedirectPath, w.HeaderMap["Location"][0]) + if value, ok := routes[test.Route]; !ok { + t.Errorf("%d> Expected key %s", i, test.Route) + } else if value == nil { + t.Errorf("%d> Expected key %s to have func", i, test.Route) + } + } +} + +func TestAuth_loginHandlerFunc_GET(t *testing.T) { + t.Parallel() + + tests := []struct { + Config *authboss.Config + }{ + {authboss.NewConfig()}, + {&authboss.Config{}}, + {&authboss.Config{ViewsPath: "views"}}, + } + + for i, test := range tests { + a := &Auth{} + if err := a.Initialize(test.Config); err != nil { + t.Errorf("%d> Unexpected config error: %v", i, err) + continue + } + + r, err := http.NewRequest("GET", "/login", nil) + if err != nil { + t.Errorf("Unexpected error '%s'", err) + } + w := httptest.NewRecorder() + + a.loginHandlerFunc(nil, w, r) + + if tpl, err := getCompiledTemplate("views/login.tpl", nil); err != nil { + t.Errorf("%d> Unexpected error '%s'", i, err) + continue + } else { + if !bytes.Equal(tpl.Bytes(), w.Body.Bytes()) { + t.Errorf("%d> Expected '%s', got '%s'", i, tpl.Bytes(), w.Body.Bytes()) + continue + } + } + } +} + +func TestAuth_loginHandlerFunc_POST(t *testing.T) { + t.Parallel() + + tests := []struct { + Username, Password string + StatusCode int + Location string + BodyData *AuthPage + }{ + {"john", "1234", http.StatusFound, "/dashboard", nil}, + {"jane", "1234", http.StatusForbidden, "", &AuthPage{"invalid username and/or password", "jane"}}, + {"mike", "", http.StatusForbidden, "", &AuthPage{"invalid username and/or password", "jane"}}, + } + + c := &authboss.Config{ + Storer: NewMockUserStorer(), + AuthLoginSuccessRoute: "/dashboard", + } + + for i, test := range tests { + a := &Auth{} + if err := a.Initialize(c); err != nil { + t.Errorf("%d> Unexpected config error: %v", i, err) + continue + } + + postData := url.Values{} + postData.Set("username", test.Username) + postData.Set("password", test.Password) + + r, err := http.NewRequest("POST", "/login", strings.NewReader(postData.Encode())) + if err != nil { + t.Errorf("%d> Unexpected error '%s'", i, err) + continue + } + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + + a.loginHandlerFunc(nil, w, r) + + if test.StatusCode != w.Code { + t.Errorf("%d> Expected status code %d, got %d", i, test.StatusCode, w.Code) + continue + } + + location := w.Header().Get("Location") + if test.Location != location { + t.Errorf("%d> Expected lcoation %s, got %s", i, test.Location, location) + continue + } + + if test.BodyData != nil { + if tpl, err := getCompiledTemplate("views/login.tpl", test.BodyData); err != nil { + t.Errorf("%d> Unexpected error '%s'", i, err) + continue + } else { + if !bytes.Equal(tpl.Bytes(), w.Body.Bytes()) { + t.Errorf("%d> Expected '%s', got '%s'", i, tpl.Bytes(), w.Body.Bytes()) + continue + } + } + } + } +} + +func TestAuth_loginHandlerFunc_OtherMethods(t *testing.T) { + t.Parallel() + + a := Auth{} + methods := []string{"HEAD", "PUT", "DELETE", "TRACE", "CONNECT"} + + for i, method := range methods { + r, err := http.NewRequest(method, "/login", nil) + if err != nil { + t.Errorf("%d> Unexpected error '%s'", i, err) + } + w := httptest.NewRecorder() + + a.loginHandlerFunc(nil, w, r) + + if http.StatusMethodNotAllowed != w.Code { + t.Errorf("%d> Expected status code %d, got %d", i, http.StatusMethodNotAllowed, w.Code) + continue + } + } +} + +func TestAuth_logoutHandlerFunc_GET(t *testing.T) { + t.Parallel() + + a := Auth{} + if err := a.Initialize(&authboss.Config{AuthLogoutRoute: "/dashboard"}); err != nil { + t.Errorf("Unexpeced config error '%s'", err) + } + r, err := http.NewRequest("GET", "/logout", nil) + if err != nil { + t.Errorf("Unexpected error '%s'", err) + } + w := httptest.NewRecorder() + + a.logoutHandlerFunc(nil, w, r) + + if http.StatusFound != w.Code { + t.Errorf("Expected status code %d, got %d", http.StatusFound, w.Code) + } + + location := w.Header().Get("Location") + if location != "/dashboard" { + t.Errorf("Expected lcoation %s, got %s", "/dashboard", location) + } +} + +func TestAuth_logoutHandlerFunc_OtherMethods(t *testing.T) { + t.Parallel() + + a := Auth{} + methods := []string{"HEAD", "POST", "PUT", "DELETE", "TRACE", "CONNECT"} + + for i, method := range methods { + r, err := http.NewRequest(method, "/logout", nil) + if err != nil { + t.Errorf("%d> Unexpected error '%s'", i, err) + } + w := httptest.NewRecorder() + + a.logoutHandlerFunc(nil, w, r) + + if http.StatusMethodNotAllowed != w.Code { + t.Errorf("%d> Expected status code %d, got %d", i, http.StatusMethodNotAllowed, w.Code) + continue } } - } diff --git a/auth/bindata.go b/auth/bindata.go index 735c5f8..96a1304 100644 --- a/auth/bindata.go +++ b/auth/bindata.go @@ -61,7 +61,7 @@ func (fi bindata_file_info) Sys() interface{} { return nil } -var _views_login_tpl = []byte("\x1f\x8b\x08\x00\x00\x09\x6e\x88\x00\xff\x8c\x52\x4f\x4f\xfc\x20\x10\xbd\xff\x92\xdf\x77\xc0\xf1\x8c\x64\xef\x94\xa3\xa7\x3d\x18\x13\x3f\x00\x2d\xb3\x85\x48\x4b\x1d\x60\x75\xbf\xbd\xb4\xc2\xba\x8d\x31\xb1\x97\xf9\xc3\xe3\xcd\xeb\x1b\xa4\x4d\x93\x57\xff\xff\x49\x8b\xda\x94\xc8\xca\x27\xef\x38\x97\xde\xcd\xaf\x2c\x5d\x16\xec\x20\xe1\x47\x12\x43\x8c\xc0\x08\x7d\x07\x31\x5d\x3c\x46\x8b\x98\x80\x59\xc2\x53\x07\x42\xe7\x64\xfb\x10\xe3\xc3\x86\x12\x8a\xf3\x95\x52\x54\x4e\xd9\x07\x73\x59\xa3\x71\x67\x36\x78\x1d\x63\x07\x3e\x8c\x6e\xe6\x83\x26\x03\x6d\xaa\x3d\xa8\x63\x18\xb9\x9b\xcb\xc5\x83\x92\x3d\xb5\x83\x53\xa0\xa9\xe6\x5b\xed\xe6\x25\xa7\x1b\x6d\xc0\x66\x3d\x95\x3c\x47\xa4\x35\x03\xb6\x78\x3d\xa0\x0d\xde\x20\x75\xf0\x72\x6d\x13\xbe\x65\x47\x68\xca\x35\xca\x08\xbf\x71\x2e\x45\xe1\x7b\x28\xca\x2a\xef\x77\xbd\xe3\x7d\xba\xb6\xff\xc8\x1b\x73\x3f\xb9\xab\xda\xcd\x01\xd8\xf9\xc1\xbe\x5c\x69\xb8\xb3\xf6\xb9\x00\x8f\x1b\xb0\x99\x21\xaa\x1b\xb5\xfc\x61\xa9\x45\xbf\xec\x14\xe8\xba\xa4\x7b\x50\xcf\x38\xba\x98\x90\xa4\xd0\xea\xb6\xff\x18\x68\x0c\x89\xb5\x1f\x5a\x8f\xdb\xb4\xc2\xbf\xad\xb2\xc5\xb6\x4a\x51\xdf\xcd\x67\x00\x00\x00\xff\xff\x0b\x6f\xce\x8d\x40\x02\x00\x00") +var _views_login_tpl = []byte("\x1f\x8b\x08\x00\x00\x09\x6e\x88\x00\xff\x8c\x51\x4d\x4f\xc4\x20\x10\xbd\x9b\xf8\x1f\xc8\x78\xde\x34\xde\x69\x8f\x9e\xf6\x60\x4c\xfc\x01\xb4\xcc\x16\x12\x5a\x70\x80\xd5\xfd\xf7\x4e\x2b\xa0\x8d\x31\x91\xcb\xbc\xf9\xe0\xf1\x78\x23\x4d\x5a\xdc\x70\x7f\x27\x47\xaf\x6f\x5b\xd4\xf6\x2a\x26\xa7\x62\xec\xc1\xf9\xd9\xae\xa7\x49\x91\x06\xee\x08\x3e\xd2\x3c\x0e\x67\x3f\x9f\xec\x2a\x3b\x86\x72\xa4\xda\xb8\x78\x5a\x0a\xde\x73\xbb\x86\x9c\x44\xba\x05\xec\x21\xe1\x47\x02\xb1\xaa\x85\x71\x8e\x48\x1b\x02\x11\x9c\x9a\xd0\x78\xa7\x91\x7a\x78\x6d\x65\xc2\xb7\x6c\x09\x35\x5f\xa3\x8c\xf0\x17\x67\x60\x85\xef\x9e\x95\x15\xde\xef\xfc\xc0\xfb\xdc\xca\xff\xe4\x8d\x79\x5c\x6c\x53\xbb\x3b\x00\x07\x3f\xc4\x97\x2b\x75\xee\xaa\x5c\xe6\xc1\xf3\x3e\x58\xcd\xe8\x8a\x1b\x25\xfd\x65\xa9\x41\x17\x0e\x0a\x94\x30\x84\x97\x1e\x1e\x60\x78\xc1\xd9\xc6\x84\x24\x3b\x35\xfc\xac\x3f\x79\x9a\x7d\x12\xf5\x43\x5b\xbb\xbe\xc6\xfc\xdb\xe6\x5a\xac\xab\xec\xca\x6e\x3f\x03\x00\x00\xff\xff\x05\x40\xcd\x07\xe4\x01\x00\x00") func views_login_tpl_bytes() ([]byte, error) { return bindata_read( @@ -76,7 +76,7 @@ func views_login_tpl() (*asset, error) { return nil, err } - info := bindata_file_info{name: "views/login.tpl", size: 576, mode: os.FileMode(438), modTime: time.Unix(1420956750, 0)} + info := bindata_file_info{name: "views/login.tpl", size: 484, mode: os.FileMode(438), modTime: time.Unix(1421030024, 0)} a := &asset{bytes: bytes, info: info} return a, nil } diff --git a/auth/mocks.go b/auth/mocks.go new file mode 100644 index 0000000..adcaeae --- /dev/null +++ b/auth/mocks.go @@ -0,0 +1,44 @@ +package auth + +import ( + "errors" + + "strings" + + "gopkg.in/authboss.v0" +) + +type MockUser struct { + Username, Password string +} + +type MockUserStorer struct { + Users []MockUser +} + +func NewMockUserStorer() *MockUserStorer { + return &MockUserStorer{ + Users: []MockUser{ + {"John", "$2a$10$0hwgO.5fThx0DOHbErIxaemMTrU3RDNJchM6ToMOmFf.hkuX4RKRK"}, // 1234 + {"Jane", "$2a$10$tzIH0BU8BpOOsf768Iv4KecouL0gPgrvCpYZpBwJozlqezfabBpr2"}, // asdf + }, + } +} + +func (s MockUserStorer) Create(key string, attr authboss.Attributes) error { + return errors.New("Not implemented") +} + +func (s MockUserStorer) Put(key string, attr authboss.Attributes) error { + return errors.New("Not implemented") +} + +func (s MockUserStorer) Get(key string, attrMeta authboss.AttributeMeta) (result interface{}, err error) { + for _, u := range s.Users { + if strings.EqualFold(u.Username, key) { + return u, nil + } + } + + return nil, errors.New("User not found") +} diff --git a/auth/views/login.tpl b/auth/views/login.tpl index ae9b05a..f9cb8be 100644 --- a/auth/views/login.tpl +++ b/auth/views/login.tpl @@ -1,7 +1,4 @@ - - -

Log-in