From f83a404206506b8e6a968062da93cd8a06e4c44a Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Thu, 19 Sep 2024 21:33:20 +0200 Subject: [PATCH] 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 {