From aa865af2e9aa971d0a4a92b239ea2b0a280f9d22 Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Thu, 24 Sep 2026 05:11:49 +0200 Subject: [PATCH] refac: reamaining auth surface rewrite, T0 --- api/dependecyInjection.go | 68 ++-- api/handlers/article/articleHandler.go | 43 ++- api/handlers/auth/authHandler.go | 176 +++++++-- api/middleware/auth.go | 38 +- api/pkg/apperr/messages.go | 6 + api/pkg/claims/claims.go | 26 ++ api/router.go | 13 +- features/account_verification.feature | 127 +++++++ features/account_verification.qa.md | 84 +++++ features/cancel_pending_registration.feature | 112 ++++++ features/cancel_pending_registration.qa.md | 76 ++++ features/request_password_reset.feature | 150 ++++++++ features/request_password_reset.qa.md | 111 ++++++ features/reset_password.feature | 183 +++++++++ features/reset_password.qa.md | 108 ++++++ features/verified_account_required.feature | 165 ++++++++ features/verified_account_required.qa.md | 120 ++++++ internal/domain/user/userDomain.go | 26 +- .../domain/user/userRepositoryInterface.go | 6 +- .../persistence/mappers/userMapper.go | 40 +- .../persistence/models/userModel.go | 21 +- .../repositories/userRepository.go | 39 +- .../useCases/CancelRegistration.go | 26 ++ .../useCases/RequestPasswordReset.go | 53 +++ .../infrastructure/useCases/ResetPassword.go | 39 ++ .../infrastructure/useCases/UpdateUser.go | 8 +- .../infrastructure/useCases/VerifyUser.go | 33 ++ test/account_verification_http_test.go | 244 ++++++++++++ test/account_verification_test.go | 81 ++++ test/article_publishing_http_test.go | 6 + test/article_title_rules_test.go | 6 + test/cancel_pending_registration_http_test.go | 230 ++++++++++++ test/cancel_pending_registration_test.go | 51 +++ test/delete_article_cascades_images_test.go | 6 + test/edit_article_body_http_test.go | 6 + test/hasher_test.go | 11 +- test/not_found_errors_test.go | 14 +- test/public_article_listing_test.go | 6 + ...ration_gate_and_verification_email_test.go | 4 + test/request_password_reset_http_test.go | 243 ++++++++++++ test/request_password_reset_test.go | 140 +++++++ test/reset_password_http_test.go | 280 ++++++++++++++ test/reset_password_test.go | 164 ++++++++ test/runtime_settings_injection_http_test.go | 35 +- ...nverified_users_and_reregistration_test.go | 7 +- test/upload_article_image_http_test.go | 5 + test/use_case_baseline_test.go | 4 +- test/user_test.go | 69 +++- test/verified_account_required_http_test.go | 352 ++++++++++++++++++ test/write_not_found_errors_test.go | 11 +- 50 files changed, 3723 insertions(+), 149 deletions(-) create mode 100644 api/pkg/claims/claims.go create mode 100644 features/account_verification.feature create mode 100644 features/account_verification.qa.md create mode 100644 features/cancel_pending_registration.feature create mode 100644 features/cancel_pending_registration.qa.md create mode 100644 features/request_password_reset.feature create mode 100644 features/request_password_reset.qa.md create mode 100644 features/reset_password.feature create mode 100644 features/reset_password.qa.md create mode 100644 features/verified_account_required.feature create mode 100644 features/verified_account_required.qa.md create mode 100644 internal/infrastructure/useCases/CancelRegistration.go create mode 100644 internal/infrastructure/useCases/RequestPasswordReset.go create mode 100644 internal/infrastructure/useCases/ResetPassword.go create mode 100644 internal/infrastructure/useCases/VerifyUser.go create mode 100644 test/account_verification_http_test.go create mode 100644 test/account_verification_test.go create mode 100644 test/cancel_pending_registration_http_test.go create mode 100644 test/cancel_pending_registration_test.go create mode 100644 test/request_password_reset_http_test.go create mode 100644 test/request_password_reset_test.go create mode 100644 test/reset_password_http_test.go create mode 100644 test/reset_password_test.go create mode 100644 test/verified_account_required_http_test.go diff --git a/api/dependecyInjection.go b/api/dependecyInjection.go index 6367f4b..ec5d9bb 100644 --- a/api/dependecyInjection.go +++ b/api/dependecyInjection.go @@ -20,21 +20,25 @@ import ( ) type UseCases struct { - Token *jwtauth.JWTAuth - CreateArticleUseCase *useCases.CreateArticleUseCase - GetArticleUseCase *useCases.GetArticleUseCase - ListArticlesUseCase *useCases.ListArticlesUseCase - UpdateArticleUseCase *useCases.UpdateArticleUseCase - DeleteArticleUseCase *useCases.DeleteArticleUseCase - GetUserUseCase *useCases.GetUserUseCase - CreateUserUseCase *useCases.CreateUserUseCase - UpdateUserUseCase *useCases.UpdateUserUseCase - AuthenticateUserUseCase *useCases.AuthenticateUserUseCase - DeleteUserUseCase *useCases.DeleteUserUseCase - ListUsersUseCase *useCases.ListUsersUseCase - SendMailUseCase *useCases.SendMailUseCase - CreateImageUseCase *useCases.CreateImageUseCase - DeleteImageUseCase *useCases.DeleteImageUseCase + Token *jwtauth.JWTAuth + CreateArticleUseCase *useCases.CreateArticleUseCase + GetArticleUseCase *useCases.GetArticleUseCase + ListArticlesUseCase *useCases.ListArticlesUseCase + UpdateArticleUseCase *useCases.UpdateArticleUseCase + DeleteArticleUseCase *useCases.DeleteArticleUseCase + GetUserUseCase *useCases.GetUserUseCase + CreateUserUseCase *useCases.CreateUserUseCase + UpdateUserUseCase *useCases.UpdateUserUseCase + AuthenticateUserUseCase *useCases.AuthenticateUserUseCase + VerifyUserUseCase *useCases.VerifyUserUseCase + CancelRegistrationUseCase *useCases.CancelRegistrationUseCase + RequestPasswordResetUseCase *useCases.RequestPasswordResetUseCase + ResetPasswordUseCase *useCases.ResetPasswordUseCase + DeleteUserUseCase *useCases.DeleteUserUseCase + ListUsersUseCase *useCases.ListUsersUseCase + SendMailUseCase *useCases.SendMailUseCase + CreateImageUseCase *useCases.CreateImageUseCase + DeleteImageUseCase *useCases.DeleteImageUseCase } type Repositories struct { @@ -73,21 +77,25 @@ func getUseCases(repos *Repositories, settings config.Settings) *UseCases { hasher := hashing.NewBcryptHasher() return &UseCases{ - Token: jwtauth.New("HS256", []byte(settings.JWTSecret), nil), - CreateArticleUseCase: useCases.NewCreateArticleUseCase(repos.ArticleRepo), - GetArticleUseCase: useCases.NewGetArticleUseCase(repos.ArticleRepo), - ListArticlesUseCase: useCases.NewListArticlesUseCase(repos.ArticleRepo), - UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(repos.ArticleRepo), - DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo), - GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), - CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), - UpdateUserUseCase: useCases.NewUpdateUserUseCase(repos.UserRepo, hasher), - AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(repos.UserRepo, hasher), - DeleteUserUseCase: useCases.NewDeleteUserUseCase(repos.UserRepo), - ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), - SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), - CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), - DeleteImageUseCase: useCases.NewDeleteImageUseCase(repos.ImageRepo), + Token: jwtauth.New("HS256", []byte(settings.JWTSecret), nil), + CreateArticleUseCase: useCases.NewCreateArticleUseCase(repos.ArticleRepo), + GetArticleUseCase: useCases.NewGetArticleUseCase(repos.ArticleRepo), + ListArticlesUseCase: useCases.NewListArticlesUseCase(repos.ArticleRepo), + UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(repos.ArticleRepo), + DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo), + GetUserUseCase: useCases.NewGetUserUseCase(repos.UserRepo), + CreateUserUseCase: useCases.NewCreateUserUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), + UpdateUserUseCase: useCases.NewUpdateUserUseCase(repos.UserRepo, hasher), + AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(repos.UserRepo, hasher), + VerifyUserUseCase: useCases.NewVerifyUserUseCase(repos.UserRepo, hasher), + CancelRegistrationUseCase: useCases.NewCancelRegistrationUseCase(repos.UserRepo), + RequestPasswordResetUseCase: useCases.NewRequestPasswordResetUseCase(repos.UserRepo, hasher, repos.MailRepo, settings.PublicURL), + ResetPasswordUseCase: useCases.NewResetPasswordUseCase(repos.UserRepo, hasher), + DeleteUserUseCase: useCases.NewDeleteUserUseCase(repos.UserRepo), + ListUsersUseCase: useCases.NewListUsersUseCase(repos.UserRepo), + SendMailUseCase: useCases.NewSendMailUseCase(repos.MailRepo), + CreateImageUseCase: useCases.NewCreateImageUseCase(repos.ImageRepo), + DeleteImageUseCase: useCases.NewDeleteImageUseCase(repos.ImageRepo), } } diff --git a/api/handlers/article/articleHandler.go b/api/handlers/article/articleHandler.go index dfaf8ff..152a738 100644 --- a/api/handlers/article/articleHandler.go +++ b/api/handlers/article/articleHandler.go @@ -2,6 +2,7 @@ package article import ( "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" validator "RenewCMS/api/pkg/validator" domain "RenewCMS/internal/domain/article" "RenewCMS/internal/infrastructure/useCases" @@ -13,15 +14,15 @@ import ( "strconv" "github.com/go-chi/chi/v5" - "github.com/go-chi/jwtauth/v5" ) type Handler struct { - CreateUseCase *useCases.CreateArticleUseCase - GetUseCase *useCases.GetArticleUseCase - ListUseCase *useCases.ListArticlesUseCase - DeleteUseCase *useCases.DeleteArticleUseCase - UpdateUseCase *useCases.UpdateArticleUseCase + CreateUseCase *useCases.CreateArticleUseCase + GetUseCase *useCases.GetArticleUseCase + ListUseCase *useCases.ListArticlesUseCase + DeleteUseCase *useCases.DeleteArticleUseCase + UpdateUseCase *useCases.UpdateArticleUseCase + GetUserUseCase *useCases.GetUserUseCase } type PostArticle struct { @@ -29,13 +30,14 @@ type PostArticle struct { Body string `json:"body" validate:"required,max=10000"` } -func NewHandler(create *useCases.CreateArticleUseCase, get *useCases.GetArticleUseCase, list *useCases.ListArticlesUseCase, delete *useCases.DeleteArticleUseCase, update *useCases.UpdateArticleUseCase) *Handler { +func NewHandler(create *useCases.CreateArticleUseCase, get *useCases.GetArticleUseCase, list *useCases.ListArticlesUseCase, delete *useCases.DeleteArticleUseCase, update *useCases.UpdateArticleUseCase, getUser *useCases.GetUserUseCase) *Handler { return &Handler{ - CreateUseCase: create, - GetUseCase: get, - ListUseCase: list, - DeleteUseCase: delete, - UpdateUseCase: update, + CreateUseCase: create, + GetUseCase: get, + ListUseCase: list, + DeleteUseCase: delete, + UpdateUseCase: update, + GetUserUseCase: getUser, } } @@ -184,15 +186,24 @@ func (h *Handler) UnpublishArticle(w http.ResponseWriter, r *http.Request) { h.setIsOnline(w, r, false) } -func requestHasValidJwt(r *http.Request) bool { - token, _, err := jwtauth.FromContext(r.Context()) - return token != nil && err == nil +func (h *Handler) callerIsVerified(r *http.Request) bool { + userId, ok := claims.UserId(r) + if !ok { + return false + } + + currentUser, err := h.GetUserUseCase.GetUser(userId) + if err != nil { + return false + } + + return currentUser.IsVerified } func (h *Handler) ListArticles(w http.ResponseWriter, r *http.Request) { var articles []domain.Article var err error - if requestHasValidJwt(r) { + if h.callerIsVerified(r) { articles, err = h.ListUseCase.ListAllArticles() } else { articles, err = h.ListUseCase.ListOnlineArticles() diff --git a/api/handlers/auth/authHandler.go b/api/handlers/auth/authHandler.go index b613107..8a485bc 100644 --- a/api/handlers/auth/authHandler.go +++ b/api/handlers/auth/authHandler.go @@ -2,6 +2,7 @@ package auth import ( "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" validator "RenewCMS/api/pkg/validator" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" @@ -14,12 +15,16 @@ import ( ) type Handler struct { - Token *jwtauth.JWTAuth - GetUseCase *useCases.GetUserUseCase - ListUseCase *useCases.ListUsersUseCase - CreateUseCase *useCases.CreateUserUseCase - AuthenticateUseCase *useCases.AuthenticateUserUseCase - IsProduction bool + Token *jwtauth.JWTAuth + GetUseCase *useCases.GetUserUseCase + ListUseCase *useCases.ListUsersUseCase + CreateUseCase *useCases.CreateUserUseCase + AuthenticateUseCase *useCases.AuthenticateUserUseCase + VerifyUseCase *useCases.VerifyUserUseCase + CancelUseCase *useCases.CancelRegistrationUseCase + RequestPasswordResetUseCase *useCases.RequestPasswordResetUseCase + ResetPasswordUseCase *useCases.ResetPasswordUseCase + IsProduction bool } type RegisterCredentials struct { @@ -33,6 +38,20 @@ type LoginCredentials struct { Password string `json:"password" validate:"required,min=8"` } +type VerifyCredentials struct { + Code string `json:"code" validate:"required"` +} + +type RequestPasswordResetCredentials struct { + Email string `json:"email" validate:"required,email"` +} + +type ResetPasswordCredentials struct { + Email string `json:"email" validate:"required,email"` + Code string `json:"code" validate:"required"` + Password string `json:"password" validate:"required,min=8"` +} + func (h *Handler) removeJwtCookie(w http.ResponseWriter) { http.SetCookie(w, &http.Cookie{ Name: "jwt", @@ -45,14 +64,18 @@ func (h *Handler) removeJwtCookie(w http.ResponseWriter) { }) } -func NewHandler(token *jwtauth.JWTAuth, get *useCases.GetUserUseCase, list *useCases.ListUsersUseCase, create *useCases.CreateUserUseCase, authenticate *useCases.AuthenticateUserUseCase, isProduction bool) *Handler { +func NewHandler(token *jwtauth.JWTAuth, get *useCases.GetUserUseCase, list *useCases.ListUsersUseCase, create *useCases.CreateUserUseCase, authenticate *useCases.AuthenticateUserUseCase, verify *useCases.VerifyUserUseCase, cancel *useCases.CancelRegistrationUseCase, requestPasswordReset *useCases.RequestPasswordResetUseCase, resetPassword *useCases.ResetPasswordUseCase, isProduction bool) *Handler { return &Handler{ - Token: token, - GetUseCase: get, - ListUseCase: list, - CreateUseCase: create, - AuthenticateUseCase: authenticate, - IsProduction: isProduction, + Token: token, + GetUseCase: get, + ListUseCase: list, + CreateUseCase: create, + AuthenticateUseCase: authenticate, + VerifyUseCase: verify, + CancelUseCase: cancel, + RequestPasswordResetUseCase: requestPasswordReset, + ResetPasswordUseCase: resetPassword, + IsProduction: isProduction, } } @@ -87,21 +110,6 @@ func (h *Handler) IsLoggedIn(r *http.Request) bool { return token != nil && err == nil } -func (h *Handler) IsVerified(r *http.Request) bool { - token, err := jwtauth.VerifyRequest( - h.Token, - r, - jwtauth.TokenFromCookie, - jwtauth.TokenFromHeader, - jwtauth.TokenFromQuery) - if err != nil { - return false - } - userId := token.PrivateClaims()["user_id"].(float64) - currentUser, _ := h.GetUseCase.GetUser(uint32(userId)) - return currentUser.IsVerified -} - func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { var credentials LoginCredentials @@ -163,6 +171,118 @@ func (h *Handler) Register(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(message) } +func (h *Handler) Verify(w http.ResponseWriter, r *http.Request) { + var credentials VerifyCredentials + err := json.NewDecoder(r.Body).Decode(&credentials) + if err != nil { + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return + } + + err = validator.Validate.Struct(credentials) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + userId, ok := claims.UserId(r) + if !ok { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + + verifiedUser, err := h.VerifyUseCase.Verify(userId, credentials.Code) + if err != nil { + if errors.Is(err, user.ErrUserNotFound) { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + if errors.Is(err, user.ErrInvalidVerificationCode) || errors.Is(err, user.ErrVerificationCodeExpired) { + http.Error(w, apperr.VerificationFailedErrorMessage, http.StatusForbidden) + return + } + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + body, _ := json.Marshal(verifiedUser) + _, _ = w.Write(body) +} + +func (h *Handler) CancelRegistration(w http.ResponseWriter, r *http.Request) { + userId, ok := claims.UserId(r) + if !ok { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + + err := h.CancelUseCase.CancelRegistration(userId) + if err != nil { + if errors.Is(err, user.ErrUserNotFound) { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + if errors.Is(err, user.ErrAlreadyVerified) { + http.Error(w, apperr.AlreadyVerifiedErrorMessage, http.StatusForbidden) + return + } + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + h.removeJwtCookie(w) + message, _ := json.Marshal(map[string]any{"message": "Registration cancelled! Account deleted and HTTPonly jwt cookie cleared"}) + _, _ = w.Write(message) +} + +func (h *Handler) RequestPasswordReset(w http.ResponseWriter, r *http.Request) { + var credentials RequestPasswordResetCredentials + err := json.NewDecoder(r.Body).Decode(&credentials) + if err != nil { + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return + } + + err = validator.Validate.Struct(credentials) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + _ = h.RequestPasswordResetUseCase.RequestPasswordReset(credentials.Email) + + message, _ := json.Marshal(map[string]any{"message": apperr.PasswordResetRequestedMessage}) + _, _ = w.Write(message) +} + +func (h *Handler) ResetPassword(w http.ResponseWriter, r *http.Request) { + var credentials ResetPasswordCredentials + err := json.NewDecoder(r.Body).Decode(&credentials) + if err != nil { + http.Error(w, apperr.BodyErrorMessage, http.StatusBadRequest) + return + } + + err = validator.Validate.Struct(credentials) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + err = h.ResetPasswordUseCase.ResetPassword(credentials.Email, credentials.Code, credentials.Password) + if err != nil { + if errors.Is(err, user.ErrInvalidPasswordResetCode) || errors.Is(err, user.ErrPasswordResetCodeExpired) { + http.Error(w, apperr.PasswordResetFailedErrorMessage, http.StatusForbidden) + return + } + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + message, _ := json.Marshal(map[string]any{"message": "Your password has been reset. You can now log in."}) + _, _ = w.Write(message) +} + func (h *Handler) Logout(w http.ResponseWriter, _ *http.Request) { h.removeJwtCookie(w) message, _ := json.Marshal(map[string]any{"message": "User logged out! HTTPonly jwt cookie deleted"}) diff --git a/api/middleware/auth.go b/api/middleware/auth.go index facf2d7..26b0e6e 100644 --- a/api/middleware/auth.go +++ b/api/middleware/auth.go @@ -1,6 +1,13 @@ package middleware -import "net/http" +import ( + "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/claims" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "net/http" +) func JsonContentTypeMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -8,3 +15,32 @@ func JsonContentTypeMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +func RequireVerifiedUser(getUser *useCases.GetUserUseCase) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + userId, ok := claims.UserId(r) + if !ok { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + + currentUser, err := getUser.GetUser(userId) + if err != nil { + if errors.Is(err, user.ErrUserNotFound) { + http.Error(w, apperr.UnauthorizedErrorMessage, http.StatusUnauthorized) + return + } + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + if !currentUser.IsVerified { + http.Error(w, apperr.VerificationRequiredErrorMessage, http.StatusForbidden) + return + } + + next.ServeHTTP(w, r) + }) + } +} diff --git a/api/pkg/apperr/messages.go b/api/pkg/apperr/messages.go index cf12b27..068551d 100644 --- a/api/pkg/apperr/messages.go +++ b/api/pkg/apperr/messages.go @@ -7,3 +7,9 @@ const ArticleNotFoundErrorMessage = "Article not found" const ArticleTitleErrorMessage = "Article title must be between 3 and 200 characters." const RegistrationClosedErrorMessage = "You are not allowed to create a user. Log in or reset database." const RegistrationFailedErrorMessage = "Registration failed. Please try again later." +const UnauthorizedErrorMessage = "You must be logged in to access this resource." +const VerificationFailedErrorMessage = "The verification code is invalid or has expired." +const AlreadyVerifiedErrorMessage = "This account is already verified and cannot be cancelled this way." +const VerificationRequiredErrorMessage = "You must verify your account to access this resource." +const PasswordResetRequestedMessage = "If an account exists for that email, a password reset link has been sent." +const PasswordResetFailedErrorMessage = "The password reset code is invalid or has expired." diff --git a/api/pkg/claims/claims.go b/api/pkg/claims/claims.go new file mode 100644 index 0000000..cd15d9f --- /dev/null +++ b/api/pkg/claims/claims.go @@ -0,0 +1,26 @@ +package claims + +import ( + "net/http" + + "github.com/go-chi/jwtauth/v5" +) + +func UserId(r *http.Request) (uint32, bool) { + _, claims, err := jwtauth.FromContext(r.Context()) + if err != nil { + return 0, false + } + + rawUserId, ok := claims["user_id"] + if !ok { + return 0, false + } + + userIdFloat, ok := rawUserId.(float64) + if !ok { + return 0, false + } + + return uint32(userIdFloat), true +} diff --git a/api/router.go b/api/router.go index de9cce4..77f97ba 100644 --- a/api/router.go +++ b/api/router.go @@ -31,7 +31,7 @@ func GetHelloWorld(w http.ResponseWriter, _ *http.Request) { func NewArticleRouter(container *UseCases) http.Handler { r := chi.NewRouter() - articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase, container.UpdateArticleUseCase) + articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase, container.UpdateArticleUseCase, container.GetUserUseCase) imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase, container.UpdateArticleUseCase) r.Group(func(r chi.Router) { @@ -39,6 +39,7 @@ func NewArticleRouter(container *UseCases) http.Handler { }) r.Group(func(r chi.Router) { r.Use(jwtauth.Authenticator(container.Token)) + r.Use(middleware.RequireVerifiedUser(container.GetUserUseCase)) r.Get("/{id}", articleHandler.GetArticle) r.Post("/", articleHandler.PostArticle) r.Delete("/{id}", articleHandler.DeleteArticle) @@ -53,11 +54,19 @@ func NewArticleRouter(container *UseCases) http.Handler { func NewAuthRouter(container *UseCases, isProduction bool) http.Handler { r := chi.NewRouter() - authHandler := auth.NewHandler(container.Token, container.GetUserUseCase, container.ListUsersUseCase, container.CreateUserUseCase, container.AuthenticateUserUseCase, isProduction) + authHandler := auth.NewHandler(container.Token, container.GetUserUseCase, container.ListUsersUseCase, container.CreateUserUseCase, container.AuthenticateUserUseCase, container.VerifyUserUseCase, container.CancelRegistrationUseCase, container.RequestPasswordResetUseCase, container.ResetPasswordUseCase, isProduction) r.Post("/register", authHandler.Register) r.Post("/login", authHandler.Login) r.Post("/logout", authHandler.Logout) + r.Post("/password-reset", authHandler.RequestPasswordReset) + r.Post("/password-reset/confirm", authHandler.ResetPassword) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(container.Token)) + r.Use(jwtauth.Authenticator(container.Token)) + r.Post("/verify", authHandler.Verify) + r.Delete("/register", authHandler.CancelRegistration) + }) return r } diff --git a/features/account_verification.feature b/features/account_verification.feature new file mode 100644 index 0000000..9eb7c9f --- /dev/null +++ b/features/account_verification.feature @@ -0,0 +1,127 @@ +Feature: A logged-in user verifies their account with the emailed code + + As the maintainer of RenewCMS + I want a logged-in user to confirm ownership of their account by submitting the verification + code that CreateUser already emailed them, have a wrong or expired code refused without + changing anything, have the stored code cleared the moment it is successfully used so it + cannot be replayed, and have a token that carries no usable user_id answered with 401 instead + of a panic + So that account verification is a real security check, not a formality, and a malformed or + hand-crafted token can never crash the server + + Background: + Given CreateUserUseCase.CreateUser already stores a bcrypt hash of a random verification code + on the new user, alongside VerificationExpiration set to two hours after creation, and emails + the raw code to the user (features/registration_gate_and_verification_email.feature) + And this slice adds POST /v1/auth/verify, mounted in api/router.go inside a group that applies + jwtauth.Verifier and jwtauth.Authenticator to the auth router, so a request with no jwt + cookie, an expired jwt, or a jwt with a bad signature never reaches the handler and is + answered 401 by jwtauth.Authenticator itself, as already happens for /v1/article routes + And the request body is decoded into a VerifyCredentials struct with one field, Code, validated + required by validator.Validate.Struct before anything else + And a new use case, VerifyUserUseCase (internal/infrastructure/useCases/VerifyUser.go), is + injected into auth.Handler as VerifyUseCase; it depends only on user.Repository and + hash.Hasher and exposes Verify(userId uint32, code string) (user.User, error) + And VerifyUserUseCase.Verify loads the user by id, refuses with the new sentinel + user.ErrInvalidVerificationCode when the stored VerificationCode is empty (already used, or + never set) or hash.Hasher.Compare finds the submitted code does not match the stored hash, + refuses with the new sentinel user.ErrVerificationCodeExpired when + time.Now() is after the stored VerificationExpiration, and otherwise calls a new + user.Repository method, ClearVerificationCode(userId uint32) (user.User, error), that sets + IsVerified true and VerificationCode "" (stored as SQL NULL, the same convention as + PasswordResetCode in features/unverified_users_and_reregistration.feature) in one write + And today, auth.Handler.IsVerified reads token.PrivateClaims()["user_id"].(float64) with a bare + type assertion that panics if the claim is absent or not a number; this slice does not touch + IsVerified, but the new Verify handler extracts user_id from the already-verified token's + claims safely, and answers 401 with a new apperr.UnauthorizedErrorMessage constant whenever + the claim is missing, is not a number, or does not match any user in the repository, instead + of panicking or trusting the value + And a wrong or expired code is mapped to 403 with a new apperr.VerificationFailedErrorMessage + constant, the same status family Login already uses for a rejected credential + And on success the handler responds 200 with the updated user as JSON (is_verified true, + verification_code "") + + Scenario: A logged-in user verifies their account with the correct emailed code + Given a user repository containing user id 7, username "alice", is_verified false, whose stored + VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour + from now + And a valid jwt cookie for user id 7 + When the client sends POST /v1/auth/verify with body {"code": "abc123"} + Then the response status is 200 + And the response body is the user as JSON with is_verified true and verification_code "" + And the user repository's record for id 7 now has is_verified true and an empty + verification_code + + Scenario: A wrong code is refused and nothing changes + Given a user repository containing user id 7, username "alice", is_verified false, whose stored + VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour + from now + And a valid jwt cookie for user id 7 + When the client sends POST /v1/auth/verify with body {"code": "wrong-code"} + Then the response status is 403 + And the response body is "The verification code is invalid or has expired." + And the user repository's record for id 7 still has is_verified false and the same stored + verification_code + + Scenario: An expired code is refused and nothing changes + Given a user repository containing user id 7, username "alice", is_verified false, whose stored + VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour + in the past + And a valid jwt cookie for user id 7 + When the client sends POST /v1/auth/verify with body {"code": "abc123"} + Then the response status is 403 + And the response body is "The verification code is invalid or has expired." + And the user repository's record for id 7 still has is_verified false and the same stored + verification_code + + Scenario: The code is cleared once used, so it cannot be replayed + Given a user repository containing user id 7, username "alice", is_verified false, whose stored + VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour + from now + And a valid jwt cookie for user id 7 + When the client sends POST /v1/auth/verify with body {"code": "abc123"} + Then the response status is 200 + When the client sends POST /v1/auth/verify again with body {"code": "abc123"} + Then the response status is 403 + And the response body is "The verification code is invalid or has expired." + + Scenario Outline: A token without a valid user_id returns 401 instead of crashing + Given a user repository containing user id 7, username "alice", is_verified false, whose stored + VerificationCode is the bcrypt hash of "abc123" and whose VerificationExpiration is one hour + from now + And + When the client sends POST /v1/auth/verify with body {"code": "abc123"} + Then the response status is 401 + And the response body is the new apperr.UnauthorizedErrorMessage text + And the handler does not panic + And the user repository's record for id 7 still has is_verified false + + Examples: + | token description | + | a validly signed jwt cookie whose claims carry no "user_id" key at all | + | a validly signed jwt cookie whose "user_id" claim is the string "seven" | + | a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user | + + Scenario: The use case itself enforces the rule, independently of any handler + Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt + hash of "right-code" and VerificationExpiration one hour from now + And a VerifyUserUseCase built directly over that fake and a hasher + When Verify is called with userId 3 and code "wrong-code" + Then the call returns user.ErrInvalidVerificationCode, asserted with errors.Is + And the fake repository's ClearVerificationCode method was never invoked + + Scenario: The use case rejects an expired code independently of any handler + Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt + hash of "right-code" and VerificationExpiration one hour in the past + And a VerifyUserUseCase built directly over that fake and a hasher + When Verify is called with userId 3 and code "right-code" + Then the call returns user.ErrVerificationCodeExpired, asserted with errors.Is + And the fake repository's ClearVerificationCode method was never invoked + + Scenario: The use case succeeds and clears the code independently of any handler + Given a user.Repository fake containing user id 3 with VerificationCode equal to the bcrypt + hash of "right-code" and VerificationExpiration one hour from now + And a VerifyUserUseCase built directly over that fake and a hasher + When Verify is called with userId 3 and code "right-code" + Then the call returns a user with IsVerified true and VerificationCode "" + And the fake repository's ClearVerificationCode method was invoked exactly once with userId 3 diff --git a/features/account_verification.qa.md b/features/account_verification.qa.md new file mode 100644 index 0000000..d0c646e --- /dev/null +++ b/features/account_verification.qa.md @@ -0,0 +1,84 @@ +# QA procedure: account_verification + +Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`). + +## Test infrastructure additions needed first + +1. Add a `ClearVerificationCode(userId uint32) (user.User, error)` method to `mockUserRepo` + mirroring the existing `UpdateVerificationStatus`/`UpdatePasswordResetCode` pattern: honor + `m.fail.trigger("ClearVerificationCode")`, look the user up (return `user.ErrUserNotFound` if + absent), set `IsVerified = true` and `VerificationCode = ""`, store it back, return it. Track a + call counter/last-argument on the mock (or reuse the existing fail-switch style) so scenarios can + assert it was or wasn't invoked. +2. `stubHasher.Compare(hashed, value)` already returns `nil` only when `hashed == "hashed:"+value` + and an error otherwise, so seed `VerificationCode` in fixtures as `"hashed:abc123"` to represent + "the bcrypt hash of abc123", and submit `"abc123"` as the matching code. + +## Setup shared by every case + +3. Call `validator.InitValidator()` once. +4. Use-case-level checks build `useCases.NewVerifyUserUseCase(repo, &stubHasher{})` where `repo` is + a `mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`. +5. HTTP-level checks wire `mockUserRepo`, `useCases.NewVerifyUserUseCase(repo, &stubHasher{})` and + the other existing use cases into `auth.NewHandler(...)` (extended with the new `VerifyUseCase` + parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. Mount + `POST /v1/auth/verify` on a fresh `chi.NewRouter()` wrapped in `jwtauth.Verifier(token)` then + `jwtauth.Authenticator(token)`, mirroring the protected group `api/router.go` adds inside + `NewAuthRouter`. No live server. Mint jwt cookies with `token.Encode(map[string]any{"user_id": + uint32(7)})` for the valid-token cases, following the convention in + `features/article_publishing.qa.md`. + +## Use-case-level checks (no HTTP) + +6. Repo has user id 3 with `VerificationCode: "hashed:right-code"`, + `VerificationExpiration: time.Now().Add(time.Hour)`. Call `Verify(3, "wrong-code")`. Assert + `errors.Is(err, user.ErrInvalidVerificationCode)`. Assert `ClearVerificationCode` was never + called on the repo. +7. Same repo, `VerificationExpiration: time.Now().Add(-time.Hour)`. Call `Verify(3, "right-code")`. + Assert `errors.Is(err, user.ErrVerificationCodeExpired)`. Assert `ClearVerificationCode` was + never called. +8. Same repo, `VerificationExpiration: time.Now().Add(time.Hour)`. Call `Verify(3, "right-code")`. + Assert `err == nil`, `result.IsVerified == true`, `result.VerificationCode == ""`. Assert the + repo's stored record for id 3 now has `IsVerified == true` and `VerificationCode == ""`. +9. Repo has user id 3 with `VerificationCode: ""` (already verified/used). Call + `Verify(3, "anything")`. Assert `errors.Is(err, user.ErrInvalidVerificationCode)`. + +## HTTP-level checks (fakes, per the shared setup above) + +10. Seed `mockUserRepo` with user id 7, username "alice", `IsVerified: false`, + `VerificationCode: "hashed:abc123"`, `VerificationExpiration: time.Now().Add(time.Hour)`. Mint a + valid jwt cookie for user id 7. `POST /v1/auth/verify` with `{"code":"abc123"}` → assert status + `200`, response body decodes to a user JSON with `is_verified: true` and + `verification_code: ""`, and the repo's record for id 7 now has `IsVerified == true` and + `VerificationCode == ""`. +11. Same seed, wrong code `{"code":"wrong-code"}` → assert status `403`, body exactly + `"The verification code is invalid or has expired."`, and the repo's record for id 7 is + unchanged (`IsVerified == false`, `VerificationCode == "hashed:abc123"`). +12. Same seed but `VerificationExpiration: time.Now().Add(-time.Hour)`, correct code + `{"code":"abc123"}` → assert status `403`, same body text as above, and the repo's record for + id 7 unchanged. +13. Replay: repeat case 10's successful call, then immediately `POST /v1/auth/verify` again with + the same body `{"code":"abc123"}` on the same router/repo → assert the second response status + is `403` with the same body text (the cleared code no longer matches). +14. Table-driven over three "no valid user_id" cases, each against the case-10 seed and asserting + status `401` and a response body equal to the new `apperr.UnauthorizedErrorMessage` constant, + without the handler panicking (a panic inside `httptest`/chi would fail the test run directly, + so simply completing with the asserted status is sufficient proof): + - mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all); + - mint the jwt with `token.Encode(map[string]any{"user_id": "seven"})` (string, not a number); + - mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no + such user in the repo). + After each sub-case, assert the repo's record for id 7 is still `IsVerified == false`. +15. A request with no `jwt` cookie at all against the same router → assert status `401` (produced + by `jwtauth.Authenticator` itself, before the handler runs); do not assert an exact body for + this one case since it is the library's own default text, not `apperr.UnauthorizedErrorMessage`. + +## Manual verification (not exercised by the Go test) + +- The real SMTP-delivered verification link and the SPA's `/register/validate?c=...` page actually + calling `POST /v1/auth/verify` with the code parsed from the URL, and redirecting or showing a + message on success/failure, are out of scope for this slice's automated test and should be + checked by hand against a running dev build. +- The pre-existing bare type assertion in `auth.Handler.IsVerified` + (`token.PrivateClaims()["user_id"].(float64)`) is untouched by this slice and remains a latent + panic if that helper is ever wired into a route; it is not exercised here. diff --git a/features/cancel_pending_registration.feature b/features/cancel_pending_registration.feature new file mode 100644 index 0000000..dccf69d --- /dev/null +++ b/features/cancel_pending_registration.feature @@ -0,0 +1,112 @@ +Feature: A logged-in, unverified user cancels their pending registration + + As the maintainer of RenewCMS + I want a logged-in user who has not yet verified their account to be able to delete that + account and clear their session in one call, while a user who is already verified cannot use + this route to delete their own account, and a token that carries no usable user_id is answered + with 401 instead of a panic + So that someone who registered by mistake, or changed their mind before verifying, can free their + username and email immediately (features/unverified_users_and_reregistration.feature), without + exposing a general self-delete for verified accounts + + Background: + Given this slice adds DELETE /v1/auth/register, mounted in api/router.go inside the same + protected group NewAuthRouter already applies to POST /v1/auth/verify: jwtauth.Verifier then + jwtauth.Authenticator, so a request with no jwt cookie, an expired jwt, or a jwt with a bad + signature never reaches the handler and is answered 401 by jwtauth.Authenticator itself + And the handler extracts user_id from the already-verified token's claims using the same safe + pattern auth.Handler.Verify already established (features/account_verification.feature): + 401 with apperr.UnauthorizedErrorMessage whenever the "user_id" claim is missing, is not a + number, or does not match any user in the repository, never a panic and never trusting the + raw claim value + And a new use case, CancelRegistrationUseCase (internal/infrastructure/useCases/CancelRegistration.go), + is injected into auth.Handler as CancelRegistrationUseCase; it depends only on user.Repository + and exposes CancelRegistration(userId uint32) error + And CancelRegistrationUseCase.CancelRegistration loads the user by id, returns the existing + sentinel user.ErrUserNotFound when no such user exists, refuses with a new sentinel + user.ErrAlreadyVerified when the loaded user's IsVerified is already true, and otherwise calls + user.Repository.Delete(userId), whose GORM implementation removes the row entirely rather than + soft-deleting it (features/unverified_users_and_reregistration.feature), so the same username + and email are immediately free for a new registration + And the handler maps user.ErrUserNotFound to 401 with apperr.UnauthorizedErrorMessage, the same + status and body Verify already uses for an unresolvable user_id, and maps + user.ErrAlreadyVerified to 403 with a new apperr.AlreadyVerifiedErrorMessage constant + And on success the handler clears the "jwt" cookie exactly the way auth.Handler.Logout already + does (Max-Age -1, empty value, Path "/") and responds 200 with {"message": "..."} + And on any error path the handler returns before touching the "jwt" cookie, so an unresolved or + already-verified request leaves the caller's existing session untouched + + Scenario: An unverified user cancels their pending registration + Given a user repository containing user id 7, username "alice", email "alice@example.com", + is_verified false + And a valid jwt cookie for user id 7 + When the client sends DELETE /v1/auth/register with that cookie + Then the response status is 200 + And the response body is {"message": "..."} + And the response clears the "jwt" cookie + And the user repository no longer contains user id 7; Get(7) returns user.ErrUserNotFound + + Scenario: Cancelling frees the username and email for a new registration + Given a user repository containing user id 7, username "alice", email "alice@example.com", + is_verified false + And a valid jwt cookie for user id 7 + When the client sends DELETE /v1/auth/register with that cookie + Then the response status is 200 + When the client sends POST /v1/auth/register with username "alice", password + "hunter2example12", email "alice@example.com" + Then the response status is 200 + And a jwt cookie is set + And the newly created user's id is different from 7 + + Scenario: An already-verified user cannot cancel their account this way + Given a user repository containing user id 7, username "alice", email "alice@example.com", + is_verified true + And a valid jwt cookie for user id 7 + When the client sends DELETE /v1/auth/register with that cookie + Then the response status is 403 + And the response body is the new apperr.AlreadyVerifiedErrorMessage text + And the response does not clear the "jwt" cookie + And the user repository's record for id 7 is unchanged: is_verified true, the row still exists + + Scenario Outline: A token without a valid user_id returns 401 instead of crashing + Given a user repository containing user id 7, username "alice", is_verified false + And + When the client sends DELETE /v1/auth/register with that cookie + Then the response status is 401 + And the response body is the apperr.UnauthorizedErrorMessage text + And the handler does not panic + And the response does not clear the "jwt" cookie + And the user repository's record for id 7 still exists + + Examples: + | token description | + | a validly signed jwt cookie whose claims carry no "user_id" key at all | + | a validly signed jwt cookie whose "user_id" claim is the string "seven" | + | a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user | + + Scenario: A request with no jwt cookie never reaches the handler + Given a user repository containing user id 7, username "alice", is_verified false + And no jwt cookie is sent + When the client sends DELETE /v1/auth/register without a cookie + Then the response status is 401 + And the user repository's record for id 7 still exists + + Scenario: The use case deletes an unverified user, independently of any handler + Given a user.Repository fake containing user id 3, is_verified false + And a CancelRegistrationUseCase built directly over that fake + When CancelRegistration is called with userId 3 + Then the call returns nil + And the fake repository no longer contains user id 3 + + Scenario: The use case refuses to delete an already-verified user, independently of any handler + Given a user.Repository fake containing user id 3, is_verified true + And a CancelRegistrationUseCase built directly over that fake + When CancelRegistration is called with userId 3 + Then the call returns user.ErrAlreadyVerified, asserted with errors.Is + And the fake repository still contains user id 3 + + Scenario: The use case reports an unknown user, independently of any handler + Given a user.Repository fake containing no user with id 42 + And a CancelRegistrationUseCase built directly over that fake + When CancelRegistration is called with userId 42 + Then the call returns user.ErrUserNotFound, asserted with errors.Is diff --git a/features/cancel_pending_registration.qa.md b/features/cancel_pending_registration.qa.md new file mode 100644 index 0000000..d3cb9e3 --- /dev/null +++ b/features/cancel_pending_registration.qa.md @@ -0,0 +1,76 @@ +# QA procedure: cancel_pending_registration + +Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`) and following the +router/handler wiring pattern already used in `test/account_verification_http_test.go`. + +## Test infrastructure reused, none new needed + +`mockUserRepo` already supports deleting an entry (used by the existing `DeleteUserUseCase` tests +in `test/use_case_baseline_test.go`) and returning `user.ErrUserNotFound` from `Get` once a user is +gone. No new mock methods are required; `CancelRegistrationUseCase` only calls `Get` and `Delete`, +both of which `mockUserRepo` already implements. + +## Setup shared by every case + +1. Call `validator.InitValidator()` once. +2. Use-case-level checks build `useCases.NewCancelRegistrationUseCase(repo)` where `repo` is a + `mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`. +3. HTTP-level checks wire `mockUserRepo`, `useCases.NewCancelRegistrationUseCase(repo)` and the + other existing use cases into `auth.NewHandler(...)` (extended with the new + `CancelRegistrationUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), + nil)`. Mount `DELETE /v1/auth/register` on a fresh `chi.NewRouter()` wrapped in + `jwtauth.Verifier(token)` then `jwtauth.Authenticator(token)`, mirroring the protected group + `api/router.go` adds inside `NewAuthRouter` (the same group `POST /v1/auth/verify` already sits + in). Mint jwt cookies with `token.Encode(map[string]any{"user_id": uint32(7)})` for the + valid-token cases. + +## Use-case-level checks (no HTTP) + +4. Repo has user id 3, `IsVerified: false`. Call `CancelRegistration(3)`. Assert `err == nil`. + Assert `repo.users` no longer has key 3 (or `repo.Get(3)` returns `user.ErrUserNotFound`). +5. Repo has user id 3, `IsVerified: true`. Call `CancelRegistration(3)`. Assert + `errors.Is(err, user.ErrAlreadyVerified)`. Assert the repo still has user id 3 with + `IsVerified: true`. +6. Repo has no user with id 42. Call `CancelRegistration(42)`. Assert + `errors.Is(err, user.ErrUserNotFound)`. + +## HTTP-level checks (fakes, per the shared setup above) + +7. Seed `mockUserRepo` with user id 7, username "alice", email "alice@example.com", + `IsVerified: false`. Mint a valid jwt cookie for user id 7. `DELETE /v1/auth/register` with that + cookie → assert status `200`, response body decodes to a JSON object with a `message` field, + the response has a `Set-Cookie` header for `jwt` with `MaxAge < 0` (or an `Expires` in the past) + and an empty `Value` (parse with `http.ParseSetCookie`, mirroring + `secureAttributeFromResponse` in `test/runtime_settings_injection_http_test.go`), and + `repo.Get(7)` now returns `user.ErrUserNotFound`. +8. Re-registration after cancel: repeat case 7's cancel call on the same router/repo, then + `POST /v1/auth/register` with `{"username":"alice","password":"hunter2example12","email":"alice@example.com"}` + → assert status `200`, a `jwt` `Set-Cookie` header is present with a non-empty value, and the + newly created user's id (read back via `repo.GetByUsername("alice")` or the use case) is + different from `7`. +9. Seed `mockUserRepo` with user id 7, username "alice", email "alice@example.com", + `IsVerified: true`. Mint a valid jwt cookie for user id 7. `DELETE /v1/auth/register` with that + cookie → assert status `403`, body exactly the `apperr.AlreadyVerifiedErrorMessage` constant, + no `Set-Cookie` header for `jwt` is present in the response, and `repo.Get(7)` still returns the + user with `IsVerified: true`. +10. Table-driven over three "no valid user_id" cases, each against a fresh seed of user id 7, + `IsVerified: false`, and asserting status `401`, a response body equal to + `apperr.UnauthorizedErrorMessage`, no `jwt` `Set-Cookie` header in the response, and + `repo.Get(7)` still succeeding afterward: + - mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all); + - mint the jwt with `token.Encode(map[string]any{"user_id": "seven"})` (string, not a number); + - mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no + such user in the repo). +11. A request with no `jwt` cookie at all against the same router → assert status `401` (produced + by `jwtauth.Authenticator` itself, before the handler runs); do not assert an exact body for + this case, and assert `repo.Get(7)` still succeeds afterward. + +## Manual verification (not exercised by the Go test) + +- The SPA-visible "cancel registration" affordance (wherever it is surfaced to a logged-in, + unverified user, e.g. on the pending-verification screen) actually calling + `DELETE /v1/auth/register` and redirecting to a logged-out state on success is out of scope for + this slice's automated test and should be checked by hand against a running dev build. +- End-to-end confirmation against the real SQLite file that the row is physically gone (not just + hidden by a soft-delete) is already covered at the repository level by + `features/unverified_users_and_reregistration.feature` and is not re-verified here. diff --git a/features/request_password_reset.feature b/features/request_password_reset.feature new file mode 100644 index 0000000..a6f2f6d --- /dev/null +++ b/features/request_password_reset.feature @@ -0,0 +1,150 @@ +Feature: Anyone can request a password reset, and the response never reveals whether the email exists + + As the maintainer of RenewCMS + I want POST /v1/auth/password-reset to always answer with the same status and body regardless of + whether the submitted email belongs to an account, and, when it does, to email that account a + reset code that expires one hour after it was generated + So that the endpoint cannot be used to discover which email addresses are registered, and a + legitimate reset link always uses the server's own configured public URL + + Background: + Given POST /v1/auth/password-reset is public and mounted alongside /register, /login and + /logout in NewAuthRouter, outside the jwt-protected group + And the request body is decoded into a RequestPasswordResetCredentials struct with one field, + Email, validated required,email by validator.Validate.Struct before anything else + And a decode failure or a validation failure (missing or malformed email) is answered exactly + like every other auth route: apperr.BodyErrorMessage with 400 for a decode failure, or the + validator's error text with 400 for a validation failure; only the "does this email exist" + question is hidden, not the shape of the request itself + And this slice adds a new use case, RequestPasswordResetUseCase + (internal/infrastructure/useCases/RequestPasswordReset.go), injected into auth.Handler as + RequestPasswordResetUseCase; its constructor takes user.Repository, hash.Hasher, mail.Repository + and the configured public URL string, the same shape as CreateUserUseCase, with the public URL + coming from config.Settings.PublicURL through api/dependecyInjection.go + And RequestPasswordReset(email string) error looks the user up with user.Repository.GetByEmail; + when no user has that email, or GetByEmail returns any other error, it does nothing further and + returns nil + And when a user is found, RequestPasswordReset generates a random raw code with uuid.NewString() + (the same generator CreateUser uses for the verification code), hashes it with hash.Hasher.Hash, + and stores the hash together with a fresh expiration of time.Now().Add(1 * time.Hour) by calling + user.Repository.UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) + (user.User, error) — this slice adds the expiration parameter to that existing repository + method and to UpdateUserUseCase.UpdatePasswordResetCode, which gains the same parameter and + passes it straight through, and adds a new PasswordResetExpiration time.Time field to + user.User, to internal/infrastructure/persistence/models.User (a plain, non-nullable column, + the same shape as VerificationExpiration) and to the UserToDomain/UserToModel mappers; no new + entry is needed in api/dependecyInjection.go's AutoMigrate call because models.User is already + migrated there + And storing a new code and expiration always overwrites whatever was stored before, so only the + most recently requested code and expiration are ever valid; a user who requests a reset twice + can only use the second code, and the first one no longer matches once overwritten + And after storing succeeds, RequestPasswordReset calls mail.Repository.Send with the user's + email, template "passwordReset" (the already-existing + internal/infrastructure/mailer/templates/passwordReset.html, which renders a link from + "{{.Host}}/register/reset/validate?c={{.VerificationCode}}&email={{.Email}}"), and a + MailPasswordResetData value whose Host is the injected public URL, whose VerificationCode is + the raw (unhashed) code, and whose Email is the user's email + And RequestPasswordReset never returns a non-nil error to its caller: a failure to hash the + code, to store it, or to send the email is swallowed the same way a not-found email is, so the + handler can never tell those cases apart either — sending the request always looks the same to + the caller regardless of whether the email exists or whether the mail send happened to succeed + And a reset applies the same way regardless of the user's IsVerified status: an unverified user + can request and receive a password reset email exactly like a verified one, since resetting a + password is unrelated to email verification + And auth.Handler.RequestPasswordReset calls RequestPasswordResetUseCase.RequestPasswordReset and + then always responds 200 with the same body, + {"message": "If an account exists for that email, a password reset link has been sent."}, + never sets or clears the jwt cookie, and never branches on the use case's return value + And this slice does not add the confirmation endpoint that consumes the code (the page behind + "/register/reset/validate?c=...&email=..." and the POST call it would make); only the request + step is in scope + + Scenario: Requesting a reset for an existing email sends the reset email and returns the generic message + Given the user repository contains a user "alice" with email "alice@example.com" and IsVerified true + When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"} + Then the response status is 200 + And the response body is {"message": "If an account exists for that email, a password reset link has been sent."} + And exactly one email was sent to "alice@example.com" using template "passwordReset" + And that email's data has VerificationCode equal to the raw code stored in the repository's + hashed PasswordResetCode for alice, not the hash itself, and Email equal to "alice@example.com" + + Scenario: Requesting a reset for an email with no account returns the identical response and sends no email + Given the user repository is empty + When the client sends POST /v1/auth/password-reset with body {"email": "nobody@example.com"} + Then the response status is 200 + And the response body is {"message": "If an account exists for that email, a password reset link has been sent."} + And no email was sent + + Scenario: The reset email's Host reflects the injected public URL + Given a config.Settings value with PublicURL "https://cms.example.com" + And the auth handler is built from that settings value + And the user repository contains a user "alice" with email "alice@example.com" + When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"} + Then the email sent to "alice@example.com" has data.Host equal to "https://cms.example.com" + + Scenario: The stored reset code expires one hour after creation + Given the user repository contains a user "alice" with email "alice@example.com" + When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"} + Then alice's stored PasswordResetExpiration is within 5 seconds of time.Now().Add(1 * time.Hour) + + Scenario: A second request overwrites the first code, invalidating it + Given the user repository contains a user "alice" with email "alice@example.com" + When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"} + Then exactly one email has been sent, carrying raw code "code-1" in its VerificationCode data + When the client sends POST /v1/auth/password-reset again with body {"email": "alice@example.com"} + Then exactly two emails have been sent in total, and the second one's VerificationCode data + differs from "code-1" + And alice's stored, hashed PasswordResetCode no longer matches "code-1" but matches the raw code + from the second email + + Scenario: A mail-send failure for an existing user still returns the same generic response + Given the user repository contains a user "alice" with email "alice@example.com" + And the mail repository is set to fail on its next Send call + When the client sends POST /v1/auth/password-reset with body {"email": "alice@example.com"} + Then the response status is 200 + And the response body is {"message": "If an account exists for that email, a password reset link has been sent."} + And no jwt cookie is set + + Scenario: An unverified user can request a password reset exactly like a verified one + Given the user repository contains a user "bob" with email "bob@example.com" and IsVerified false + When the client sends POST /v1/auth/password-reset with body {"email": "bob@example.com"} + Then the response status is 200 + And the response body is {"message": "If an account exists for that email, a password reset link has been sent."} + And exactly one email was sent to "bob@example.com" using template "passwordReset" + + Scenario Outline: A malformed request is still rejected, since this is not an enumeration concern + When the client sends POST /v1/auth/password-reset with body + Then the response status is 400 + + Examples: + | body | + | {} | + | {"email": "not-an-email"} | + | not-json | + + Scenario: The use case itself hashes the code, stores it with a one-hour expiration, and sends the email + Given a user.Repository fake containing user id 3, email "alice@example.com" + And a RequestPasswordResetUseCase built directly over that fake, a hasher, a mail.Repository + fake and public URL "https://cms.example.com" + When RequestPasswordReset is called with "alice@example.com" + Then the call returns nil + And the fake repository's UpdatePasswordResetCode method was invoked exactly once with userId 3, + a hashed code, and an expiration within 5 seconds of time.Now().Add(1 * time.Hour) + And exactly one email was sent to "alice@example.com" using template "passwordReset" whose data + carries the same raw code that was hashed, and Host "https://cms.example.com" + + Scenario: The use case swallows an unknown email and returns nil without touching the repository or mailer + Given an empty user.Repository fake + And a RequestPasswordResetUseCase built directly over that fake, a hasher, and a mail.Repository fake + When RequestPasswordReset is called with "nobody@example.com" + Then the call returns nil + And the fake repository's UpdatePasswordResetCode method was never invoked + And no email was sent + + Scenario: The use case swallows a mail repository failure and still returns nil + Given a user.Repository fake containing user id 3, email "alice@example.com" + And a RequestPasswordResetUseCase built directly over that fake, a hasher, and a mail.Repository + fake set to fail on its next Send call + When RequestPasswordReset is called with "alice@example.com" + Then the call returns nil + And the fake repository's UpdatePasswordResetCode method was invoked exactly once with userId 3 diff --git a/features/request_password_reset.qa.md b/features/request_password_reset.qa.md new file mode 100644 index 0000000..0ccc9d9 --- /dev/null +++ b/features/request_password_reset.qa.md @@ -0,0 +1,111 @@ +# QA procedure: request_password_reset + +Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`) and +`test/mail_test.go` (`mockMailRepo`, `recordedMail`, `failSwitch`). + +## Test infrastructure additions needed first + +1. Update `mockUserRepo.UpdatePasswordResetCode` to the new three-argument signature + `UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error)`: + honor `m.fail.trigger("UpdatePasswordResetCode")`, look the user up (return `user.ErrUserNotFound` + if absent), set both `PasswordResetCode = code` and `PasswordResetExpiration = expiration`, store + it back, return it. Track a call counter and the last `(userId, code, expiration)` triple (mirroring + the existing `clearVerificationCodeCalls`/`clearVerificationCodeUserIds` fields) so scenarios can + assert it was or wasn't invoked and with what arguments. +2. Update every existing caller of the old two-argument signature to pass an expiration: this is a + breaking, cross-cutting change reaching `internal/infrastructure/useCases/UpdateUser.go`, + `internal/infrastructure/persistence/repositories/userRepository.go`, + `internal/domain/user/userRepositoryInterface.go`, `test/hasher_test.go` (the + `UpdatePasswordResetCode` case in `TestUpdateUserUseCaseHashesUpdatedValues`), + `test/write_not_found_errors_test.go`, and `test/use_case_baseline_test.go`. None of these are new + to this slice; note them here so the coder updates all call sites together instead of missing one. +3. Add `PasswordResetExpiration time.Time` to `user.User`, `models.User` and both mapper functions, + the same shape as `VerificationExpiration` (a plain, non-nullable column with no explicit `gorm` + tag beyond the struct field). +4. `stubHasher.Compare(hashed, value)` returns `nil` only when `hashed == "hashed:"+value`, so a + scenario asserting "the stored code no longer matches the first raw code but matches the second" + can call `stubHasher{}.Compare(repo.users[id].PasswordResetCode, "code-1")` and expect an error, + then `.Compare(..., secondRawCode)` and expect `nil`. + +## Setup shared by every case + +5. Call `validator.InitValidator()` once. +6. Use-case-level checks build + `useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com")` + where `repo` is a `mockUserRepo` and `mailRepo` is a `mockMailRepo`. Use `mailRepo.fail.failNext("Send")` + to force the next `Send` call to fail. +7. HTTP-level checks wire `mockUserRepo`, `mockMailRepo`, + `useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, publicURL)` and the other + existing use cases into `auth.NewHandler(...)` (extended with the new + `RequestPasswordResetUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), + nil)`. Mount `POST /v1/auth/password-reset` unprotected on a fresh `chi.NewRouter()` mirroring the + public group `NewAuthRouter` already uses for `/register`, `/login` and `/logout`. No live server, + no real SMTP. Default `publicURL` for cases that don't care about its value is + `"https://cms.example.com"`. +8. The expected response body for every "generic response" assertion is the exact JSON + `{"message":"If an account exists for that email, a password reset link has been sent."}`; decode + both the expected and actual bodies (or compare the raw bytes exactly, matching how `Login` and + `Register`'s success bodies are asserted elsewhere) rather than doing a loose substring check. + +## Use-case-level checks (no HTTP) + +9. Repo pre-populated with one user, id 3, email "alice@example.com". Call + `RequestPasswordReset("alice@example.com")`. Assert the return value is `nil`. Assert + `UpdatePasswordResetCode` was invoked exactly once with `userId == 3`, a `code` matching + `"hashed:" + ` and an `expiration` within 5 seconds of `time.Now().Add(time.Hour)`. + Assert `mailRepo.SentEmails` has exactly one entry: `Address == "alice@example.com"`, + `Template == "passwordReset"`, and its `Data` (asserted via a type assertion to + `useCases.MailPasswordResetData`) has `VerificationCode` equal to the same raw code that hashes to + the stored value, and `Host == "https://cms.example.com"`. +10. On an empty repo, call `RequestPasswordReset("nobody@example.com")`. Assert the return value is + `nil`. Assert `UpdatePasswordResetCode`'s call counter is still 0. Assert `mailRepo.SentEmails` is + empty. +11. Repo pre-populated with user id 3, email "alice@example.com". Set `mailRepo.fail.failNext("Send")`. + Call `RequestPasswordReset("alice@example.com")`. Assert the return value is `nil` (not the mail + repository's failure). Assert `UpdatePasswordResetCode` was still invoked exactly once (the store + happens before the send). +12. Repo pre-populated with two unverified users, "alice" and "bob" (`IsVerified: false`). Call + `RequestPasswordReset` for alice's email. Assert the return value is `nil` and exactly one email + was sent, showing IsVerified plays no role in the use case's behavior. + +## HTTP-level checks (fakes, per the shared setup above) + +13. Seed `mockUserRepo` with user "alice", email "alice@example.com", `IsVerified: true`. + `POST /v1/auth/password-reset` with `{"email":"alice@example.com"}` → assert status `200`, body + exactly matches the generic JSON from step 8, and `mockMailRepo.SentEmails` has exactly one entry + with `Address == "alice@example.com"` and `Template == "passwordReset"`. +14. On an empty `mockUserRepo`, `POST /v1/auth/password-reset` with `{"email":"nobody@example.com"}` → + assert status `200`, body byte-for-byte identical to step 13's body, and + `mockMailRepo.SentEmails` is empty. +15. Build the handler with `publicURL = "https://cms.example.com"`. Seed a user "alice", + email "alice@example.com". `POST /v1/auth/password-reset` with `{"email":"alice@example.com"}` → + assert the recorded mail's `Data.Host` equals `"https://cms.example.com"`. +16. Seed a user "alice", email "alice@example.com". `POST /v1/auth/password-reset` with her email → + assert `mockUserRepo`'s stored record has `PasswordResetExpiration` within 5 seconds of + `time.Now().Add(time.Hour)`. +17. Seed a user "alice", email "alice@example.com". `POST /v1/auth/password-reset` with her email + twice in a row. Assert `mockMailRepo.SentEmails` has exactly two entries and their + `VerificationCode` data values differ. Assert the repo's stored `PasswordResetCode` no longer + matches the first email's raw code via `stubHasher{}.Compare` (expect an error) but does match + the second email's raw code (expect `nil`). +18. Seed a user "alice", email "alice@example.com". Set `mailRepo.fail.failNext("Send")`. + `POST /v1/auth/password-reset` with her email → assert status `200`, body identical to step 13's + body, and no `Set-Cookie: jwt=...` header present. +19. Seed a user "bob", email "bob@example.com", `IsVerified: false`. `POST /v1/auth/password-reset` + with his email → assert status `200`, body identical to step 13's body, and exactly one email was + sent to "bob@example.com" with template "passwordReset". +20. Table-driven malformed-request cases against a `mockUserRepo` seeded with "alice" so a leak would + be visible if it occurred: `{}` (missing email) and `{"email":"not-an-email"}` → assert status + `400`. A non-JSON body `not-json` → assert status `400`. For all three, assert no email was sent. + +## Manual verification (not exercised by the Go test) + +- The real SMTP send and the rendered `passwordReset.html` link are never exercised automatically; + verify by hand against a real or sandboxed SMTP server that the link matches `PUBLIC_URL` from the + deployed `.env` and that visiting `/register/reset/validate?c=...&email=...` reaches the SPA. +- This slice does not add the confirmation endpoint that consumes the code, nor any SPA page/route + for it; both are out of scope and unverified here. +- The response-time difference between an existing and a non-existing email (hashing, storing and + emailing only happen when a user is found) is a known, unaddressed timing side channel; this slice + only equalizes the HTTP status and body, not response latency, and no automated timing assertion is + included since the suite has no precedent for timing-sensitive tests. diff --git a/features/reset_password.feature b/features/reset_password.feature new file mode 100644 index 0000000..1add87b --- /dev/null +++ b/features/reset_password.feature @@ -0,0 +1,183 @@ +Feature: A user sets a new password with their email and reset code, and the code cannot be replayed + + As the maintainer of RenewCMS + I want POST /v1/auth/password-reset/confirm to accept the email, reset code and new password + from the link RequestPasswordReset already emailed, set the new password only when the code + matches and has not expired, clear the stored code the moment it is used so it can never be + replayed, and answer an unknown email exactly like a wrong or expired code + So that a forgotten password can be reset without exposing which emails are registered, and a + reset code is single-use by construction rather than by convention + + Background: + Given RequestPasswordResetUseCase (features/request_password_reset.feature) already stores a + bcrypt hash of a random reset code on the user, together with PasswordResetExpiration set to + one hour after the request, and emails the raw code and the user's email in a link to + "{PublicURL}/register/reset/validate?c={code}&email={email}" + And this slice adds POST /v1/auth/password-reset/confirm, mounted alongside /register, /login, + /logout and /password-reset in NewAuthRouter, public and outside the jwt-protected group, + since a user resetting a forgotten password cannot be expected to present a jwt + And the request body is decoded into a ResetPasswordCredentials struct with three fields — + Email (validated required,email), Code (validated required) and Password (validated + required,min=8, the same length rule RegisterCredentials already applies to its own Password + field) — by validator.Validate.Struct before anything else + And a decode failure or a validation failure is answered exactly like every other auth route: + apperr.BodyErrorMessage with 400 for a decode failure, or the validator's error text with 400 + for a validation failure; these are shape checks on the request itself, not the + "does this account or code exist" question this slice otherwise hides + And this slice adds a new use case, ResetPasswordUseCase + (internal/infrastructure/useCases/ResetPassword.go), injected into auth.Handler as + ResetPasswordUseCase; its constructor takes only user.Repository and hash.Hasher, the same + shape as VerifyUserUseCase's constructor + And ResetPassword(email, code, newPassword string) error looks the user up with + user.Repository.GetByEmail; when no user has that email, or GetByEmail returns any other + error, it refuses with the new sentinel user.ErrInvalidPasswordResetCode instead of letting + the lookup failure itself distinguish the case, so an unknown email produces exactly the same + outcome as a wrong code + And when a user is found, ResetPassword refuses with the same user.ErrInvalidPasswordResetCode + when the stored PasswordResetCode is empty (never requested, or already used) or when + hash.Hasher.Compare finds the submitted code does not match the stored hash — the same + empty-string-never-matches rule VerifyUserUseCase already applies to VerificationCode, so a + cleared code short-circuits before any hash comparison happens, and is never compared against + a hash of the empty string + And ResetPassword refuses with the new sentinel user.ErrPasswordResetCodeExpired when + time.Now() is after the stored PasswordResetExpiration; the repository is not touched in + either refusal case + And otherwise ResetPassword hashes newPassword with hash.Hasher.Hash and calls a new + user.Repository method, ResetPassword(userId uint32, hashedPassword string) (user.User, + error), that in one write sets Password to the hash, PasswordResetCode to "" (stored as SQL + NULL, the same convention VerificationCode already uses) and PasswordResetExpiration to the + zero time.Time — the same "clear together in one write" pattern ClearVerificationCode already + uses for IsVerified and VerificationCode + And UpdateUserUseCase.UpdatePasswordResetCode, the older generic path that hashes whatever code + it is given, no longer hashes an empty code: given "" it passes "" straight through to + user.Repository.UpdatePasswordResetCode (stored as SQL NULL) without calling hash.Hasher.Hash, + so no code path can ever store a hash of the empty string as a reset code + And because the code is cleared rather than replaced with a hash of an empty string, and an + empty stored code never matches any submitted code, the same code can never be used a second + time even if an attacker knows it; requesting a new reset (RequestPasswordReset) is the only + way to obtain a usable code afterward + And auth.Handler.ResetPassword maps both user.ErrInvalidPasswordResetCode and + user.ErrPasswordResetCodeExpired to the exact same response — 403 with a new + apperr.PasswordResetFailedErrorMessage constant — so a client can never tell an unknown email + apart from a wrong or expired code; any other error is answered 500 "Internal server error" + And on success the handler responds 200 with + {"message": "Your password has been reset. You can now log in."}; it never sets or clears the + jwt cookie and never returns the user record, so a successful reset does not log the caller in + And this slice does not touch RequestPasswordReset, CreateUser or any SPA page; the SPA route + behind "/register/reset/validate?c=...&email=..." calling this endpoint remains a manual/SPA + concern + + Scenario: A correct, unexpired code sets the new password and cannot be reused + Given the user repository contains a user "alice" with email "alice@example.com", whose stored + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour from now + When the client sends POST /v1/auth/password-reset/confirm with body + {"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"} + Then the response status is 200 + And the response body is {"message": "Your password has been reset. You can now log in."} + And no jwt cookie is set + And the user repository's record for alice now has a Password hash that is no longer the old + one, an empty PasswordResetCode, and a zero PasswordResetExpiration + When the client sends POST /v1/auth/password-reset/confirm again with the same body + Then the response status is 403 + And the response body is the new apperr.PasswordResetFailedErrorMessage text + And the user repository's record for alice still has the password set by the first request + + Scenario: A wrong code is refused with the generic message and nothing changes + Given the user repository contains a user "alice" with email "alice@example.com", whose stored + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour from now + When the client sends POST /v1/auth/password-reset/confirm with body + {"email": "alice@example.com", "code": "wrong-code", "password": "newpassword1"} + Then the response status is 403 + And the response body is the new apperr.PasswordResetFailedErrorMessage text + And the user repository's record for alice is unchanged: the same stored PasswordResetCode and + the same Password hash as before + + Scenario: An expired code is refused with the same generic message and nothing changes + Given the user repository contains a user "alice" with email "alice@example.com", whose stored + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour in the past + When the client sends POST /v1/auth/password-reset/confirm with body + {"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"} + Then the response status is 403 + And the response body is the new apperr.PasswordResetFailedErrorMessage text + And the user repository's record for alice is unchanged + + Scenario: An unknown email is refused with the exact same status and body as a wrong code + Given the user repository is empty + When the client sends POST /v1/auth/password-reset/confirm with body + {"email": "nobody@example.com", "code": "any-code", "password": "newpassword1"} + Then the response status is 403 + And the response body is the new apperr.PasswordResetFailedErrorMessage text, byte-for-byte + identical to the body a wrong code produces for a known email + + Scenario: A user who never requested a reset gets the same refusal, since the stored code is empty + Given the user repository contains a user "alice" with email "alice@example.com" and an empty + PasswordResetCode + When the client sends POST /v1/auth/password-reset/confirm with body + {"email": "alice@example.com", "code": "anything", "password": "newpassword1"} + Then the response status is 403 + And the response body is the new apperr.PasswordResetFailedErrorMessage text + + Scenario Outline: A malformed or too-short request is rejected before any lookup happens + When the client sends POST /v1/auth/password-reset/confirm with body + Then the response status is 400 + + Examples: + | body | + | {} | + | {"email": "not-an-email", "code": "c", "password": "newpassword1"} | + | {"email": "alice@example.com", "code": "", "password": "newpassword1"} | + | {"email": "alice@example.com", "code": "reset-code-1", "password": "short"} | + | not-json | + + Scenario: The use case itself sets the password and clears the code in one write, independently of any handler + Given a user.Repository fake containing user id 3, email "alice@example.com", whose + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour from now + And a ResetPasswordUseCase built directly over that fake and a hasher + When ResetPassword is called with "alice@example.com", "reset-code-1" and "newpassword1" + Then the call returns nil + And the fake repository's ResetPassword method was invoked exactly once with userId 3 and a + hash of "newpassword1" + + Scenario: The use case rejects a wrong code without touching the repository's ResetPassword method + Given a user.Repository fake containing user id 3, email "alice@example.com", whose + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour from now + And a ResetPasswordUseCase built directly over that fake and a hasher + When ResetPassword is called with "alice@example.com", "wrong-code" and "newpassword1" + Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is + And the fake repository's ResetPassword method was never invoked + + Scenario: The use case rejects an expired code without touching the repository's ResetPassword method + Given a user.Repository fake containing user id 3, email "alice@example.com", whose + PasswordResetCode is the bcrypt hash of "reset-code-1" and whose PasswordResetExpiration is + one hour in the past + And a ResetPasswordUseCase built directly over that fake and a hasher + When ResetPassword is called with "alice@example.com", "reset-code-1" and "newpassword1" + Then the call returns user.ErrPasswordResetCodeExpired, asserted with errors.Is + And the fake repository's ResetPassword method was never invoked + + Scenario: The use case treats an unknown email the same as a wrong code + Given an empty user.Repository fake + And a ResetPasswordUseCase built directly over that fake and a hasher + When ResetPassword is called with "nobody@example.com", "any-code" and "newpassword1" + Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is + And the fake repository's ResetPassword method was never invoked + + Scenario: The use case treats an empty stored code the same as a wrong code, never calling Compare with an empty hash + Given a user.Repository fake containing user id 3, email "alice@example.com", whose + PasswordResetCode is empty + And a ResetPasswordUseCase built directly over that fake and a hasher + When ResetPassword is called with "alice@example.com", "anything" and "newpassword1" + Then the call returns user.ErrInvalidPasswordResetCode, asserted with errors.Is + And the fake repository's ResetPassword method was never invoked + + Scenario: The generic reset-code update stores an empty code as empty instead of hashing it + Given a user.Repository fake containing user id 3, email "alice@example.com" + And an UpdateUserUseCase built directly over that fake and a hasher + When UpdatePasswordResetCode is called with userId 3, code "" and the zero time.Time + Then the hasher's Hash method was never invoked + And the fake repository's stored PasswordResetCode for id 3 is "" diff --git a/features/reset_password.qa.md b/features/reset_password.qa.md new file mode 100644 index 0000000..d5b66b2 --- /dev/null +++ b/features/reset_password.qa.md @@ -0,0 +1,108 @@ +# QA procedure: reset_password + +Package: `test` (black-box), extending `test/user_test.go` (`mockUserRepo`, `stubHasher`). + +## Test infrastructure additions needed first + +1. Add a `ResetPassword(userId uint32, hashedPassword string) (user.User, error)` method to + `mockUserRepo`, mirroring the `ClearVerificationCode`/`UpdatePasswordResetCode` pattern: honor + `m.fail.trigger("ResetPassword")`, look the user up (return `user.ErrUserNotFound` if absent), + set `Password = hashedPassword`, `PasswordResetCode = ""` and `PasswordResetExpiration = + time.Time{}`, store it back, return it. Track a call counter and the last `(userId, + hashedPassword)` pair (e.g. `resetPasswordCalls`, `lastResetPasswordUserId`, + `lastResetPasswordHash`), the same shape as the existing `clearVerificationCodeCalls` fields, so + scenarios can assert it was or wasn't invoked and with what arguments. +2. `stubHasher.Compare(hashed, value)` returns `nil` only when `hashed == "hashed:"+value` and an + error otherwise, and `stubHasher.Hash(value)` returns `"hashed:"+value`, so seed + `PasswordResetCode` in fixtures as `"hashed:reset-code-1"` to represent "the bcrypt hash of + reset-code-1", and submit `"reset-code-1"` as the matching code. + +## Setup shared by every case + +3. Call `validator.InitValidator()` once. +4. Use-case-level checks build `useCases.NewResetPasswordUseCase(repo, &stubHasher{})` where `repo` + is a `mockUserRepo` pre-populated directly via `repo.users[id] = user.User{...}`. +5. HTTP-level checks wire `mockUserRepo`, `useCases.NewResetPasswordUseCase(repo, &stubHasher{})` + and the other existing use cases into `auth.NewHandler(...)` (extended with the new + `ResetPasswordUseCase` parameter) with a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. + Mount `POST /v1/auth/password-reset/confirm` unprotected on a fresh `chi.NewRouter()` mirroring + the public group `NewAuthRouter` already uses for `/register`, `/login`, `/logout` and + `/password-reset`. No live server, no real SMTP. +6. The expected response body for every "generic refusal" assertion is the exact plain-text value + of `apperr.PasswordResetFailedErrorMessage`, compared byte-for-byte via `http.Error`'s output + (trailing newline included, the same way `test/account_verification_http_test.go` compares + `apperr.VerificationFailedErrorMessage`). + +## Use-case-level checks (no HTTP) + +7. Repo has user id 3, email "alice@example.com", `PasswordResetCode: "hashed:reset-code-1"`, + `PasswordResetExpiration: time.Now().Add(time.Hour)`. Call `ResetPassword("alice@example.com", + "reset-code-1", "newpassword1")`. Assert the return value is `nil`. Assert the fake's + `ResetPassword` method was invoked exactly once with `userId == 3` and a hash equal to + `"hashed:newpassword1"`. +8. Same seed. Call `ResetPassword("alice@example.com", "wrong-code", "newpassword1")`. Assert + `errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword` method was + never invoked. +9. Same seed but `PasswordResetExpiration: time.Now().Add(-time.Hour)`. Call + `ResetPassword("alice@example.com", "reset-code-1", "newpassword1")`. Assert + `errors.Is(err, user.ErrPasswordResetCodeExpired)`. Assert the fake's `ResetPassword` method was + never invoked. +10. On an empty repo, call `ResetPassword("nobody@example.com", "any-code", "newpassword1")`. + Assert `errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword` + method was never invoked. +11. Repo has user id 3, email "alice@example.com", `PasswordResetCode: ""`. Call + `ResetPassword("alice@example.com", "anything", "newpassword1")`. Assert + `errors.Is(err, user.ErrInvalidPasswordResetCode)`. Assert the fake's `ResetPassword` method + was never invoked. +11b. Repo has user id 3, email "alice@example.com". Build `UpdateUserUseCase` over it with a + hasher that counts `Hash` calls. Call `UpdatePasswordResetCode(3, "", time.Time{})`. Assert + `Hash` was never invoked and the stored `PasswordResetCode` for id 3 is `""`. + +## HTTP-level checks (fakes, per the shared setup above) + +12. Seed `mockUserRepo` with user "alice", email "alice@example.com", + `PasswordResetCode: "hashed:reset-code-1"`, `PasswordResetExpiration: + time.Now().Add(time.Hour)`. `POST /v1/auth/password-reset/confirm` with + `{"email":"alice@example.com","code":"reset-code-1","password":"newpassword1"}` → assert + status `200`, body exactly `{"message":"Your password has been reset. You can now log in."}`, + and no `Set-Cookie: jwt=...` header present. Then assert the repo's record for alice has + `Password == "hashed:newpassword1"`, `PasswordResetCode == ""` and `PasswordResetExpiration` + equal to `time.Time{}`. +13. Immediately repeat step 12's exact request on the same router/repo (the code was already + cleared) → assert status `403`, body equal to `apperr.PasswordResetFailedErrorMessage`, and the + repo's record for alice still has `Password == "hashed:newpassword1"` (the value set by the + first request, unchanged by the second). +14. Seed a fresh `mockUserRepo`/router with user "alice" as in step 12. `POST + /v1/auth/password-reset/confirm` with `{"email":"alice@example.com","code":"wrong-code","password":"newpassword1"}` + → assert status `403`, body equal to `apperr.PasswordResetFailedErrorMessage`, and the repo's + record for alice is unchanged (`PasswordResetCode` still `"hashed:reset-code-1"`, `Password` + still its original seeded value). +15. Seed with `PasswordResetExpiration: time.Now().Add(-time.Hour)`. `POST + /v1/auth/password-reset/confirm` with the correct code → assert status `403`, body equal to + `apperr.PasswordResetFailedErrorMessage`, and the record is unchanged. +16. On an empty `mockUserRepo`, `POST /v1/auth/password-reset/confirm` with + `{"email":"nobody@example.com","code":"any-code","password":"newpassword1"}` → assert status + `403` and body byte-for-byte identical to step 14's body. +17. Seed a user "alice" with `PasswordResetCode: ""`. `POST /v1/auth/password-reset/confirm` with + `{"email":"alice@example.com","code":"anything","password":"newpassword1"}` → assert status + `403`, body equal to `apperr.PasswordResetFailedErrorMessage`. +18. Table-driven over the five malformed/too-short bodies from the feature's Examples table, each + against a `mockUserRepo` seeded with "alice" (`PasswordResetCode: "hashed:reset-code-1"`, + unexpired) so a leak would be visible if it occurred: `{}`, `{"email":"not-an-email",...}`, + `{"email":"alice@example.com","code":"",...}`, a `password` shorter than 8 characters, and the + non-JSON body `not-json` → assert status `400` for all five, and assert the repo's record for + alice is unchanged in every case (in particular `ResetPassword`'s call counter stays at 0). + +## Manual verification (not exercised by the Go test) + +- The SPA page behind `/register/reset/validate?c=...&email=...` actually reading the query + parameters, prompting for a new password, and calling + `POST /v1/auth/password-reset/confirm` is out of scope for this slice's automated test and + should be checked by hand against a running dev build. +- The response-time difference between an unknown email (short-circuits at `GetByEmail`) and a + known email with a wrong code (also compares a hash) is a known, unaddressed timing side + channel; this slice only equalizes the HTTP status and body, not response latency, matching the + same caveat already recorded in `features/request_password_reset.qa.md`. +- This slice does not log the user in on a successful reset; verify by hand, if desired, that the + SPA redirects to the login page rather than assuming an authenticated session after a successful + reset. diff --git a/features/verified_account_required.feature b/features/verified_account_required.feature new file mode 100644 index 0000000..0a71e06 --- /dev/null +++ b/features/verified_account_required.feature @@ -0,0 +1,165 @@ +Feature: Protected article routes and the offline-inclusive listing require a verified account + + As the maintainer of RenewCMS + I want every protected article route, and the "see offline articles too" behavior of + GET /v1/article/, to require an account that has completed email verification, not merely + a valid jwt session + So that a user who registered but never confirmed their email cannot create, read, edit, + publish, unpublish, delete or attach images to articles, and cannot see unpublished drafts + through the listing endpoint, while a verified user's existing workflow is unchanged + + Background: + Given before this slice, the protected group inside NewArticleRouter (api/router.go) applies + only jwtauth.Verifier then jwtauth.Authenticator to GET /v1/article/{id}, POST /v1/article/, + DELETE /v1/article/{id}, PATCH /v1/article/{id}, PATCH /v1/article/{id}/publish, + PATCH /v1/article/{id}/unpublish and POST /v1/article/{id}/image, so any caller with a + validly signed, unexpired jwt cookie reaches the handler regardless of that account's + is_verified status + And this slice adds a third middleware, api/middleware.RequireVerifiedUser(getUser + *useCases.GetUserUseCase), mounted after jwtauth.Authenticator in that same group, so the + order becomes Verifier, Authenticator, RequireVerifiedUser for all seven routes above + And RequireVerifiedUser reads user_id from the request's jwt claims using a new exported + helper, api/pkg/claims.UserId(r *http.Request) (uint32, bool), which replaces the + identical private userIdFromClaims method api/handlers/auth.Handler.Verify and + CancelRegistration already used (features/account_verification.feature, + features/cancel_pending_registration.feature); those two handlers are updated to call the + shared helper, and their observable 401 behavior on a missing or non-numeric user_id claim + is unchanged + And when claims.UserId reports no usable user_id, RequireVerifiedUser answers 401 with + apperr.UnauthorizedErrorMessage without calling GetUserUseCase, the same body and status the + protected auth routes already use for the same condition + And otherwise RequireVerifiedUser calls GetUserUseCase.GetUser(userId); when that returns + user.ErrUserNotFound (a token minted for an account since deleted), it answers 401 with + apperr.UnauthorizedErrorMessage; when it returns any other error, it answers 500 with body + "Internal server error"; when it returns a user whose IsVerified is false, it answers 403 + with a new apperr.VerificationRequiredErrorMessage constant, "You must verify your account + to access this resource."; when IsVerified is true, it calls next.ServeHTTP unchanged, so + the wrapped handler runs exactly as it did before this slice + And this slice deletes the dead api/handlers/auth.Handler.IsVerified method: it was never + wired to any route, and it performed the bare token.PrivateClaims()["user_id"].(float64) + assertion this slice avoids everywhere else; RequireVerifiedUser is the behavior it was + meant to provide, so it is removed rather than kept unused + And this slice also changes GET /v1/article/, which sits under jwtauth.Verifier only (no + Authenticator, per features/public_article_listing.feature) and therefore is never rejected + outright: article.Handler gains a GetUserUseCase field, wired from container.GetUserUseCase + in api/router.go's NewArticleRouter, and Handler.ListArticles now calls ListAllArticles only + when the request carries a resolvable user_id (via the same claims.UserId helper) whose + GetUserUseCase.GetUser lookup succeeds and returns IsVerified true; any other case (no jwt, + malformed jwt, unresolvable user_id, deleted account, or an account that exists but is not + yet verified) falls back to ListOnlineArticles, the same fallback + features/public_article_listing.feature already documents for a malformed cookie; this + listing route never answers 401 or 403 for an unverified caller, it just narrows the result + And a repository failure while ListArticles resolves the caller (GetUserUseCase.GetUser + returning an error other than user.ErrUserNotFound) is treated the same as "not verified" + for the purpose of choosing which list to show, so the listing still answers 200 with the + online-only array rather than failing the request; only a failure inside ListOnlineArticles + or ListAllArticles itself still answers 500, unchanged from features/public_article_listing.feature + + Scenario Outline: An authenticated but unverified user is refused on every protected article route + Given a user repository containing user id 1, is_verified false + And a valid jwt cookie for user id 1 + And the article repository contains an article with id 1 titled "Backyard Renovation Update" + When the client sends with that cookie + Then the response status is 403 + And the response body is the apperr.VerificationRequiredErrorMessage text + + Examples: + | method | path | + | GET | /v1/article/1 | + | POST | /v1/article/ | + | DELETE | /v1/article/1 | + | PATCH | /v1/article/1 | + | PATCH | /v1/article/1/publish | + | PATCH | /v1/article/1/unpublish | + + Scenario: An unverified user is also refused when attaching an image + Given a user repository containing user id 1, is_verified false + And a valid jwt cookie for user id 1 + And the article repository contains an article with id 1 titled "Backyard Renovation Update" + When the client sends POST /v1/article/1/image with that cookie and a multipart form field + "file" holding a file named "before.png" with content type "image/png" + Then the response status is 403 + And the response body is the apperr.VerificationRequiredErrorMessage text + And the image repository's Create is never called + + Scenario: A verified user still exercises the protected routes exactly as before + Given a user repository containing user id 1, is_verified true + And a valid jwt cookie for user id 1 + When the client sends POST /v1/article/ with that cookie and body + {"title": "Valid Title", "body": "Some content"} + Then the response status is 201 + And the response body is the created article as JSON + + Scenario Outline: A token that resolves to no usable account still answers 401, never 403 + Given a user repository containing user id 1, is_verified false + And + And the article repository contains an article with id 1 titled "Backyard Renovation Update" + When the client sends POST /v1/article/ with that cookie and body + {"title": "Valid Title", "body": "Some content"} + Then the response status is 401 + And the response body is the apperr.UnauthorizedErrorMessage text + + Examples: + | token description | + | a validly signed jwt cookie whose claims carry no "user_id" key at all | + | a validly signed jwt cookie whose "user_id" claim is the string "one" | + | a validly signed jwt cookie whose "user_id" claim is the number 999, which does not match any user | + + Scenario Outline: A request with no jwt cookie is still refused with 401, never 403 + Given no jwt cookie is sent + And the article repository contains an article with id 1 titled "Backyard Renovation Update" + When the client sends without a cookie + Then the response status is 401 + + Examples: + | method | path | + | GET | /v1/article/1 | + | POST | /v1/article/ | + + Scenario: A repository failure while resolving the caller's verification status answers 500 + Given a valid jwt cookie for user id 1 + And the user repository fails on its next call to Get with an error other than + user.ErrUserNotFound + And the article repository contains an article with id 1 titled "Backyard Renovation Update" + When the client sends DELETE /v1/article/1 with that cookie + Then the response status is 500 + And the response body is the text "Internal server error" + And the article repository's article with id 1 still exists + + Scenario: Listing falls back to the online-only list for an authenticated but unverified caller + Given a user repository containing user id 1, is_verified false + And a valid jwt cookie for user id 1 + And the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing exactly the article with id 1 + And the article with id 2 is absent from the response body + + Scenario: Listing still returns every article for an authenticated and verified caller + Given a user repository containing user id 1, is_verified true + And a valid jwt cookie for user id 1 + And the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing both the article with id 1 and the article with id 2 + + Scenario: Listing falls back to the online-only list when the caller's account no longer exists + Given the user repository contains no user with id 999 + And a valid jwt cookie for user id 999 + And the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing exactly the article with id 1 + + Scenario: Listing falls back to the online-only list rather than failing when the user lookup errors + Given a valid jwt cookie for user id 1 + And the user repository fails on its next call to Get with an error other than + user.ErrUserNotFound + And the article repository contains an online article titled "Backyard Renovation Update" with id 1 + And the same repository contains an offline article titled "Draft: Kitchen Remodel" with id 2 + When the client sends GET /v1/article/ with that cookie + Then the response status is 200 + And the response body is a JSON array containing exactly the article with id 1 diff --git a/features/verified_account_required.qa.md b/features/verified_account_required.qa.md new file mode 100644 index 0000000..38a62b7 --- /dev/null +++ b/features/verified_account_required.qa.md @@ -0,0 +1,120 @@ +# QA procedure: verified_account_required + +Package: `test` (black-box), extending `test/article_test.go` (`mockArticleRepo`) and +`test/user_test.go` (`mockUserRepo`). This slice's router mount touches both the article and auth +routers, so the HTTP-level checks live in a new `test/verified_account_required_http_test.go`. + +## Test infrastructure additions needed first + +1. `mockUserRepo.Get` already honors `m.fail.trigger("Get")` and already returns + `user.ErrUserNotFound` for an absent id (`test/user_test.go`), so no new mock methods are + required on the user side. +2. `mockArticleRepo` already supports seeding via `repo.articles[id] = article.Article{...}` and a + `failNext(...)` helper (`test/article_test.go`); no changes needed there either. +3. Build the middleware under test directly: `middleware.RequireVerifiedUser(getUser + *useCases.GetUserUseCase) func(http.Handler) http.Handler`, exported from `api/middleware`, and + `claims.UserId(r *http.Request) (uint32, bool)`, exported from `api/pkg/claims`. Construct + `useCases.NewGetUserUseCase(repo)` over a `mockUserRepo`. + +## Setup shared by every case + +4. Call `validator.InitValidator()` once. +5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. +6. Wire `article.NewHandler(create, get, list, delete, update, getUser)` (extended with the new + `GetUserUseCase` parameter) over a `mockArticleRepo` and the `mockUserRepo`/`GetUserUseCase` + from step 3. +7. Mount the routes on a fresh `chi.NewRouter()` mirroring the new `api/router.go` shape: + - `r.Use(jwtauth.Verifier(token))` on the whole `/article` group (parses, never rejects) + - `r.Get("/", articleHandler.ListArticles)` directly under that Verifier-only group + - a nested `r.Group` adding `jwtauth.Authenticator(token)` then + `middleware.RequireVerifiedUser(getUser)`, containing `r.Get("/{id}", ...)`, + `r.Post("/", ...)`, `r.Delete("/{id}", ...)`, `r.Patch("/{id}", ...)`, + `r.Patch("/{id}/publish", ...)`, `r.Patch("/{id}/unpublish", ...)`, + `r.Post("/{id}/image", ...)` + No live server. +8. Mint jwt cookies with `token.Encode(map[string]any{"user_id": uint32(1)})` for the valid-token + cases, following the convention in `features/article_publishing.qa.md`. +9. Seed `mockUserRepo` per scenario via `repo.users[1] = user.User{ID: 1, IsVerified: ...}`. + +## HTTP checks: protected routes reject an unverified caller + +10. Seed `mockUserRepo` with user id 1, `IsVerified: false`; seed `mockArticleRepo` with article id + 1. Table-driven over `GET /v1/article/1`, `POST /v1/article/` (body + `{"title":"Valid Title","body":"Some content"}`), `DELETE /v1/article/1`, + `PATCH /v1/article/1` (body `{"body":"Updated"}`), `PATCH /v1/article/1/publish`, + `PATCH /v1/article/1/unpublish`, each sent with the valid cookie from step 8 → assert status + `403` and body exactly `apperr.VerificationRequiredErrorMessage`. Assert the article repository + was not mutated by any of the write calls. +11. Same seed, `POST /v1/article/1/image` with the valid cookie and a multipart form field `"file"` + holding `before.png` / `image/png` → assert status `403`, body + `apperr.VerificationRequiredErrorMessage`, and the image repository's `Create` is never called + (mirroring the assertion style in `test/upload_article_image_test.go`). + +## HTTP checks: a verified caller is unaffected + +12. Seed `mockUserRepo` with user id 1, `IsVerified: true`. `POST /v1/article/` with the valid + cookie and body `{"title":"Valid Title","body":"Some content"}` → assert status `201` and the + response body decodes to the created `article.Article`. + +## HTTP checks: an unresolvable caller still answers 401, not 403 + +13. Table-driven over three token shapes, each against a fresh seed of user id 1, + `IsVerified: false`, sent as `POST /v1/article/` with body + `{"title":"Valid Title","body":"Some content"}`, asserting status `401` and body exactly + `apperr.UnauthorizedErrorMessage`: + - mint the jwt with `token.Encode(map[string]any{"role": "user"})` (no `user_id` key at all); + - mint the jwt with `token.Encode(map[string]any{"user_id": "one"})` (string, not a number); + - mint the jwt with `token.Encode(map[string]any{"user_id": uint32(999)})` (well-formed but no + such user in the repo). + +## HTTP checks: no session at all is still 401 + +14. `GET /v1/article/1` and `POST /v1/article/` with no `jwt` cookie → assert status `401` for + both (produced by `jwtauth.Authenticator` itself, before `RequireVerifiedUser` runs); do not + assert an exact body. + +## HTTP checks: a user-repository failure while resolving the caller + +15. Mint a valid jwt cookie for user id 1. Configure `mockUserRepo`'s `failSwitch` so the next call + to `Get` returns a generic error (not `user.ErrUserNotFound`), via the existing + `failNext("Get")`-style helper. `DELETE /v1/article/1` with that cookie, article repository + seeded with article id 1 → assert status `500`, body exactly `"Internal server error"`, and + that the article repository's article with id 1 still exists (the delete use case was never + reached). + +## HTTP checks: the offline-inclusive listing is gated by verification + +16. Seed `mockUserRepo` with user id 1, `IsVerified: false`; seed `mockArticleRepo` with an online + article id 1 and an offline article id 2. `GET /v1/article/` with the valid cookie from step 8 + → assert status `200`; decode the JSON array and assert it has exactly one element, id `1`. +17. Same article seed, `mockUserRepo` user id 1 with `IsVerified: true` → `GET /v1/article/` with + the same cookie → assert status `200`; decode the JSON array and assert both ids `1` and `2` + are present. +18. Same article seed, no user with id 999 in `mockUserRepo`, jwt cookie minted for user id 999 → + `GET /v1/article/` with that cookie → assert status `200`; assert exactly id `1` is present + (fallback to online-only on `user.ErrUserNotFound`). +19. Same article seed, valid cookie for user id 1, `mockUserRepo`'s `failSwitch` set so the next + `Get` call returns a generic error → `GET /v1/article/` with that cookie → assert status `200`; + assert exactly id `1` is present (a user-lookup failure falls back to online-only rather than + failing the request). + +## Regression check on existing fixtures + +20. `features/article_publishing.feature`, `features/edit_article_body.feature`, + `features/upload_article_image.feature` and the corresponding `test/*_test.go` files mint a + valid jwt cookie for "a user with id 1" without asserting anything about that user's + `IsVerified` status. After this slice, those HTTP tests' `mockUserRepo` fixture must seed user + id 1 with `IsVerified: true` (or the equivalent id used in each file), or every one of those + protected-route assertions will start failing with `403` instead of the expected success/404/400 + status. This is a required update to those test files' setup, not a new behavior; call it out + explicitly when touching them so the regression is caught at review time rather than by a + surprise `task test` failure. + +## Manual verification (not exercised by the Go test) + +- The SPA-visible behaviour of an unverified, logged-in user landing on the back-office article + screens (create/edit/publish/image-upload) and seeing a 403 instead of the normal editing UI, + and the front-office/back-office listing showing only published articles to that same unverified + session, is out of scope for this slice's automated test and should be checked by hand against a + running dev build with an unverified test account. +- No SMTP involvement. diff --git a/internal/domain/user/userDomain.go b/internal/domain/user/userDomain.go index 81e1361..98947cf 100644 --- a/internal/domain/user/userDomain.go +++ b/internal/domain/user/userDomain.go @@ -8,16 +8,22 @@ import ( var ErrUserNotFound = errors.New("user not found") var ErrInvalidCredentials = errors.New("invalid credentials") var ErrRegistrationClosed = errors.New("registration closed") +var ErrInvalidVerificationCode = errors.New("invalid verification code") +var ErrVerificationCodeExpired = errors.New("verification code expired") +var ErrAlreadyVerified = errors.New("user already verified") +var ErrInvalidPasswordResetCode = errors.New("invalid password reset code") +var ErrPasswordResetCodeExpired = errors.New("password reset code expired") type User struct { - ID uint32 `json:"id"` - Username string `json:"username"` - Password string `json:"password"` - PasswordResetCode string `json:"password_reset_code"` - Email string `json:"email"` - IsVerified bool `json:"is_verified"` - VerificationCode string `json:"verification_code"` - VerificationExpiration time.Time `json:"verification_expiration"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID uint32 `json:"id"` + Username string `json:"username"` + Password string `json:"password"` + PasswordResetCode string `json:"password_reset_code"` + Email string `json:"email"` + IsVerified bool `json:"is_verified"` + VerificationCode string `json:"verification_code"` + VerificationExpiration time.Time `json:"verification_expiration"` + PasswordResetExpiration time.Time `json:"password_reset_expiration"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } diff --git a/internal/domain/user/userRepositoryInterface.go b/internal/domain/user/userRepositoryInterface.go index 6c4a4a3..880ed43 100644 --- a/internal/domain/user/userRepositoryInterface.go +++ b/internal/domain/user/userRepositoryInterface.go @@ -1,5 +1,7 @@ package user +import "time" + type Repository interface { Get(id uint32) (User, error) GetByUsername(username string) (User, error) @@ -9,5 +11,7 @@ type Repository interface { Delete(id uint32) error UpdateVerificationStatus(userId uint32, isVerified bool) (User, error) UpdatePassword(userId uint32, password string) (User, error) - UpdatePasswordResetCode(userId uint32, code string) (User, error) + UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (User, error) + ClearVerificationCode(userId uint32) (User, error) + ResetPassword(userId uint32, hashedPassword string) (User, error) } diff --git a/internal/infrastructure/persistence/mappers/userMapper.go b/internal/infrastructure/persistence/mappers/userMapper.go index 66c7564..63723eb 100644 --- a/internal/infrastructure/persistence/mappers/userMapper.go +++ b/internal/infrastructure/persistence/mappers/userMapper.go @@ -21,29 +21,31 @@ func DomainToNullableString(value string) *string { func UserToDomain(m models.User) user.User { return user.User{ - ID: m.ID, - Username: m.Username, - Password: m.Password, - PasswordResetCode: nullableStringToDomain(m.PasswordResetCode), - Email: m.Email, - IsVerified: m.IsVerified, - VerificationCode: nullableStringToDomain(m.VerificationCode), - VerificationExpiration: m.VerificationExpiration, - CreatedAt: m.CreatedAt, - UpdatedAt: m.UpdatedAt, + ID: m.ID, + Username: m.Username, + Password: m.Password, + PasswordResetCode: nullableStringToDomain(m.PasswordResetCode), + Email: m.Email, + IsVerified: m.IsVerified, + VerificationCode: nullableStringToDomain(m.VerificationCode), + VerificationExpiration: m.VerificationExpiration, + PasswordResetExpiration: m.PasswordResetExpiration, + CreatedAt: m.CreatedAt, + UpdatedAt: m.UpdatedAt, } } func UserToModel(a user.User) models.User { return models.User{ - Username: a.Username, - Password: a.Password, - PasswordResetCode: DomainToNullableString(a.PasswordResetCode), - Email: a.Email, - IsVerified: a.IsVerified, - VerificationCode: DomainToNullableString(a.VerificationCode), - VerificationExpiration: a.VerificationExpiration, - CreatedAt: a.CreatedAt, - UpdatedAt: a.UpdatedAt, + Username: a.Username, + Password: a.Password, + PasswordResetCode: DomainToNullableString(a.PasswordResetCode), + Email: a.Email, + IsVerified: a.IsVerified, + VerificationCode: DomainToNullableString(a.VerificationCode), + VerificationExpiration: a.VerificationExpiration, + PasswordResetExpiration: a.PasswordResetExpiration, + CreatedAt: a.CreatedAt, + UpdatedAt: a.UpdatedAt, } } diff --git a/internal/infrastructure/persistence/models/userModel.go b/internal/infrastructure/persistence/models/userModel.go index 01674aa..dc35ed8 100644 --- a/internal/infrastructure/persistence/models/userModel.go +++ b/internal/infrastructure/persistence/models/userModel.go @@ -5,14 +5,15 @@ import ( ) type User struct { - ID uint32 `gorm:"primaryKey;autoIncrement"` - Username string `gorm:"unique;not null"` - Password string `gorm:"not null"` - PasswordResetCode *string `gorm:"unique"` - Email string `gorm:"unique;not null"` - IsVerified bool `gorm:"default=false;not null"` - VerificationCode *string `gorm:"unique"` - VerificationExpiration time.Time - CreatedAt time.Time `gorm:"autoCreateTime"` - UpdatedAt time.Time `gorm:"autoUpdateTime"` + ID uint32 `gorm:"primaryKey;autoIncrement"` + Username string `gorm:"unique;not null"` + Password string `gorm:"not null"` + PasswordResetCode *string `gorm:"unique"` + Email string `gorm:"unique;not null"` + IsVerified bool `gorm:"default=false;not null"` + VerificationCode *string `gorm:"unique"` + VerificationExpiration time.Time + PasswordResetExpiration time.Time + CreatedAt time.Time `gorm:"autoCreateTime"` + UpdatedAt time.Time `gorm:"autoUpdateTime"` } diff --git a/internal/infrastructure/persistence/repositories/userRepository.go b/internal/infrastructure/persistence/repositories/userRepository.go index 5a192e2..8c94c47 100644 --- a/internal/infrastructure/persistence/repositories/userRepository.go +++ b/internal/infrastructure/persistence/repositories/userRepository.go @@ -4,6 +4,7 @@ import ( domainUser "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/persistence/mappers" entity "RenewCMS/internal/infrastructure/persistence/models" + "time" "gorm.io/gorm" ) @@ -115,7 +116,7 @@ func (u *UserRepository) UpdatePassword(userId uint32, password string) (domainU return mappers.UserToDomain(localUser), nil } -func (u *UserRepository) UpdatePasswordResetCode(userId uint32, code string) (domainUser.User, error) { +func (u *UserRepository) UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (domainUser.User, error) { var localUser entity.User err := u.db.Model(&entity.User{}).First(&localUser, userId).Error if err != nil { @@ -123,6 +124,42 @@ func (u *UserRepository) UpdatePasswordResetCode(userId uint32, code string) (do } localUser.PasswordResetCode = mappers.DomainToNullableString(code) + localUser.PasswordResetExpiration = expiration + err = u.db.Save(&localUser).Error + if err != nil { + return domainUser.User{}, err + } + + return mappers.UserToDomain(localUser), nil +} + +func (u *UserRepository) ClearVerificationCode(userId uint32) (domainUser.User, error) { + var localUser entity.User + err := u.db.Model(&entity.User{}).First(&localUser, userId).Error + if err != nil { + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) + } + + localUser.IsVerified = true + localUser.VerificationCode = mappers.DomainToNullableString("") + err = u.db.Save(&localUser).Error + if err != nil { + return domainUser.User{}, err + } + + return mappers.UserToDomain(localUser), nil +} + +func (u *UserRepository) ResetPassword(userId uint32, hashedPassword string) (domainUser.User, error) { + var localUser entity.User + err := u.db.Model(&entity.User{}).First(&localUser, userId).Error + if err != nil { + return domainUser.User{}, mapNotFoundErr(err, domainUser.ErrUserNotFound) + } + + localUser.Password = hashedPassword + localUser.PasswordResetCode = mappers.DomainToNullableString("") + localUser.PasswordResetExpiration = time.Time{} err = u.db.Save(&localUser).Error if err != nil { return domainUser.User{}, err diff --git a/internal/infrastructure/useCases/CancelRegistration.go b/internal/infrastructure/useCases/CancelRegistration.go new file mode 100644 index 0000000..dbc05ac --- /dev/null +++ b/internal/infrastructure/useCases/CancelRegistration.go @@ -0,0 +1,26 @@ +package useCases + +import ( + "RenewCMS/internal/domain/user" +) + +type CancelRegistrationUseCase struct { + userRepository user.Repository +} + +func NewCancelRegistrationUseCase(userRepository user.Repository) *CancelRegistrationUseCase { + return &CancelRegistrationUseCase{userRepository} +} + +func (c *CancelRegistrationUseCase) CancelRegistration(userId uint32) error { + dbUser, err := c.userRepository.Get(userId) + if err != nil { + return err + } + + if dbUser.IsVerified { + return user.ErrAlreadyVerified + } + + return c.userRepository.Delete(userId) +} diff --git a/internal/infrastructure/useCases/RequestPasswordReset.go b/internal/infrastructure/useCases/RequestPasswordReset.go new file mode 100644 index 0000000..d1f0c09 --- /dev/null +++ b/internal/infrastructure/useCases/RequestPasswordReset.go @@ -0,0 +1,53 @@ +package useCases + +import ( + "RenewCMS/internal/domain/hash" + "RenewCMS/internal/domain/mail" + "RenewCMS/internal/domain/user" + "time" + + "github.com/google/uuid" +) + +type RequestPasswordResetUseCase struct { + userRepository user.Repository + hasher hash.Hasher + mailRepository mail.Repository + publicURL string +} + +type MailPasswordResetData struct { + Host string + VerificationCode string + Email string +} + +func NewRequestPasswordResetUseCase(userRepository user.Repository, hasher hash.Hasher, mailRepository mail.Repository, publicURL string) *RequestPasswordResetUseCase { + return &RequestPasswordResetUseCase{userRepository, hasher, mailRepository, publicURL} +} + +func (g *RequestPasswordResetUseCase) RequestPasswordReset(email string) error { + foundUser, err := g.userRepository.GetByEmail(email) + if err != nil { + return nil + } + + rawCode := uuid.NewString() + + hashedCode, err := g.hasher.Hash(rawCode) + if err != nil { + return nil + } + + if _, err := g.userRepository.UpdatePasswordResetCode(foundUser.ID, hashedCode, time.Now().Add(time.Hour)); err != nil { + return nil + } + + _ = g.mailRepository.Send(foundUser.Email, "passwordReset", MailPasswordResetData{ + Host: g.publicURL, + VerificationCode: rawCode, + Email: foundUser.Email, + }) + + return nil +} diff --git a/internal/infrastructure/useCases/ResetPassword.go b/internal/infrastructure/useCases/ResetPassword.go new file mode 100644 index 0000000..22e9d1d --- /dev/null +++ b/internal/infrastructure/useCases/ResetPassword.go @@ -0,0 +1,39 @@ +package useCases + +import ( + "RenewCMS/internal/domain/hash" + "RenewCMS/internal/domain/user" + "time" +) + +type ResetPasswordUseCase struct { + userRepository user.Repository + hasher hash.Hasher +} + +func NewResetPasswordUseCase(userRepository user.Repository, hasher hash.Hasher) *ResetPasswordUseCase { + return &ResetPasswordUseCase{userRepository, hasher} +} + +func (r *ResetPasswordUseCase) ResetPassword(email string, code string, newPassword string) error { + dbUser, err := r.userRepository.GetByEmail(email) + if err != nil { + return user.ErrInvalidPasswordResetCode + } + + if dbUser.PasswordResetCode == "" || r.hasher.Compare(dbUser.PasswordResetCode, code) != nil { + return user.ErrInvalidPasswordResetCode + } + + if time.Now().After(dbUser.PasswordResetExpiration) { + return user.ErrPasswordResetCodeExpired + } + + hashedPassword, err := r.hasher.Hash(newPassword) + if err != nil { + return err + } + + _, err = r.userRepository.ResetPassword(dbUser.ID, hashedPassword) + return err +} diff --git a/internal/infrastructure/useCases/UpdateUser.go b/internal/infrastructure/useCases/UpdateUser.go index 4caf505..dac0815 100644 --- a/internal/infrastructure/useCases/UpdateUser.go +++ b/internal/infrastructure/useCases/UpdateUser.go @@ -3,6 +3,7 @@ package useCases import ( "RenewCMS/internal/domain/hash" "RenewCMS/internal/domain/user" + "time" ) type UpdateUserUseCase struct { @@ -18,12 +19,15 @@ func (g *UpdateUserUseCase) UpdateVerificationStatus(userId uint32, isVerified b return g.userRepository.UpdateVerificationStatus(userId, isVerified) } -func (g *UpdateUserUseCase) UpdatePasswordResetCode(userId uint32, code string) (user.User, error) { +func (g *UpdateUserUseCase) UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error) { + if code == "" { + return g.userRepository.UpdatePasswordResetCode(userId, "", expiration) + } hashedCode, err := g.hasher.Hash(code) if err != nil { return user.User{}, err } - return g.userRepository.UpdatePasswordResetCode(userId, hashedCode) + return g.userRepository.UpdatePasswordResetCode(userId, hashedCode, expiration) } func (g *UpdateUserUseCase) UpdatePassword(userId uint32, password string) (user.User, error) { diff --git a/internal/infrastructure/useCases/VerifyUser.go b/internal/infrastructure/useCases/VerifyUser.go new file mode 100644 index 0000000..9c007e8 --- /dev/null +++ b/internal/infrastructure/useCases/VerifyUser.go @@ -0,0 +1,33 @@ +package useCases + +import ( + "RenewCMS/internal/domain/hash" + "RenewCMS/internal/domain/user" + "time" +) + +type VerifyUserUseCase struct { + userRepository user.Repository + hasher hash.Hasher +} + +func NewVerifyUserUseCase(userRepository user.Repository, hasher hash.Hasher) *VerifyUserUseCase { + return &VerifyUserUseCase{userRepository, hasher} +} + +func (v *VerifyUserUseCase) Verify(userId uint32, code string) (user.User, error) { + dbUser, err := v.userRepository.Get(userId) + if err != nil { + return user.User{}, err + } + + if dbUser.VerificationCode == "" || v.hasher.Compare(dbUser.VerificationCode, code) != nil { + return user.User{}, user.ErrInvalidVerificationCode + } + + if time.Now().After(dbUser.VerificationExpiration) { + return user.User{}, user.ErrVerificationCodeExpired + } + + return v.userRepository.ClearVerificationCode(userId) +} diff --git a/test/account_verification_http_test.go b/test/account_verification_http_test.go new file mode 100644 index 0000000..8489c54 --- /dev/null +++ b/test/account_verification_http_test.go @@ -0,0 +1,244 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newAccountVerificationRouter(userRepo *mockUserRepo, mailRepo *mockMailRepo, token *jwtauth.JWTAuth) *chi.Mux { + hasher := &stubHasher{} + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), + true, + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/auth", func(r chi.Router) { + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Post("/verify", authHandler.Verify) + }) + }) + }) + return r +} + +func newAccountVerificationTestUserRepo() *mockUserRepo { + repo := newMockUserRepo() + repo.users[7] = user.User{ + ID: 7, + Username: "alice", + IsVerified: false, + VerificationCode: "hashed:abc123", + VerificationExpiration: time.Now().Add(time.Hour), + } + repo.nextID = 7 + return repo +} + +func sendVerifyRequest(router *chi.Mux, token *jwtauth.JWTAuth, claims map[string]any, code string) *httptest.ResponseRecorder { + body, _ := json.Marshal(map[string]string{"code": code}) + req := httptest.NewRequest(http.MethodPost, "/v1/auth/verify", bytes.NewReader(body)) + if claims != nil { + _, tokenString, _ := token.Encode(claims) + req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func TestAccountVerificationHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + t.Run("A logged-in user verifies their account with the correct emailed code", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "abc123") + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + var got user.User + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if !got.IsVerified { + t.Errorf("expected is_verified true") + } + if got.VerificationCode != "" { + t.Errorf("expected verification_code empty, got %q", got.VerificationCode) + } + stored := repo.users[7] + if !stored.IsVerified || stored.VerificationCode != "" { + t.Errorf("expected stored record updated, got %+v", stored) + } + }) + + t.Run("A wrong code is refused and nothing changes", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "wrong-code") + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.VerificationFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + stored := repo.users[7] + if stored.IsVerified || stored.VerificationCode != "hashed:abc123" { + t.Errorf("expected stored record unchanged, got %+v", stored) + } + }) + + t.Run("An expired code is refused and nothing changes", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + expired := repo.users[7] + expired.VerificationExpiration = time.Now().Add(-time.Hour) + repo.users[7] = expired + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "abc123") + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.VerificationFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + stored := repo.users[7] + if stored.IsVerified || stored.VerificationCode != "hashed:abc123" { + t.Errorf("expected stored record unchanged, got %+v", stored) + } + }) + + t.Run("The code is cleared once used, so it cannot be replayed", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + first := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "abc123") + if first.Code != http.StatusOK { + t.Fatalf("expected first status 200, got %d: %s", first.Code, first.Body.String()) + } + + second := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "abc123") + if second.Code != http.StatusForbidden { + t.Fatalf("expected second status 403, got %d: %s", second.Code, second.Body.String()) + } + if second.Body.String() != apperr.VerificationFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", second.Body.String()) + } + }) + + t.Run("A token without a valid user_id returns 401 instead of crashing", func(t *testing.T) { + cases := []struct { + name string + claims map[string]any + }{ + {"no user_id key at all", map[string]any{"role": "user"}}, + {"user_id is a string", map[string]any{"user_id": "seven"}}, + {"user_id does not match any user", map[string]any{"user_id": uint32(999)}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, tc.claims, "abc123") + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + stored := repo.users[7] + if stored.IsVerified { + t.Errorf("expected stored record unchanged, got %+v", stored) + } + }) + } + }) + + t.Run("A repository failure unrelated to the code returns 500", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + repo.fail.failNext("ClearVerificationCode") + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "abc123") + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != "Internal server error\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("A malformed request body returns 400", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + req := httptest.NewRequest(http.MethodPost, "/v1/auth/verify", bytes.NewReader([]byte("not-json"))) + _, tokenString, _ := token.Encode(map[string]any{"user_id": uint32(7)}) + req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.BodyErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("A missing code fails validation and returns 400", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, map[string]any{"user_id": uint32(7)}, "") + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d: %s", rec.Code, rec.Body.String()) + } + }) + + t.Run("A request with no jwt cookie is answered 401 by jwtauth itself", func(t *testing.T) { + repo := newAccountVerificationTestUserRepo() + router := newAccountVerificationRouter(repo, newMockMailRepo(), token) + + rec := sendVerifyRequest(router, token, nil, "abc123") + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d: %s", rec.Code, rec.Body.String()) + } + }) +} diff --git a/test/account_verification_test.go b/test/account_verification_test.go new file mode 100644 index 0000000..43103e0 --- /dev/null +++ b/test/account_verification_test.go @@ -0,0 +1,81 @@ +package test + +import ( + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "testing" + "time" +) + +func TestAccountVerificationUseCase(t *testing.T) { + t.Run("The use case itself enforces the rule, independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ + ID: 3, + VerificationCode: "hashed:right-code", + VerificationExpiration: time.Now().Add(time.Hour), + } + uc := useCases.NewVerifyUserUseCase(repo, &stubHasher{}) + + _, err := uc.Verify(3, "wrong-code") + + if !errors.Is(err, user.ErrInvalidVerificationCode) { + t.Fatalf("expected ErrInvalidVerificationCode, got %v", err) + } + if repo.clearVerificationCodeCalls != 0 { + t.Errorf("expected ClearVerificationCode to never be called, got %d calls", repo.clearVerificationCodeCalls) + } + }) + + t.Run("The use case rejects an expired code independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ + ID: 3, + VerificationCode: "hashed:right-code", + VerificationExpiration: time.Now().Add(-time.Hour), + } + uc := useCases.NewVerifyUserUseCase(repo, &stubHasher{}) + + _, err := uc.Verify(3, "right-code") + + if !errors.Is(err, user.ErrVerificationCodeExpired) { + t.Fatalf("expected ErrVerificationCodeExpired, got %v", err) + } + if repo.clearVerificationCodeCalls != 0 { + t.Errorf("expected ClearVerificationCode to never be called, got %d calls", repo.clearVerificationCodeCalls) + } + }) + + t.Run("The use case succeeds and clears the code independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ + ID: 3, + VerificationCode: "hashed:right-code", + VerificationExpiration: time.Now().Add(time.Hour), + } + uc := useCases.NewVerifyUserUseCase(repo, &stubHasher{}) + + result, err := uc.Verify(3, "right-code") + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if !result.IsVerified { + t.Errorf("expected IsVerified true") + } + if result.VerificationCode != "" { + t.Errorf("expected VerificationCode empty, got %q", result.VerificationCode) + } + if repo.clearVerificationCodeCalls != 1 { + t.Errorf("expected ClearVerificationCode to be called exactly once, got %d calls", repo.clearVerificationCodeCalls) + } + if len(repo.clearVerificationCodeUserIds) != 1 || repo.clearVerificationCodeUserIds[0] != 3 { + t.Errorf("expected ClearVerificationCode to be called with userId 3, got %v", repo.clearVerificationCodeUserIds) + } + stored := repo.users[3] + if !stored.IsVerified || stored.VerificationCode != "" { + t.Errorf("expected stored record updated, got %+v", stored) + } + }) +} diff --git a/test/article_publishing_http_test.go b/test/article_publishing_http_test.go index 6e8c17c..3d72ec9 100644 --- a/test/article_publishing_http_test.go +++ b/test/article_publishing_http_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -18,12 +19,16 @@ import ( ) func newArticlePublishingRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) r := chi.NewRouter() @@ -34,6 +39,7 @@ func newArticlePublishingRouter(articleRepo *mockArticleRepo, token *jwtauth.JWT r.Get("/", articleHandler.ListArticles) r.Group(func(r chi.Router) { r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Get("/{id}", articleHandler.GetArticle) r.Post("/", articleHandler.PostArticle) r.Delete("/{id}", articleHandler.DeleteArticle) diff --git a/test/article_title_rules_test.go b/test/article_title_rules_test.go index e5c6fb8..fd4b71b 100644 --- a/test/article_title_rules_test.go +++ b/test/article_title_rules_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -21,12 +22,16 @@ import ( ) func newArticleTitleRulesRouter(repo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(repo), useCases.NewGetArticleUseCase(repo), useCases.NewListArticlesUseCase(repo), useCases.NewDeleteArticleUseCase(repo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(repo), + getUserUseCase, ) r := chi.NewRouter() @@ -34,6 +39,7 @@ func newArticleTitleRulesRouter(repo *mockArticleRepo, token *jwtauth.JWTAuth) * r.Group(func(r chi.Router) { r.Use(jwtauth.Verifier(token)) r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Route("/article", func(r chi.Router) { r.Post("/", articleHandler.PostArticle) }) diff --git a/test/cancel_pending_registration_http_test.go b/test/cancel_pending_registration_http_test.go new file mode 100644 index 0000000..8e22a16 --- /dev/null +++ b/test/cancel_pending_registration_http_test.go @@ -0,0 +1,230 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newCancelRegistrationRouter(userRepo *mockUserRepo, mailRepo *mockMailRepo, token *jwtauth.JWTAuth) *chi.Mux { + hasher := &stubHasher{} + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), + true, + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/auth", func(r chi.Router) { + r.Post("/register", authHandler.Register) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Use(jwtauth.Authenticator(token)) + r.Post("/verify", authHandler.Verify) + r.Delete("/register", authHandler.CancelRegistration) + }) + }) + }) + return r +} + +func newCancelRegistrationTestUserRepo(isVerified bool) *mockUserRepo { + repo := newMockUserRepo() + repo.users[7] = user.User{ + ID: 7, + Username: "alice", + Email: "alice@example.com", + IsVerified: isVerified, + } + repo.nextID = 7 + return repo +} + +func sendCancelRegistrationRequest(router *chi.Mux, token *jwtauth.JWTAuth, claims map[string]any) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodDelete, "/v1/auth/register", nil) + if claims != nil { + _, tokenString, _ := token.Encode(claims) + req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func jwtSetCookie(t *testing.T, rec *httptest.ResponseRecorder) (*http.Cookie, bool) { + t.Helper() + for _, value := range rec.Result().Header.Values("Set-Cookie") { + cookie, err := http.ParseSetCookie(value) + if err != nil { + t.Fatalf("failed to parse Set-Cookie header %q: %v", value, err) + } + if cookie.Name == "jwt" { + return cookie, true + } + } + return nil, false +} + +func TestCancelPendingRegistrationHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + t.Run("An unverified user cancels their pending registration", func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(false) + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + rec := sendCancelRegistrationRequest(router, token, map[string]any{"user_id": uint32(7)}) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if _, ok := body["message"]; !ok { + t.Errorf("expected a message field in the response body") + } + cookie, ok := jwtSetCookie(t, rec) + if !ok { + t.Fatalf("expected a jwt Set-Cookie header") + } + if cookie.Value != "" || cookie.MaxAge >= 0 { + t.Errorf("expected the jwt cookie to be cleared, got %+v", cookie) + } + if _, err := repo.Get(7); err == nil { + t.Errorf("expected user 7 to no longer exist") + } else if err != user.ErrUserNotFound { + t.Errorf("expected user.ErrUserNotFound, got %v", err) + } + }) + + t.Run("Cancelling frees the username and email for a new registration", func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(false) + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + cancelRec := sendCancelRegistrationRequest(router, token, map[string]any{"user_id": uint32(7)}) + if cancelRec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", cancelRec.Code, cancelRec.Body.String()) + } + + registerRec := registerRequest(t, router, "alice", "hunter2example12", "alice@example.com") + if registerRec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", registerRec.Code, registerRec.Body.String()) + } + if _, ok := jwtSetCookie(t, registerRec); !ok { + t.Fatalf("expected a jwt Set-Cookie header on registration") + } + + newUser, err := repo.GetByUsername("alice") + if err != nil { + t.Fatalf("expected alice to be registered again: %v", err) + } + if newUser.ID == 7 { + t.Errorf("expected a new id different from 7, got %d", newUser.ID) + } + }) + + t.Run("An already-verified user cannot cancel their account this way", func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(true) + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + rec := sendCancelRegistrationRequest(router, token, map[string]any{"user_id": uint32(7)}) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.AlreadyVerifiedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if _, ok := jwtSetCookie(t, rec); ok { + t.Errorf("expected no jwt Set-Cookie header") + } + stored, err := repo.Get(7) + if err != nil { + t.Fatalf("expected user 7 to still exist: %v", err) + } + if !stored.IsVerified { + t.Errorf("expected stored user to remain verified") + } + }) + + t.Run("A token without a valid user_id returns 401 instead of crashing", func(t *testing.T) { + cases := []struct { + name string + claims map[string]any + }{ + {"no user_id key at all", map[string]any{"role": "user"}}, + {"user_id is a string", map[string]any{"user_id": "seven"}}, + {"user_id does not match any user", map[string]any{"user_id": uint32(999)}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(false) + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + rec := sendCancelRegistrationRequest(router, token, tc.claims) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if _, ok := jwtSetCookie(t, rec); ok { + t.Errorf("expected no jwt Set-Cookie header") + } + if _, err := repo.Get(7); err != nil { + t.Errorf("expected user 7 to still exist: %v", err) + } + }) + } + }) + + t.Run("A repository failure unrelated to verification returns 500", func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(false) + repo.fail.failNext("Delete") + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + rec := sendCancelRegistrationRequest(router, token, map[string]any{"user_id": uint32(7)}) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != "Internal server error\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("A request with no jwt cookie never reaches the handler", func(t *testing.T) { + repo := newCancelRegistrationTestUserRepo(false) + router := newCancelRegistrationRouter(repo, newMockMailRepo(), token) + + rec := sendCancelRegistrationRequest(router, token, nil) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d: %s", rec.Code, rec.Body.String()) + } + if _, err := repo.Get(7); err != nil { + t.Errorf("expected user 7 to still exist: %v", err) + } + }) +} diff --git a/test/cancel_pending_registration_test.go b/test/cancel_pending_registration_test.go new file mode 100644 index 0000000..6eb4518 --- /dev/null +++ b/test/cancel_pending_registration_test.go @@ -0,0 +1,51 @@ +package test + +import ( + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "testing" +) + +func TestCancelRegistrationUseCase(t *testing.T) { + t.Run("The use case deletes an unverified user, independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ID: 3, IsVerified: false} + uc := useCases.NewCancelRegistrationUseCase(repo) + + err := uc.CancelRegistration(3) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if _, ok := repo.users[3]; ok { + t.Errorf("expected user 3 to be removed from the repository") + } + }) + + t.Run("The use case refuses to delete an already-verified user, independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ID: 3, IsVerified: true} + uc := useCases.NewCancelRegistrationUseCase(repo) + + err := uc.CancelRegistration(3) + + if !errors.Is(err, user.ErrAlreadyVerified) { + t.Fatalf("expected user.ErrAlreadyVerified, got %v", err) + } + if _, ok := repo.users[3]; !ok { + t.Errorf("expected user 3 to still be present in the repository") + } + }) + + t.Run("The use case reports an unknown user, independently of any handler", func(t *testing.T) { + repo := newMockUserRepo() + uc := useCases.NewCancelRegistrationUseCase(repo) + + err := uc.CancelRegistration(42) + + if !errors.Is(err, user.ErrUserNotFound) { + t.Fatalf("expected user.ErrUserNotFound, got %v", err) + } + }) +} diff --git a/test/delete_article_cascades_images_test.go b/test/delete_article_cascades_images_test.go index a31c04d..127de76 100644 --- a/test/delete_article_cascades_images_test.go +++ b/test/delete_article_cascades_images_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -82,12 +83,16 @@ func TestDeleteArticleUseCaseCascadesImageDeletion(t *testing.T) { } func newDeleteArticleCascadesImagesRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) r := chi.NewRouter() @@ -95,6 +100,7 @@ func newDeleteArticleCascadesImagesRouter(articleRepo *mockArticleRepo, imageRep r.Group(func(r chi.Router) { r.Use(jwtauth.Verifier(token)) r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Route("/article", func(r chi.Router) { r.Delete("/{id}", articleHandler.DeleteArticle) }) diff --git a/test/edit_article_body_http_test.go b/test/edit_article_body_http_test.go index ba77fe5..35ca079 100644 --- a/test/edit_article_body_http_test.go +++ b/test/edit_article_body_http_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -19,12 +20,16 @@ import ( ) func newEditArticleBodyRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) r := chi.NewRouter() @@ -34,6 +39,7 @@ func newEditArticleBodyRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAu r.Use(jwtauth.Verifier(token)) r.Group(func(r chi.Router) { r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Patch("/{id}", articleHandler.UpdateArticleBody) }) }) diff --git a/test/hasher_test.go b/test/hasher_test.go index 9e9d5f6..ab07ef2 100644 --- a/test/hasher_test.go +++ b/test/hasher_test.go @@ -14,6 +14,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/go-chi/chi/v5" "github.com/go-chi/jwtauth/v5" @@ -95,7 +96,7 @@ func TestUpdateUserUseCaseHashesUpdatedValues(t *testing.T) { method: "UpdatePasswordResetCode", plaintext: "reset-code-123", run: func(uc *useCases.UpdateUserUseCase, plaintext string) (user.User, error) { - return uc.UpdatePasswordResetCode(1, plaintext) + return uc.UpdatePasswordResetCode(1, plaintext, time.Now().Add(time.Hour)) }, stored: func(u user.User) string { return u.PasswordResetCode }, }, @@ -218,6 +219,10 @@ func newHasherRouter(userRepo *mockUserRepo, hasher *stubHasher) *chi.Mux { useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), true, ) @@ -238,6 +243,10 @@ func newRealHasherRouter(userRepo *mockUserRepo) *chi.Mux { useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), true, ) diff --git a/test/not_found_errors_test.go b/test/not_found_errors_test.go index ad42172..09c7ed3 100644 --- a/test/not_found_errors_test.go +++ b/test/not_found_errors_test.go @@ -3,6 +3,7 @@ package test import ( "RenewCMS/api/handlers/article" "RenewCMS/api/handlers/auth" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -124,12 +125,14 @@ func TestUserRepositoryReportsMissingUserAsDomainError(t *testing.T) { } func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, token *jwtauth.JWTAuth) *chi.Mux { + getUserUseCase := useCases.NewGetUserUseCase(userRepo) articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) hasher := hashing.NewBcryptHasher() authHandler := auth.NewHandler( @@ -138,6 +141,10 @@ func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRep useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), true, ) @@ -146,6 +153,7 @@ func newNotFoundErrorsRouter(articleRepo *mockArticleRepo, userRepo *mockUserRep r.Group(func(r chi.Router) { r.Use(jwtauth.Verifier(token)) r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Route("/article", func(r chi.Router) { r.Get("/{id}", articleHandler.GetArticle) }) @@ -178,7 +186,7 @@ func TestNotFoundErrorsHttp(t *testing.T) { } t.Run("Fetching a nonexistent article answers 404, not 500", func(t *testing.T) { - router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + router := newNotFoundErrorsRouter(newArticleRepo(), newVerifiedUserRepo(), token) req := httptest.NewRequest(http.MethodGet, "/v1/article/9999", nil) setJwtCookie(t, req, token, 1) rec := httptest.NewRecorder() @@ -193,7 +201,7 @@ func TestNotFoundErrorsHttp(t *testing.T) { }) t.Run("Fetching an offline article still answers 404 with the same message", func(t *testing.T) { - router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + router := newNotFoundErrorsRouter(newArticleRepo(), newVerifiedUserRepo(), token) req := httptest.NewRequest(http.MethodGet, "/v1/article/2", nil) setJwtCookie(t, req, token, 1) rec := httptest.NewRecorder() @@ -208,7 +216,7 @@ func TestNotFoundErrorsHttp(t *testing.T) { }) t.Run("Fetching an online article still answers 200", func(t *testing.T) { - router := newNotFoundErrorsRouter(newArticleRepo(), newMockUserRepo(), token) + router := newNotFoundErrorsRouter(newArticleRepo(), newVerifiedUserRepo(), token) req := httptest.NewRequest(http.MethodGet, "/v1/article/1", nil) setJwtCookie(t, req, token, 1) rec := httptest.NewRecorder() diff --git a/test/public_article_listing_test.go b/test/public_article_listing_test.go index ab5f563..67a6db1 100644 --- a/test/public_article_listing_test.go +++ b/test/public_article_listing_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" domainArticle "RenewCMS/internal/domain/article" "RenewCMS/internal/infrastructure/useCases" "bytes" @@ -37,12 +38,16 @@ func (w *erroringResponseWriter) Write([]byte) (int, error) { func (w *erroringResponseWriter) WriteHeader(int) {} func newPublicArticleListingRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) r := chi.NewRouter() @@ -54,6 +59,7 @@ func newPublicArticleListingRouter(articleRepo *mockArticleRepo, token *jwtauth. }) r.Group(func(r chi.Router) { r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Get("/{id}", articleHandler.GetArticle) r.Post("/", articleHandler.PostArticle) r.Delete("/{id}", articleHandler.DeleteArticle) diff --git a/test/registration_gate_and_verification_email_test.go b/test/registration_gate_and_verification_email_test.go index b697172..99621ae 100644 --- a/test/registration_gate_and_verification_email_test.go +++ b/test/registration_gate_and_verification_email_test.go @@ -46,6 +46,10 @@ func newRegistrationGateRouter(userRepo *mockUserRepo, mailRepo *mockMailRepo, p useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, publicURL), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, publicURL), + useCases.NewResetPasswordUseCase(userRepo, hasher), true, ) diff --git a/test/request_password_reset_http_test.go b/test/request_password_reset_http_test.go new file mode 100644 index 0000000..d6163bf --- /dev/null +++ b/test/request_password_reset_http_test.go @@ -0,0 +1,243 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newRequestPasswordResetRouter(userRepo *mockUserRepo, mailRepo *mockMailRepo, publicURL string) *chi.Mux { + hasher := &stubHasher{} + token := jwtauth.New("HS256", []byte("test-secret"), nil) + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, publicURL), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, publicURL), + useCases.NewResetPasswordUseCase(userRepo, hasher), + true, + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/auth", func(r chi.Router) { + r.Post("/password-reset", authHandler.RequestPasswordReset) + }) + }) + return r +} + +func sendPasswordResetRequest(router *chi.Mux, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodPost, "/v1/auth/password-reset", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, req) + return recorder +} + +const requestPasswordResetGenericBody = `{"message":"If an account exists for that email, a password reset link has been sent."}` + +func TestRequestPasswordResetHTTP(t *testing.T) { + validator.InitValidator() + + t.Run("Requesting a reset for an existing email sends the reset email and returns the generic message", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com", IsVerified: true} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + recorder := sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + + if recorder.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", recorder.Code) + } + if strings.TrimSpace(recorder.Body.String()) != requestPasswordResetGenericBody { + t.Errorf("unexpected body: %s", recorder.Body.String()) + } + if len(mailRepo.SentEmails) != 1 { + t.Fatalf("expected exactly one email, got %d", len(mailRepo.SentEmails)) + } + if mailRepo.SentEmails[0].Address != "alice@example.com" { + t.Errorf("expected email to alice@example.com, got %s", mailRepo.SentEmails[0].Address) + } + if mailRepo.SentEmails[0].Template != "passwordReset" { + t.Errorf("expected template passwordReset, got %s", mailRepo.SentEmails[0].Template) + } + }) + + t.Run("Requesting a reset for an email with no account returns the identical response and sends no email", func(t *testing.T) { + userRepo := newMockUserRepo() + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + recorder := sendPasswordResetRequest(router, `{"email": "nobody@example.com"}`) + + if recorder.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", recorder.Code) + } + if strings.TrimSpace(recorder.Body.String()) != requestPasswordResetGenericBody { + t.Errorf("unexpected body: %s", recorder.Body.String()) + } + if len(mailRepo.SentEmails) != 0 { + t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) + } + }) + + t.Run("The reset email's Host reflects the injected public URL", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com"} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + + if len(mailRepo.SentEmails) != 1 { + t.Fatalf("expected exactly one email, got %d", len(mailRepo.SentEmails)) + } + data, ok := mailRepo.SentEmails[0].Data.(useCases.MailPasswordResetData) + if !ok { + t.Fatalf("expected data of type MailPasswordResetData, got %T", mailRepo.SentEmails[0].Data) + } + if data.Host != "https://cms.example.com" { + t.Errorf("expected Host https://cms.example.com, got %s", data.Host) + } + }) + + t.Run("The stored reset code expires one hour after creation", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com"} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + + stored := userRepo.users[1] + want := time.Now().Add(time.Hour) + if stored.PasswordResetExpiration.Sub(want).Abs() > 5*time.Second { + t.Errorf("expected expiration near %v, got %v", want, stored.PasswordResetExpiration) + } + }) + + t.Run("A second request overwrites the first code, invalidating it", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com"} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + + if len(mailRepo.SentEmails) != 2 { + t.Fatalf("expected exactly two emails, got %d", len(mailRepo.SentEmails)) + } + firstData := mailRepo.SentEmails[0].Data.(useCases.MailPasswordResetData) + secondData := mailRepo.SentEmails[1].Data.(useCases.MailPasswordResetData) + if firstData.VerificationCode == secondData.VerificationCode { + t.Errorf("expected the second code to differ from the first") + } + + hasher := &stubHasher{} + stored := userRepo.users[1] + if hasher.Compare(stored.PasswordResetCode, firstData.VerificationCode) == nil { + t.Errorf("expected the stored code to no longer match the first raw code") + } + if hasher.Compare(stored.PasswordResetCode, secondData.VerificationCode) != nil { + t.Errorf("expected the stored code to match the second raw code") + } + }) + + t.Run("A mail-send failure for an existing user still returns the same generic response", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com"} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + mailRepo.fail.failNext("Send") + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + recorder := sendPasswordResetRequest(router, `{"email": "alice@example.com"}`) + + if recorder.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", recorder.Code) + } + if strings.TrimSpace(recorder.Body.String()) != requestPasswordResetGenericBody { + t.Errorf("unexpected body: %s", recorder.Body.String()) + } + if recorder.Header().Get("Set-Cookie") != "" { + t.Errorf("expected no jwt cookie to be set, got %s", recorder.Header().Get("Set-Cookie")) + } + }) + + t.Run("An unverified user can request a password reset exactly like a verified one", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "bob", Email: "bob@example.com", IsVerified: false} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + recorder := sendPasswordResetRequest(router, `{"email": "bob@example.com"}`) + + if recorder.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", recorder.Code) + } + if strings.TrimSpace(recorder.Body.String()) != requestPasswordResetGenericBody { + t.Errorf("unexpected body: %s", recorder.Body.String()) + } + if len(mailRepo.SentEmails) != 1 { + t.Fatalf("expected exactly one email, got %d", len(mailRepo.SentEmails)) + } + if mailRepo.SentEmails[0].Address != "bob@example.com" { + t.Errorf("expected email to bob@example.com, got %s", mailRepo.SentEmails[0].Address) + } + if mailRepo.SentEmails[0].Template != "passwordReset" { + t.Errorf("expected template passwordReset, got %s", mailRepo.SentEmails[0].Template) + } + }) + + t.Run("A malformed request is still rejected, since this is not an enumeration concern", func(t *testing.T) { + cases := []struct { + name string + body string + }{ + {"missing email", `{}`}, + {"malformed email", `{"email": "not-an-email"}`}, + {"non-json body", `not-json`}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com"} + userRepo.nextID = 1 + mailRepo := newMockMailRepo() + router := newRequestPasswordResetRouter(userRepo, mailRepo, "https://cms.example.com") + + recorder := sendPasswordResetRequest(router, tc.body) + + if recorder.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", recorder.Code) + } + if len(mailRepo.SentEmails) != 0 { + t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) + } + }) + } + }) +} diff --git a/test/request_password_reset_test.go b/test/request_password_reset_test.go new file mode 100644 index 0000000..f0e642a --- /dev/null +++ b/test/request_password_reset_test.go @@ -0,0 +1,140 @@ +package test + +import ( + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "testing" + "time" +) + +func newRequestPasswordResetTestUserRepo() *mockUserRepo { + repo := newMockUserRepo() + repo.users[3] = user.User{ID: 3, Email: "alice@example.com"} + repo.nextID = 3 + return repo +} + +func TestRequestPasswordResetUseCase(t *testing.T) { + t.Run("The use case itself hashes the code, stores it with a one-hour expiration, and sends the email", func(t *testing.T) { + repo := newRequestPasswordResetTestUserRepo() + mailRepo := newMockMailRepo() + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("alice@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + if repo.updatePasswordResetCodeCalls != 1 { + t.Fatalf("expected UpdatePasswordResetCode to be called once, got %d", repo.updatePasswordResetCodeCalls) + } + if repo.lastUpdatePasswordResetCodeUserId != 3 { + t.Errorf("expected userId 3, got %d", repo.lastUpdatePasswordResetCodeUserId) + } + if got, want := repo.lastUpdatePasswordResetCodeExpiration, time.Now().Add(time.Hour); got.Sub(want).Abs() > 5*time.Second { + t.Errorf("expected expiration near %v, got %v", want, got) + } + + if len(mailRepo.SentEmails) != 1 { + t.Fatalf("expected exactly one email, got %d", len(mailRepo.SentEmails)) + } + sent := mailRepo.SentEmails[0] + if sent.Address != "alice@example.com" { + t.Errorf("expected email to alice@example.com, got %s", sent.Address) + } + if sent.Template != "passwordReset" { + t.Errorf("expected template passwordReset, got %s", sent.Template) + } + data, ok := sent.Data.(useCases.MailPasswordResetData) + if !ok { + t.Fatalf("expected data of type MailPasswordResetData, got %T", sent.Data) + } + if data.Host != "https://cms.example.com" { + t.Errorf("expected Host https://cms.example.com, got %s", data.Host) + } + if (&stubHasher{}).Compare(repo.lastUpdatePasswordResetCodeCode, data.VerificationCode) != nil { + t.Errorf("expected stored hash to match the raw code sent in the email") + } + }) + + t.Run("The use case swallows an unknown email and returns nil without touching the repository or mailer", func(t *testing.T) { + repo := newMockUserRepo() + mailRepo := newMockMailRepo() + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("nobody@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if repo.updatePasswordResetCodeCalls != 0 { + t.Errorf("expected UpdatePasswordResetCode to never be called, got %d calls", repo.updatePasswordResetCodeCalls) + } + if len(mailRepo.SentEmails) != 0 { + t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) + } + }) + + t.Run("The use case swallows a mail repository failure and still returns nil", func(t *testing.T) { + repo := newRequestPasswordResetTestUserRepo() + mailRepo := newMockMailRepo() + mailRepo.fail.failNext("Send") + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("alice@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if repo.updatePasswordResetCodeCalls != 1 { + t.Errorf("expected UpdatePasswordResetCode to be called once, got %d", repo.updatePasswordResetCodeCalls) + } + }) + + t.Run("The use case swallows a hash failure and returns nil without storing or sending", func(t *testing.T) { + repo := newRequestPasswordResetTestUserRepo() + mailRepo := newMockMailRepo() + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{hashErr: errStubHashFailure}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("alice@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if repo.updatePasswordResetCodeCalls != 0 { + t.Errorf("expected UpdatePasswordResetCode to never be called, got %d calls", repo.updatePasswordResetCodeCalls) + } + if len(mailRepo.SentEmails) != 0 { + t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) + } + }) + + t.Run("The use case swallows a store failure and returns nil without sending", func(t *testing.T) { + repo := newRequestPasswordResetTestUserRepo() + repo.fail.failNext("UpdatePasswordResetCode") + mailRepo := newMockMailRepo() + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("alice@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if len(mailRepo.SentEmails) != 0 { + t.Errorf("expected no email to be sent, got %d", len(mailRepo.SentEmails)) + } + }) + + t.Run("An unverified user can request a password reset exactly like a verified one", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[1] = user.User{ID: 1, Email: "alice@example.com", IsVerified: false} + repo.users[2] = user.User{ID: 2, Email: "bob@example.com", IsVerified: false} + repo.nextID = 2 + mailRepo := newMockMailRepo() + uc := useCases.NewRequestPasswordResetUseCase(repo, &stubHasher{}, mailRepo, "https://cms.example.com") + + err := uc.RequestPasswordReset("alice@example.com") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if len(mailRepo.SentEmails) != 1 { + t.Fatalf("expected exactly one email, got %d", len(mailRepo.SentEmails)) + } + }) +} diff --git a/test/reset_password_http_test.go b/test/reset_password_http_test.go new file mode 100644 index 0000000..d58ecce --- /dev/null +++ b/test/reset_password_http_test.go @@ -0,0 +1,280 @@ +package test + +import ( + "RenewCMS/api/handlers/auth" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newResetPasswordRouter(userRepo *mockUserRepo, hasher *stubHasher) *chi.Mux { + token := jwtauth.New("HS256", []byte("test-secret"), nil) + mailRepo := newMockMailRepo() + authHandler := auth.NewHandler( + token, + useCases.NewGetUserUseCase(userRepo), + useCases.NewListUsersUseCase(userRepo), + useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), + true, + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/auth", func(r chi.Router) { + r.Post("/password-reset/confirm", authHandler.ResetPassword) + }) + }) + return r +} + +func sendResetPasswordRequest(router *chi.Mux, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodPost, "/v1/auth/password-reset/confirm", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func newAliceResetPasswordRepo(expiration time.Time) *mockUserRepo { + repo := newMockUserRepo() + repo.users[1] = user.User{ + ID: 1, + Username: "alice", + Email: "alice@example.com", + Password: "hashed:oldpassword", + PasswordResetCode: "hashed:reset-code-1", + PasswordResetExpiration: expiration, + } + repo.nextID = 1 + return repo +} + +func TestResetPasswordHTTP(t *testing.T) { + validator.InitValidator() + + t.Run("A correct, unexpired code sets the new password and cannot be reused", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + firstBody := `{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}` + first := sendResetPasswordRequest(router, firstBody) + + if first.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", first.Code, first.Body.String()) + } + if first.Body.String() != `{"message":"Your password has been reset. You can now log in."}` { + t.Errorf("unexpected body: %s", first.Body.String()) + } + if first.Header().Get("Set-Cookie") != "" { + t.Errorf("expected no jwt cookie, got %s", first.Header().Get("Set-Cookie")) + } + + stored := userRepo.users[1] + if stored.Password != "hashed:newpassword1" { + t.Errorf("expected stored password %q, got %q", "hashed:newpassword1", stored.Password) + } + if stored.PasswordResetCode != "" { + t.Errorf("expected empty PasswordResetCode, got %q", stored.PasswordResetCode) + } + if !stored.PasswordResetExpiration.IsZero() { + t.Errorf("expected zero PasswordResetExpiration, got %v", stored.PasswordResetExpiration) + } + + second := sendResetPasswordRequest(router, firstBody) + + if second.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", second.Code, second.Body.String()) + } + if second.Body.String() != apperr.PasswordResetFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", second.Body.String()) + } + if userRepo.users[1].Password != "hashed:newpassword1" { + t.Errorf("expected password to remain from the first request, got %q", userRepo.users[1].Password) + } + }) + + t.Run("A wrong code is refused with the generic message and nothing changes", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "wrong-code", "password": "newpassword1"}`) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.PasswordResetFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + stored := userRepo.users[1] + if stored.PasswordResetCode != "hashed:reset-code-1" || stored.Password != "hashed:oldpassword" { + t.Errorf("expected record unchanged, got %+v", stored) + } + }) + + t.Run("An expired code is refused with the same generic message and nothing changes", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(-time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}`) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.PasswordResetFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + stored := userRepo.users[1] + if stored.PasswordResetCode != "hashed:reset-code-1" || stored.Password != "hashed:oldpassword" { + t.Errorf("expected record unchanged, got %+v", stored) + } + }) + + t.Run("An unknown email is refused with the exact same status and body as a wrong code", func(t *testing.T) { + userRepo := newMockUserRepo() + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "nobody@example.com", "code": "any-code", "password": "newpassword1"}`) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.PasswordResetFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("A user who never requested a reset gets the same refusal, since the stored code is empty", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.users[1] = user.User{ID: 1, Username: "alice", Email: "alice@example.com", PasswordResetCode: ""} + userRepo.nextID = 1 + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "anything", "password": "newpassword1"}`) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.PasswordResetFailedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("A hashing failure is reported as an internal server error", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{hashErr: errRepositoryFailure} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}`) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d: %s", rec.Code, rec.Body.String()) + } + if userRepo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", userRepo.resetPasswordCalls) + } + }) + + t.Run("A repository failure while writing the reset is reported as an internal server error", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + userRepo.fail.failNext("ResetPassword") + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "reset-code-1", "password": "newpassword1"}`) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d: %s", rec.Code, rec.Body.String()) + } + }) + + t.Run("A 7-character password is rejected as too short and nothing changes", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "reset-code-1", "password": "1234567"}`) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d: %s", rec.Code, rec.Body.String()) + } + if userRepo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", userRepo.resetPasswordCalls) + } + stored := userRepo.users[1] + if stored.PasswordResetCode != "hashed:reset-code-1" || stored.Password != "hashed:oldpassword" { + t.Errorf("expected record unchanged, got %+v", stored) + } + }) + + t.Run("An exactly-8-character password with a valid code succeeds", func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, `{"email": "alice@example.com", "code": "reset-code-1", "password": "12345678"}`) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + stored := userRepo.users[1] + if stored.Password != "hashed:12345678" { + t.Errorf("expected stored password %q, got %q", "hashed:12345678", stored.Password) + } + if stored.PasswordResetCode != "" { + t.Errorf("expected empty PasswordResetCode, got %q", stored.PasswordResetCode) + } + }) + + t.Run("A malformed or too-short request is rejected before any lookup happens", func(t *testing.T) { + cases := []struct { + name string + body string + }{ + {"empty body", `{}`}, + {"malformed email", `{"email": "not-an-email", "code": "c", "password": "newpassword1"}`}, + {"empty code", `{"email": "alice@example.com", "code": "", "password": "newpassword1"}`}, + {"short password", `{"email": "alice@example.com", "code": "reset-code-1", "password": "short"}`}, + {"non-json body", `not-json`}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + userRepo := newAliceResetPasswordRepo(time.Now().Add(time.Hour)) + hasher := &stubHasher{} + router := newResetPasswordRouter(userRepo, hasher) + + rec := sendResetPasswordRequest(router, tc.body) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d: %s", rec.Code, rec.Body.String()) + } + if userRepo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", userRepo.resetPasswordCalls) + } + stored := userRepo.users[1] + if stored.PasswordResetCode != "hashed:reset-code-1" || stored.Password != "hashed:oldpassword" { + t.Errorf("expected record unchanged, got %+v", stored) + } + }) + } + }) +} diff --git a/test/reset_password_test.go b/test/reset_password_test.go new file mode 100644 index 0000000..9871d1d --- /dev/null +++ b/test/reset_password_test.go @@ -0,0 +1,164 @@ +package test + +import ( + "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "errors" + "testing" + "time" +) + +func TestResetPasswordUseCase(t *testing.T) { + newRepo := func(expiration time.Time) *mockUserRepo { + repo := newMockUserRepo() + repo.users[3] = user.User{ + ID: 3, + Email: "alice@example.com", + PasswordResetCode: "hashed:reset-code-1", + PasswordResetExpiration: expiration, + } + return repo + } + + t.Run("The use case itself sets the password and clears the code in one write, independently of any handler", func(t *testing.T) { + repo := newRepo(time.Now().Add(time.Hour)) + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("alice@example.com", "reset-code-1", "newpassword1") + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if repo.resetPasswordCalls != 1 { + t.Fatalf("expected ResetPassword to be called exactly once, got %d", repo.resetPasswordCalls) + } + if repo.lastResetPasswordUserId != 3 { + t.Errorf("expected userId 3, got %d", repo.lastResetPasswordUserId) + } + if repo.lastResetPasswordHash != "hashed:newpassword1" { + t.Errorf("expected hash %q, got %q", "hashed:newpassword1", repo.lastResetPasswordHash) + } + }) + + t.Run("The use case rejects a wrong code without touching the repository's ResetPassword method", func(t *testing.T) { + repo := newRepo(time.Now().Add(time.Hour)) + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("alice@example.com", "wrong-code", "newpassword1") + + if !errors.Is(err, user.ErrInvalidPasswordResetCode) { + t.Errorf("expected user.ErrInvalidPasswordResetCode, got %v", err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + }) + + t.Run("The use case rejects an expired code without touching the repository's ResetPassword method", func(t *testing.T) { + repo := newRepo(time.Now().Add(-time.Hour)) + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("alice@example.com", "reset-code-1", "newpassword1") + + if !errors.Is(err, user.ErrPasswordResetCodeExpired) { + t.Errorf("expected user.ErrPasswordResetCodeExpired, got %v", err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + }) + + t.Run("The use case treats an unknown email the same as a wrong code", func(t *testing.T) { + repo := newMockUserRepo() + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("nobody@example.com", "any-code", "newpassword1") + + if !errors.Is(err, user.ErrInvalidPasswordResetCode) { + t.Errorf("expected user.ErrInvalidPasswordResetCode, got %v", err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + }) + + t.Run("The use case treats an empty stored code the same as a wrong code, never calling Compare with an empty hash", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ID: 3, Email: "alice@example.com", PasswordResetCode: ""} + hasher := &stubHasher{} + uc := useCases.NewResetPasswordUseCase(repo, hasher) + + err := uc.ResetPassword("alice@example.com", "anything", "newpassword1") + + if !errors.Is(err, user.ErrInvalidPasswordResetCode) { + t.Errorf("expected user.ErrInvalidPasswordResetCode, got %v", err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + if hasher.compareCalls != 0 { + t.Errorf("expected Compare to never be called, got %d calls", hasher.compareCalls) + } + }) + + t.Run("The use case treats any GetByEmail failure, not just not-found, as an invalid code", func(t *testing.T) { + repo := newRepo(time.Now().Add(time.Hour)) + repo.fail.failNextWith("GetByEmail", errors.New("db is down")) + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("alice@example.com", "reset-code-1", "newpassword1") + + if !errors.Is(err, user.ErrInvalidPasswordResetCode) { + t.Errorf("expected user.ErrInvalidPasswordResetCode, got %v", err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + }) + + t.Run("A hashing failure propagates and the repository's ResetPassword method is never called", func(t *testing.T) { + repo := newRepo(time.Now().Add(time.Hour)) + hashErr := errors.New("hash blew up") + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{hashErr: hashErr}) + + err := uc.ResetPassword("alice@example.com", "reset-code-1", "newpassword1") + + if !errors.Is(err, hashErr) { + t.Errorf("expected %v, got %v", hashErr, err) + } + if repo.resetPasswordCalls != 0 { + t.Errorf("expected ResetPassword to never be called, got %d", repo.resetPasswordCalls) + } + }) + + t.Run("A failure in the repository's ResetPassword method propagates", func(t *testing.T) { + repo := newRepo(time.Now().Add(time.Hour)) + repo.fail.failNext("ResetPassword") + uc := useCases.NewResetPasswordUseCase(repo, &stubHasher{}) + + err := uc.ResetPassword("alice@example.com", "reset-code-1", "newpassword1") + + if !errors.Is(err, errRepositoryFailure) { + t.Errorf("expected %v, got %v", errRepositoryFailure, err) + } + }) + + t.Run("The generic reset-code update stores an empty code as empty instead of hashing it", func(t *testing.T) { + repo := newMockUserRepo() + repo.users[3] = user.User{ID: 3, Email: "alice@example.com"} + hasher := &stubHasher{} + uc := useCases.NewUpdateUserUseCase(repo, hasher) + + _, err := uc.UpdatePasswordResetCode(3, "", time.Time{}) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if hasher.hashCalls != 0 { + t.Errorf("expected Hash to never be called, got %d calls", hasher.hashCalls) + } + if repo.users[3].PasswordResetCode != "" { + t.Errorf("expected stored PasswordResetCode to be empty, got %q", repo.users[3].PasswordResetCode) + } + }) +} diff --git a/test/runtime_settings_injection_http_test.go b/test/runtime_settings_injection_http_test.go index 1c37dce..7506ca1 100644 --- a/test/runtime_settings_injection_http_test.go +++ b/test/runtime_settings_injection_http_test.go @@ -42,6 +42,10 @@ func newSecureCookieAuthRouter(t *testing.T, isProduction bool) (*chi.Mux, *mock useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), isProduction, ) @@ -133,21 +137,22 @@ func newRuntimeSettingsContainer(t *testing.T) *api.UseCases { token := jwtauth.New("HS256", []byte("test-secret"), nil) return &api.UseCases{ - Token: token, - CreateArticleUseCase: useCases.NewCreateArticleUseCase(articleRepo), - GetArticleUseCase: useCases.NewGetArticleUseCase(articleRepo), - ListArticlesUseCase: useCases.NewListArticlesUseCase(articleRepo), - UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(articleRepo), - DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), - GetUserUseCase: useCases.NewGetUserUseCase(userRepo), - CreateUserUseCase: useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), - UpdateUserUseCase: useCases.NewUpdateUserUseCase(userRepo, hasher), - AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(userRepo, hasher), - DeleteUserUseCase: useCases.NewDeleteUserUseCase(userRepo), - ListUsersUseCase: useCases.NewListUsersUseCase(userRepo), - SendMailUseCase: useCases.NewSendMailUseCase(mailRepo), - CreateImageUseCase: useCases.NewCreateImageUseCase(imageRepo), - DeleteImageUseCase: useCases.NewDeleteImageUseCase(imageRepo), + Token: token, + CreateArticleUseCase: useCases.NewCreateArticleUseCase(articleRepo), + GetArticleUseCase: useCases.NewGetArticleUseCase(articleRepo), + ListArticlesUseCase: useCases.NewListArticlesUseCase(articleRepo), + UpdateArticleUseCase: useCases.NewUpdateArticleUseCase(articleRepo), + DeleteArticleUseCase: useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), + GetUserUseCase: useCases.NewGetUserUseCase(userRepo), + CreateUserUseCase: useCases.NewCreateUserUseCase(userRepo, hasher, mailRepo, "https://cms.example.com"), + UpdateUserUseCase: useCases.NewUpdateUserUseCase(userRepo, hasher), + AuthenticateUserUseCase: useCases.NewAuthenticateUserUseCase(userRepo, hasher), + CancelRegistrationUseCase: useCases.NewCancelRegistrationUseCase(userRepo), + DeleteUserUseCase: useCases.NewDeleteUserUseCase(userRepo), + ListUsersUseCase: useCases.NewListUsersUseCase(userRepo), + SendMailUseCase: useCases.NewSendMailUseCase(mailRepo), + CreateImageUseCase: useCases.NewCreateImageUseCase(imageRepo), + DeleteImageUseCase: useCases.NewDeleteImageUseCase(imageRepo), } } diff --git a/test/unverified_users_and_reregistration_test.go b/test/unverified_users_and_reregistration_test.go index 5c4965e..8f88e9b 100644 --- a/test/unverified_users_and_reregistration_test.go +++ b/test/unverified_users_and_reregistration_test.go @@ -14,6 +14,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/go-chi/chi/v5" "github.com/go-chi/jwtauth/v5" @@ -135,7 +136,7 @@ func TestUnverifiedUsersAndReregistrationRepository(t *testing.T) { t.Fatalf("expected bob creation to succeed, got %v", err) } - _, err = repo.UpdatePasswordResetCode(bob.ID, "alice-code") + _, err = repo.UpdatePasswordResetCode(bob.ID, "alice-code", time.Now().Add(time.Hour)) if err == nil { t.Fatal("expected UpdatePasswordResetCode for bob to fail because the code is already active for alice") } @@ -158,6 +159,10 @@ func newUnverifiedRegistrationRouter(userRepo *mockUserRepo, token *jwtauth.JWTA useCases.NewListUsersUseCase(userRepo), useCases.NewCreateUserUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), useCases.NewAuthenticateUserUseCase(userRepo, hasher), + useCases.NewVerifyUserUseCase(userRepo, hasher), + useCases.NewCancelRegistrationUseCase(userRepo), + useCases.NewRequestPasswordResetUseCase(userRepo, hasher, newMockMailRepo(), "https://cms.example.com"), + useCases.NewResetPasswordUseCase(userRepo, hasher), true, ) diff --git a/test/upload_article_image_http_test.go b/test/upload_article_image_http_test.go index 8d2b3cd..b908806 100644 --- a/test/upload_article_image_http_test.go +++ b/test/upload_article_image_http_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/image" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -21,6 +22,9 @@ import ( ) func newUploadArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + imageHandler := image.NewHandler( useCases.NewCreateImageUseCase(imageRepo), useCases.NewGetArticleUseCase(articleRepo), @@ -34,6 +38,7 @@ func newUploadArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockIm r.Use(jwtauth.Verifier(token)) r.Group(func(r chi.Router) { r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Post("/{id}/image", imageHandler.PostImage) }) }) diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index b551a76..caab5bf 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -640,7 +640,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePasswordResetCode_id1", func(t *testing.T) { repo := newRepo() uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - result, err := uc.UpdatePasswordResetCode(1, "reset-code-123") + result, err := uc.UpdatePasswordResetCode(1, "reset-code-123", time.Now().Add(time.Hour)) if err != nil { t.Fatalf("expected no error, got %v", err) } @@ -652,7 +652,7 @@ func TestUpdateUserDelegatesEachOperation(t *testing.T) { t.Run("UpdatePasswordResetCode_id9", func(t *testing.T) { repo := newRepo() uc := useCases.NewUpdateUserUseCase(repo, &stubHasher{}) - _, err := uc.UpdatePasswordResetCode(9, "reset-code-123") + _, err := uc.UpdatePasswordResetCode(9, "reset-code-123", time.Now().Add(time.Hour)) if !errors.Is(err, user.ErrUserNotFound) { t.Errorf("expected user.ErrUserNotFound, got %v", err) } diff --git a/test/user_test.go b/test/user_test.go index 57f03f9..1462d7a 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -6,13 +6,17 @@ import ( "errors" "sort" "testing" + "time" ) type stubHasher struct { - hashErr error + hashErr error + hashCalls int + compareCalls int } func (s *stubHasher) Hash(value string) (string, error) { + s.hashCalls++ if s.hashErr != nil { return "", s.hashErr } @@ -20,6 +24,7 @@ func (s *stubHasher) Hash(value string) (string, error) { } func (s *stubHasher) Compare(hashed string, value string) error { + s.compareCalls++ if hashed == "hashed:"+value { return nil } @@ -27,15 +32,30 @@ func (s *stubHasher) Compare(hashed string, value string) error { } type mockUserRepo struct { - users map[uint32]user.User - nextID uint32 - fail failSwitch + users map[uint32]user.User + nextID uint32 + fail failSwitch + clearVerificationCodeCalls int + clearVerificationCodeUserIds []uint32 + updatePasswordResetCodeCalls int + lastUpdatePasswordResetCodeUserId uint32 + lastUpdatePasswordResetCodeCode string + lastUpdatePasswordResetCodeExpiration time.Time + resetPasswordCalls int + lastResetPasswordUserId uint32 + lastResetPasswordHash string } func newMockUserRepo() *mockUserRepo { return &mockUserRepo{users: make(map[uint32]user.User)} } +func newVerifiedUserRepo() *mockUserRepo { + repo := newMockUserRepo() + repo.users[1] = user.User{ID: 1, IsVerified: true} + return repo +} + func (m *mockUserRepo) sortedIds() []uint32 { ids := make([]uint32, 0, len(m.users)) for id := range m.users { @@ -137,7 +157,11 @@ func (m *mockUserRepo) UpdatePassword(userId uint32, password string) (user.User return u, nil } -func (m *mockUserRepo) UpdatePasswordResetCode(userId uint32, code string) (user.User, error) { +func (m *mockUserRepo) UpdatePasswordResetCode(userId uint32, code string, expiration time.Time) (user.User, error) { + m.updatePasswordResetCodeCalls++ + m.lastUpdatePasswordResetCodeUserId = userId + m.lastUpdatePasswordResetCodeCode = code + m.lastUpdatePasswordResetCodeExpiration = expiration if err := m.fail.trigger("UpdatePasswordResetCode"); err != nil { return user.User{}, err } @@ -146,6 +170,41 @@ func (m *mockUserRepo) UpdatePasswordResetCode(userId uint32, code string) (user return user.User{}, user.ErrUserNotFound } u.PasswordResetCode = code + u.PasswordResetExpiration = expiration + m.users[userId] = u + return u, nil +} + +func (m *mockUserRepo) ClearVerificationCode(userId uint32) (user.User, error) { + m.clearVerificationCodeCalls++ + m.clearVerificationCodeUserIds = append(m.clearVerificationCodeUserIds, userId) + if err := m.fail.trigger("ClearVerificationCode"); err != nil { + return user.User{}, err + } + u, ok := m.users[userId] + if !ok { + return user.User{}, user.ErrUserNotFound + } + u.IsVerified = true + u.VerificationCode = "" + m.users[userId] = u + return u, nil +} + +func (m *mockUserRepo) ResetPassword(userId uint32, hashedPassword string) (user.User, error) { + m.resetPasswordCalls++ + m.lastResetPasswordUserId = userId + m.lastResetPasswordHash = hashedPassword + if err := m.fail.trigger("ResetPassword"); err != nil { + return user.User{}, err + } + u, ok := m.users[userId] + if !ok { + return user.User{}, user.ErrUserNotFound + } + u.Password = hashedPassword + u.PasswordResetCode = "" + u.PasswordResetExpiration = time.Time{} m.users[userId] = u return u, nil } diff --git a/test/verified_account_required_http_test.go b/test/verified_account_required_http_test.go new file mode 100644 index 0000000..af5afb7 --- /dev/null +++ b/test/verified_account_required_http_test.go @@ -0,0 +1,352 @@ +package test + +import ( + "RenewCMS/api/handlers/article" + "RenewCMS/api/handlers/image" + "RenewCMS/api/middleware" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + domainArticle "RenewCMS/internal/domain/article" + domainUser "RenewCMS/internal/domain/user" + "RenewCMS/internal/infrastructure/useCases" + "bytes" + "encoding/json" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mockUserRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( + useCases.NewCreateArticleUseCase(articleRepo), + useCases.NewGetArticleUseCase(articleRepo), + useCases.NewListArticlesUseCase(articleRepo), + useCases.NewDeleteArticleUseCase(articleRepo, imageRepo), + useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, + ) + imageHandler := image.NewHandler( + useCases.NewCreateImageUseCase(imageRepo), + useCases.NewGetArticleUseCase(articleRepo), + useCases.NewUpdateArticleUseCase(articleRepo), + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/article", func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Get("/", articleHandler.ListArticles) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) + r.Get("/{id}", articleHandler.GetArticle) + r.Post("/", articleHandler.PostArticle) + r.Delete("/{id}", articleHandler.DeleteArticle) + r.Patch("/{id}", articleHandler.UpdateArticleBody) + r.Patch("/{id}/publish", articleHandler.PublishArticle) + r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) + r.Post("/{id}/image", imageHandler.PostImage) + }) + }) + }) + return r +} + +func seedVerifiedAccountRequiredArticleRepo() *mockArticleRepo { + repo := newMockArticleRepo() + repo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} + return repo +} + +func seedVerifiedAccountRequiredUserRepo(isVerified bool) *mockUserRepo { + repo := newMockUserRepo() + repo.users[1] = domainUser.User{ID: 1, IsVerified: isVerified} + return repo +} + +func mintVerifiedAccountRequiredCookie(t *testing.T, token *jwtauth.JWTAuth, claims map[string]any) *http.Cookie { + t.Helper() + _, tokenString, err := token.Encode(claims) + if err != nil { + t.Fatalf("failed to encode jwt: %v", err) + } + return &http.Cookie{Name: "jwt", Value: tokenString} +} + +func newVerifiedAccountRequiredRequest(method, target string, body string) *http.Request { + if body == "" { + return httptest.NewRequest(method, target, nil) + } + req := httptest.NewRequest(method, target, bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + return req +} + +func TestVerifiedAccountRequiredHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + protectedRouteCases := []struct { + method string + path string + body string + }{ + {http.MethodGet, "/v1/article/1", ""}, + {http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`}, + {http.MethodDelete, "/v1/article/1", ""}, + {http.MethodPatch, "/v1/article/1", `{"body": "Updated"}`}, + {http.MethodPatch, "/v1/article/1/publish", ""}, + {http.MethodPatch, "/v1/article/1/unpublish", ""}, + } + for _, tc := range protectedRouteCases { + t.Run("An authenticated but unverified user is refused on every protected article route: "+tc.method+" "+tc.path, func(t *testing.T) { + articleRepo := seedVerifiedAccountRequiredArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(tc.method, tc.path, tc.body) + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.VerificationRequiredErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if got := articleRepo.articles[1]; got.Title != "Backyard Renovation Update" || got.Body != "" || got.IsOnline { + t.Errorf("expected article 1 to be untouched, got %+v", got) + } + }) + } + + t.Run("An unverified user is also refused when attaching an image", func(t *testing.T) { + articleRepo := seedVerifiedAccountRequiredArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + imageRepo := newMockImageRepo() + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, imageRepo, token) + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, err := writer.CreatePart(map[string][]string{ + "Content-Disposition": {`form-data; name="file"; filename="before.png"`}, + "Content-Type": {"image/png"}, + }) + if err != nil { + t.Fatalf("failed to create multipart part: %v", err) + } + if _, err := io.WriteString(part, "fake-image-data"); err != nil { + t.Fatalf("failed to write file content: %v", err) + } + if err := writer.Close(); err != nil { + t.Fatalf("failed to close multipart writer: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/v1/article/1/image", body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.VerificationRequiredErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if imageRepo.images == nil { + t.Fatalf("expected imageRepo.images to be initialized") + } + if len(imageRepo.images) != 0 { + t.Errorf("expected the image repository's Create to never be called, got %d images", len(imageRepo.images)) + } + }) + + t.Run("A verified user still exercises the protected routes exactly as before", func(t *testing.T) { + articleRepo := newMockArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(true) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) + } + var got domainArticle.Article + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if got.Title != "Valid Title" || got.Body != "Some content" { + t.Errorf("unexpected article: %+v", got) + } + }) + + unresolvableTokenCases := []struct { + name string + claims map[string]any + }{ + {"a validly signed jwt cookie whose claims carry no user_id key at all", map[string]any{"role": "user"}}, + {`a validly signed jwt cookie whose user_id claim is the string "one"`, map[string]any{"user_id": "one"}}, + {"a validly signed jwt cookie whose user_id claim is the number 999, which does not match any user", map[string]any{"user_id": uint32(999)}}, + } + for _, tc := range unresolvableTokenCases { + t.Run("A token that resolves to no usable account still answers 401, never 403: "+tc.name, func(t *testing.T) { + articleRepo := seedVerifiedAccountRequiredArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodPost, "/v1/article/", `{"title": "Valid Title", "body": "Some content"}`) + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, tc.claims)) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.String() != apperr.UnauthorizedErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + } + + noCookieCases := []struct { + method string + path string + }{ + {http.MethodGet, "/v1/article/1"}, + {http.MethodPost, "/v1/article/"}, + } + for _, tc := range noCookieCases { + t.Run("A request with no jwt cookie is still refused with 401, never 403: "+tc.method+" "+tc.path, func(t *testing.T) { + articleRepo := seedVerifiedAccountRequiredArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(tc.method, tc.path, "") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d, body %q", rec.Code, rec.Body.String()) + } + }) + } + + t.Run("A repository failure while resolving the caller's verification status answers 500", func(t *testing.T) { + articleRepo := seedVerifiedAccountRequiredArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + userRepo.fail.failNext("Get") + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodDelete, "/v1/article/1", "") + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status 500, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.String() != "Internal server error\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if _, exists := articleRepo.articles[1]; !exists { + t.Errorf("expected article 1 to still exist") + } + }) + + seedListingArticleRepo := func() *mockArticleRepo { + repo := newMockArticleRepo() + repo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", IsOnline: true} + repo.articles[2] = domainArticle.Article{ID: 2, Title: "Draft: Kitchen Remodel", IsOnline: false} + return repo + } + + assertOnlyArticleOneListed := func(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + var got []domainArticle.Article + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if len(got) != 1 || got[0].ID != 1 { + t.Errorf("expected exactly the article with id 1, got %+v", got) + } + } + + t.Run("Listing falls back to the online-only list for an authenticated but unverified caller", func(t *testing.T) { + articleRepo := seedListingArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(false) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + assertOnlyArticleOneListed(t, rec) + }) + + t.Run("Listing still returns every article for an authenticated and verified caller", func(t *testing.T) { + articleRepo := seedListingArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(true) + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body %q", rec.Code, rec.Body.String()) + } + var got []domainArticle.Article + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + ids := map[uint32]bool{} + for _, a := range got { + ids[a.ID] = true + } + if !ids[1] || !ids[2] { + t.Errorf("expected ids 1 and 2, got %+v", got) + } + }) + + t.Run("Listing falls back to the online-only list when the caller's account no longer exists", func(t *testing.T) { + articleRepo := seedListingArticleRepo() + userRepo := newMockUserRepo() + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(999)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + assertOnlyArticleOneListed(t, rec) + }) + + t.Run("Listing falls back to the online-only list rather than failing when the user lookup errors", func(t *testing.T) { + articleRepo := seedListingArticleRepo() + userRepo := seedVerifiedAccountRequiredUserRepo(true) + userRepo.fail.failNext("Get") + router := newVerifiedAccountRequiredRouter(articleRepo, userRepo, newMockImageRepo(), token) + + req := newVerifiedAccountRequiredRequest(http.MethodGet, "/v1/article/", "") + req.AddCookie(mintVerifiedAccountRequiredCookie(t, token, map[string]any{"user_id": uint32(1)})) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + assertOnlyArticleOneListed(t, rec) + }) +} diff --git a/test/write_not_found_errors_test.go b/test/write_not_found_errors_test.go index c275a0f..3c5e7b2 100644 --- a/test/write_not_found_errors_test.go +++ b/test/write_not_found_errors_test.go @@ -2,6 +2,7 @@ package test import ( "RenewCMS/api/handlers/article" + "RenewCMS/api/middleware" "RenewCMS/api/pkg/apperr" validator "RenewCMS/api/pkg/validator" domainArticle "RenewCMS/internal/domain/article" @@ -14,6 +15,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/go-chi/chi/v5" "github.com/go-chi/jwtauth/v5" @@ -125,7 +127,7 @@ func TestTheUserRepositoryReportsAMissingUserAsADomainErrorOnAWrite(t *testing.T t.Run("UpdatePasswordResetCode and code newHashedResetCode", func(t *testing.T) { repo := newSeededUserRepository(t) - _, err := repo.UpdatePasswordResetCode(9, "newHashedResetCode") + _, err := repo.UpdatePasswordResetCode(9, "newHashedResetCode", time.Now().Add(time.Hour)) assertWriteResultIsUserNotFound(t, err) }) @@ -179,7 +181,7 @@ func TestTheFakesMirrorTheRealRepositoriesOnEveryWriteNotFoundPath(t *testing.T) if _, err := userRepo.UpdatePassword(9, "newHashedPassword"); !errors.Is(err, domainUser.ErrUserNotFound) { t.Errorf("expected user.ErrUserNotFound, got %v", err) } - if _, err := userRepo.UpdatePasswordResetCode(9, "newHashedResetCode"); !errors.Is(err, domainUser.ErrUserNotFound) { + if _, err := userRepo.UpdatePasswordResetCode(9, "newHashedResetCode", time.Now().Add(time.Hour)); !errors.Is(err, domainUser.ErrUserNotFound) { t.Errorf("expected user.ErrUserNotFound, got %v", err) } if err := userRepo.Delete(9); !errors.Is(err, domainUser.ErrUserNotFound) { @@ -188,12 +190,16 @@ func TestTheFakesMirrorTheRealRepositoriesOnEveryWriteNotFoundPath(t *testing.T) } func newDeleteArticleRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + articleHandler := article.NewHandler( useCases.NewCreateArticleUseCase(articleRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewListArticlesUseCase(articleRepo), useCases.NewDeleteArticleUseCase(articleRepo, newMockImageRepo()), useCases.NewUpdateArticleUseCase(articleRepo), + getUserUseCase, ) r := chi.NewRouter() @@ -201,6 +207,7 @@ func newDeleteArticleRouter(articleRepo *mockArticleRepo, token *jwtauth.JWTAuth r.Group(func(r chi.Router) { r.Use(jwtauth.Verifier(token)) r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) r.Route("/article", func(r chi.Router) { r.Delete("/{id}", articleHandler.DeleteArticle) })