Merge pull request #7 from Questspace-v2/user/2

feat!: Check old password for update user
Questspace-v2 · Oct 30, 2023 · a18edc8 · a18edc8
2 parents e262b5f + 02b3d35
commit a18edc8
Show file tree

Hide file tree

Showing 10 changed files with 105 additions and 59 deletions.
diff --git a/docs/docs.go b/docs/docs.go
@@ -135,10 +135,14 @@ const docTemplate = `{
                     "type": "string",
                     "example": "b5ee72a3-54dd-c4b8-551c-4bdc0204cedb"
                 },
-                "password": {
+                "new_password": {
                     "type": "string",
                     "example": "complex_password_here"
                 },
+                "old_password": {
+                    "type": "string",
+                    "example": "12345"
+                },
                 "username": {
                     "type": "string",
                     "example": "svayp11"

diff --git a/docs/swagger.json b/docs/swagger.json
@@ -124,10 +124,14 @@
                     "type": "string",
                     "example": "b5ee72a3-54dd-c4b8-551c-4bdc0204cedb"
                 },
-                "password": {
+                "new_password": {
                     "type": "string",
                     "example": "complex_password_here"
                 },
+                "old_password": {
+                    "type": "string",
+                    "example": "12345"
+                },
                 "username": {
                     "type": "string",
                     "example": "svayp11"

diff --git a/docs/swagger.yaml b/docs/swagger.yaml
@@ -19,9 +19,12 @@ definitions:
       id:
         example: b5ee72a3-54dd-c4b8-551c-4bdc0204cedb
         type: string
-      password:
+      new_password:
         example: complex_password_here
         type: string
+      old_password:
+        example: "12345"
+        type: string
       username:
         example: svayp11
         type: string

diff --git a/internal/handlers/user/create_handler.go b/internal/handlers/user/create_handler.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"hash"
 	"net/http"
+	"questspace/internal/hasher"
 	"questspace/internal/validate"
 	aerrors "questspace/pkg/application/errors"
 	"questspace/pkg/storage"
@@ -53,9 +54,7 @@ func (h CreateHandler) Handle(c *gin.Context) error {
 	if req.AvatarURL == "" {
 		req.AvatarURL = defaultAvatarURL
 	}
-	h.hasher.Write([]byte(req.Password))
-	req.Password = string(h.hasher.Sum(nil))
-	h.hasher.Reset()
+	req.Password = hasher.HashString(h.hasher, req.Password)
 	user, err := h.storage.CreateUser(c, &req)
 	if err != nil {
 		if errors.Is(err, storage.ErrExists) {

diff --git a/internal/handlers/user/create_handler_test.go b/internal/handlers/user/create_handler_test.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"questspace/internal/hasher"
 	"questspace/pkg/application"
 	"questspace/pkg/storage"
 	"questspace/pkg/storage/mocks"
@@ -87,8 +88,8 @@ func TestCreateHandler_CommonCases(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	userStorage := mocks.NewMockUserStorage(ctrl)
 	router := gin.Default()
-	hasher := sha256.New()
-	handler := NewCreateHandler(userStorage, http.Client{}, hasher)
+	pwHasher := sha256.New()
+	handler := NewCreateHandler(userStorage, http.Client{}, pwHasher)
 	router.POST("/test", application.AsGinHandler(handler.Handle))
 
 	for _, tc := range testCases {
@@ -102,17 +103,16 @@ func TestCreateHandler_CommonCases(t *testing.T) {
 				defer img.Close()
 				tc.req.AvatarURL = img.URL
 			}
-			hasher.Write([]byte(tc.req.Password))
 			raw, err := json.Marshal(tc.req)
 			require.NoError(t, err)
 			request, err := http.NewRequest(http.MethodPost, "/test", bytes.NewReader(raw))
 			require.NoError(t, err)
 			actualReq := &storage.CreateUserRequest{
 				Username:  tc.req.Username,
-				Password:  string(hasher.Sum(nil)),
+				Password:  hasher.HashString(pwHasher, tc.req.Password),
 				AvatarURL: tc.req.AvatarURL,
 			}
-			hasher.Reset()
+			pwHasher.Reset()
 			if tc.wantStore {
 				userStorage.EXPECT().CreateUser(gomock.Any(), actualReq).Return(nil, tc.storeErr)
 			}
@@ -127,22 +127,20 @@ func TestCreateHandler_SetsDefaultURL(t *testing.T) {
 	gin.SetMode(gin.TestMode)
 	ctrl := gomock.NewController(t)
 	userStorage := mocks.NewMockUserStorage(ctrl)
-	hasher := sha256.New()
+	pwHasher := sha256.New()
 	rr := httptest.NewRecorder()
 	router := gin.Default()
-	handler := NewCreateHandler(userStorage, http.Client{}, hasher)
+	handler := NewCreateHandler(userStorage, http.Client{}, pwHasher)
 	router.POST("/test", application.AsGinHandler(handler.Handle))
 	req := &storage.CreateUserRequest{
 		Username: "user",
 		Password: "password",
 	}
-	hasher.Write([]byte(req.Password))
 	storageReq := &storage.CreateUserRequest{
-		Username:  "user",
-		Password:  string(hasher.Sum(nil)),
+		Username:  req.Username,
+		Password:  hasher.HashString(pwHasher, req.Password),
 		AvatarURL: defaultAvatarURL,
 	}
-	hasher.Reset()
 	raw, err := json.Marshal(req)
 	require.NoError(t, err)
 	request, err := http.NewRequest(http.MethodPost, "/test", bytes.NewReader(raw))

diff --git a/internal/handlers/user/update_handler.go b/internal/handlers/user/update_handler.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"hash"
 	"net/http"
+	"questspace/internal/hasher"
 	"questspace/internal/validate"
 	aerrors "questspace/pkg/application/errors"
 	"questspace/pkg/storage"
@@ -46,16 +47,20 @@ func (h UpdateHandler) Handle(c *gin.Context) error {
 	if err := json.Unmarshal(data, &req); err != nil {
 		return xerrors.Errorf("failed to unmarshall request: %w", err)
 	}
+	if req.OldPassword == "" {
+		return aerrors.ErrBadRequest
+	}
 	req.Id = c.Param("id")
 
 	if req.AvatarURL != "" {
 		if err := validate.ImageURL(h.fetcher, req.AvatarURL); err != nil {
 			return xerrors.Errorf("failed to validate an image: %w", err)
 		}
 	}
-	h.hasher.Write([]byte(req.Password))
-	req.Password = string(h.hasher.Sum(nil))
-	h.hasher.Reset()
+	req.OldPassword = hasher.HashString(h.hasher, req.OldPassword)
+	if req.NewPassword != "" {
+		req.NewPassword = hasher.HashString(h.hasher, req.NewPassword)
+	}
 	user, err := h.storage.UpdateUser(c, &req)
 	if err != nil {
 		if errors.Is(err, storage.ErrNotFound) {

diff --git a/internal/handlers/user/update_handler_test.go b/internal/handlers/user/update_handler_test.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"questspace/internal/hasher"
 	"questspace/pkg/application"
 	"questspace/pkg/storage"
 	"questspace/pkg/storage/mocks"
@@ -30,20 +31,20 @@ func TestUpdateHandler(t *testing.T) {
 			name:    "ok",
 			imgType: "image/svg",
 			req: &storage.UpdateUserRequest{
-				Id:       "1",
-				Username: "user",
-				Password: "password",
+				Id:          "1",
+				Username:    "user",
+				OldPassword: "password",
 			},
 			wantUpd:    true,
 			statusCode: http.StatusOK,
 		},
 		{
 			name: "ok with custom avatar link",
 			req: &storage.UpdateUserRequest{
-				Id:        "1",
-				Username:  "user",
-				Password:  "password",
-				AvatarURL: "https://some.domain.com/avatar.png",
+				Id:          "1",
+				Username:    "user",
+				OldPassword: "password",
+				AvatarURL:   "https://some.domain.com/avatar.png",
 			},
 			wantUpd:    true,
 			statusCode: http.StatusOK,
@@ -52,9 +53,9 @@ func TestUpdateHandler(t *testing.T) {
 			name:    "not found",
 			imgType: "image/svg",
 			req: &storage.UpdateUserRequest{
-				Id:       "non_existent_id",
-				Username: "user",
-				Password: "password",
+				Id:          "non_existent_id",
+				Username:    "user",
+				OldPassword: "password",
 			},
 			wantUpd:    true,
 			updErr:     storage.ErrNotFound,
@@ -64,9 +65,9 @@ func TestUpdateHandler(t *testing.T) {
 			name:    "not an image",
 			imgType: "application/json",
 			req: &storage.UpdateUserRequest{
-				Id:       "1",
-				Username: "user",
-				Password: "password",
+				Id:          "1",
+				Username:    "user",
+				OldPassword: "password",
 			},
 			wantUpd:    false,
 			statusCode: http.StatusUnprocessableEntity,
@@ -75,9 +76,9 @@ func TestUpdateHandler(t *testing.T) {
 			name:    "internal error",
 			imgType: "image/jpg",
 			req: &storage.UpdateUserRequest{
-				Id:       "1",
-				Username: "user",
-				Password: "password",
+				Id:          "1",
+				Username:    "user",
+				OldPassword: "password",
 			},
 			wantUpd:    true,
 			updErr:     xerrors.New("oops"),
@@ -89,8 +90,8 @@ func TestUpdateHandler(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	userStorage := mocks.NewMockUserStorage(ctrl)
 	router := gin.Default()
-	hasher := sha256.New()
-	handler := NewUpdateHandler(userStorage, http.Client{}, hasher)
+	pwHasher := sha256.New()
+	handler := NewUpdateHandler(userStorage, http.Client{}, pwHasher)
 	router.POST("/test/:id", application.AsGinHandler(handler.Handle))
 
 	for _, tc := range testCases {
@@ -104,19 +105,17 @@ func TestUpdateHandler(t *testing.T) {
 				defer img.Close()
 				tc.req.AvatarURL = img.URL
 			}
-			hasher.Write([]byte(tc.req.Password))
 			raw, err := json.Marshal(tc.req)
 			require.NoError(t, err)
 			request, err := http.NewRequest(http.MethodPost, "/test/"+tc.req.Id, bytes.NewReader(raw))
 			require.NoError(t, err)
 
 			actualReq := &storage.UpdateUserRequest{
-				Id:        tc.req.Id,
-				Username:  tc.req.Username,
-				Password:  string(hasher.Sum(nil)),
-				AvatarURL: tc.req.AvatarURL,
+				Id:          tc.req.Id,
+				Username:    tc.req.Username,
+				OldPassword: hasher.HashString(pwHasher, tc.req.OldPassword),
+				AvatarURL:   tc.req.AvatarURL,
 			}
-			hasher.Reset()
 
 			if tc.wantUpd {
 				userStorage.EXPECT().UpdateUser(gomock.Any(), actualReq).Return(&storage.User{}, tc.updErr)

diff --git a/internal/hasher/hasher.go b/internal/hasher/hasher.go
@@ -0,0 +1,10 @@
+package hasher
+
+import "hash"
+
+func HashString(h hash.Hash, str string) string {
+	h.Write([]byte(str))
+	sum := h.Sum(nil)
+	h.Reset()
+	return string(sum)
+}
diff --git a/internal/pgdb/client/client.go b/internal/pgdb/client/client.go
@@ -2,6 +2,7 @@ package pgdb
 
 import (
 	"context"
+	"database/sql"
 	"errors"
 	"questspace/pkg/storage"
 
@@ -107,12 +108,34 @@ func (c *Client) GetUser(ctx context.Context, req *storage.GetUserRequest) (*sto
 }
 
 func (c *Client) UpdateUser(ctx context.Context, req *storage.UpdateUserRequest) (*storage.User, error) {
-	if req.Username == "" && req.Password == "" && req.AvatarURL == "" {
-		return c.GetUser(ctx, &storage.GetUserRequest{Id: req.Id})
-	}
 	if err := c.conn.WaitUntilReady(ctx); err != nil {
 		return nil, xerrors.Errorf("failed to await db readiness: %w", err)
 	}
+	pwQuery := sq.
+		Select("password").
+		From(`"user"`).
+		Where(sq.Eq{"id": req.Id}).
+		PlaceholderFormat(sq.Dollar)
+	queryStr, args, err := pwQuery.ToSql()
+	if err != nil {
+		return nil, xerrors.Errorf("failed to build query string: %w", err)
+	}
+	row := c.conn.QueryRowEx(ctx, queryStr, nil, args...)
+	var oldPassword []byte
+	if err := row.Scan(&oldPassword); err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return nil, storage.ErrNotFound
+		}
+		return nil, xerrors.Errorf("failed to get stored password: %w", err)
+	}
+	// TODO(svayp11): Use more suitable error
+	if string(oldPassword) != req.OldPassword {
+		return nil, storage.ErrExists
+	}
+
+	if req.Username == "" && req.NewPassword == "" && req.AvatarURL == "" {
+		return c.GetUser(ctx, &storage.GetUserRequest{Id: req.Id})
+	}
 
 	query := sq.
 		Update(`"user"`).
@@ -122,28 +145,28 @@ func (c *Client) UpdateUser(ctx context.Context, req *storage.UpdateUserRequest)
 	if req.Username != "" {
 		query = query.Set("username", req.Username)
 	}
-	if req.Password != "" {
-		query = query.Set("password", []byte(req.Password))
+	if req.NewPassword != "" {
+		query = query.Set("password", []byte(req.NewPassword))
 	}
 	if req.AvatarURL != "" {
 		query = query.Set("avatar_url", req.AvatarURL)
 	}
 
-	queryStr, args, err := query.ToSql()
+	queryStr, args, err = query.ToSql()
 	if err != nil {
 		return nil, xerrors.Errorf("failed to build query string: %w", err)
 	}
-	row, err := c.conn.QueryEx(ctx, queryStr, nil, args...)
+	rows, err := c.conn.QueryEx(ctx, queryStr, nil, args...)
 	if err != nil {
 		return nil, xerrors.Errorf("failed to execute query %s: %w", queryStr, err)
 	}
-	defer row.Close()
-	if !row.Next() {
-		return nil, storage.ErrNotFound
+	defer rows.Close()
+	if !rows.Next() {
+		return nil, xerrors.Errorf("failed to insert row: %w", rows.Err())
 	}
 
 	user := &storage.User{}
-	if err := row.Scan(&user.Id, &user.Username, &user.AvatarURL); err != nil {
+	if err := rows.Scan(&user.Id, &user.Username, &user.AvatarURL); err != nil {
 		return nil, xerrors.Errorf("failed to scan row: %w", err)
 	}
 	return user, nil

diff --git a/pkg/storage/requests.go b/pkg/storage/requests.go
@@ -14,10 +14,11 @@ type GetUserRequest struct {
 }
 
 type UpdateUserRequest struct {
-	Id        string `json:"id" example:"b5ee72a3-54dd-c4b8-551c-4bdc0204cedb"`
-	Username  string `json:"username" example:"svayp11"`
-	Password  string `json:"password,omitempty" example:"complex_password_here"`
-	AvatarURL string `json:"avatar_url,omitempty" example:"https://i.pinimg.com/originals/7a/62/cb/7a62cb80e20da2d68a37b8db26833dc0.jpg"`
+	Id          string `json:"id" example:"b5ee72a3-54dd-c4b8-551c-4bdc0204cedb"`
+	Username    string `json:"username" example:"svayp11"`
+	OldPassword string `json:"old_password" example:"12345"`
+	NewPassword string `json:"new_password,omitempty" example:"complex_password_here"`
+	AvatarURL   string `json:"avatar_url,omitempty" example:"https://i.pinimg.com/originals/7a/62/cb/7a62cb80e20da2d68a37b8db26833dc0.jpg"`
 }
 
 type CreateQuestRequest struct {