From d18473568ed682887a74385c13557d277d38ebdf Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Thu, 4 Apr 2019 01:41:14 +0200 Subject: [PATCH] Basic credentials set/check --- internal/rules/resources_mock_test.go | 12 ++ system/cli/users.go | 51 ++++++++- system/internal/repository/credentials.go | 8 ++ .../internal/repository/mocks/credentials.go | 12 ++ system/internal/service/auth.go | 105 +++++++++++++++++- system/internal/service/auth_test.go | 84 +++++++++++++- 6 files changed, 260 insertions(+), 12 deletions(-) diff --git a/internal/rules/resources_mock_test.go b/internal/rules/resources_mock_test.go index 6e0119d65..6adaea5c7 100644 --- a/internal/rules/resources_mock_test.go +++ b/internal/rules/resources_mock_test.go @@ -99,3 +99,15 @@ func (m *MockResourcesInterface) Delete(roleID uint64) error { func (mr *MockResourcesInterfaceMockRecorder) Delete(roleID interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockResourcesInterface)(nil).Delete), roleID) } + +// Reset mocks base method +func (m *MockResourcesInterface) Reset(rules []Rule) error { + ret := m.ctrl.Call(m, "Reset", rules) + ret0, _ := ret[0].(error) + return ret0 +} + +// Reset indicates an expected call of Reset +func (mr *MockResourcesInterfaceMockRecorder) Reset(rules interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reset", reflect.TypeOf((*MockResourcesInterface)(nil).Reset), rules) +} diff --git a/system/cli/users.go b/system/cli/users.go index 7074480c6..c9fef8565 100644 --- a/system/cli/users.go +++ b/system/cli/users.go @@ -3,12 +3,15 @@ package cli import ( "context" "fmt" - "os" + "syscall" + "github.com/davecgh/go-spew/spew" "github.com/spf13/cobra" "github.com/titpetric/factory" + "golang.org/x/crypto/ssh/terminal" "github.com/crusttech/crust/system/internal/repository" + "github.com/crusttech/crust/system/internal/service" "github.com/crusttech/crust/system/types" ) @@ -20,7 +23,7 @@ func usersCmd(ctx context.Context, db *factory.DB) *cobra.Command { } // List users. - cmdUsersList := &cobra.Command{ + listCmd := &cobra.Command{ Use: "list", Short: "List users", Run: func(cmd *cobra.Command, args []string) { @@ -31,8 +34,7 @@ func usersCmd(ctx context.Context, db *factory.DB) *cobra.Command { users, err := userRepo.Find(uf) if err != nil { - fmt.Printf("Error: %v\n", err) - os.Exit(1) + exit(cmd, err) } fmt.Println(" Created Updated Email") @@ -54,7 +56,46 @@ func usersCmd(ctx context.Context, db *factory.DB) *cobra.Command { }, } - cmd.AddCommand(cmdUsersList) + addCmd := &cobra.Command{ + Use: "add [email]", + Short: "Add new user", + Args: cobra.MinimumNArgs(1), + Run: func(cmd *cobra.Command, args []string) { + var ( + userRepo = repository.User(ctx, db) + authSvc = service.Auth(ctx) + + // @todo email validation + user = &types.User{Email: args[0]} + + err error + password []byte + ) + + if user, err = userRepo.Create(user); err != nil { + exit(cmd, err) + } + + cmd.Printf("User created [%d].\n", user.ID) + + cmd.Print("Set password password: ") + if password, err = terminal.ReadPassword(syscall.Stdin); err != nil { + exit(cmd, err) + } + + if len(password) == 0 { + // Password not set, that's ok too. + exit(cmd, nil) + } + + spew.Dump(password) + if err = authSvc.ChangePassword(user, password); err != nil { + exit(cmd, err) + } + }, + } + + cmd.AddCommand(listCmd, addCmd) return cmd } diff --git a/system/internal/repository/credentials.go b/system/internal/repository/credentials.go index adbe43fec..68de68dfd 100644 --- a/system/internal/repository/credentials.go +++ b/system/internal/repository/credentials.go @@ -23,6 +23,7 @@ type ( Create(c *types.Credentials) (*types.Credentials, error) Update(c *types.Credentials) (*types.Credentials, error) DeleteByID(id uint64) error + DeleteByKind(ownerID uint64, kind string) (err error) } credentials struct { @@ -105,3 +106,10 @@ func (r *credentials) Update(c *types.Credentials) (*types.Credentials, error) { func (r *credentials) DeleteByID(id uint64) error { return r.updateColumnByID(r.tblname, "deleted_at", time.Now(), id) } + +func (r *credentials) DeleteByKind(ownerID uint64, kind string) (err error) { + return exec(r.db().Exec( + fmt.Sprintf("UPDATE %s SET deleted_at = NOW() WHERE rel_owner = ? AND kind = ?", r.tblname), + ownerID, + kind)) +} diff --git a/system/internal/repository/mocks/credentials.go b/system/internal/repository/mocks/credentials.go index f30979ece..4f846fde3 100644 --- a/system/internal/repository/mocks/credentials.go +++ b/system/internal/repository/mocks/credentials.go @@ -150,3 +150,15 @@ func (m *MockCredentialsRepository) DeleteByID(id uint64) error { func (mr *MockCredentialsRepositoryMockRecorder) DeleteByID(id interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteByID", reflect.TypeOf((*MockCredentialsRepository)(nil).DeleteByID), id) } + +// DeleteByKind mocks base method +func (m *MockCredentialsRepository) DeleteByKind(ownerID uint64, kind string) error { + ret := m.ctrl.Call(m, "DeleteByKind", ownerID, kind) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteByKind indicates an expected call of DeleteByKind +func (mr *MockCredentialsRepositoryMockRecorder) DeleteByKind(ownerID, kind interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteByKind", reflect.TypeOf((*MockCredentialsRepository)(nil).DeleteByKind), ownerID, kind) +} diff --git a/system/internal/service/auth.go b/system/internal/service/auth.go index 079705848..ceaff9485 100644 --- a/system/internal/service/auth.go +++ b/system/internal/service/auth.go @@ -3,10 +3,12 @@ package service import ( "context" "log" + "regexp" "time" "github.com/markbates/goth" "github.com/pkg/errors" + "golang.org/x/crypto/bcrypt" "github.com/crusttech/crust/system/internal/repository" "github.com/crusttech/crust/system/types" @@ -29,13 +31,21 @@ type ( External(profile goth.User) (*types.User, error) - CheckPassword(username, password string) (*types.User, error) - ChangePassword(user *types.User, password string) error + CheckPassword(email string, password []byte) (*types.User, error) + ChangePassword(user *types.User, password []byte) error CheckCredentials(credentialsID uint64, secret string) (*types.User, error) RevokeCredentialsByID(user *types.User, credentialsID uint64) error } ) +const ( + CredentialsTypePassword = "password" +) + +var ( + reEmail = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") +) + func defaultProviderValidator(provider string) error { _, err := goth.GetProvider(provider) return err @@ -203,15 +213,98 @@ func (svc *auth) External(profile goth.User) (u *types.User, err error) { // CheckPassword verifies username/password combination // // Expects plain text password as an input -func (svc *auth) CheckPassword(username, password string) (*types.User, error) { - panic("svc.auth.CheckPassword, not implemented") +func (svc *auth) CheckPassword(email string, password []byte) (u *types.User, err error) { + if err = svc.validateCredentials(email, password); err != nil { + return + } + + return u, svc.db.Transaction(func() error { + var ( + cc types.CredentialsSet + ) + + u, err = svc.users.FindByEmail(email) + if err != repository.ErrUserNotFound { + return errors.New("invalid username/password combination") + } + + if err != nil { + return errors.Wrap(err, "could not check password") + } + + cc, err := svc.credentials.FindByKind(u.ID, CredentialsTypePassword) + if err != nil { + return errors.Wrap(err, "could not find credentials") + } + + return svc.checkPassword(password, cc) + }) +} + +// validateCredentials does basic format & length check +func (svc auth) validateCredentials(email string, password []byte) error { + if !reEmail.MatchString(email) { + return errors.New("invalid email format") + } + + if len(password) == 0 { + return errors.New("empty password") + } + + return nil +} + +func (svc auth) checkPassword(password []byte, cc types.CredentialsSet) (err error) { + // We need only valid credentials (skip deleted, expired) + cc, _ = cc.Filter(func(c *types.Credentials) (b bool, e error) { + return c.Valid(), nil + }) + + for _, c := range cc { + if len(c.Credentials) == 0 { + continue + } + + err = bcrypt.CompareHashAndPassword([]byte(c.Credentials), password) + if err == bcrypt.ErrMismatchedHashAndPassword { + // Mismatch, continue with the checking + continue + } else if err != nil { + // Some other error + return errors.Wrap(err, "could not compare passwords") + } else { + // Password matched one of credentials + return nil + } + } + + return errors.New("invalid username/password combination") } // ChangePassword (soft) deletes old password entry and creates a new one // // Expects plain text password as an input -func (svc *auth) ChangePassword(user *types.User, password string) error { - panic("svc.auth.ChangePassword, not implemented") +func (svc *auth) ChangePassword(user *types.User, password []byte) (err error) { + var hash []byte + + hash, err = bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return errors.Wrap(err, "could not hash password") + } + + return svc.db.Transaction(func() error { + if err = svc.credentials.DeleteByKind(user.ID, CredentialsTypePassword); err != nil { + return errors.Wrap(err, "could not remove old passswords") + } + + _, err = svc.credentials.Create(&types.Credentials{ + OwnerID: user.ID, + Kind: CredentialsTypePassword, + Credentials: string(hash), + }) + + return errors.Wrap(err, "could not create new password") + }) } // CheckCredentials searches for credentials/secret combination and returns loaded user if successful diff --git a/system/internal/service/auth_test.go b/system/internal/service/auth_test.go index 8661373b7..2ae1d5452 100644 --- a/system/internal/service/auth_test.go +++ b/system/internal/service/auth_test.go @@ -1,4 +1,4 @@ -// +build unit +// +-build unit package service @@ -8,6 +8,7 @@ import ( "github.com/golang/mock/gomock" "github.com/markbates/goth" + "golang.org/x/crypto/bcrypt" "github.com/crusttech/crust/internal/test" "github.com/crusttech/crust/system/internal/repository" @@ -105,3 +106,84 @@ func TestAuth_External_NonExisting(t *testing.T) { test.Assert(t, auser.ID == u.ID, "Did not receive expected user") } } + +func Test_auth_validateCredentials(t *testing.T) { + type args struct { + email string + password []byte + } + tests := []struct { + name string + args args + wantErr bool + }{ + {name: "no email", args: args{"", []byte("")}, wantErr: true}, + {name: "bad email", args: args{"test", []byte("")}, wantErr: true}, + {name: "no pass", args: args{"test@domain.tld", []byte("")}, wantErr: true}, + {name: "all good", args: args{"test@domain.tld", []byte("password")}, wantErr: false}, + } + svc := auth{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := svc.validateCredentials(tt.args.email, tt.args.password); (err != nil) != tt.wantErr { + t.Errorf("auth.validateCredentials() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_auth_checkPassword(t *testing.T) { + plainPassword := []byte(" ... plain password ... ") + hashedPassword, _ := bcrypt.GenerateFromPassword(plainPassword, bcrypt.DefaultCost) + type args struct { + password []byte + cc types.CredentialsSet + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty set", + wantErr: true, + args: args{}}, + { + name: "bad pwd", + wantErr: true, + args: args{ + password: []byte(" foo "), + cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: string(hashedPassword)}}}}, + { + name: "invalid credentials", + wantErr: true, + args: args{ + password: []byte(" foo "), + cc: types.CredentialsSet{&types.Credentials{ID: 0, Credentials: string(hashedPassword)}}}}, + { + name: "ok", + wantErr: false, + args: args{ + password: plainPassword, + cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: string(hashedPassword)}}}}, + { + name: "multipass", + wantErr: false, + args: args{ + password: plainPassword, + cc: types.CredentialsSet{ + &types.Credentials{ID: 0, Credentials: string(hashedPassword)}, + &types.Credentials{ID: 1, Credentials: "$2a$10$8sOZxfZinxnu3bAtpkqEx.wBBwOfci6aG1szgUyxm5.BL2WiLu.ni"}, + &types.Credentials{ID: 2, Credentials: string(hashedPassword)}, + &types.Credentials{ID: 3, Credentials: ""}, + }}}, + } + svc := auth{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := svc.checkPassword(tt.args.password, tt.args.cc); (err != nil) != tt.wantErr { + t.Errorf("auth.checkPassword() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}