From 4e16476016553ec06ca93f7e3d898a88ef7a7852 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Thu, 4 Jul 2024 17:08:52 +0200 Subject: [PATCH 1/7] Adds an endpoint and handlers to change user account password --- internal/api/handlers.go | 29 +++++++++++++++++++++ internal/api/handlers_test.go | 48 +++++++++++++++++++++++++---------- internal/certdb/certdb.go | 8 +++--- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 12a284f..7869e91 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -34,6 +34,7 @@ func NewGoCertRouter(env *Environment) http.Handler { apiV1Router.HandleFunc("GET /accounts/{id}", GetUserAccount(env)) apiV1Router.HandleFunc("GET /accounts", GetUserAccounts(env)) apiV1Router.HandleFunc("POST /accounts", PostUserAccount(env)) + apiV1Router.HandleFunc("POST /accounts/{id}", ChangeUserAccountPassword(env)) m := metrics.NewMetricsSubsystem(env.DB) frontendHandler := newFrontendFileServer() @@ -360,6 +361,34 @@ func PostUserAccount(env *Environment) http.HandlerFunc { } } +func ChangeUserAccountPassword(env *Environment) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + id := r.PathValue("id") + var user certdb.User + if err := json.NewDecoder(r.Body).Decode(&user); err != nil { + logErrorAndWriteResponse("Invalid JSON format", http.StatusBadRequest, w) + return + } + if user.Password == "" { + logErrorAndWriteResponse("Password is required", http.StatusBadRequest, w) + return + } + ret, err := env.DB.UpdateUser(id, user.Password) + if err != nil { + if errors.Is(err, certdb.ErrIdNotFound) { + logErrorAndWriteResponse(err.Error(), http.StatusNotFound, w) + return + } + logErrorAndWriteResponse(err.Error(), http.StatusInternalServerError, w) + return + } + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte(strconv.FormatInt(ret, 10))); err != nil { + logErrorAndWriteResponse(err.Error(), http.StatusInternalServerError, w) + } + } +} + // logErrorAndWriteResponse is a helper function that logs any error and writes it back as an http response func logErrorAndWriteResponse(msg string, status int, w http.ResponseWriter) { errMsg := fmt.Sprintf("error: %s", msg) diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go index 4e30dbb..abfe64e 100644 --- a/internal/api/handlers_test.go +++ b/internal/api/handlers_test.go @@ -101,10 +101,12 @@ const ( ) const ( - adminUser = `{"username": "testadmin", "password": "admin"}` - validUser = `{"username": "testuser", "password": "user"}` - noPasswordUser = `{"username": "nopass", "password": ""}` - invalidUser = `{"username": "", "password": ""}` + adminUser = `{"username": "testadmin", "password": "admin"}` + validUser = `{"username": "testuser", "password": "user"}` + invalidUser = `{"username": "", "password": ""}` + noPasswordUser = `{"username": "nopass", "password": ""}` + adminUserNewPassword = `{"id": 1, "password": "newpassword"}` + userNewInvalidPassword = `{"id": 1, "password": ""}` ) func TestGoCertCertificatesHandlers(t *testing.T) { @@ -268,7 +270,7 @@ func TestGoCertCertificatesHandlers(t *testing.T) { method: "POST", path: "/api/v1/certificate_requests/2/certificate", data: validCert2, - response: "4", + response: "1", status: http.StatusCreated, }, { @@ -284,7 +286,7 @@ func TestGoCertCertificatesHandlers(t *testing.T) { method: "POST", path: "/api/v1/certificate_requests/4/certificate/reject", data: "", - response: "4", + response: "1", status: http.StatusAccepted, }, { @@ -300,7 +302,7 @@ func TestGoCertCertificatesHandlers(t *testing.T) { method: "DELETE", path: "/api/v1/certificate_requests/2/certificate", data: "", - response: "4", + response: "1", status: http.StatusAccepted, }, { @@ -369,12 +371,6 @@ func TestGoCertUsersHandlers(t *testing.T) { ts := httptest.NewTLSServer(server.NewGoCertRouter(env)) defer ts.Close() - originalFunc := server.GeneratePassword - server.GeneratePassword = func(length int) (string, error) { - return "generatedPassword", nil - } - defer func() { server.GeneratePassword = originalFunc }() - client := ts.Client() testCases := []struct { @@ -414,7 +410,7 @@ func TestGoCertUsersHandlers(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: noPasswordUser, - response: "{\"id\":3,\"password\":\"generatedPassword\"}", + response: "{\"id\":3,\"password\":", status: http.StatusCreated, }, { @@ -441,6 +437,30 @@ func TestGoCertUsersHandlers(t *testing.T) { response: "error: Username is required", status: http.StatusBadRequest, }, + { + desc: "Change password success", + method: "POST", + path: "/api/v1/accounts/1", + data: adminUserNewPassword, + response: "1", + status: http.StatusOK, + }, + { + desc: "Change password failure no user", + method: "POST", + path: "/api/v1/accounts/100", + data: adminUserNewPassword, + response: "id not found", + status: http.StatusNotFound, + }, + { + desc: "Change password failure", + method: "POST", + path: "/api/v1/accounts/1", + data: userNewInvalidPassword, + response: "Password is required", + status: http.StatusBadRequest, + }, } for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { diff --git a/internal/certdb/certdb.go b/internal/certdb/certdb.go index 0d306b3..191e812 100644 --- a/internal/certdb/certdb.go +++ b/internal/certdb/certdb.go @@ -130,11 +130,11 @@ func (db *CertificateRequestsRepository) UpdateCSR(id string, cert string) (int6 if err != nil { return 0, err } - insertId, err := result.LastInsertId() + affectedRows, err := result.RowsAffected() if err != nil { return 0, err } - return insertId, nil + return affectedRows, nil } // DeleteCSR removes a CSR from the database alongside the certificate that may have been generated for it. @@ -219,11 +219,11 @@ func (db *CertificateRequestsRepository) UpdateUser(id, password string) (int64, if err != nil { return 0, err } - insertId, err := result.LastInsertId() + affectedRows, err := result.RowsAffected() if err != nil { return 0, err } - return insertId, nil + return affectedRows, nil } // DeleteUser removes a user from the table. From a3ff1d6f011af4029cb9542b8ec4845b76103d16 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Fri, 5 Jul 2024 09:04:37 +0200 Subject: [PATCH 2/7] Fixes url --- internal/api/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 7869e91..6a3f71a 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -34,7 +34,7 @@ func NewGoCertRouter(env *Environment) http.Handler { apiV1Router.HandleFunc("GET /accounts/{id}", GetUserAccount(env)) apiV1Router.HandleFunc("GET /accounts", GetUserAccounts(env)) apiV1Router.HandleFunc("POST /accounts", PostUserAccount(env)) - apiV1Router.HandleFunc("POST /accounts/{id}", ChangeUserAccountPassword(env)) + apiV1Router.HandleFunc("POST /accounts/{id}/change_password", ChangeUserAccountPassword(env)) m := metrics.NewMetricsSubsystem(env.DB) frontendHandler := newFrontendFileServer() From 2868302646b437a5dcbfb9f3c12624562a6d6da5 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Fri, 5 Jul 2024 09:41:06 +0200 Subject: [PATCH 3/7] Fixes tests --- internal/api/handlers_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go index abfe64e..ff9accd 100644 --- a/internal/api/handlers_test.go +++ b/internal/api/handlers_test.go @@ -440,7 +440,7 @@ func TestGoCertUsersHandlers(t *testing.T) { { desc: "Change password success", method: "POST", - path: "/api/v1/accounts/1", + path: "/api/v1/accounts/1/change_password", data: adminUserNewPassword, response: "1", status: http.StatusOK, @@ -448,7 +448,7 @@ func TestGoCertUsersHandlers(t *testing.T) { { desc: "Change password failure no user", method: "POST", - path: "/api/v1/accounts/100", + path: "/api/v1/accounts/100/change_password", data: adminUserNewPassword, response: "id not found", status: http.StatusNotFound, @@ -456,7 +456,7 @@ func TestGoCertUsersHandlers(t *testing.T) { { desc: "Change password failure", method: "POST", - path: "/api/v1/accounts/1", + path: "/api/v1/accounts/1/change_password", data: userNewInvalidPassword, response: "Password is required", status: http.StatusBadRequest, From d6b1d7b6129d5908078ec923b3e790a275af67a6 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Fri, 5 Jul 2024 11:38:31 +0200 Subject: [PATCH 4/7] Validate password --- internal/api/handlers.go | 79 +++++++++++++++++++++++++++++++---- internal/api/handlers_test.go | 32 ++++++++++---- 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 6a3f71a..f6b62f4 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -9,7 +9,9 @@ import ( "io/fs" "log" "math/big" + mrand "math/rand" "net/http" + "regexp" "strconv" "strings" @@ -322,13 +324,21 @@ func PostUserAccount(env *Environment) http.HandlerFunc { return } if user.Password == "" { - generatedPassword, err := GeneratePassword(8) + generatedPassword, err := GeneratePassword() if err != nil { logErrorAndWriteResponse("Failed to generate password", http.StatusInternalServerError, w) return } user.Password = generatedPassword } + if !validatePassword(user.Password) { + logErrorAndWriteResponse( + "Password does not meet requirements. It must include at least one capital letter, one lowercase letter, and either a number or a symbol.", + http.StatusBadRequest, + w, + ) + return + } users, err := env.DB.RetrieveAllUsers() if err != nil { logErrorAndWriteResponse("Failed to retrieve users: "+err.Error(), http.StatusInternalServerError, w) @@ -373,6 +383,14 @@ func ChangeUserAccountPassword(env *Environment) http.HandlerFunc { logErrorAndWriteResponse("Password is required", http.StatusBadRequest, w) return } + if !validatePassword(user.Password) { + logErrorAndWriteResponse( + "Password does not meet requirements. It must include at least one capital letter, one lowercase letter, and either a number or a symbol.", + http.StatusBadRequest, + w, + ) + return + } ret, err := env.DB.UpdateUser(id, user.Password) if err != nil { if errors.Is(err, certdb.ErrIdNotFound) { @@ -399,15 +417,62 @@ func logErrorAndWriteResponse(msg string, status int, w http.ResponseWriter) { } } -var GeneratePassword = func(length int) (string, error) { - const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789&*?@" - b := make([]byte, length) - for i := range b { +func getRandomChars(charset string, length int) (string, error) { + result := make([]byte, length) + for i := range result { n, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset)))) if err != nil { return "", err } - b[i] = charset[n.Int64()] + result[i] = charset[n.Int64()] + } + return string(result), nil +} + +// Generates a random 8 chars long password that contains uppercase and lowercase characters and numbers or symbols. +var GeneratePassword = func() (string, error) { + const ( + uppercaseSet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + lowercaseSet = "abcdefghijklmnopqrstuvwxyz" + numbersAndSymbolsSet = "0123456789&*?@" + allCharsSet = uppercaseSet + lowercaseSet + numbersAndSymbolsSet + ) + uppercase, err := getRandomChars(uppercaseSet, 2) + if err != nil { + return "", err + } + lowercase, err := getRandomChars(lowercaseSet, 2) + if err != nil { + return "", err } - return string(b), nil + numbersOrSymbols, err := getRandomChars(numbersAndSymbolsSet, 2) + if err != nil { + return "", err + } + allChars, err := getRandomChars(allCharsSet, 2) + if err != nil { + return "", err + } + res := []rune(uppercase + lowercase + numbersOrSymbols + allChars) + mrand.Shuffle(len(res), func(i, j int) { + res[i], res[j] = res[j], res[i] + }) + return string(res), nil +} + +func validatePassword(password string) bool { + if len(password) < 8 { + return false + } + hasCapital := regexp.MustCompile(`[A-Z]`).MatchString(password) + if !hasCapital { + return false + } + hasLower := regexp.MustCompile(`[a-z]`).MatchString(password) + if !hasLower { + return false + } + hasNumberOrSymbol := regexp.MustCompile(`[0-9!@#$%^&*()_+\-=\[\]{};':"|,.<>?~]`).MatchString(password) + + return hasNumberOrSymbol } diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go index ff9accd..a5e68cf 100644 --- a/internal/api/handlers_test.go +++ b/internal/api/handlers_test.go @@ -101,12 +101,13 @@ const ( ) const ( - adminUser = `{"username": "testadmin", "password": "admin"}` - validUser = `{"username": "testuser", "password": "user"}` + adminUser = `{"username": "testadmin", "password": "Admin123"}` + validUser = `{"username": "testuser", "password": "userPass!"}` invalidUser = `{"username": "", "password": ""}` noPasswordUser = `{"username": "nopass", "password": ""}` - adminUserNewPassword = `{"id": 1, "password": "newpassword"}` - userNewInvalidPassword = `{"id": 1, "password": ""}` + adminUserNewPassword = `{"id": 1, "password": "newPassword1"}` + userNewInvalidPassword = `{"id": 1, "password": "password"}` + userMissingPassword = `{"id": 1, "password": ""}` ) func TestGoCertCertificatesHandlers(t *testing.T) { @@ -369,6 +370,11 @@ func TestGoCertUsersHandlers(t *testing.T) { env := &server.Environment{} env.DB = testdb ts := httptest.NewTLSServer(server.NewGoCertRouter(env)) + originalFunc := server.GeneratePassword + server.GeneratePassword = func() (string, error) { + return "generatedPassword!", nil + } + defer func() { server.GeneratePassword = originalFunc }() defer ts.Close() client := ts.Client() @@ -386,7 +392,7 @@ func TestGoCertUsersHandlers(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: adminUser, - response: "{\"id\":1,\"password\":\"admin\"}", + response: "{\"id\":1,\"password\":\"Admin123\"}", status: http.StatusCreated, }, { @@ -402,7 +408,7 @@ func TestGoCertUsersHandlers(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: validUser, - response: "{\"id\":2,\"password\":\"user\"}", + response: "{\"id\":2,\"password\":\"userPass!\"}", status: http.StatusCreated, }, { @@ -410,7 +416,7 @@ func TestGoCertUsersHandlers(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: noPasswordUser, - response: "{\"id\":3,\"password\":", + response: "{\"id\":3,\"password\":\"generatedPassword!\"}", status: http.StatusCreated, }, { @@ -454,13 +460,21 @@ func TestGoCertUsersHandlers(t *testing.T) { status: http.StatusNotFound, }, { - desc: "Change password failure", + desc: "Change password failure missing password", method: "POST", path: "/api/v1/accounts/1/change_password", - data: userNewInvalidPassword, + data: userMissingPassword, response: "Password is required", status: http.StatusBadRequest, }, + { + desc: "Change password failure bad password", + method: "POST", + path: "/api/v1/accounts/1/change_password", + data: userNewInvalidPassword, + response: "Password does not meet requirements. It must include at least one capital letter, one lowercase letter, and either a number or a symbol.", + status: http.StatusBadRequest, + }, } for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { From 3c4be56abeabf8fcb96c26edb84229eb8de36bb4 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Fri, 5 Jul 2024 14:43:34 +0200 Subject: [PATCH 5/7] Increases the length of the password generated --- internal/api/handlers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index f6b62f4..7ed4767 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -429,7 +429,7 @@ func getRandomChars(charset string, length int) (string, error) { return string(result), nil } -// Generates a random 8 chars long password that contains uppercase and lowercase characters and numbers or symbols. +// Generates a random 16 chars long password that contains uppercase and lowercase characters and numbers or symbols. var GeneratePassword = func() (string, error) { const ( uppercaseSet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" @@ -449,7 +449,7 @@ var GeneratePassword = func() (string, error) { if err != nil { return "", err } - allChars, err := getRandomChars(allCharsSet, 2) + allChars, err := getRandomChars(allCharsSet, 10) if err != nil { return "", err } From 6053068e5d3da61791eace1c2857b592c2ced30c Mon Sep 17 00:00:00 2001 From: yazansalti Date: Tue, 9 Jul 2024 12:01:31 +0200 Subject: [PATCH 6/7] Removes mocks --- internal/api/handlers.go | 4 ++-- internal/api/handlers_test.go | 22 +++++++++------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index b82c522..4a19cf1 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -330,7 +330,7 @@ func PostUserAccount(env *Environment) http.HandlerFunc { return } if user.Password == "" { - generatedPassword, err := GeneratePassword() + generatedPassword, err := generatePassword() if err != nil { logErrorAndWriteResponse("Failed to generate password", http.StatusInternalServerError, w) return @@ -498,7 +498,7 @@ func getRandomChars(charset string, length int) (string, error) { } // Generates a random 16 chars long password that contains uppercase and lowercase characters and numbers or symbols. -var GeneratePassword = func() (string, error) { +func generatePassword() (string, error) { const ( uppercaseSet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" lowercaseSet = "abcdefghijklmnopqrstuvwxyz" diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go index 74c5a30..5ac9414 100644 --- a/internal/api/handlers_test.go +++ b/internal/api/handlers_test.go @@ -6,6 +6,7 @@ import ( "log" "net/http" "net/http/httptest" + "regexp" "strings" "testing" @@ -374,11 +375,6 @@ func TestGoCertUsersHandlers(t *testing.T) { env := &server.Environment{} env.DB = testdb ts := httptest.NewTLSServer(server.NewGoCertRouter(env)) - originalFunc := server.GeneratePassword - server.GeneratePassword = func() (string, error) { - return "generatedPassword!", nil - } - defer func() { server.GeneratePassword = originalFunc }() defer ts.Close() client := ts.Client() @@ -420,7 +416,7 @@ func TestGoCertUsersHandlers(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: noPasswordUser, - response: "{\"id\":3,\"password\":\"generatedPassword!\"}", + response: "{\"id\":3,\"password\":", status: http.StatusCreated, }, { @@ -514,6 +510,12 @@ func TestGoCertUsersHandlers(t *testing.T) { if res.StatusCode != tC.status || !strings.Contains(string(resBody), tC.response) { t.Errorf("expected response did not match.\nExpected vs Received status code: %d vs %d\nExpected vs Received body: \n%s\nvs\n%s\n", tC.status, res.StatusCode, tC.response, string(resBody)) } + if tC.desc == "Create no password user success" { + match, _ := regexp.MatchString(`"password":"[!-~]{16}"`, string(resBody)) + if !match { + t.Errorf("password does not match expected format or length: got %s", string(resBody)) + } + } }) } } @@ -529,12 +531,6 @@ func TestLogin(t *testing.T) { ts := httptest.NewTLSServer(server.NewGoCertRouter(env)) defer ts.Close() - originalFunc := server.GeneratePassword - server.GeneratePassword = func() (string, error) { - return "generatedPassword", nil - } - defer func() { server.GeneratePassword = originalFunc }() - client := ts.Client() testCases := []struct { @@ -550,7 +546,7 @@ func TestLogin(t *testing.T) { method: "POST", path: "/api/v1/accounts", data: adminUser, - response: "{\"id\":1,\"password\":\"admin\"}", + response: "{\"id\":1,\"password\":\"Admin123\"}", status: http.StatusCreated, }, { From d22b82a15ecdb08261d84d5887fca92ea51db536 Mon Sep 17 00:00:00 2001 From: yazansalti Date: Tue, 9 Jul 2024 13:55:52 +0200 Subject: [PATCH 7/7] Remove symbols that need escaping from generated password --- internal/api/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 4a19cf1..77f7591 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -502,7 +502,7 @@ func generatePassword() (string, error) { const ( uppercaseSet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" lowercaseSet = "abcdefghijklmnopqrstuvwxyz" - numbersAndSymbolsSet = "0123456789&*?@" + numbersAndSymbolsSet = "0123456789*?@" allCharsSet = uppercaseSet + lowercaseSet + numbersAndSymbolsSet ) uppercase, err := getRandomChars(uppercaseSet, 2)