From a41265be982be801d85421449e1eb9420fe76b52 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Wed, 18 Sep 2024 21:49:49 +0200 Subject: [PATCH 1/7] fix: restructure handler yet again #181 --- handler/auth.go | 15 +++++++++++++-- handler/default.go | 12 +++++++++--- handler/workout.go | 21 ++++++++++++++++----- service/auth.go | 1 + 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/handler/auth.go b/handler/auth.go index 9bb57cb..71fa20b 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -7,8 +7,9 @@ import ( "net/http" ) -func handleAuth(db *sql.DB, router *http.ServeMux) { - // Don't use auth middleware for these routes, as it makes redirecting very difficult, if the mail is not yet verified +func authUi(db *sql.DB) http.Handler { + router := http.NewServeMux() + router.Handle("/auth/signin", service.HandleSignInPage(db)) router.Handle("/auth/signup", service.HandleSignUpPage(db)) router.Handle("/auth/verify", service.HandleSignUpVerifyPage(db)) // Hint for the user to verify their email @@ -16,6 +17,14 @@ func handleAuth(db *sql.DB, router *http.ServeMux) { router.Handle("/auth/verify-email", service.HandleSignUpVerifyResponsePage(db)) // The link contained in the email router.Handle("/auth/change-password", service.HandleChangePasswordPage(db)) router.Handle("/auth/reset-password", service.HandleResetPasswordPage(db)) + router.Handle("/", service.HandleIndexAnd404(db)) + + return router +} + +func authApi(db *sql.DB) http.Handler { + router := http.NewServeMux() + router.Handle("/api/auth/signup", service.HandleSignUpComp(db)) router.Handle("/api/auth/signin", service.HandleSignInComp(db)) router.Handle("/api/auth/signout", service.HandleSignOutComp(db)) @@ -24,4 +33,6 @@ func handleAuth(db *sql.DB, router *http.ServeMux) { router.Handle("/api/auth/change-password", service.HandleChangePasswordComp(db)) router.Handle("/api/auth/reset-password", service.HandleResetPasswordComp(db)) router.Handle("/api/auth/reset-password-actual", service.HandleActualResetPasswordComp(db)) + + return router } diff --git a/handler/default.go b/handler/default.go index e877d59..d4194b9 100644 --- a/handler/default.go +++ b/handler/default.go @@ -16,11 +16,17 @@ func GetHandler(db *sql.DB) http.Handler { // Serve static files (CSS, JS and images) router.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("./static/")))) - handleWorkout(db, router) + router.Handle("/auth/", authUi(db)) + router.Handle("/api/auth/", authApi(db)) - handleAuth(db, router) + router.Handle("/workout", auth(db, workoutUi(db))) + router.Handle("/api/workout", auth(db, workoutApi(db))) + // Needed a second time with trailing slash, otherwise either /api/workout or /api/workout/{id} does not match + router.Handle("/api/workout/", auth(db, workoutApi(db))) - return middleware.Logging(middleware.EnableCors(router)) + return middleware.Logging( + middleware.EnableCors( + router)) } func auth(db *sql.DB, h http.Handler) http.Handler { diff --git a/handler/workout.go b/handler/workout.go index d78212c..ed3672e 100644 --- a/handler/workout.go +++ b/handler/workout.go @@ -7,9 +7,20 @@ import ( "net/http" ) -func handleWorkout(db *sql.DB, router *http.ServeMux) { - router.Handle("/workout", auth(db, service.HandleWorkoutPage(db))) - router.Handle("POST /api/workout", auth(db, service.HandleWorkoutNewComp(db))) - router.Handle("GET /api/workout", auth(db, service.HandleWorkoutGetComp(db))) - router.Handle("DELETE /api/workout/{id}", auth(db, service.HandleWorkoutDeleteComp(db))) +func workoutUi(db *sql.DB) http.Handler { + router := http.NewServeMux() + + router.Handle("/workout", service.HandleWorkoutPage(db)) + + return router +} + +func workoutApi(db *sql.DB) http.Handler { + router := http.NewServeMux() + + router.Handle("POST /api/workout", service.HandleWorkoutNewComp(db)) + router.Handle("GET /api/workout", service.HandleWorkoutGetComp(db)) + router.Handle("DELETE /api/workout/{id}", service.HandleWorkoutDeleteComp(db)) + + return router } diff --git a/service/auth.go b/service/auth.go index 024ec09..e069846 100644 --- a/service/auth.go +++ b/service/auth.go @@ -585,6 +585,7 @@ func HandleResetPasswordComp(db *sql.DB) http.HandlerFunc { utils.TriggerToast(w, r, "info", "If the email exists, an email has been sent") } } + func sendVerificationEmail(db *sql.DB, userId string, email string) { var token string -- 2.49.1 From dbe687c105fcdb8982b30d09519ed694fee576c4 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Wed, 18 Sep 2024 21:50:24 +0200 Subject: [PATCH 2/7] chore(auth): add test for passwort checking #181 --- service/auth_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 service/auth_test.go diff --git a/service/auth_test.go b/service/auth_test.go new file mode 100644 index 0000000..7b62f3b --- /dev/null +++ b/service/auth_test.go @@ -0,0 +1,40 @@ +package service + +import ( + "testing" +) + +func TestValidPasswords(t *testing.T) { + passwords := []string{ + "aB!'2d2y", //normal + "v-#:j`fQurudEEUk#xA)uzI-B+'eZW3`F*5Eaf+{YID#PWuD.TbyH'f Date: Wed, 18 Sep 2024 23:07:01 +0200 Subject: [PATCH 3/7] chore(auth): add test for retrieving session from db #181 --- handler/auth.go | 2 +- handler/default.go | 38 ++++++++++++-- middleware/auth.go | 3 +- service/auth.go | 69 ++++++++++++++++++++---- service/auth_test.go | 110 +++++++++++++++++++++++++++++++++++++++ service/index_and_404.go | 32 ------------ types/types.go | 13 ++++- utils/db.go | 9 +++- utils/http.go | 35 +------------ 9 files changed, 229 insertions(+), 82 deletions(-) delete mode 100644 service/index_and_404.go diff --git a/handler/auth.go b/handler/auth.go index 71fa20b..8d28f66 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -17,7 +17,7 @@ func authUi(db *sql.DB) http.Handler { router.Handle("/auth/verify-email", service.HandleSignUpVerifyResponsePage(db)) // The link contained in the email router.Handle("/auth/change-password", service.HandleChangePasswordPage(db)) router.Handle("/auth/reset-password", service.HandleResetPasswordPage(db)) - router.Handle("/", service.HandleIndexAnd404(db)) + router.Handle("/", handleNotFound(db)) return router } diff --git a/handler/default.go b/handler/default.go index d4194b9..0f6aa66 100644 --- a/handler/default.go +++ b/handler/default.go @@ -3,17 +3,19 @@ package handler import ( "me-fit/middleware" "me-fit/service" + "me-fit/template" + "me-fit/utils" "database/sql" "net/http" ) func GetHandler(db *sql.DB) http.Handler { - var router = http.NewServeMux() + router := http.NewServeMux() - router.HandleFunc("/", service.HandleIndexAnd404(db)) + router.HandleFunc("/$", handleIndex(db)) + router.HandleFunc("/", handleNotFound(db)) - // Serve static files (CSS, JS and images) router.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("./static/")))) router.Handle("/auth/", authUi(db)) @@ -32,3 +34,33 @@ func GetHandler(db *sql.DB) http.Handler { func auth(db *sql.DB, h http.Handler) http.Handler { return middleware.EnsureValidSession(db, h) } + +func handleIndex(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + err := template. + Layout(template.Index(), service.UserInfoComp(user)). + Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render index", err) + http.Error(w, "Failed to render index", http.StatusInternalServerError) + } + } +} + +func handleNotFound(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + err := template. + Layout(template.NotFound(), service.UserInfoComp(user)). + Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render index", err) + http.Error(w, "Failed to render index", http.StatusInternalServerError) + } + + w.WriteHeader(http.StatusNotFound) + } +} diff --git a/middleware/auth.go b/middleware/auth.go index e48c010..4c7786a 100644 --- a/middleware/auth.go +++ b/middleware/auth.go @@ -1,6 +1,7 @@ package middleware import ( + "me-fit/service" "me-fit/utils" "context" @@ -12,7 +13,7 @@ func EnsureValidSession(db *sql.DB, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := service.GetUserFromRequest(db, r) if user == nil { utils.DoRedirect(w, r, "/auth/signin") return diff --git a/service/auth.go b/service/auth.go index e069846..ce45e59 100644 --- a/service/auth.go +++ b/service/auth.go @@ -24,9 +24,42 @@ import ( "golang.org/x/crypto/argon2" ) +// TESTED + +func GetUserFromSessionId(db *sql.DB, sessionId types.SessionId) *types.User { + if sessionId == "" { + return nil + } + + var ( + createdAt time.Time + userId uuid.UUID + email string + emailVerified bool + ) + + err := db.QueryRow(` + SELECT u.user_uuid, u.email, u.email_verified, s.created_at + FROM session s + INNER JOIN user u ON s.user_uuid = u.user_uuid + WHERE session_id = ?`, sessionId).Scan(&userId, &email, &emailVerified, &createdAt) + if err != nil { + slog.Warn("Could not verify session: " + err.Error()) + return nil + } + + if createdAt.Add(time.Duration(8 * time.Hour)).Before(time.Now()) { + return nil + } else { + return types.NewUser(userId, email, sessionId, emailVerified) + } +} + +// NOT TESTED + func HandleSignInPage(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { userComp := UserInfoComp(nil) @@ -48,7 +81,7 @@ func HandleSignInPage(db *sql.DB) http.HandlerFunc { func HandleSignUpPage(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { userComp := UserInfoComp(nil) @@ -70,7 +103,7 @@ func HandleSignUpPage(db *sql.DB) http.HandlerFunc { func HandleSignUpVerifyPage(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { utils.DoRedirect(w, r, "/auth/signin") } else if user.EmailVerified { @@ -90,7 +123,7 @@ func HandleSignUpVerifyPage(db *sql.DB) http.HandlerFunc { func HandleDeleteAccountPage(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // An unverified email should be able to delete their account - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { utils.DoRedirect(w, r, "/auth/signin") } else { @@ -152,7 +185,7 @@ func HandleChangePasswordPage(db *sql.DB) http.HandlerFunc { isPasswordReset := r.URL.Query().Has("token") - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil && !isPasswordReset { utils.DoRedirect(w, r, "/auth/signin") } else { @@ -170,7 +203,7 @@ func HandleChangePasswordPage(db *sql.DB) http.HandlerFunc { func HandleResetPasswordPage(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user != nil { utils.DoRedirect(w, r, "/auth/signin") } else { @@ -314,7 +347,7 @@ func HandleSignInComp(db *sql.DB) http.HandlerFunc { func HandleSignOutComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user != nil { _, err := db.Exec("DELETE FROM session WHERE session_id = ?", user.SessionId) @@ -343,7 +376,7 @@ func HandleSignOutComp(db *sql.DB) http.HandlerFunc { func HandleDeleteAccountComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { utils.DoRedirect(w, r, "/auth/signin") return @@ -409,7 +442,7 @@ func HandleDeleteAccountComp(db *sql.DB) http.HandlerFunc { func HandleVerifyResendComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil || user.EmailVerified { utils.DoRedirect(w, r, "/auth/signin") return @@ -424,7 +457,7 @@ func HandleVerifyResendComp(db *sql.DB) http.HandlerFunc { func HandleChangePasswordComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) + user := GetUserFromRequest(db, r) if user == nil { utils.DoRedirect(w, r, "/auth/signin") return @@ -669,3 +702,19 @@ func checkPassword(password string) error { return nil } } + +//TODO: delete + +func getSessionID(r *http.Request) types.SessionId { + for _, c := range r.Cookies() { + if c.Name == "id" { + return types.SessionId(c.Value) + } + } + return "" +} + +func GetUserFromRequest(db *sql.DB, r *http.Request) *types.User { + sessionId := getSessionID(r) + return GetUserFromSessionId(db, sessionId) +} diff --git a/service/auth_test.go b/service/auth_test.go index 7b62f3b..1665b6d 100644 --- a/service/auth_test.go +++ b/service/auth_test.go @@ -1,9 +1,119 @@ package service import ( + "me-fit/types" + "me-fit/utils" + + "database/sql" "testing" + + "github.com/google/uuid" ) +func mustSetup(t *testing.T) *sql.DB { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("Could not open Database data.db: %v", err) + } + utils.MustRunMigrationsTest(db, "../") + return db +} + +func TestGetUserFromSessionIfSessionNotExpired(t *testing.T) { + db := mustSetup(t) + defer db.Close() + + expected := types.NewUser(uuid.New(), "email", "session_id", true) + + db.Exec(`INSERT INTO user ( + user_uuid, email, email_verified, email_verified_at, + is_admin, password, salt, created_at) + VAlUES ( + ?, ?, 1, datetime(), + 0, "password", "salt", datetime())`, expected.Id, expected.Email) + db.Exec(`INSERT INTO session (session_id, user_uuid, created_at) VALUES (?, ?, datetime('now', '-2 hour'))`, expected.SessionId, expected.Id) + + actual := GetUserFromSessionId(db, expected.SessionId) + + if *actual != *expected { + t.Errorf("Expected %v, got %v", *expected, *actual) + } +} + +func TestGetUserFromSessionIfSessionInFuture(t *testing.T) { + db := mustSetup(t) + defer db.Close() + + expected := types.NewUser(uuid.New(), "email", "session_id", true) + + db.Exec(`INSERT INTO user ( + user_uuid, email, email_verified, email_verified_at, + is_admin, password, salt, created_at) + VAlUES ( + ?, ?, 1, datetime(), + 0, "password", "salt", datetime())`, expected.Id, expected.Email) + db.Exec(`INSERT INTO session (session_id, user_uuid, created_at) VALUES (?, ?, datetime('now', '+2 hour'))`, expected.SessionId, expected.Id) + + actual := GetUserFromSessionId(db, expected.SessionId) + + if *actual != *expected { + t.Errorf("Expected %v, got %v", *expected, *actual) + } +} + +func TestFailGetUserFromSessionIfSessionExpired(t *testing.T) { + db := mustSetup(t) + defer db.Close() + + expected := types.NewUser(uuid.New(), "email", "session_id", true) + + db.Exec(`INSERT INTO user ( + user_uuid, email, email_verified, email_verified_at, + is_admin, password, salt, created_at) + VAlUES ( + ?, ?, 1, datetime(), + 0, "password", "salt", datetime())`, expected.Id, expected.Email) + db.Exec(`INSERT INTO session (session_id, user_uuid, created_at) VALUES (?, ?, datetime('now', '-8 hour', '-1 minute'))`, expected.SessionId, expected.Id) + + actual := GetUserFromSessionId(db, expected.SessionId) + + if actual != nil { + t.Errorf("Expected nil, got %v", *actual) + } +} + +func TestGetUserFromSessionShouldFindCorrectUserBySessionId(t *testing.T) { + db := mustSetup(t) + defer db.Close() + + expected := types.NewUser(uuid.New(), "email", "session_id", true) + userId2 := uuid.New() + + db.Exec(`INSERT INTO user ( + user_uuid, email, email_verified, email_verified_at, + is_admin, password, salt, created_at) + VAlUES ( + ?, ?, 1, datetime(), + 0, "password", "salt", datetime()), + ( + ?, ?, 1, datetime(), + 0, "password", "salt", datetime()) + `, expected.Id, expected.Email, userId2, "email2") + db.Exec(` + INSERT INTO session ( + session_id, user_uuid, created_at) + VALUES + (?, ?, datetime('now')), + (?, ?, datetime('now')) + `, expected.SessionId, expected.Id, expected.SessionId+"x", userId2) + + actual := GetUserFromSessionId(db, expected.SessionId) + + if *actual != *expected { + t.Errorf("Expected %v, got %v", *expected, *actual) + } +} + func TestValidPasswords(t *testing.T) { passwords := []string{ "aB!'2d2y", //normal diff --git a/service/index_and_404.go b/service/index_and_404.go deleted file mode 100644 index 480ca6d..0000000 --- a/service/index_and_404.go +++ /dev/null @@ -1,32 +0,0 @@ -package service - -import ( - "database/sql" - "me-fit/template" - "me-fit/utils" - "net/http" - - "github.com/a-h/templ" -) - -func HandleIndexAnd404(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := utils.GetUserFromSession(db, r) - - var comp templ.Component = nil - userComp := UserInfoComp(user) - - if r.URL.Path != "/" { - comp = template.Layout(template.NotFound(), userComp) - w.WriteHeader(http.StatusNotFound) - } else { - comp = template.Layout(template.Index(), userComp) - } - - err := comp.Render(r.Context(), w) - if err != nil { - utils.LogError("Failed to render index", err) - http.Error(w, "Failed to render index", http.StatusInternalServerError) - } - } -} diff --git a/types/types.go b/types/types.go index 2c5324f..013f1e7 100644 --- a/types/types.go +++ b/types/types.go @@ -2,9 +2,20 @@ package types import "github.com/google/uuid" +type SessionId string + type User struct { Id uuid.UUID Email string - SessionId string + SessionId SessionId EmailVerified bool } + +func NewUser(id uuid.UUID, email string, sessionId SessionId, emailVerified bool) *User { + return &User{ + Id: id, + Email: email, + SessionId: sessionId, + EmailVerified: emailVerified, + } +} diff --git a/utils/db.go b/utils/db.go index 90f782e..57e54c9 100644 --- a/utils/db.go +++ b/utils/db.go @@ -10,13 +10,20 @@ import ( ) func MustRunMigrations(db *sql.DB) { + mustRunMigrationsInternal(db, "") +} +func MustRunMigrationsTest(db *sql.DB, pathPrefix string) { + mustRunMigrationsInternal(db, "../") +} + +func mustRunMigrationsInternal(db *sql.DB, pathPrefix string) { driver, err := sqlite3.WithInstance(db, &sqlite3.Config{}) if err != nil { log.Fatal(err) } m, err := migrate.NewWithDatabaseInstance( - "file://./migration/", + "file://./"+pathPrefix+"migration/", "", driver) if err != nil { diff --git a/utils/http.go b/utils/http.go index 406d0de..5a3a517 100644 --- a/utils/http.go +++ b/utils/http.go @@ -1,12 +1,10 @@ package utils import ( - "database/sql" "fmt" "log/slog" "me-fit/types" "net/http" - "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -61,39 +59,10 @@ func GetUser(r *http.Request) *types.User { } } -func GetUserFromSession(db *sql.DB, r *http.Request) *types.User { - sessionId := getSessionID(r) - if sessionId == "" { - return nil - } - - var user types.User - var createdAt time.Time - - user.SessionId = sessionId - - err := db.QueryRow(` - SELECT u.user_uuid, u.email, u.email_verified, s.created_at - FROM session s - INNER JOIN user u ON s.user_uuid = u.user_uuid - WHERE session_id = ?`, sessionId).Scan(&user.Id, &user.Email, &user.EmailVerified, &createdAt) - if err != nil { - slog.Warn("Could not verify session: " + err.Error()) - return nil - } - - if createdAt.Add(time.Duration(8 * time.Hour)).Before(time.Now()) { - return nil - } else { - return &user - } - -} - -func getSessionID(r *http.Request) string { +func GetSessionID(r *http.Request) types.SessionId { for _, c := range r.Cookies() { if c.Name == "id" { - return c.Value + return types.SessionId(c.Value) } } return "" -- 2.49.1 From f83a404206506b8e6a968062da93cd8a06e4c44a Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Thu, 19 Sep 2024 21:33:20 +0200 Subject: [PATCH 4/7] chore(auth): more refactoring --- handler/auth.go | 208 ++++++++++++++++++++++- handler/default.go | 8 +- service/auth.go | 404 +++++++++++++++------------------------------ 3 files changed, 334 insertions(+), 286 deletions(-) diff --git a/handler/auth.go b/handler/auth.go index 8d28f66..44c2135 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -2,6 +2,10 @@ package handler import ( "me-fit/service" + "me-fit/template" + "me-fit/template/auth" + "me-fit/types" + "me-fit/utils" "database/sql" "net/http" @@ -10,13 +14,13 @@ import ( func authUi(db *sql.DB) http.Handler { router := http.NewServeMux() - router.Handle("/auth/signin", service.HandleSignInPage(db)) - router.Handle("/auth/signup", service.HandleSignUpPage(db)) - router.Handle("/auth/verify", service.HandleSignUpVerifyPage(db)) // Hint for the user to verify their email - router.Handle("/auth/delete-account", service.HandleDeleteAccountPage(db)) - router.Handle("/auth/verify-email", service.HandleSignUpVerifyResponsePage(db)) // The link contained in the email - router.Handle("/auth/change-password", service.HandleChangePasswordPage(db)) - router.Handle("/auth/reset-password", service.HandleResetPasswordPage(db)) + router.Handle("/auth/signin", handleSignInPage(db)) + router.Handle("/auth/signup", handleSignUpPage(db)) + router.Handle("/auth/verify", handleSignUpVerifyPage(db)) // Hint for the user to verify their email + router.Handle("/auth/delete-account", handleDeleteAccountPage(db)) + router.Handle("/auth/verify-email", handleSignUpVerifyResponsePage(db)) // The link contained in the email + router.Handle("/auth/change-password", handleChangePasswordPage(db)) + router.Handle("/auth/reset-password", handleResetPasswordPage(db)) router.Handle("/", handleNotFound(db)) return router @@ -25,8 +29,8 @@ func authUi(db *sql.DB) http.Handler { func authApi(db *sql.DB) http.Handler { router := http.NewServeMux() - router.Handle("/api/auth/signup", service.HandleSignUpComp(db)) - router.Handle("/api/auth/signin", service.HandleSignInComp(db)) + router.Handle("/api/auth/signup", handleSignUp(db)) + router.Handle("/api/auth/signin", handleSignIn(db)) router.Handle("/api/auth/signout", service.HandleSignOutComp(db)) router.Handle("/api/auth/delete-account", service.HandleDeleteAccountComp(db)) router.Handle("/api/auth/verify-resend", service.HandleVerifyResendComp(db)) @@ -36,3 +40,189 @@ func authApi(db *sql.DB) http.Handler { return router } + +func handleSignInPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + if user == nil { + userComp := service.UserInfoComp(nil) + signIn := auth.SignInOrUpComp(true) + err := template.Layout(signIn, userComp).Render(r.Context(), w) + + if err != nil { + utils.LogError("Failed to render sign in 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 handleSignUpPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + if user == nil { + userComp := service.UserInfoComp(nil) + signUpComp := auth.SignInOrUpComp(false) + err := template.Layout(signUpComp, userComp).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 handleSignUpVerifyPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + if user == nil { + utils.DoRedirect(w, r, "/auth/signin") + } else if user.EmailVerified { + utils.DoRedirect(w, r, "/") + } else { + userComp := service.UserInfoComp(user) + signIn := auth.VerifyComp() + err := template.Layout(signIn, userComp).Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render verify page", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + } + } +} + +func handleDeleteAccountPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + // An unverified email should be able to delete their account + user := service.GetUserFromRequest(db, r) + if user == nil { + utils.DoRedirect(w, r, "/auth/signin") + } else { + userComp := service.UserInfoComp(user) + comp := auth.DeleteAccountComp() + err := template.Layout(comp, userComp).Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render delete account page", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + } + } +} + +func handleSignUpVerifyResponsePage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + token := r.URL.Query().Get("token") + if token == "" { + utils.DoRedirect(w, r, "/auth/verify") + return + } + + err := service.ValidateEmail(db, token) + if err != nil { + utils.LogError("Could not validate email", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + + utils.DoRedirect(w, r, "/") + } +} + +func handleChangePasswordPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + isPasswordReset := r.URL.Query().Has("token") + + user := service.GetUserFromRequest(db, r) + if user == nil && !isPasswordReset { + utils.DoRedirect(w, r, "/auth/signin") + } else { + userComp := service.UserInfoComp(user) + comp := auth.ChangePasswordComp(isPasswordReset) + err := template.Layout(comp, userComp).Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render change password page", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + } + } +} + +func handleResetPasswordPage(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + user := service.GetUserFromRequest(db, r) + if user != nil { + utils.DoRedirect(w, r, "/auth/signin") + } else { + userComp := service.UserInfoComp(nil) + comp := auth.ResetPasswordComp() + err := template.Layout(comp, userComp).Render(r.Context(), w) + if err != nil { + utils.LogError("Failed to render change password page", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + } + } +} + +func handleSignUp(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + var email = r.FormValue("email") + var password = r.FormValue("password") + + sessionId, err := service.SignUp(db, email, password) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + http.SetCookie(w, createSessionCookie(*sessionId)) + + utils.DoRedirect(w, r, "/auth/verify") + } +} + +func createSessionCookie(sessionId types.SessionId) *http.Cookie { + cookie := http.Cookie{ + Name: "id", + Value: string(sessionId), + MaxAge: 60 * 60 * 8, // 8 hours + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + Path: "/", + } + + return &cookie +} + +func handleSignIn(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + var email = r.FormValue("email") + var password = r.FormValue("password") + + sessionId, err := service.SignIn(db, email, password) + if err != nil { + utils.TriggerToast(w, r, "error", "Invalid username or password") + } + + http.SetCookie(w, createSessionCookie(*sessionId)) + + utils.DoRedirect(w, r, "/auth/verify") + } +} diff --git a/handler/default.go b/handler/default.go index 0f6aa66..1470263 100644 --- a/handler/default.go +++ b/handler/default.go @@ -21,17 +21,17 @@ func GetHandler(db *sql.DB) http.Handler { router.Handle("/auth/", authUi(db)) router.Handle("/api/auth/", authApi(db)) - router.Handle("/workout", auth(db, workoutUi(db))) - router.Handle("/api/workout", auth(db, workoutApi(db))) + router.Handle("/workout", authMiddleware(db, workoutUi(db))) + router.Handle("/api/workout", authMiddleware(db, workoutApi(db))) // Needed a second time with trailing slash, otherwise either /api/workout or /api/workout/{id} does not match - router.Handle("/api/workout/", auth(db, workoutApi(db))) + router.Handle("/api/workout/", authMiddleware(db, workoutApi(db))) return middleware.Logging( middleware.EnableCors( router)) } -func auth(db *sql.DB, h http.Handler) http.Handler { +func authMiddleware(db *sql.DB, h http.Handler) http.Handler { return middleware.EnsureValidSession(db, h) } diff --git a/service/auth.go b/service/auth.go index ce45e59..d0f1d84 100644 --- a/service/auth.go +++ b/service/auth.go @@ -13,7 +13,6 @@ import ( "strings" "time" - "me-fit/template" "me-fit/template/auth" tempMail "me-fit/template/mail" "me-fit/types" @@ -25,6 +24,98 @@ import ( ) // TESTED +// NOT TESTED + +func SignUp(db *sql.DB, email string, password string) (*types.SessionId, error) { + _, err := mail.ParseAddress(email) + if err != nil { + return nil, errors.New("Invalid email") + } + + err = checkPassword(password) + if err != nil { + return nil, err + } + + userId, err := uuid.NewRandom() + if err != nil { + return nil, errors.Join(errors.New("Could not generate UUID for new user"), err) + } + + salt := make([]byte, 16) + _, err = rand.Read(salt) + if err != nil { + return nil, errors.Join(errors.New("Could not generate salt for new user"), err) + } + + 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") { + return nil, errors.New("Bad Request") + } + + return nil, errors.Join(errors.New("Could not insert user"), err) + } + + sessionId, err := createSession(db, userId) + if err != nil { + return nil, err + } + + // Send verification email as a goroutine + go sendVerificationEmail(db, userId.String(), email) + return sessionId, nil +} + +func SignIn(db *sql.DB, email string, password string) (*types.SessionId, error) { + start := time.Now() + + sessionId, err := internalSignIn(db, email, password) + + duration := time.Since(start) + timeToWait := 100 - duration.Milliseconds() + // It is important to sleep for a while to prevent timing attacks + // If the email is correct, the server will calculate the hash, which will take some time + // This way an attacker could guess emails when comparing the response time + // Because of that, we cant use WriteHeader in the middle of the function. We have to wait until the end + // Unfortunatly this makes the code harder to read + time.Sleep(time.Duration(timeToWait) * time.Millisecond) + + return sessionId, err +} + +func internalSignIn(db *sql.DB, email string, password string) (*types.SessionId, error) { + var ( + userId uuid.UUID + savedHash []byte + salt []byte + ) + + err := db.QueryRow("SELECT user_uuid, password, salt FROM user WHERE email = ?", email).Scan(&userId, &savedHash, &salt) + if err != nil { + return nil, err + } + + new_hash := getHashPassword(password, salt) + + if subtle.ConstantTimeCompare(new_hash, savedHash) == 0 { + return nil, errors.New("Invalid Request") + } + + sessionId, err := createSession(db, userId) + if err != nil { + return nil, err + } + + return sessionId, nil +} func GetUserFromSessionId(db *sql.DB, sessionId types.SessionId) *types.User { if sessionId == "" { @@ -55,167 +146,31 @@ func GetUserFromSessionId(db *sql.DB, sessionId types.SessionId) *types.User { } } -// NOT TESTED +func ValidateEmail(db *sql.DB, token string) error { + result, err := db.Exec(` + UPDATE user + SET email_verified = true, email_verified_at = datetime() + WHERE user_uuid = ( + SELECT user_uuid + FROM user_token + WHERE type = "email_verify" + AND token = ? + );`, token) -func HandleSignInPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - - if user == nil { - userComp := UserInfoComp(nil) - signIn := auth.SignInOrUpComp(true) - err := template.Layout(signIn, userComp).Render(r.Context(), w) - - if err != nil { - utils.LogError("Failed to render sign in 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, "/") - } + if err != nil { + return errors.Join(errors.New("Could not update user on verify response"), err) } -} -func HandleSignUpPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - - if user == nil { - userComp := UserInfoComp(nil) - signUpComp := auth.SignInOrUpComp(false) - err := template.Layout(signUpComp, userComp).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, "/") - } + i, err := result.RowsAffected() + if err != nil { + return errors.Join(errors.New("Could not get rows affected on verify response"), err) } -} -func HandleSignUpVerifyPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - if user == nil { - utils.DoRedirect(w, r, "/auth/signin") - } else if user.EmailVerified { - utils.DoRedirect(w, r, "/") - } else { - userComp := UserInfoComp(user) - signIn := auth.VerifyComp() - err := template.Layout(signIn, userComp).Render(r.Context(), w) - if err != nil { - utils.LogError("Failed to render verify page", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } - } + if i == 0 { + return errors.New("Token is invalid") } -} -func HandleDeleteAccountPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - // An unverified email should be able to delete their account - user := GetUserFromRequest(db, r) - if user == nil { - utils.DoRedirect(w, r, "/auth/signin") - } else { - userComp := UserInfoComp(user) - comp := auth.DeleteAccountComp() - err := template.Layout(comp, userComp).Render(r.Context(), w) - if err != nil { - utils.LogError("Failed to render delete account page", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } - } - } -} - -func HandleSignUpVerifyResponsePage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - - token := r.URL.Query().Get("token") - - if token == "" { - utils.DoRedirect(w, r, "/auth/verify") - return - } - - result, err := db.Exec(` - UPDATE user - SET email_verified = true, email_verified_at = datetime() - WHERE user_uuid = ( - SELECT user_uuid - FROM user_token - WHERE type = "email_verify" - AND token = ? - ); - `, token) - - if err != nil { - utils.LogError("Could not update user on verify response", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - return - } - - i, err := result.RowsAffected() - if err != nil { - utils.LogError("Could not get rows affected on verify response", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - return - } - - if i == 0 { - utils.DoRedirect(w, r, "/") - } else { - utils.DoRedirect(w, r, "/auth/signin") - } - } -} - -func HandleChangePasswordPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - - isPasswordReset := r.URL.Query().Has("token") - - user := GetUserFromRequest(db, r) - if user == nil && !isPasswordReset { - utils.DoRedirect(w, r, "/auth/signin") - } else { - userComp := UserInfoComp(user) - comp := auth.ChangePasswordComp(isPasswordReset) - err := template.Layout(comp, userComp).Render(r.Context(), w) - if err != nil { - utils.LogError("Failed to render change password page", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } - } - } -} - -func HandleResetPasswordPage(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - - user := GetUserFromRequest(db, r) - if user != nil { - utils.DoRedirect(w, r, "/auth/signin") - } else { - userComp := UserInfoComp(nil) - comp := auth.ResetPasswordComp() - err := template.Layout(comp, userComp).Render(r.Context(), w) - if err != nil { - utils.LogError("Failed to render change password page", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } - } - } + return nil } func UserInfoComp(user *types.User) templ.Component { @@ -227,124 +182,6 @@ func UserInfoComp(user *types.User) templ.Component { } } -func HandleSignUpComp(db *sql.DB) 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 - } - - result := tryCreateSessionAndSetCookie(r, w, db, userId) - if !result { - return - } - - // Send verification email as a goroutine - go sendVerificationEmail(db, userId.String(), email) - - utils.DoRedirect(w, r, "/auth/verify") - } -} - -func HandleSignInComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - var email = r.FormValue("email") - var password = r.FormValue("password") - - var result bool = true - start := time.Now() - - var ( - userId uuid.UUID - savedHash []byte - salt []byte - emailVerified bool - ) - err := db.QueryRow("SELECT user_uuid, password, salt, email_verified FROM user WHERE email = ?", email).Scan(&userId, &savedHash, &salt, &emailVerified) - if err != nil { - result = false - } - - if result { - new_hash := getHashPassword(password, salt) - - if subtle.ConstantTimeCompare(new_hash, savedHash) == 0 { - result = false - } - } - - if result { - result := tryCreateSessionAndSetCookie(r, w, db, userId) - if !result { - return - } - } - - duration := time.Since(start) - timeToWait := 100 - duration.Milliseconds() - // It is important to sleep for a while to prevent timing attacks - // If the email is correct, the server will calculate the hash, which will take some time - // This way an attacker could guess emails when comparing the response time - // Because of that, we cant use WriteHeader in the middle of the function. We have to wait until the end - // Unfortunatly this makes the code harder to read - time.Sleep(time.Duration(timeToWait) * time.Millisecond) - - if result { - if !emailVerified { - utils.DoRedirect(w, r, "/auth/verify") - } else { - utils.DoRedirect(w, r, "/") - } - } else { - auth.Error("Invalid email or password").Render(r.Context(), w) - } - } -} - func HandleSignOutComp(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { user := GetUserFromRequest(db, r) @@ -651,6 +488,27 @@ func sendVerificationEmail(db *sql.DB, userId string, email string) { utils.SendMail(email, "Welcome to ME-FIT", w.String()) } +func createSession(db *sql.DB, user_uuid uuid.UUID) (*types.SessionId, error) { + sessionId, err := utils.RandomToken() + if err != nil { + return nil, errors.Join(errors.New("Could not generate session ID"), err) + } + + // Delete old inactive sessions + _, err = db.Exec("DELETE FROM session WHERE created_at < datetime('now','-8 hours') AND user_uuid = ?", user_uuid) + if err != nil { + utils.LogError("Could not delete old sessions", err) + } + + _, err = db.Exec("INSERT INTO session (session_id, user_uuid, created_at) VALUES (?, ?, datetime())", sessionId, user_uuid) + if err != nil { + return nil, errors.Join(errors.New("Could not insert session"), err) + } + + sessionIdType := types.SessionId(sessionId) + return &sessionIdType, nil +} + func tryCreateSessionAndSetCookie(r *http.Request, w http.ResponseWriter, db *sql.DB, user_uuid uuid.UUID) bool { sessionId, err := utils.RandomToken() if err != nil { -- 2.49.1 From 7e3e9388e21dcc6ab67bc2f2ec5af21217566010 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Thu, 19 Sep 2024 23:20:17 +0200 Subject: [PATCH 5/7] chore(auth): refactor the last auth handlers --- handler/auth.go | 153 +++++++++++++++++++- service/auth.go | 371 +++++++++++++++++------------------------------- 2 files changed, 278 insertions(+), 246 deletions(-) diff --git a/handler/auth.go b/handler/auth.go index 44c2135..ed9a9ef 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -1,14 +1,18 @@ package handler import ( + "context" "me-fit/service" "me-fit/template" "me-fit/template/auth" + mail "me-fit/template/mail" "me-fit/types" "me-fit/utils" + "strings" "database/sql" "net/http" + "net/url" ) func authUi(db *sql.DB) http.Handler { @@ -31,12 +35,12 @@ func authApi(db *sql.DB) http.Handler { router.Handle("/api/auth/signup", handleSignUp(db)) router.Handle("/api/auth/signin", handleSignIn(db)) - router.Handle("/api/auth/signout", service.HandleSignOutComp(db)) - router.Handle("/api/auth/delete-account", service.HandleDeleteAccountComp(db)) - router.Handle("/api/auth/verify-resend", service.HandleVerifyResendComp(db)) - router.Handle("/api/auth/change-password", service.HandleChangePasswordComp(db)) - router.Handle("/api/auth/reset-password", service.HandleResetPasswordComp(db)) - router.Handle("/api/auth/reset-password-actual", service.HandleActualResetPasswordComp(db)) + router.Handle("/api/auth/signout", handleSignOut(db)) + router.Handle("/api/auth/delete-account", handleDeleteAccount(db)) + router.Handle("/api/auth/verify-resend", handleVerifyResend(db)) + router.Handle("/api/auth/change-password", handleChangePassword(db)) + router.Handle("/api/auth/reset-password", handleResetPassword(db)) + router.Handle("/api/auth/reset-password-actual", handleActualResetPassword(db)) return router } @@ -226,3 +230,140 @@ func handleSignIn(db *sql.DB) http.HandlerFunc { utils.DoRedirect(w, r, "/auth/verify") } } + +func handleSignOut(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + err := service.SignOut(db, user) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + utils.TriggerToast(w, r, "error", "Internal Server Error") + return + } + + c := http.Cookie{ + Name: "id", + Value: "", + MaxAge: -1, + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + Path: "/", + } + + http.SetCookie(w, &c) + utils.DoRedirect(w, r, "/") + } +} + +func handleDeleteAccount(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + + if user == nil { + utils.DoRedirect(w, r, "/auth/signin") + return + } + + password := r.FormValue("password") + + err := service.DeleteAccount(db, user, password) + if err != nil { + utils.LogError("Could not delete account", err) + utils.TriggerToast(w, r, "error", err.Error()) + } + + utils.DoRedirect(w, r, "/") + } +} + +func handleVerifyResend(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + user := service.GetUserFromRequest(db, r) + if user == nil || user.EmailVerified { + utils.DoRedirect(w, r, "/auth/signin") + return + } + + go service.SendVerificationEmail(db, user.Id.String(), user.Email) + + w.Write([]byte("

Verification email sent

")) + } +} + +func handleChangePassword(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + user := service.GetUserFromRequest(db, r) + if user == nil { + utils.DoRedirect(w, r, "/auth/signin") + return + } + + currPass := r.FormValue("current-password") + newPass := r.FormValue("new-password") + + err := service.ChangePassword(db, user, currPass, newPass) + if err != nil { + utils.TriggerToast(w, r, "error", err.Error()) + return + } + + utils.TriggerToast(w, r, "success", "Password changed") + } +} + +func handleResetPassword(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + email := r.FormValue("email") + + token, err := service.ResetPassword(db, email) + if err != nil { + utils.TriggerToast(w, r, "error", err.Error()) + return + } + + if token != "" { + var string strings.Builder + err = mail.ResetPassword(token).Render(context.Background(), &string) + if err != nil { + utils.LogError("Could not render reset password email", err) + utils.TriggerToast(w, r, "error", "Internal Server Error") + return + } + utils.SendMail(email, "Reset Password", string.String()) + } + + utils.TriggerToast(w, r, "info", "If the email exists, an email has been sent") + } +} + +func handleActualResetPassword(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + pageUrl, err := url.Parse(r.Header.Get("HX-Current-URL")) + if err != nil { + utils.LogError("Could not get current URL", err) + utils.TriggerToast(w, r, "error", "Internal Server Error") + return + } + + token := pageUrl.Query().Get("token") + if token == "" { + utils.TriggerToast(w, r, "error", "No token") + return + } + + newPass := r.FormValue("new-password") + + service.ActualResetPassword(db, token, newPass) + if err != nil { + utils.TriggerToast(w, r, "error", err.Error()) + return + } + + utils.TriggerToast(w, r, "success", "Password changed") + } +} diff --git a/service/auth.go b/service/auth.go index d0f1d84..27312f3 100644 --- a/service/auth.go +++ b/service/auth.go @@ -9,7 +9,6 @@ import ( "log/slog" "net/http" "net/mail" - "net/url" "strings" "time" @@ -70,7 +69,7 @@ func SignUp(db *sql.DB, email string, password string) (*types.SessionId, error) } // Send verification email as a goroutine - go sendVerificationEmail(db, userId.String(), email) + go SendVerificationEmail(db, userId.String(), email) return sessionId, nil } @@ -182,199 +181,113 @@ func UserInfoComp(user *types.User) templ.Component { } } -func HandleSignOutComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - - if user != nil { - _, err := db.Exec("DELETE FROM session WHERE session_id = ?", user.SessionId) - if err != nil { - utils.LogError("Could not delete session", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - } - - c := http.Cookie{ - Name: "id", - Value: "", - MaxAge: -1, - Secure: true, - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - Path: "/", - } - - http.SetCookie(w, &c) - utils.DoRedirect(w, r, "/") +func SignOut(db *sql.DB, user *types.User) error { + if user == nil { + return nil } + + _, err := db.Exec("DELETE FROM session WHERE session_id = ?", user.SessionId) + if err != nil { + return errors.Join(errors.New("Could not delete session"), err) + } + + return nil } -func HandleDeleteAccountComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - if user == nil { - utils.DoRedirect(w, r, "/auth/signin") - return - } +func DeleteAccount(db *sql.DB, user *types.User, password string) error { - password := r.FormValue("password") - if password == "" { - utils.TriggerToast(w, r, "error", "Password is required") - return - } - - var ( - storedHash []byte - salt []byte - ) - - 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") - return - } - - currHash := getHashPassword(password, salt) - if subtle.ConstantTimeCompare(currHash, storedHash) == 0 { - utils.TriggerToast(w, r, "error", "Password is not correct") - return - } - - _, err = db.Exec("DELETE FROM workout WHERE user_id = ?", user.Id) - if err != nil { - utils.LogError("Could not delete workouts", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - _, err = db.Exec("DELETE FROM user_token WHERE user_uuid = ?", user.Id) - if err != nil { - utils.LogError("Could not delete user tokens", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - _, err = db.Exec("DELETE FROM session WHERE user_uuid = ?", user.Id) - if err != nil { - utils.LogError("Could not delete sessions", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - _, err = db.Exec("DELETE FROM user WHERE user_uuid = ?", user.Id) - if err != nil { - utils.LogError("Could not delete user", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - go utils.SendMail(user.Email, "Account deleted", "Your account has been deleted") - - utils.DoRedirect(w, r, "/") + if password == "" { + return errors.New("Please enter your password") } + + var ( + storedHash []byte + salt []byte + ) + + err := db.QueryRow("SELECT password, salt FROM user WHERE user_uuid = ?", user.Id).Scan(&storedHash, &salt) + if err != nil { + return errors.Join(errors.New("Could not get password"), err) + } + + currHash := getHashPassword(password, salt) + if subtle.ConstantTimeCompare(currHash, storedHash) == 0 { + return errors.New("Password is not correct") + } + + _, err = db.Exec("DELETE FROM workout WHERE user_id = ?", user.Id) + if err != nil { + return errors.Join(errors.New("Could not delete workouts"), err) + } + + _, err = db.Exec("DELETE FROM user_token WHERE user_uuid = ?", user.Id) + if err != nil { + return errors.Join(errors.New("Could not delete tokens"), err) + } + + _, err = db.Exec("DELETE FROM session WHERE user_uuid = ?", user.Id) + if err != nil { + return errors.Join(errors.New("Could not delete sessions"), err) + } + + _, err = db.Exec("DELETE FROM user WHERE user_uuid = ?", user.Id) + if err != nil { + return errors.Join(errors.New("Could not delete user"), err) + } + + go utils.SendMail(user.Email, "Account deleted", "Your account has been deleted") + return nil } -func HandleVerifyResendComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - user := GetUserFromRequest(db, r) - if user == nil || user.EmailVerified { - utils.DoRedirect(w, r, "/auth/signin") - return - } +func ChangePassword(db *sql.DB, user *types.User, currPass string, newPass string) error { - go sendVerificationEmail(db, user.Id.String(), user.Email) - - w.Write([]byte("

Verification email sent

")) + err := checkPassword(newPass) + if err != nil { + return err } + + if currPass == newPass { + return errors.New("New password can not be the same as the current password") + } + + var ( + storedHash []byte + salt []byte + ) + + err = db.QueryRow("SELECT password, salt FROM user WHERE user_uuid = ?", user.Id).Scan(&storedHash, &salt) + if err != nil { + return errors.Join(errors.New("Could not get password"), err) + } + + currHash := getHashPassword(currPass, salt) + if subtle.ConstantTimeCompare(currHash, storedHash) == 0 { + return errors.New("Current password is not correct") + } + + newHash := getHashPassword(newPass, salt) + + _, err = db.Exec("UPDATE user SET password = ? WHERE user_uuid = ?", newHash, user.Id) + if err != nil { + return errors.Join(errors.New("Could not update password"), err) + } + + return nil } -func HandleChangePasswordComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { +func ActualResetPassword(db *sql.DB, token string, newPass string) error { - user := GetUserFromRequest(db, r) - if user == nil { - utils.DoRedirect(w, r, "/auth/signin") - return - } - - currPass := r.FormValue("current-password") - newPass := r.FormValue("new-password") - - err := checkPassword(newPass) - if err != nil { - utils.TriggerToast(w, r, "error", err.Error()) - return - } - - if currPass == newPass { - utils.TriggerToast(w, r, "error", "Please use a new password") - return - } - - var ( - storedHash []byte - salt []byte - ) - - 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") - return - } - - currHash := getHashPassword(currPass, salt) - if subtle.ConstantTimeCompare(currHash, storedHash) == 0 { - utils.TriggerToast(w, r, "error", "Current Password is not correct") - return - } - - newHash := getHashPassword(newPass, salt) - - _, err = db.Exec("UPDATE user SET password = ? WHERE user_uuid = ?", newHash, user.Id) - if err != nil { - utils.LogError("Could not update password", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - utils.TriggerToast(w, r, "success", "Password changed") + err := checkPassword(newPass) + if err != nil { + return err } -} -func HandleActualResetPasswordComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { + var ( + userId uuid.UUID + salt []byte + ) - pageUrl, err := url.Parse(r.Header.Get("HX-Current-URL")) - if err != nil { - utils.LogError("Could not get current URL", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - token := pageUrl.Query().Get("token") - if token == "" { - utils.TriggerToast(w, r, "error", "No token") - return - } - - newPass := r.FormValue("new-password") - - err = checkPassword(newPass) - if err != nil { - utils.TriggerToast(w, r, "error", err.Error()) - return - } - - var ( - userId uuid.UUID - salt []byte - ) - - err = db.QueryRow(` + err = db.QueryRow(` SELECT u.user_uuid, salt FROM user_token t INNER JOIN user u ON t.user_uuid = u.user_uuid @@ -382,81 +295,59 @@ func HandleActualResetPasswordComp(db *sql.DB) http.HandlerFunc { AND t.type = 'password_reset' AND t.expires_at > datetime() `, token).Scan(&userId, &salt) - if err != nil { - slog.Warn("Could not get user from token: " + err.Error()) - utils.TriggerToast(w, r, "error", "Invalid token") - return - } - - _, err = db.Exec("DELETE FROM user_token WHERE token = ? AND type = 'password_reset'", token) - if err != nil { - utils.LogError("Could not delete token", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - passHash := getHashPassword(newPass, salt) - - _, err = db.Exec("UPDATE user SET password = ? WHERE user_uuid = ?", passHash, userId) - if err != nil { - utils.LogError("Could not update password", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - - utils.TriggerToast(w, r, "success", "Password changed") + if err != nil { + return errors.Join(errors.New("Could not get user from token"), err) } + + _, err = db.Exec("DELETE FROM user_token WHERE token = ? AND type = 'password_reset'", token) + if err != nil { + return errors.Join(errors.New("Could not delete token"), err) + } + + passHash := getHashPassword(newPass, salt) + + _, err = db.Exec("UPDATE user SET password = ? WHERE user_uuid = ?", passHash, userId) + if err != nil { + return errors.Join(errors.New("Could not update password"), err) + } + + return nil } -func HandleResetPasswordComp(db *sql.DB) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - email := r.FormValue("email") - if email == "" { - utils.TriggerToast(w, r, "error", "Please enter an email") - return - } +func ResetPassword(db *sql.DB, email string) (string, error) { - token, err := utils.RandomToken() - if err != nil { - utils.LogError("Could not generate token", err) - return - } + if email == "" { + return "", errors.New("Please enter an email") + } - res, err := db.Exec(` + token, err := utils.RandomToken() + if err != nil { + return "", errors.Join(errors.New("Could not generate token"), err) + } + + res, err := db.Exec(` INSERT INTO user_token (user_uuid, type, token, created_at, expires_at) SELECT user_uuid, 'password_reset', ?, datetime(), datetime('now', '+15 minute') FROM user WHERE email = ? `, token, email) - if err != nil { - utils.LogError("Could not insert token", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } + if err != nil { + return "", errors.Join(errors.New("Could not insert token"), err) + } - i, err := res.RowsAffected() - if err != nil { - utils.LogError("Could not get rows affected", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } + i, err := res.RowsAffected() + if err != nil { + return "", errors.Join(errors.New("Could not get rows affected"), err) + } - if i != 0 { - var mail strings.Builder - err = tempMail.ResetPassword(token).Render(context.Background(), &mail) - if err != nil { - utils.LogError("Could not render reset password email", err) - utils.TriggerToast(w, r, "error", "Internal Server Error") - return - } - utils.SendMail(email, "Reset Password", mail.String()) - } - - utils.TriggerToast(w, r, "info", "If the email exists, an email has been sent") + if i == 0 { + return "", nil + } else { + return token, nil } } -func sendVerificationEmail(db *sql.DB, userId string, email string) { +func SendVerificationEmail(db *sql.DB, userId string, email string) { var token string err := db.QueryRow("SELECT token FROM user_token WHERE user_uuid = ? AND type = 'email_verify'", userId).Scan(&token) -- 2.49.1 From e53a0e5cf73678796f4e09edd4bce398c92db4bf Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Fri, 20 Sep 2024 22:19:02 +0200 Subject: [PATCH 6/7] chore(auth): add service structs and test error handling --- handler/auth.go | 48 +++++++------- handler/default.go | 11 +++- service/auth.go | 154 ++++++++++++++++++++++++++++----------------- utils/ctypto.go | 16 ----- 4 files changed, 130 insertions(+), 99 deletions(-) delete mode 100644 utils/ctypto.go diff --git a/handler/auth.go b/handler/auth.go index ed9a9ef..16b5af0 100644 --- a/handler/auth.go +++ b/handler/auth.go @@ -15,32 +15,37 @@ import ( "net/url" ) -func authUi(db *sql.DB) http.Handler { +type AuthHandler struct { + db *sql.DB + service *service.AuthService +} + +func (a *AuthHandler) authUi() http.Handler { router := http.NewServeMux() - router.Handle("/auth/signin", handleSignInPage(db)) - router.Handle("/auth/signup", handleSignUpPage(db)) - router.Handle("/auth/verify", handleSignUpVerifyPage(db)) // Hint for the user to verify their email - router.Handle("/auth/delete-account", handleDeleteAccountPage(db)) - router.Handle("/auth/verify-email", handleSignUpVerifyResponsePage(db)) // The link contained in the email - router.Handle("/auth/change-password", handleChangePasswordPage(db)) - router.Handle("/auth/reset-password", handleResetPasswordPage(db)) - router.Handle("/", handleNotFound(db)) + router.Handle("/auth/signin", handleSignInPage(a.db)) + router.Handle("/auth/signup", handleSignUpPage(a.db)) + router.Handle("/auth/verify", handleSignUpVerifyPage(a.db)) // Hint for the user to verify their email + router.Handle("/auth/delete-account", handleDeleteAccountPage(a.db)) + router.Handle("/auth/verify-email", handleSignUpVerifyResponsePage(a.db)) // The link contained in the email + router.Handle("/auth/change-password", handleChangePasswordPage(a.db)) + router.Handle("/auth/reset-password", handleResetPasswordPage(a.db)) + router.Handle("/", handleNotFound(a.db)) return router } -func authApi(db *sql.DB) http.Handler { +func (a *AuthHandler) authApi() http.Handler { router := http.NewServeMux() - router.Handle("/api/auth/signup", handleSignUp(db)) - router.Handle("/api/auth/signin", handleSignIn(db)) - router.Handle("/api/auth/signout", handleSignOut(db)) - router.Handle("/api/auth/delete-account", handleDeleteAccount(db)) - router.Handle("/api/auth/verify-resend", handleVerifyResend(db)) - router.Handle("/api/auth/change-password", handleChangePassword(db)) - router.Handle("/api/auth/reset-password", handleResetPassword(db)) - router.Handle("/api/auth/reset-password-actual", handleActualResetPassword(db)) + router.Handle("/api/auth/signup", handleSignUp(a.db)) + router.Handle("/api/auth/signin", a.handleSignIn()) + router.Handle("/api/auth/signout", handleSignOut(a.db)) + router.Handle("/api/auth/delete-account", handleDeleteAccount(a.db)) + router.Handle("/api/auth/verify-resend", handleVerifyResend(a.db)) + router.Handle("/api/auth/change-password", handleChangePassword(a.db)) + router.Handle("/api/auth/reset-password", handleResetPassword(a.db)) + router.Handle("/api/auth/reset-password-actual", handleActualResetPassword(a.db)) return router } @@ -215,14 +220,15 @@ func createSessionCookie(sessionId types.SessionId) *http.Cookie { return &cookie } -func handleSignIn(db *sql.DB) http.HandlerFunc { +func (a *AuthHandler) handleSignIn() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var email = r.FormValue("email") var password = r.FormValue("password") - sessionId, err := service.SignIn(db, email, password) + sessionId, err := a.service.SignIn(email, password) if err != nil { - utils.TriggerToast(w, r, "error", "Invalid username or password") + utils.TriggerToast(w, r, "error", err.Error()) + return } http.SetCookie(w, createSessionCookie(*sessionId)) diff --git a/handler/default.go b/handler/default.go index 1470263..083582c 100644 --- a/handler/default.go +++ b/handler/default.go @@ -13,13 +13,18 @@ import ( func GetHandler(db *sql.DB) http.Handler { router := http.NewServeMux() - router.HandleFunc("/$", handleIndex(db)) + router.HandleFunc("/{$}", handleIndex(db)) router.HandleFunc("/", handleNotFound(db)) router.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("./static/")))) - router.Handle("/auth/", authUi(db)) - router.Handle("/api/auth/", authApi(db)) + authHandler := AuthHandler{ + db: db, + service: service.NewAuthService(db), + } + + router.Handle("/auth/", authHandler.authUi()) + router.Handle("/api/auth/", authHandler.authApi()) router.Handle("/workout", authMiddleware(db, workoutUi(db))) router.Handle("/api/workout", authMiddleware(db, workoutApi(db))) diff --git a/service/auth.go b/service/auth.go index 27312f3..d8258a5 100644 --- a/service/auth.go +++ b/service/auth.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "crypto/subtle" "database/sql" + "encoding/base64" "errors" "log/slog" "net/http" @@ -23,60 +24,28 @@ import ( ) // TESTED + +var ( + ErrInternalServer = errors.New("Internal Server Error") + ErrInvalidUsernameOrPassword = errors.New("Invalid username or password") +) + // NOT TESTED -func SignUp(db *sql.DB, email string, password string) (*types.SessionId, error) { - _, err := mail.ParseAddress(email) - if err != nil { - return nil, errors.New("Invalid email") - } - - err = checkPassword(password) - if err != nil { - return nil, err - } - - userId, err := uuid.NewRandom() - if err != nil { - return nil, errors.Join(errors.New("Could not generate UUID for new user"), err) - } - - salt := make([]byte, 16) - _, err = rand.Read(salt) - if err != nil { - return nil, errors.Join(errors.New("Could not generate salt for new user"), err) - } - - 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") { - return nil, errors.New("Bad Request") - } - - return nil, errors.Join(errors.New("Could not insert user"), err) - } - - sessionId, err := createSession(db, userId) - if err != nil { - return nil, err - } - - // Send verification email as a goroutine - go SendVerificationEmail(db, userId.String(), email) - return sessionId, nil +type AuthService struct { + db *sql.DB } -func SignIn(db *sql.DB, email string, password string) (*types.SessionId, error) { +func NewAuthService(db *sql.DB) *AuthService { + return &AuthService{ + db: db, + } +} + +func (a *AuthService) SignIn(email string, password string) (*types.SessionId, error) { start := time.Now() - sessionId, err := internalSignIn(db, email, password) + sessionId, err := a.internalSignIn(email, password) duration := time.Since(start) timeToWait := 100 - duration.Milliseconds() @@ -90,25 +59,29 @@ func SignIn(db *sql.DB, email string, password string) (*types.SessionId, error) return sessionId, err } -func internalSignIn(db *sql.DB, email string, password string) (*types.SessionId, error) { +func (a *AuthService) internalSignIn(email string, password string) (*types.SessionId, error) { var ( userId uuid.UUID savedHash []byte salt []byte ) - err := db.QueryRow("SELECT user_uuid, password, salt FROM user WHERE email = ?", email).Scan(&userId, &savedHash, &salt) + err := a.db.QueryRow("SELECT user_uuid, password, salt FROM user WHERE email = ?", email).Scan(&userId, &savedHash, &salt) if err != nil { - return nil, err + if err == sql.ErrNoRows { + return nil, ErrInvalidUsernameOrPassword + } + utils.LogError("Could not query user on sign in", err) + return nil, ErrInternalServer } new_hash := getHashPassword(password, salt) if subtle.ConstantTimeCompare(new_hash, savedHash) == 0 { - return nil, errors.New("Invalid Request") + return nil, errors.Join(ErrInvalidUsernameOrPassword) } - sessionId, err := createSession(db, userId) + sessionId, err := createSession(a.db, userId) if err != nil { return nil, err } @@ -144,6 +117,56 @@ func GetUserFromSessionId(db *sql.DB, sessionId types.SessionId) *types.User { return types.NewUser(userId, email, sessionId, emailVerified) } } +func SignUp(db *sql.DB, email string, password string) (*types.SessionId, error) { + _, err := mail.ParseAddress(email) + if err != nil { + return nil, errors.Join(errors.New("Invalid email")) + } + + err = checkPassword(password) + if err != nil { + return nil, err + } + + userId, err := uuid.NewRandom() + if err != nil { + utils.LogError("Could not generate UUID for new user", err) + return nil, ErrInternalServer + } + + salt := make([]byte, 16) + _, err = rand.Read(salt) + if err != nil { + utils.LogError("Could not generate salt for new user", err) + return nil, ErrInternalServer + } + + 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") { + return nil, errors.New("Bad Request") + } + + utils.LogError("Could not insert user", err) + return nil, ErrInternalServer + } + + sessionId, err := createSession(db, userId) + if err != nil { + return nil, err + } + + // Send verification email as a goroutine + go SendVerificationEmail(db, userId.String(), email) + return sessionId, nil +} func ValidateEmail(db *sql.DB, token string) error { result, err := db.Exec(` @@ -320,7 +343,7 @@ func ResetPassword(db *sql.DB, email string) (string, error) { return "", errors.New("Please enter an email") } - token, err := utils.RandomToken() + token, err := randomToken() if err != nil { return "", errors.Join(errors.New("Could not generate token"), err) } @@ -357,7 +380,7 @@ func SendVerificationEmail(db *sql.DB, userId string, email string) { } if token == "" { - token, err := utils.RandomToken() + token, err := randomToken() if err != nil { utils.LogError("Could not generate token", err) return @@ -380,20 +403,22 @@ func SendVerificationEmail(db *sql.DB, userId string, email string) { } func createSession(db *sql.DB, user_uuid uuid.UUID) (*types.SessionId, error) { - sessionId, err := utils.RandomToken() + sessionId, err := randomToken() if err != nil { - return nil, errors.Join(errors.New("Could not generate session ID"), err) + return nil, err } // Delete old inactive sessions _, err = db.Exec("DELETE FROM session WHERE created_at < datetime('now','-8 hours') AND user_uuid = ?", user_uuid) if err != nil { utils.LogError("Could not delete old sessions", err) + return nil, ErrInternalServer } _, err = db.Exec("INSERT INTO session (session_id, user_uuid, created_at) VALUES (?, ?, datetime())", sessionId, user_uuid) if err != nil { - return nil, errors.Join(errors.New("Could not insert session"), err) + utils.LogError("Could not insert new session", err) + return nil, ErrInternalServer } sessionIdType := types.SessionId(sessionId) @@ -401,7 +426,7 @@ func createSession(db *sql.DB, user_uuid uuid.UUID) (*types.SessionId, error) { } func tryCreateSessionAndSetCookie(r *http.Request, w http.ResponseWriter, db *sql.DB, user_uuid uuid.UUID) bool { - sessionId, err := utils.RandomToken() + sessionId, err := randomToken() if err != nil { utils.LogError("Could not generate session ID", err) auth.Error("Internal Server Error").Render(r.Context(), w) @@ -467,3 +492,14 @@ func GetUserFromRequest(db *sql.DB, r *http.Request) *types.User { sessionId := getSessionID(r) return GetUserFromSessionId(db, sessionId) } + +func randomToken() (string, error) { + b := make([]byte, 32) + _, err := rand.Read(b) + if err != nil { + utils.LogError("Could not generate random token", err) + return "", ErrInternalServer + } + + return base64.StdEncoding.EncodeToString(b), nil +} 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 cbaee078904332c311f24c9df623acb2291e2476 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Wed, 25 Sep 2024 21:31:57 +0200 Subject: [PATCH 7/7] chore(auth): add service structs and test error handling --- db/auth.go | 29 +++++++++++++ service/auth.go | 107 +++++++++++++++++++++++++----------------------- 2 files changed, 84 insertions(+), 52 deletions(-) create mode 100644 db/auth.go diff --git a/db/auth.go b/db/auth.go new file mode 100644 index 0000000..eb609de --- /dev/null +++ b/db/auth.go @@ -0,0 +1,29 @@ +package db + +import ( + "database/sql" + "errors" + "strings" + + "github.com/google/uuid" +) + +type AuthDb interface { + InsertUser(userIs uuid.UUID, email string, password string) error +} +type AuthDbSqlite struct { + db *sql.DB +} + +func (a *AuthDbSqlite) InsertUser(userId uuid.UUID, email string, passwordHash []byte, salt []byte) error { + + _, err := a.db.Exec("INSERT INTO user (user_uuid, email, email_verified, is_admin, password, salt, created_at) VALUES (?, ?, FALSE, FALSE, ?, ?, datetime())", userId, email, passwordHash, salt) + if err != nil { + if strings.Contains(err.Error(), "email") { + return errors.New("Bad Request") + } + + utils.LogError("Could not insert user", err) + return ErrInternalServer + } +} diff --git a/service/auth.go b/service/auth.go index d8258a5..13b049c 100644 --- a/service/auth.go +++ b/service/auth.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "me-fit/db" "me-fit/template/auth" tempMail "me-fit/template/mail" "me-fit/types" @@ -33,7 +34,8 @@ var ( // NOT TESTED type AuthService struct { - db *sql.DB + db *sql.DB + dbSer *db.AuthDb } func NewAuthService(db *sql.DB) *AuthService { @@ -42,6 +44,58 @@ func NewAuthService(db *sql.DB) *AuthService { } } +func (a *AuthService) SignUp(email string, password string) (*types.SessionId, error) { + _, err := mail.ParseAddress(email) + if err != nil { + return nil, errors.Join(errors.New("Invalid email")) + } + + err = checkPassword(password) + if err != nil { + return nil, err + } + + userId, err := uuid.NewRandom() + if err != nil { + utils.LogError("Could not generate UUID for new user", err) + return nil, ErrInternalServer + } + + salt := make([]byte, 16) + _, err = rand.Read(salt) + if err != nil { + utils.LogError("Could not generate salt for new user", err) + return nil, ErrInternalServer + } + + hash := getHashPassword(password, salt) + + err := a.dbSer.InsertUser(a.db, userId, email, hash, salt) + _, err = a.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") { + return nil, errors.New("Bad Request") + } + + utils.LogError("Could not insert user", err) + return nil, ErrInternalServer + } + + sessionId, err := createSession(a.db, userId) + if err != nil { + return nil, err + } + + // Send verification email as a goroutine + go SendVerificationEmail(a.db, userId.String(), email) + return sessionId, nil +} + func (a *AuthService) SignIn(email string, password string) (*types.SessionId, error) { start := time.Now() @@ -117,57 +171,6 @@ func GetUserFromSessionId(db *sql.DB, sessionId types.SessionId) *types.User { return types.NewUser(userId, email, sessionId, emailVerified) } } -func SignUp(db *sql.DB, email string, password string) (*types.SessionId, error) { - _, err := mail.ParseAddress(email) - if err != nil { - return nil, errors.Join(errors.New("Invalid email")) - } - - err = checkPassword(password) - if err != nil { - return nil, err - } - - userId, err := uuid.NewRandom() - if err != nil { - utils.LogError("Could not generate UUID for new user", err) - return nil, ErrInternalServer - } - - salt := make([]byte, 16) - _, err = rand.Read(salt) - if err != nil { - utils.LogError("Could not generate salt for new user", err) - return nil, ErrInternalServer - } - - 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") { - return nil, errors.New("Bad Request") - } - - utils.LogError("Could not insert user", err) - return nil, ErrInternalServer - } - - sessionId, err := createSession(db, userId) - if err != nil { - return nil, err - } - - // Send verification email as a goroutine - go SendVerificationEmail(db, userId.String(), email) - return sessionId, nil -} - func ValidateEmail(db *sql.DB, token string) error { result, err := db.Exec(` UPDATE user -- 2.49.1