From 5b4160b09fe64bdeeaa8237623928d934da17da3 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Fri, 4 Oct 2024 23:25:57 +0200 Subject: [PATCH 1/6] fix: migrate sigin to testable code #181 --- db/auth.go | 52 ++++++++++++ db/auth_test.go | 56 ++++++++++++- handler/auth.go | 57 ++++++++++++- handler/default.go | 4 +- service/auth.go | 194 +++++++++++++++++-------------------------- service/auth_test.go | 22 ++--- 6 files changed, 252 insertions(+), 133 deletions(-) diff --git a/db/auth.go b/db/auth.go index 76c010b..d99a2b0 100644 --- a/db/auth.go +++ b/db/auth.go @@ -6,6 +6,7 @@ import ( "database/sql" "errors" + "strings" "time" "github.com/google/uuid" @@ -13,6 +14,7 @@ import ( var ( ErrUserNotFound = errors.New("User not found") + ErrUserExists = errors.New("User already exists") ) type User struct { @@ -41,6 +43,10 @@ func NewUser(id uuid.UUID, email string, emailVerified bool, emailVerifiedAt *ti type DbAuth interface { GetUser(email string) (*User, error) + InsertUser(user *User) error + + GetEmailVerificationToken(userId uuid.UUID) (string, error) + InsertEmailVerificationToken(userId uuid.UUID, token string) error } type DbAuthSqlite struct { @@ -77,3 +83,49 @@ func (db DbAuthSqlite) GetUser(email string) (*User, error) { return NewUser(userId, email, emailVerified, emailVerifiedAt, isAdmin, password, salt, createdAt), nil } +func (db DbAuthSqlite) InsertUser(user *User) error { + _, err := db.db.Exec(` + INSERT INTO user (user_uuid, email, email_verified, email_verified_at, is_admin, password, salt, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + user.Id, user.Email, user.EmailVerified, user.EmailVerifiedAt, user.IsAdmin, user.Password, user.Salt, user.CreateAt) + + if err != nil { + if strings.Contains(err.Error(), "email") { + return ErrUserExists + } + + utils.LogError("SQL error InsertUser", err) + return types.ErrInternal + } + + return nil +} + +func (db DbAuthSqlite) GetEmailVerificationToken(userId uuid.UUID) (string, error) { + var token string + + err := db.db.QueryRow(` + SELECT token + FROM user_token + WHERE user_uuid = ? + AND type = 'email_verify'`, userId).Scan(&token) + + if err != nil && err != sql.ErrNoRows { + utils.LogError("Could not get token", err) + return "", types.ErrInternal + } + + return token, nil +} +func (db DbAuthSqlite) InsertEmailVerificationToken(userId uuid.UUID, token string) error { + _, err := db.db.Exec(` + INSERT INTO user_token (user_uuid, type, token, created_at) + VALUES (?, 'email_verify', ?, datetime())`, userId, token) + + if err != nil { + utils.LogError("Could not insert token", err) + return types.ErrInternal + } + + return nil +} diff --git a/db/auth_test.go b/db/auth_test.go index 48c6168..95e714d 100644 --- a/db/auth_test.go +++ b/db/auth_test.go @@ -4,6 +4,7 @@ import ( "me-fit/utils" "database/sql" + "errors" "reflect" "testing" "time" @@ -16,6 +17,9 @@ func setupDb(t *testing.T) *sql.DB { if err != nil { t.Fatalf("Error opening database: %v", err) } + t.Cleanup(func() { + db.Close() + }) err = utils.RunMigrations(db, "../") if err != nil { @@ -31,7 +35,6 @@ func TestGetUser(t *testing.T) { t.Run("should return UserNotFound", func(t *testing.T) { t.Parallel() db := setupDb(t) - defer db.Close() underTest := DbAuthSqlite{db: db} @@ -44,7 +47,6 @@ func TestGetUser(t *testing.T) { t.Run("should find user in database", func(t *testing.T) { t.Parallel() db := setupDb(t) - defer db.Close() underTest := DbAuthSqlite{db: db} @@ -69,5 +71,53 @@ func TestGetUser(t *testing.T) { t.Errorf("Expected %v, got %v", user, actual) } }) - +} +func TestInsertUser(t *testing.T) { + t.Parallel() + + t.Run("should insert user", func(t *testing.T) { + t.Parallel() + db := setupDb(t) + + underTest := DbAuthSqlite{db: db} + + verifiedAt := time.Date(2020, 1, 5, 13, 0, 0, 0, time.UTC) + createAt := time.Date(2020, 1, 5, 12, 0, 0, 0, time.UTC) + user := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) + + err := underTest.InsertUser(user) + if err != nil { + t.Fatalf("Error inserting user: %v", err) + } + + actual, err := underTest.GetUser(user.Email) + if err != nil { + t.Fatalf("Error getting user: %v", err) + } + + if !reflect.DeepEqual(user, actual) { + t.Errorf("Expected %v, got %v", user, actual) + } + }) + + t.Run("should throw error if user already exists", func(t *testing.T) { + t.Parallel() + db := setupDb(t) + + underTest := DbAuthSqlite{db: db} + + verifiedAt := time.Date(2020, 1, 5, 13, 0, 0, 0, time.UTC) + createAt := time.Date(2020, 1, 5, 12, 0, 0, 0, time.UTC) + user := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) + + err := underTest.InsertUser(user) + if err != nil { + t.Fatalf("Error inserting user: %v", err) + } + + err = underTest.InsertUser(user) + if !errors.Is(err, ErrUserExists) { + t.Fatalf("Error inserting user: %v", err) + } + }) } diff --git a/handler/auth.go b/handler/auth.go index 9267fee..6c7529a 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -8,6 +8,7 @@ import ( "me-fit/utils" "database/sql" + "errors" "net/http" "time" ) @@ -33,13 +34,13 @@ func NewHandlerAuth(db *sql.DB, service service.ServiceAuth, serverSettings *typ func (handler HandlerAuthImpl) handle(router *http.ServeMux) { // Don't use auth middleware for these routes, as it makes redirecting very difficult, if the mail is not yet verified router.Handle("/auth/signin", handler.handleSignInPage()) - router.Handle("/auth/signup", service.HandleSignUpPage(handler.db, handler.serverSettings)) + router.Handle("/auth/signup", handler.handleSignUpPage()) router.Handle("/auth/verify", service.HandleSignUpVerifyPage(handler.db, handler.serverSettings)) // Hint for the user to verify their email router.Handle("/auth/delete-account", service.HandleDeleteAccountPage(handler.db, handler.serverSettings)) router.Handle("/auth/verify-email", service.HandleSignUpVerifyResponsePage(handler.db)) // The link contained in the email router.Handle("/auth/change-password", service.HandleChangePasswordPage(handler.db, handler.serverSettings)) router.Handle("/auth/reset-password", service.HandleResetPasswordPage(handler.db, handler.serverSettings)) - router.Handle("/api/auth/signup", service.HandleSignUpComp(handler.db, handler.serverSettings)) + router.Handle("/api/auth/signup", handler.handleSignUp()) router.Handle("/api/auth/signin", handler.handleSignIn()) router.Handle("/api/auth/signout", service.HandleSignOutComp(handler.db)) router.Handle("/api/auth/delete-account", service.HandleDeleteAccountComp(handler.db, handler.serverSettings)) @@ -74,7 +75,6 @@ func (handler HandlerAuthImpl) handleSignInPage() http.HandlerFunc { } } } - func (handler HandlerAuthImpl) handleSignIn() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { user, err := utils.WaitMinimumTime(securityWaitDuration, func() (*service.User, error) { @@ -112,3 +112,54 @@ func (handler HandlerAuthImpl) handleSignIn() http.HandlerFunc { } } } + +func (handler HandlerAuthImpl) handleSignUpPage() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := utils.GetUserFromSession(handler.db, r) + + if user == nil { + userComp := service.UserInfoComp(nil) + signUpComp := auth.SignInOrUpComp(false) + err := template.Layout(signUpComp, userComp, handler.serverSettings.Environment).Render(r.Context(), w) + + if err != nil { + utils.LogError("Failed to render sign up page", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + + } else if !user.EmailVerified { + utils.DoRedirect(w, r, "/auth/verify") + } else { + utils.DoRedirect(w, r, "/") + } + } +} +func (handler HandlerAuthImpl) handleSignUp() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + var email = r.FormValue("email") + var password = r.FormValue("password") + + _, err := utils.WaitMinimumTime(securityWaitDuration, func() (interface{}, error) { + user, err := handler.service.SignUp(email, password) + if err != nil { + return nil, err + } + + go handler.service.SendVerificationMail(user) + return nil, nil + }) + + if err != nil { + if errors.Is(err, types.ErrInternal) { + utils.TriggerToast(w, r, "error", "An error occurred") + return + } else if errors.Is(err, service.ErrInvalidEmail) { + utils.TriggerToast(w, r, "error", "The email provided is invalid") + return + } + // If the "service.ErrAccountExists", then just continue + } + + utils.TriggerToast(w, r, "success", "A link to activate your account has been emailed to the address provided.") + } +} diff --git a/handler/default.go b/handler/default.go index be45239..af9eb74 100644 --- a/handler/default.go +++ b/handler/default.go @@ -15,7 +15,9 @@ func GetHandler(d *sql.DB, serverSettings *types.ServerSettings) http.Handler { router.HandleFunc("/", service.HandleIndexAnd404(d, serverSettings)) - handlerAuth := NewHandlerAuth(d, service.NewServiceAuthImpl(db.NewDbAuthSqlite(d)), serverSettings) + dbAuth := db.NewDbAuthSqlite(d) + serviceAuth := service.NewServiceAuthImpl(dbAuth, serverSettings) + handlerAuth := NewHandlerAuth(d, serviceAuth, serverSettings) // Serve static files (CSS, JS and images) router.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("./static/")))) diff --git a/service/auth.go b/service/auth.go index c9a8cd8..10edde2 100644 --- a/service/auth.go +++ b/service/auth.go @@ -11,6 +11,7 @@ import ( "net/mail" "net/url" "strings" + "time" "me-fit/db" "me-fit/template" @@ -27,6 +28,8 @@ import ( var ( ErrInvaidCredentials = errors.New("Invalid email or password") ErrPasswordComplexity = errors.New("Password needs to be 8 characters long, contain at least one number, one special, one uppercase and one lowercase character") + ErrInvalidEmail = errors.New("Invalid email") + ErrAccountExists = errors.New("Account already exists") ) type User struct { @@ -45,20 +48,25 @@ func NewUser(user *db.User) *User { type ServiceAuth interface { SignIn(email string, password string) (*User, error) + SignUp(email string, password string) (*User, error) + SendVerificationMail(user *User) } type ServiceAuthImpl struct { - dbAuth db.DbAuth + dbAuth db.DbAuth + serverSettings *types.ServerSettings + mailService MailService } -func NewServiceAuthImpl(dbAuth db.DbAuth) *ServiceAuthImpl { +func NewServiceAuthImpl(dbAuth db.DbAuth, serverSettings *types.ServerSettings) *ServiceAuthImpl { return &ServiceAuthImpl{ - dbAuth: dbAuth, + dbAuth: dbAuth, + serverSettings: serverSettings, + mailService: NewMailService(serverSettings), } } func (service ServiceAuthImpl) SignIn(email string, password string) (*User, error) { - user, err := service.dbAuth.GetUser(email) if err != nil { if errors.Is(err, db.ErrUserNotFound) { @@ -77,30 +85,79 @@ func (service ServiceAuthImpl) SignIn(email string, password string) (*User, err return NewUser(user), nil } -// TODO +func (service ServiceAuthImpl) SignUp(email string, password string) (*User, error) { + _, err := mail.ParseAddress(email) + if err != nil { + return nil, ErrInvalidEmail + } -func HandleSignUpPage(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + err = checkPassword(password) + if err != nil { + return nil, err + } - if user == nil { - userComp := UserInfoComp(nil) - signUpComp := auth.SignInOrUpComp(false) - err := template.Layout(signUpComp, userComp, serverSettings.Environment).Render(r.Context(), w) + userId, err := uuid.NewRandom() + if err != nil { + utils.LogError("Could not generate UUID", err) + return nil, types.ErrInternal + } - if err != nil { - utils.LogError("Failed to render sign up page", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } + salt := make([]byte, 16) + _, err = rand.Read(salt) + if err != nil { + utils.LogError("Could not generate salt", err) + return nil, types.ErrInternal + } - } else if !user.EmailVerified { - utils.DoRedirect(w, r, "/auth/verify") + hash := GetHashPassword(password, salt) + + dbUser := db.NewUser(userId, email, false, nil, false, hash, salt, time.Now()) + + err = service.dbAuth.InsertUser(dbUser) + if err != nil { + if err == db.ErrUserExists { + return nil, ErrAccountExists } else { - utils.DoRedirect(w, r, "/") + return nil, types.ErrInternal } } + + return NewUser(dbUser), nil } +func (service ServiceAuthImpl) SendVerificationMail(user *User) { + var token string + + token, err := service.dbAuth.GetEmailVerificationToken(user.Id) + if err != nil { + return + } + + if token == "" { + token, err := utils.RandomToken() + if err != nil { + utils.LogError("Could not generate token", err) + return + } + + err = service.dbAuth.InsertEmailVerificationToken(user.Id, token) + if err != nil { + return + } + } + + var w strings.Builder + err = tempMail.Register(service.serverSettings.BaseUrl, token).Render(context.Background(), &w) + if err != nil { + utils.LogError("Could not render welcome email", err) + return + } + + service.mailService.SendMail(user.Email, "Welcome to ME-FIT", w.String()) +} + +// TODO + func HandleSignUpVerifyPage(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { user := utils.GetUserFromSession(db, r) @@ -227,69 +284,6 @@ func UserInfoComp(user *types.User) templ.Component { } } -func HandleSignUpComp(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - var email = r.FormValue("email") - var password = r.FormValue("password") - - _, err := mail.ParseAddress(email) - if err != nil { - http.Error(w, "Invalid email", http.StatusBadRequest) - return - } - - err = checkPassword(password) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - - userId, err := uuid.NewRandom() - if err != nil { - utils.LogError("Could not generate UUID", err) - auth.Error("Internal Server Error").Render(r.Context(), w) - return - } - - salt := make([]byte, 16) - _, err = rand.Read(salt) - if err != nil { - utils.LogError("Could not generate salt", err) - auth.Error("Internal Server Error").Render(r.Context(), w) - return - } - - hash := GetHashPassword(password, salt) - - _, err = db.Exec("INSERT INTO user (user_uuid, email, email_verified, is_admin, password, salt, created_at) VALUES (?, ?, FALSE, FALSE, ?, ?, datetime())", userId, email, hash, salt) - if err != nil { - // This does leak information about the email being in use, though not specifically stated - // It needs to be refacoteres to "If the email is not already in use, an email has been send to your address", or something - // The happy path, currently a redirect, needs to send the same message! - // Then it is also important to have the same compute time in both paths - // Otherwise an attacker could guess emails when comparing the response time - if strings.Contains(err.Error(), "email") { - auth.Error("Bad Request").Render(r.Context(), w) - return - } - - utils.LogError("Could not insert user", err) - auth.Error("Internal Server Error").Render(r.Context(), w) - return - } - - err = TryCreateSessionAndSetCookie(r, w, db, userId) - if err != nil { - return - } - - // Send verification email as a goroutine - go sendVerificationEmail(db, userId.String(), email, serverSettings) - - utils.DoRedirect(w, r, "/auth/verify") - } -} - func HandleSignOutComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { user := utils.GetUserFromSession(db, r) @@ -394,7 +388,8 @@ func HandleVerifyResendComp(db *sql.DB, serverSettings *types.ServerSettings) ht return } - go sendVerificationEmail(db, user.Id.String(), user.Email, serverSettings) + // TODO + // go sendVerificationEmail(db, user.Id.String(), user.Email, serverSettings) w.Write([]byte("

Verification email sent

")) } @@ -566,39 +561,6 @@ func HandleResetPasswordComp(db *sql.DB, serverSettings *types.ServerSettings) h } } -func sendVerificationEmail(db *sql.DB, userId string, email string, serverSettings *types.ServerSettings) { - - var token string - err := db.QueryRow("SELECT token FROM user_token WHERE user_uuid = ? AND type = 'email_verify'", userId).Scan(&token) - if err != nil && err != sql.ErrNoRows { - utils.LogError("Could not get token", err) - return - } - - if token == "" { - token, err := utils.RandomToken() - if err != nil { - utils.LogError("Could not generate token", err) - return - } - - _, err = db.Exec("INSERT INTO user_token (user_uuid, type, token, created_at) VALUES (?, 'email_verify', ?, datetime())", userId, token) - if err != nil { - utils.LogError("Could not insert token", err) - return - } - } - - var w strings.Builder - err = tempMail.Register(serverSettings.BaseUrl, token).Render(context.Background(), &w) - if err != nil { - utils.LogError("Could not render welcome email", err) - return - } - mailService := NewMailService(serverSettings) - mailService.SendMail(email, "Welcome to ME-FIT", w.String()) -} - func TryCreateSessionAndSetCookie(r *http.Request, w http.ResponseWriter, db *sql.DB, user_uuid uuid.UUID) error { sessionId, err := utils.RandomToken() if err != nil { diff --git a/service/auth_test.go b/service/auth_test.go index 0e12e14..bf39eaa 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -2,7 +2,7 @@ package service import ( "me-fit/db" - m "me-fit/mocks" + "me-fit/mocks" "me-fit/types" "errors" @@ -18,7 +18,6 @@ func TestSignIn(t *testing.T) { t.Parallel() salt := []byte("salt") verifiedAt := time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC) - user := db.NewUser( uuid.New(), "test@test.de", @@ -30,10 +29,10 @@ func TestSignIn(t *testing.T) { time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), ) - mockDbAuth := m.NewMockDbAuth(t) + mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test@test.de").Return(user, nil) - underTest := NewServiceAuthImpl(mockDbAuth) + underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) actualUser, err := underTest.SignIn(user.Email, "password") if err != nil { @@ -54,6 +53,7 @@ func TestSignIn(t *testing.T) { t.Parallel() salt := []byte("salt") verifiedAt := time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC) + user := db.NewUser( uuid.New(), "test@test.de", @@ -65,10 +65,10 @@ func TestSignIn(t *testing.T) { time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), ) - mockDbAuth := m.NewMockDbAuth(t) + mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser(user.Email).Return(user, nil) - underTest := NewServiceAuthImpl(mockDbAuth) + underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test@test.de", "wrong password") if err != ErrInvaidCredentials { @@ -77,10 +77,11 @@ func TestSignIn(t *testing.T) { }) t.Run("should return ErrInvalidCretentials if user has not been found", func(t *testing.T) { t.Parallel() - mockDbAuth := m.NewMockDbAuth(t) + + mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, db.ErrUserNotFound) - underTest := NewServiceAuthImpl(mockDbAuth) + underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") if err != ErrInvaidCredentials { @@ -89,10 +90,11 @@ func TestSignIn(t *testing.T) { }) t.Run("should forward ErrInternal on any other error", func(t *testing.T) { t.Parallel() - mockDbAuth := m.NewMockDbAuth(t) + + mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some error")) - underTest := NewServiceAuthImpl(mockDbAuth) + underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") if err != types.ErrInternal { -- 2.49.1 From d36f880a013c7cef9a15f41c33cceb701a792d16 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Sat, 5 Oct 2024 12:58:34 +0200 Subject: [PATCH 2/6] fix: use testify for assertions #181 --- service/auth_test.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/service/auth_test.go b/service/auth_test.go index bf39eaa..691fb7e 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" ) func TestSignIn(t *testing.T) { @@ -35,18 +36,15 @@ func TestSignIn(t *testing.T) { underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) actualUser, err := underTest.SignIn(user.Email, "password") - if err != nil { - t.Errorf("Expected nil, got %v", err) - } + assert.Nil(t, err) expectedUser := User{ Id: user.Id, Email: user.Email, EmailVerified: user.EmailVerified, } - if *actualUser != expectedUser { - t.Errorf("Expected %v, got %v", expectedUser, actualUser) - } + + assert.Equal(t, expectedUser, *actualUser) }) t.Run("should return ErrInvalidCretentials if password is not correct", func(t *testing.T) { @@ -71,9 +69,8 @@ func TestSignIn(t *testing.T) { underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test@test.de", "wrong password") - if err != ErrInvaidCredentials { - t.Errorf("Expected %v, got %v", ErrInvaidCredentials, err) - } + + assert.Equal(t, ErrInvaidCredentials, err) }) t.Run("should return ErrInvalidCretentials if user has not been found", func(t *testing.T) { t.Parallel() @@ -84,21 +81,18 @@ func TestSignIn(t *testing.T) { underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") - if err != ErrInvaidCredentials { - t.Errorf("Expected %v, got %v", ErrInvaidCredentials, err) - } + assert.Equal(t, ErrInvaidCredentials, err) }) t.Run("should forward ErrInternal on any other error", func(t *testing.T) { t.Parallel() mockDbAuth := mocks.NewMockDbAuth(t) - mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some error")) + mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some undefined error")) underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") - if err != types.ErrInternal { - t.Errorf("Expected %v, got %v", types.ErrInternal, err) - } + + assert.Equal(t, types.ErrInternal, err) }) } -- 2.49.1 From 6b033e2c2e232e0dd39c51d2773dd18eef2154bc Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Sat, 5 Oct 2024 13:49:43 +0200 Subject: [PATCH 3/6] fix: create RandomGenerator interface and struct for testing purpose #181 --- .mockery.yaml | 3 ++ handler/default.go | 3 +- service/auth.go | 62 +++++++++++++++++-------------------- service/auth_test.go | 48 +++++++++++++++++++++++++--- service/random_generator.go | 48 ++++++++++++++++++++++++++++ utils/ctypto.go | 16 ---------- 6 files changed, 125 insertions(+), 55 deletions(-) create mode 100644 service/random_generator.go delete mode 100644 utils/ctypto.go diff --git a/.mockery.yaml b/.mockery.yaml index 9adcc90..b6a04fb 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -2,6 +2,9 @@ with-expecter: True dir: mocks/ outpkg: mocks packages: + me-fit/service: + interfaces: + RandomGenerator: me-fit/db: interfaces: DbAuth: diff --git a/handler/default.go b/handler/default.go index af9eb74..c19cc5f 100644 --- a/handler/default.go +++ b/handler/default.go @@ -15,8 +15,9 @@ func GetHandler(d *sql.DB, serverSettings *types.ServerSettings) http.Handler { router.HandleFunc("/", service.HandleIndexAnd404(d, serverSettings)) + randomGenerator := service.NewRandomGeneratorImpl() dbAuth := db.NewDbAuthSqlite(d) - serviceAuth := service.NewServiceAuthImpl(dbAuth, serverSettings) + serviceAuth := service.NewServiceAuthImpl(dbAuth, randomGenerator, serverSettings) handlerAuth := NewHandlerAuth(d, serviceAuth, serverSettings) // Serve static files (CSS, JS and images) diff --git a/service/auth.go b/service/auth.go index 10edde2..99dcda5 100644 --- a/service/auth.go +++ b/service/auth.go @@ -2,7 +2,6 @@ package service import ( "context" - "crypto/rand" "crypto/subtle" "database/sql" "errors" @@ -26,10 +25,10 @@ import ( ) var ( - ErrInvaidCredentials = errors.New("Invalid email or password") - ErrPasswordComplexity = errors.New("Password needs to be 8 characters long, contain at least one number, one special, one uppercase and one lowercase character") - ErrInvalidEmail = errors.New("Invalid email") - ErrAccountExists = errors.New("Account already exists") + ErrInvaidCredentials = errors.New("Invalid email or password") + ErrInvalidPassword = errors.New("Password needs to be 8 characters long, contain at least one number, one special, one uppercase and one lowercase character") + ErrInvalidEmail = errors.New("Invalid email") + ErrAccountExists = errors.New("Account already exists") ) type User struct { @@ -53,16 +52,18 @@ type ServiceAuth interface { } type ServiceAuthImpl struct { - dbAuth db.DbAuth - serverSettings *types.ServerSettings - mailService MailService + dbAuth db.DbAuth + randomGenerator RandomGenerator + serverSettings *types.ServerSettings + mailService MailService } -func NewServiceAuthImpl(dbAuth db.DbAuth, serverSettings *types.ServerSettings) *ServiceAuthImpl { +func NewServiceAuthImpl(dbAuth db.DbAuth, randomGenerator RandomGenerator, serverSettings *types.ServerSettings) *ServiceAuthImpl { return &ServiceAuthImpl{ - dbAuth: dbAuth, - serverSettings: serverSettings, - mailService: NewMailService(serverSettings), + dbAuth: dbAuth, + randomGenerator: randomGenerator, + serverSettings: serverSettings, + mailService: NewMailService(serverSettings), } } @@ -91,9 +92,8 @@ func (service ServiceAuthImpl) SignUp(email string, password string) (*User, err return nil, ErrInvalidEmail } - err = checkPassword(password) - if err != nil { - return nil, err + if !isPasswordValid(password) { + return nil, ErrInvalidPassword } userId, err := uuid.NewRandom() @@ -102,10 +102,8 @@ func (service ServiceAuthImpl) SignUp(email string, password string) (*User, err return nil, types.ErrInternal } - salt := make([]byte, 16) - _, err = rand.Read(salt) + salt, err := service.randomGenerator.Bytes(16) if err != nil { - utils.LogError("Could not generate salt", err) return nil, types.ErrInternal } @@ -134,9 +132,8 @@ func (service ServiceAuthImpl) SendVerificationMail(user *User) { } if token == "" { - token, err := utils.RandomToken() + token, err := service.randomGenerator.String(32) if err != nil { - utils.LogError("Could not generate token", err) return } @@ -407,9 +404,8 @@ func HandleChangePasswordComp(db *sql.DB) http.HandlerFunc { currPass := r.FormValue("current-password") newPass := r.FormValue("new-password") - err := checkPassword(newPass) - if err != nil { - utils.TriggerToast(w, r, "error", err.Error()) + if !isPasswordValid(newPass) { + utils.TriggerToast(w, r, "error", ErrInvalidPassword.Error()) return } @@ -423,7 +419,7 @@ func HandleChangePasswordComp(db *sql.DB) http.HandlerFunc { salt []byte ) - err = db.QueryRow("SELECT password, salt FROM user WHERE user_uuid = ?", user.Id).Scan(&storedHash, &salt) + err := db.QueryRow("SELECT password, salt FROM user WHERE user_uuid = ?", user.Id).Scan(&storedHash, &salt) if err != nil { utils.LogError("Could not get password", err) utils.TriggerToast(w, r, "error", "Internal Server Error") @@ -467,9 +463,8 @@ func HandleActualResetPasswordComp(db *sql.DB) http.HandlerFunc { newPass := r.FormValue("new-password") - err = checkPassword(newPass) - if err != nil { - utils.TriggerToast(w, r, "error", err.Error()) + if !isPasswordValid(newPass) { + utils.TriggerToast(w, r, "error", ErrInvalidPassword.Error()) return } @@ -511,6 +506,7 @@ func HandleActualResetPasswordComp(db *sql.DB) http.HandlerFunc { utils.TriggerToast(w, r, "success", "Password changed") } } + func HandleResetPasswordComp(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { mailService := NewMailService(serverSettings) return func(w http.ResponseWriter, r *http.Request) { @@ -521,9 +517,8 @@ func HandleResetPasswordComp(db *sql.DB, serverSettings *types.ServerSettings) h return } - token, err := utils.RandomToken() + token, err := NewRandomGeneratorImpl().String(32) if err != nil { - utils.LogError("Could not generate token", err) return } @@ -562,9 +557,8 @@ func HandleResetPasswordComp(db *sql.DB, serverSettings *types.ServerSettings) h } func TryCreateSessionAndSetCookie(r *http.Request, w http.ResponseWriter, db *sql.DB, user_uuid uuid.UUID) error { - sessionId, err := utils.RandomToken() + sessionId, err := NewRandomGeneratorImpl().String(32) if err != nil { - utils.LogError("Could not generate session ID", err) return types.ErrInternal } @@ -599,15 +593,15 @@ func GetHashPassword(password string, salt []byte) []byte { return argon2.IDKey([]byte(password), salt, 1, 64*1024, 1, 16) } -func checkPassword(password string) error { +func isPasswordValid(password string) bool { if len(password) < 8 || !strings.ContainsAny(password, "0123456789") || !strings.ContainsAny(password, "ABCDEFGHIJKLMNOPQRSTUVWXYZ") || !strings.ContainsAny(password, "abcdefghijklmnopqrstuvwxyz") || !strings.ContainsAny(password, "!@#$%^&*()_+-=[]{}\\|;:'\",.<>/?") { - return ErrPasswordComplexity + return false } else { - return nil + return true } } diff --git a/service/auth_test.go b/service/auth_test.go index 691fb7e..2b2ebb2 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -32,8 +32,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test@test.de").Return(user, nil) + mockRandom := mocks.NewMockRandomGenerator(t) - underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) actualUser, err := underTest.SignIn(user.Email, "password") assert.Nil(t, err) @@ -65,8 +66,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser(user.Email).Return(user, nil) + mockRandom := mocks.NewMockRandomGenerator(t) - underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) _, err := underTest.SignIn("test@test.de", "wrong password") @@ -77,8 +79,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, db.ErrUserNotFound) + mockRandom := mocks.NewMockRandomGenerator(t) - underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") assert.Equal(t, ErrInvaidCredentials, err) @@ -88,11 +91,48 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some undefined error")) + mockRandom := mocks.NewMockRandomGenerator(t) - underTest := NewServiceAuthImpl(mockDbAuth, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") assert.Equal(t, types.ErrInternal, err) }) } + +func TestSignUp(t *testing.T) { + t.Parallel() + t.Run("should check for correct email address", func(t *testing.T) { + t.Parallel() + + mockDbAuth := mocks.NewMockDbAuth(t) + mockRandom := mocks.NewMockRandomGenerator(t) + + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + + _, err := underTest.SignUp("invalid email address", "SomeStrongPassword123!") + + assert.Equal(t, ErrInvalidEmail, err) + }) + t.Run("should check for password complexity", func(t *testing.T) { + t.Parallel() + + mockDbAuth := mocks.NewMockDbAuth(t) + mockRandom := mocks.NewMockRandomGenerator(t) + + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + + weakPasswords := []string{ + "123!ab", // too short + "no_upper_case_123", + "NO_LOWER_CASE_123", + "noSpecialChar123", + } + + for _, password := range weakPasswords { + _, err := underTest.SignUp("some@valid.email", password) + assert.Equal(t, ErrInvalidPassword, err) + } + }) +} diff --git a/service/random_generator.go b/service/random_generator.go new file mode 100644 index 0000000..4e4e4c5 --- /dev/null +++ b/service/random_generator.go @@ -0,0 +1,48 @@ +package service + +import ( + "me-fit/types" + + "crypto/rand" + "encoding/base64" + "log/slog" + + "github.com/google/uuid" +) + +type RandomGenerator interface { + Bytes(size int) ([]byte, error) + String(size int) (string, error) + UUID() (uuid.UUID, error) +} + +type RandomGeneratorImpl struct { +} + +func NewRandomGeneratorImpl() *RandomGeneratorImpl { + return &RandomGeneratorImpl{} +} + +func (r *RandomGeneratorImpl) Bytes(size int) ([]byte, error) { + b := make([]byte, 32) + _, err := rand.Read(b) + if err != nil { + slog.Error("Error generating random bytes: " + err.Error()) + return []byte{}, types.ErrInternal + } + + return b, nil +} + +func (r *RandomGeneratorImpl) String(size int) (string, error) { + bytes, err := r.Bytes(size) + if err != nil { + return "", types.ErrInternal + } + + return base64.StdEncoding.EncodeToString(bytes), nil +} + +func (r *RandomGeneratorImpl) UUID() (uuid.UUID, error) { + return uuid.NewRandom() +} diff --git a/utils/ctypto.go b/utils/ctypto.go deleted file mode 100644 index 4f3cffe..0000000 --- a/utils/ctypto.go +++ /dev/null @@ -1,16 +0,0 @@ -package utils - -import ( - "crypto/rand" - "encoding/base64" -) - -func RandomToken() (string, error) { - b := make([]byte, 32) - _, err := rand.Read(b) - if err != nil { - return "", err - } - - return base64.StdEncoding.EncodeToString(b), nil -} -- 2.49.1 From 3232632200b7b6434d640358f9973b472ca82ac1 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Sat, 5 Oct 2024 23:40:37 +0200 Subject: [PATCH 4/6] fix: new test and extract time.Now to mockable Clock #181 --- .mockery.yaml | 1 + handler/default.go | 3 ++- service/auth.go | 10 ++++---- service/auth_test.go | 54 +++++++++++++++++++++++++++++++++++++++----- service/clock.go | 17 ++++++++++++++ 5 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 service/clock.go diff --git a/.mockery.yaml b/.mockery.yaml index b6a04fb..50c6e2e 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -5,6 +5,7 @@ packages: me-fit/service: interfaces: RandomGenerator: + Clock: me-fit/db: interfaces: DbAuth: diff --git a/handler/default.go b/handler/default.go index c19cc5f..6244f08 100644 --- a/handler/default.go +++ b/handler/default.go @@ -16,8 +16,9 @@ func GetHandler(d *sql.DB, serverSettings *types.ServerSettings) http.Handler { router.HandleFunc("/", service.HandleIndexAnd404(d, serverSettings)) randomGenerator := service.NewRandomGeneratorImpl() + clock := service.NewClockImpl() dbAuth := db.NewDbAuthSqlite(d) - serviceAuth := service.NewServiceAuthImpl(dbAuth, randomGenerator, serverSettings) + serviceAuth := service.NewServiceAuthImpl(dbAuth, randomGenerator, clock, serverSettings) handlerAuth := NewHandlerAuth(d, serviceAuth, serverSettings) // Serve static files (CSS, JS and images) diff --git a/service/auth.go b/service/auth.go index 99dcda5..7998b64 100644 --- a/service/auth.go +++ b/service/auth.go @@ -10,7 +10,6 @@ import ( "net/mail" "net/url" "strings" - "time" "me-fit/db" "me-fit/template" @@ -54,14 +53,16 @@ type ServiceAuth interface { type ServiceAuthImpl struct { dbAuth db.DbAuth randomGenerator RandomGenerator + clock Clock serverSettings *types.ServerSettings mailService MailService } -func NewServiceAuthImpl(dbAuth db.DbAuth, randomGenerator RandomGenerator, serverSettings *types.ServerSettings) *ServiceAuthImpl { +func NewServiceAuthImpl(dbAuth db.DbAuth, randomGenerator RandomGenerator, clock Clock, serverSettings *types.ServerSettings) *ServiceAuthImpl { return &ServiceAuthImpl{ dbAuth: dbAuth, randomGenerator: randomGenerator, + clock: clock, serverSettings: serverSettings, mailService: NewMailService(serverSettings), } @@ -96,9 +97,8 @@ func (service ServiceAuthImpl) SignUp(email string, password string) (*User, err return nil, ErrInvalidPassword } - userId, err := uuid.NewRandom() + userId, err := service.randomGenerator.UUID() if err != nil { - utils.LogError("Could not generate UUID", err) return nil, types.ErrInternal } @@ -109,7 +109,7 @@ func (service ServiceAuthImpl) SignUp(email string, password string) (*User, err hash := GetHashPassword(password, salt) - dbUser := db.NewUser(userId, email, false, nil, false, hash, salt, time.Now()) + dbUser := db.NewUser(userId, email, false, nil, false, hash, salt, service.clock.Now()) err = service.dbAuth.InsertUser(dbUser) if err != nil { diff --git a/service/auth_test.go b/service/auth_test.go index 2b2ebb2..78cb7ef 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -14,6 +14,7 @@ import ( ) func TestSignIn(t *testing.T) { + t.Parallel() t.Run("should return user if password is correct", func(t *testing.T) { t.Parallel() @@ -33,8 +34,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test@test.de").Return(user, nil) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) actualUser, err := underTest.SignIn(user.Email, "password") assert.Nil(t, err) @@ -67,8 +69,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser(user.Email).Return(user, nil) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) _, err := underTest.SignIn("test@test.de", "wrong password") @@ -80,8 +83,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, db.ErrUserNotFound) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") assert.Equal(t, ErrInvaidCredentials, err) @@ -92,8 +96,9 @@ func TestSignIn(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some undefined error")) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") @@ -108,8 +113,9 @@ func TestSignUp(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) _, err := underTest.SignUp("invalid email address", "SomeStrongPassword123!") @@ -120,8 +126,9 @@ func TestSignUp(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) weakPasswords := []string{ "123!ab", // too short @@ -135,4 +142,39 @@ func TestSignUp(t *testing.T) { assert.Equal(t, ErrInvalidPassword, err) } }) + t.Run("should signup correctly", func(t *testing.T) { + t.Parallel() + + mockDbAuth := mocks.NewMockDbAuth(t) + mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) + + expected := User{ + Id: uuid.New(), + Email: "some@valid.email", + EmailVerified: false, + } + + random := NewRandomGeneratorImpl() + salt, err := random.Bytes(16) + assert.Nil(t, err) + password := "SomeStrongPassword123!" + + mockRandom.EXPECT().UUID().Return(expected.Id, nil) + mockRandom.EXPECT().Bytes(16).Return(salt, nil) + + createTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + + mockClock.EXPECT().Now().Return(createTime) + + mockDbAuth.EXPECT().InsertUser(db.NewUser(expected.Id, expected.Email, false, nil, false, GetHashPassword(password, salt), salt, createTime)).Return(nil) + + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + + actual, err := underTest.SignUp(expected.Email, password) + + assert.Nil(t, err) + + assert.Equal(t, expected, *actual) + }) } diff --git a/service/clock.go b/service/clock.go new file mode 100644 index 0000000..cf7cae9 --- /dev/null +++ b/service/clock.go @@ -0,0 +1,17 @@ +package service + +import "time" + +type Clock interface { + Now() time.Time +} + +type ClockImpl struct{} + +func NewClockImpl() Clock { + return &ClockImpl{} +} + +func (c *ClockImpl) Now() time.Time { + return time.Now() +} -- 2.49.1 From 4dfd29eac110e5b088546034a04dff7a18817c77 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Sat, 5 Oct 2024 23:54:33 +0200 Subject: [PATCH 5/6] fix: missing db auth tests #181 --- db/auth_test.go | 101 +++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 56 deletions(-) diff --git a/db/auth_test.go b/db/auth_test.go index 95e714d..d47c319 100644 --- a/db/auth_test.go +++ b/db/auth_test.go @@ -4,12 +4,11 @@ import ( "me-fit/utils" "database/sql" - "errors" - "reflect" "testing" "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" ) func setupDb(t *testing.T) *sql.DB { @@ -29,7 +28,7 @@ func setupDb(t *testing.T) *sql.DB { return db } -func TestGetUser(t *testing.T) { +func TestUser(t *testing.T) { t.Parallel() t.Run("should return UserNotFound", func(t *testing.T) { @@ -39,12 +38,10 @@ func TestGetUser(t *testing.T) { underTest := DbAuthSqlite{db: db} _, err := underTest.GetUser("someNonExistentEmail") - if err != ErrUserNotFound { - t.Errorf("Expected UserNotFound, got %v", err) - } + assert.Equal(t, ErrUserNotFound, err) }) - t.Run("should find user in database", func(t *testing.T) { + t.Run("should insert and get user", func(t *testing.T) { t.Parallel() db := setupDb(t) @@ -52,52 +49,15 @@ func TestGetUser(t *testing.T) { verifiedAt := time.Date(2020, 1, 5, 13, 0, 0, 0, time.UTC) createAt := time.Date(2020, 1, 5, 12, 0, 0, 0, time.UTC) - user := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) + expected := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) - _, err := db.Exec(` - INSERT INTO user (user_uuid, email, email_verified, email_verified_at, is_admin, password, salt, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - `, user.Id, user.Email, user.EmailVerified, user.EmailVerifiedAt, user.IsAdmin, user.Password, user.Salt, user.CreateAt) - if err != nil { - t.Fatalf("Error inserting user: %v", err) - } + err := underTest.InsertUser(expected) + assert.Nil(t, err) - actual, err := underTest.GetUser(user.Email) - if err != nil { - t.Fatalf("Error getting user: %v", err) - } + actual, err := underTest.GetUser(expected.Email) + assert.Nil(t, err) - if !reflect.DeepEqual(user, actual) { - t.Errorf("Expected %v, got %v", user, actual) - } - }) -} -func TestInsertUser(t *testing.T) { - t.Parallel() - - t.Run("should insert user", func(t *testing.T) { - t.Parallel() - db := setupDb(t) - - underTest := DbAuthSqlite{db: db} - - verifiedAt := time.Date(2020, 1, 5, 13, 0, 0, 0, time.UTC) - createAt := time.Date(2020, 1, 5, 12, 0, 0, 0, time.UTC) - user := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) - - err := underTest.InsertUser(user) - if err != nil { - t.Fatalf("Error inserting user: %v", err) - } - - actual, err := underTest.GetUser(user.Email) - if err != nil { - t.Fatalf("Error getting user: %v", err) - } - - if !reflect.DeepEqual(user, actual) { - t.Errorf("Expected %v, got %v", user, actual) - } + assert.Equal(t, expected, actual) }) t.Run("should throw error if user already exists", func(t *testing.T) { @@ -111,13 +71,42 @@ func TestInsertUser(t *testing.T) { user := NewUser(uuid.New(), "some@email.de", true, &verifiedAt, false, []byte("somePass"), []byte("someSalt"), createAt) err := underTest.InsertUser(user) - if err != nil { - t.Fatalf("Error inserting user: %v", err) - } + assert.Nil(t, err) err = underTest.InsertUser(user) - if !errors.Is(err, ErrUserExists) { - t.Fatalf("Error inserting user: %v", err) - } + assert.Equal(t, ErrUserExists, err) + }) +} + +func TestEmailVerification(t *testing.T) { + t.Parallel() + + t.Run("should return empty string if no token is safed", func(t *testing.T) { + t.Parallel() + db := setupDb(t) + + underTest := DbAuthSqlite{db: db} + + token, err := underTest.GetEmailVerificationToken(uuid.New()) + + assert.Nil(t, err) + assert.Equal(t, "", token) + }) + t.Run("should insert and return token", func(t *testing.T) { + t.Parallel() + db := setupDb(t) + + underTest := DbAuthSqlite{db: db} + + userId := uuid.New() + expectedToken := "someToken" + + err := underTest.InsertEmailVerificationToken(userId, expectedToken) + assert.Nil(t, err) + + actualToken, err := underTest.GetEmailVerificationToken(userId) + assert.Nil(t, err) + + assert.Equal(t, expectedToken, actualToken) }) } -- 2.49.1 From 0fab1e1f2eb2421d3c46b064f9e9e3704e99b744 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Sun, 6 Oct 2024 10:05:00 +0200 Subject: [PATCH 6/6] fix: missing service tests #181 --- .mockery.yaml | 1 + handler/auth.go | 2 +- handler/default.go | 3 +- service/auth.go | 20 +++++------ service/auth_test.go | 80 ++++++++++++++++++++++++++++++++++++++++---- service/mail.go | 12 ++++--- 6 files changed, 95 insertions(+), 23 deletions(-) diff --git a/.mockery.yaml b/.mockery.yaml index 50c6e2e..b5f727d 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -6,6 +6,7 @@ packages: interfaces: RandomGenerator: Clock: + MailService: me-fit/db: interfaces: DbAuth: diff --git a/handler/auth.go b/handler/auth.go index 6c7529a..ea155fb 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -145,7 +145,7 @@ func (handler HandlerAuthImpl) handleSignUp() http.HandlerFunc { return nil, err } - go handler.service.SendVerificationMail(user) + go handler.service.SendVerificationMail(user.Id, user.Email) return nil, nil }) diff --git a/handler/default.go b/handler/default.go index 6244f08..ea0cf5a 100644 --- a/handler/default.go +++ b/handler/default.go @@ -18,7 +18,8 @@ func GetHandler(d *sql.DB, serverSettings *types.ServerSettings) http.Handler { randomGenerator := service.NewRandomGeneratorImpl() clock := service.NewClockImpl() dbAuth := db.NewDbAuthSqlite(d) - serviceAuth := service.NewServiceAuthImpl(dbAuth, randomGenerator, clock, serverSettings) + mailService := service.NewMailServiceImpl(serverSettings) + serviceAuth := service.NewServiceAuthImpl(dbAuth, randomGenerator, clock, mailService, serverSettings) handlerAuth := NewHandlerAuth(d, serviceAuth, serverSettings) // Serve static files (CSS, JS and images) diff --git a/service/auth.go b/service/auth.go index 7998b64..c1f3663 100644 --- a/service/auth.go +++ b/service/auth.go @@ -47,24 +47,24 @@ func NewUser(user *db.User) *User { type ServiceAuth interface { SignIn(email string, password string) (*User, error) SignUp(email string, password string) (*User, error) - SendVerificationMail(user *User) + SendVerificationMail(userId uuid.UUID, email string) } type ServiceAuthImpl struct { dbAuth db.DbAuth randomGenerator RandomGenerator clock Clock - serverSettings *types.ServerSettings mailService MailService + serverSettings *types.ServerSettings } -func NewServiceAuthImpl(dbAuth db.DbAuth, randomGenerator RandomGenerator, clock Clock, serverSettings *types.ServerSettings) *ServiceAuthImpl { +func NewServiceAuthImpl(dbAuth db.DbAuth, randomGenerator RandomGenerator, clock Clock, mailService MailService, serverSettings *types.ServerSettings) *ServiceAuthImpl { return &ServiceAuthImpl{ dbAuth: dbAuth, randomGenerator: randomGenerator, clock: clock, + mailService: mailService, serverSettings: serverSettings, - mailService: NewMailService(serverSettings), } } @@ -123,10 +123,10 @@ func (service ServiceAuthImpl) SignUp(email string, password string) (*User, err return NewUser(dbUser), nil } -func (service ServiceAuthImpl) SendVerificationMail(user *User) { +func (service ServiceAuthImpl) SendVerificationMail(userId uuid.UUID, email string) { var token string - token, err := service.dbAuth.GetEmailVerificationToken(user.Id) + token, err := service.dbAuth.GetEmailVerificationToken(userId) if err != nil { return } @@ -137,7 +137,7 @@ func (service ServiceAuthImpl) SendVerificationMail(user *User) { return } - err = service.dbAuth.InsertEmailVerificationToken(user.Id, token) + err = service.dbAuth.InsertEmailVerificationToken(userId, token) if err != nil { return } @@ -150,7 +150,7 @@ func (service ServiceAuthImpl) SendVerificationMail(user *User) { return } - service.mailService.SendMail(user.Email, "Welcome to ME-FIT", w.String()) + service.mailService.SendMail(email, "Welcome to ME-FIT", w.String()) } // TODO @@ -311,7 +311,7 @@ func HandleSignOutComp(db *sql.DB) http.HandlerFunc { } func HandleDeleteAccountComp(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { - mailService := NewMailService(serverSettings) + mailService := NewMailServiceImpl(serverSettings) return func(w http.ResponseWriter, r *http.Request) { user := utils.GetUserFromSession(db, r) if user == nil { @@ -508,7 +508,7 @@ func HandleActualResetPasswordComp(db *sql.DB) http.HandlerFunc { } func HandleResetPasswordComp(db *sql.DB, serverSettings *types.ServerSettings) http.HandlerFunc { - mailService := NewMailService(serverSettings) + mailService := NewMailServiceImpl(serverSettings) return func(w http.ResponseWriter, r *http.Request) { email := r.FormValue("email") diff --git a/service/auth_test.go b/service/auth_test.go index 78cb7ef..aeede82 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -4,6 +4,7 @@ import ( "me-fit/db" "me-fit/mocks" "me-fit/types" + "strings" "errors" "testing" @@ -11,6 +12,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestSignIn(t *testing.T) { @@ -35,8 +37,9 @@ func TestSignIn(t *testing.T) { mockDbAuth.EXPECT().GetUser("test@test.de").Return(user, nil) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) actualUser, err := underTest.SignIn(user.Email, "password") assert.Nil(t, err) @@ -70,8 +73,9 @@ func TestSignIn(t *testing.T) { mockDbAuth.EXPECT().GetUser(user.Email).Return(user, nil) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) _, err := underTest.SignIn("test@test.de", "wrong password") @@ -84,8 +88,9 @@ func TestSignIn(t *testing.T) { mockDbAuth.EXPECT().GetUser("test").Return(nil, db.ErrUserNotFound) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") assert.Equal(t, ErrInvaidCredentials, err) @@ -97,8 +102,9 @@ func TestSignIn(t *testing.T) { mockDbAuth.EXPECT().GetUser("test").Return(nil, errors.New("Some undefined error")) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) _, err := underTest.SignIn("test", "test") @@ -114,8 +120,9 @@ func TestSignUp(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) _, err := underTest.SignUp("invalid email address", "SomeStrongPassword123!") @@ -127,8 +134,9 @@ func TestSignUp(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) weakPasswords := []string{ "123!ab", // too short @@ -148,6 +156,7 @@ func TestSignUp(t *testing.T) { mockDbAuth := mocks.NewMockDbAuth(t) mockRandom := mocks.NewMockRandomGenerator(t) mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) expected := User{ Id: uuid.New(), @@ -169,7 +178,7 @@ func TestSignUp(t *testing.T) { mockDbAuth.EXPECT().InsertUser(db.NewUser(expected.Id, expected.Email, false, nil, false, GetHashPassword(password, salt), salt, createTime)).Return(nil) - underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, &types.ServerSettings{}) + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) actual, err := underTest.SignUp(expected.Email, password) @@ -177,4 +186,61 @@ func TestSignUp(t *testing.T) { assert.Equal(t, expected, *actual) }) + t.Run("should return ErrAccountExists", func(t *testing.T) { + t.Parallel() + + mockDbAuth := mocks.NewMockDbAuth(t) + mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) + + user := User{ + Id: uuid.New(), + Email: "some@valid.email", + } + + random := NewRandomGeneratorImpl() + salt, err := random.Bytes(16) + assert.Nil(t, err) + password := "SomeStrongPassword123!" + + mockRandom.EXPECT().UUID().Return(user.Id, nil) + mockRandom.EXPECT().Bytes(16).Return(salt, nil) + + createTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + + mockClock.EXPECT().Now().Return(createTime) + + mockDbAuth.EXPECT().InsertUser(db.NewUser(user.Id, user.Email, false, nil, false, GetHashPassword(password, salt), salt, createTime)).Return(db.ErrUserExists) + + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) + + _, err = underTest.SignUp(user.Email, password) + assert.Equal(t, ErrAccountExists, err) + }) +} + +func TestSendVerificationMail(t *testing.T) { + + t.Parallel() + t.Run("should use stored token and send mail", func(t *testing.T) { + t.Parallel() + + token := "someRandomTokenToUse" + email := "some@email.de" + userId := uuid.New() + + mockDbAuth := mocks.NewMockDbAuth(t) + mockRandom := mocks.NewMockRandomGenerator(t) + mockClock := mocks.NewMockClock(t) + mockMail := mocks.NewMockMailService(t) + + mockDbAuth.EXPECT().GetEmailVerificationToken(userId).Return(token, nil) + + mockMail.EXPECT().SendMail(email, "Welcome to ME-FIT", mock.MatchedBy(func(message string) bool { return strings.Contains(message, token) })).Return(nil) + + underTest := NewServiceAuthImpl(mockDbAuth, mockRandom, mockClock, mockMail, &types.ServerSettings{}) + + underTest.SendVerificationMail(userId, email) + }) } diff --git a/service/mail.go b/service/mail.go index f60d0b4..0505edb 100644 --- a/service/mail.go +++ b/service/mail.go @@ -6,15 +6,19 @@ import ( "net/smtp" ) -type MailService struct { +type MailService interface { + SendMail(to string, subject string, message string) error +} + +type MailServiceImpl struct { serverSettings *types.ServerSettings } -func NewMailService(serverSettings *types.ServerSettings) MailService { - return MailService{serverSettings: serverSettings} +func NewMailServiceImpl(serverSettings *types.ServerSettings) MailServiceImpl { + return MailServiceImpl{serverSettings: serverSettings} } -func (m MailService) SendMail(to string, subject string, message string) error { +func (m MailServiceImpl) SendMail(to string, subject string, message string) error { if m.serverSettings.Smtp == nil { return nil } -- 2.49.1