Просмотр исходного кода

implement PUT /user/password

Create this endpoint so that server operators can (re)set user
passwords.
Mike 2 лет назад
Родитель
Сommit
e578066e18
6 измененных файлов с 408 добавлено и 66 удалено
  1. 6 0
      README.md
  2. 52 29
      api.yml
  3. 93 12
      server/http/mgmt_api.go
  4. 137 20
      server/http/mgmt_api_test.go
  5. 46 0
      server/http/mock_user_manager_test.go
  6. 74 5
      state/user_store.go

+ 6 - 0
README.md

@@ -50,6 +50,12 @@ curl http://localhost:8080/user
 curl -d'{"screen_name":"myScreenName", "password":"thepassword"}' http://localhost:8080/user
 ```
 
+### Change Password
+
+```shell
+curl -X PUT -d'{"screen_name":"myScreenName", "password":"thenewpassword"}' http://localhost:8080/user/password
+```
+
 ### List Active Sessions
 
 This request lists sessions for all logged in users.

+ 52 - 29
api.yml

@@ -16,7 +16,11 @@ paths:
               schema:
                 type: array
                 items:
-                  $ref: '#/components/schemas/User'
+                  type: object
+                  properties:
+                    screen_name:
+                      type: string
+                      description: The user's screen name.
     post:
       summary: Create a new user
       description: Create a new user account with a screen name and password.
@@ -25,12 +29,21 @@ paths:
         content:
           application/json:
             schema:
-              $ref: '#/components/schemas/User'
+              type: object
+              properties:
+                screen_name:
+                  type: string
+                  description: The user's screen name.
+                password:
+                  type: string
+                  description: The user's password for account creation.
       responses:
         '201':
           description: User account created successfully.
         '400':
           description: Bad request. Invalid input data.
+        '409':
+          description: Conflict. A user with the specified screen name already exists.
 
   /session:
     get:
@@ -42,31 +55,41 @@ paths:
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/SessionResponse'
+                type: object
+                properties:
+                  count:
+                    type: integer
+                    description: The number of active sessions.
+                  sessions:
+                    type: array
+                    items:
+                      type: object
+                      properties:
+                        screen_name:
+                          type: string
+                          description: The screen name associated with the session.
 
-components:
-  schemas:
-    User:
-      type: object
-      properties:
-        screen_name:
-          type: string
-          description: The user's screen name.
-
-    SessionResponse:
-      type: object
-      properties:
-        count:
-          type: integer
-          description: The number of active sessions.
-        sessions:
-          type: array
-          items:
-            $ref: '#/components/schemas/Session'
-
-    Session:
-      type: object
-      properties:
-        screen_name:
-          type: string
-          description: The screen name associated with the session.
+  /user/password:
+    put:
+      summary: Set a user's password
+      description: Update the password for a user specified by their screen name.
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                screen_name:
+                  type: string
+                  description: The screen name of the user whose password is to be updated.
+                password:
+                  type: string
+                  description: The new password for the user.
+      responses:
+        '204':
+          description: Password updated successfully.
+        '400':
+          description: Bad request. Invalid input data.
+        '404':
+          description: User not found.

+ 93 - 12
server/http/mgmt_api.go

@@ -2,6 +2,7 @@ package http
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -13,7 +14,7 @@ import (
 	"github.com/mk6i/retro-aim-server/state"
 )
 
-type createUser struct {
+type userWithPassword struct {
 	state.User
 	Password string `json:"password,omitempty"`
 }
@@ -30,6 +31,7 @@ type onlineUsers struct {
 type UserManager interface {
 	AllUsers() ([]state.User, error)
 	InsertUser(u state.User) error
+	SetUserPassword(u state.User) error
 }
 
 type SessionRetriever interface {
@@ -38,8 +40,14 @@ type SessionRetriever interface {
 
 func StartManagementAPI(userManager UserManager, sessionRetriever SessionRetriever, logger *slog.Logger) {
 	mux := http.NewServeMux()
+	newUser := func() state.User {
+		return state.User{AuthKey: uuid.New().String()}
+	}
 	mux.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
-		userHandler(w, r, userManager, logger)
+		userHandler(w, r, userManager, newUser, logger)
+	})
+	mux.HandleFunc("/user/password", func(w http.ResponseWriter, r *http.Request) {
+		userPasswordHandler(w, r, userManager, newUser, logger)
 	})
 	mux.HandleFunc("/session", func(w http.ResponseWriter, r *http.Request) {
 		sessionHandler(w, r, sessionRetriever)
@@ -54,17 +62,74 @@ func StartManagementAPI(userManager UserManager, sessionRetriever SessionRetriev
 	}
 }
 
-func userHandler(w http.ResponseWriter, r *http.Request, userManager UserManager, logger *slog.Logger) {
+func userHandler(
+	w http.ResponseWriter,
+	r *http.Request,
+	userManager UserManager,
+	newUser func() state.User,
+	logger *slog.Logger,
+) {
 	switch r.Method {
 	case http.MethodGet:
 		getUserHandler(w, r, userManager, logger)
 	case http.MethodPost:
-		postUserHandler(w, r, userManager, logger)
+		postUserHandler(w, r, userManager, newUser, logger)
+	default:
+		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+	}
+}
+
+func userPasswordHandler(
+	w http.ResponseWriter,
+	r *http.Request,
+	userManager UserManager,
+	userFactory func() state.User,
+	logger *slog.Logger,
+) {
+	switch r.Method {
+	case http.MethodPut:
+		putUserPasswordHandler(w, r, userManager, userFactory, logger)
 	default:
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 	}
 }
 
+// putUserPasswordHandler handles the PUT /user/password endpoint.
+func putUserPasswordHandler(
+	w http.ResponseWriter,
+	r *http.Request,
+	userManager UserManager,
+	newUser func() state.User,
+	logger *slog.Logger,
+) {
+	user := userWithPassword{
+		User: newUser(),
+	}
+	if err := json.NewDecoder(r.Body).Decode(&user); err != nil {
+		http.Error(w, "malformed input", http.StatusBadRequest)
+		return
+	}
+	if err := user.HashPassword(user.Password); err != nil {
+		logger.Error("error hashing user password in PUT /user/password", "err", err.Error())
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
+	if err := userManager.SetUserPassword(user.User); err != nil {
+		switch {
+		case errors.Is(err, state.ErrNoUser):
+			http.Error(w, "user does not exist", http.StatusNotFound)
+			return
+		case err != nil:
+			logger.Error("error updating user password PUT /user/password", "err", err.Error())
+			http.Error(w, "internal server error", http.StatusInternalServerError)
+			return
+		}
+	}
+
+	w.WriteHeader(http.StatusNoContent)
+}
+
 // sessionHandler handles GET /session
 func sessionHandler(w http.ResponseWriter, r *http.Request, sessionRetriever SessionRetriever) {
 	w.Header().Set("Content-Type", "application/json")
@@ -108,17 +173,33 @@ func getUserHandler(w http.ResponseWriter, _ *http.Request, userManager UserMana
 }
 
 // postUserHandler handles the POST /user endpoint.
-func postUserHandler(w http.ResponseWriter, r *http.Request, userManager UserManager, logger *slog.Logger) {
-	var newUser createUser
-	if err := json.NewDecoder(r.Body).Decode(&newUser); err != nil {
+func postUserHandler(
+	w http.ResponseWriter,
+	r *http.Request,
+	userManager UserManager,
+	newUser func() state.User,
+	logger *slog.Logger,
+) {
+	user := userWithPassword{
+		User: newUser(),
+	}
+	if err := json.NewDecoder(r.Body).Decode(&user); err != nil {
 		http.Error(w, "malformed input", http.StatusBadRequest)
 		return
 	}
-	newUser.AuthKey = uuid.New().String()
-	// todo does the request contain authkey?
-	newUser.HashPassword(newUser.Password)
-	if err := userManager.InsertUser(newUser.User); err != nil {
-		logger.Error("error in GET /user", "err", err.Error())
+	if err := user.HashPassword(user.Password); err != nil {
+		logger.Error("error hashing user password in POST /user", "err", err.Error())
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
+	err := userManager.InsertUser(user.User)
+	switch {
+	case errors.Is(err, state.ErrDupUser):
+		http.Error(w, "user already exists", http.StatusConflict)
+		return
+	case err != nil:
+		logger.Error("error inserting user POST /user", "err", err.Error())
 		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
 	}

+ 137 - 20
server/http/mgmt_api_test.go

@@ -8,9 +8,9 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/mk6i/retro-aim-server/state"
+	"github.com/stretchr/testify/assert"
 
-	"github.com/stretchr/testify/mock"
+	"github.com/mk6i/retro-aim-server/state"
 )
 
 func TestSessionHandler_GET(t *testing.T) {
@@ -125,7 +125,7 @@ func TestUserHandler_GET(t *testing.T) {
 				AllUsers().
 				Return(tc.users, tc.userHandlerErr)
 
-			userHandler(responseRecorder, request, userManager, slog.Default())
+			userHandler(responseRecorder, request, userManager, nil, slog.Default())
 
 			if responseRecorder.Code != tc.statusCode {
 				t.Errorf("Want status '%d', got '%d'", tc.statusCode, responseRecorder.Code)
@@ -150,31 +150,54 @@ func TestUserHandler_POST(t *testing.T) {
 		{
 			name: "with valid user",
 			body: `{"screen_name":"userA", "password":"thepassword"}`,
-			user: state.User{
-				ScreenName: "userA",
-			},
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
 			want:       `User account created successfully.`,
 			statusCode: http.StatusCreated,
 		},
 		{
-			name: "with malformed body",
-			body: `{"screen_name":"userA", "password":"thepassword"`,
-			user: state.User{
-				ScreenName: "userA",
-			},
+			name:       "with malformed body",
+			body:       `{"screen_name":"userA", "password":"thepassword"`,
+			user:       state.User{},
 			want:       `malformed input`,
 			statusCode: http.StatusBadRequest,
 		},
 		{
 			name: "user handler error",
 			body: `{"screen_name":"userA", "password":"thepassword"}`,
-			user: state.User{
-				ScreenName: "userA",
-			},
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
 			userHandlerErr: io.EOF,
 			want:           `internal server error`,
 			statusCode:     http.StatusInternalServerError,
 		},
+		{
+			name: "duplicate user",
+			body: `{"screen_name":"userA", "password":"thepassword"}`,
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
+			userHandlerErr: state.ErrDupUser,
+			want:           `user already exists`,
+			statusCode:     http.StatusConflict,
+		},
 	}
 
 	for _, tc := range tt {
@@ -183,12 +206,106 @@ func TestUserHandler_POST(t *testing.T) {
 			responseRecorder := httptest.NewRecorder()
 
 			userManager := newMockUserManager(t)
-			userManager.EXPECT().
-				InsertUser(mock.Anything). // todo make this more concrete
-				Return(tc.userHandlerErr).
-				Maybe()
+			if tc.user.ScreenName != "" {
+				userManager.EXPECT().
+					InsertUser(tc.user).
+					Return(tc.userHandlerErr)
+			}
+
+			newUser := func() state.User {
+				return tc.user
+			}
+			userHandler(responseRecorder, request, userManager, newUser, slog.Default())
 
-			userHandler(responseRecorder, request, userManager, slog.Default())
+			if responseRecorder.Code != tc.statusCode {
+				t.Errorf("want status '%d', got '%d'", tc.statusCode, responseRecorder.Code)
+			}
+
+			if strings.TrimSpace(responseRecorder.Body.String()) != tc.want {
+				t.Errorf("want '%s', got '%s'", tc.want, responseRecorder.Body)
+			}
+		})
+	}
+}
+
+func TestUserPasswordHandler_PUT(t *testing.T) {
+	tt := []struct {
+		name           string
+		body           string
+		user           state.User
+		userHandlerErr error
+		want           string
+		statusCode     int
+	}{
+		{
+			name: "with valid password",
+			body: `{"screen_name":"userA", "password":"thepassword"}`,
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
+			want:       ``,
+			statusCode: http.StatusNoContent,
+		},
+		{
+			name:       "with malformed body",
+			body:       `{"screen_name":"userA", "password":"thepassword"`,
+			user:       state.User{},
+			want:       `malformed input`,
+			statusCode: http.StatusBadRequest,
+		},
+		{
+			name: "user password handler error",
+			body: `{"screen_name":"userA", "password":"thepassword"}`,
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
+			userHandlerErr: io.EOF,
+			want:           `internal server error`,
+			statusCode:     http.StatusInternalServerError,
+		},
+		{
+			name: "user doesn't exist",
+			body: `{"screen_name":"userA", "password":"thepassword"}`,
+			user: func() state.User {
+				user := state.User{
+					AuthKey:    "theAuthKey",
+					ScreenName: "userA",
+				}
+				assert.NoError(t, user.HashPassword("thepassword"))
+				return user
+			}(),
+			userHandlerErr: state.ErrNoUser,
+			want:           `user does not exist`,
+			statusCode:     http.StatusNotFound,
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			request := httptest.NewRequest(http.MethodPut, "/user", strings.NewReader(tc.body))
+			responseRecorder := httptest.NewRecorder()
+
+			userManager := newMockUserManager(t)
+			if tc.user.ScreenName != "" {
+				userManager.EXPECT().
+					SetUserPassword(tc.user).
+					Return(tc.userHandlerErr)
+			}
+
+			uf := func() state.User {
+				return tc.user
+			}
+			userPasswordHandler(responseRecorder, request, userManager, uf, slog.Default())
 
 			if responseRecorder.Code != tc.statusCode {
 				t.Errorf("want status '%d', got '%d'", tc.statusCode, responseRecorder.Code)
@@ -205,7 +322,7 @@ func TestUserHandler_DisallowedMethod(t *testing.T) {
 	request := httptest.NewRequest(http.MethodPut, "/user", nil)
 	responseRecorder := httptest.NewRecorder()
 
-	userHandler(responseRecorder, request, nil, nil)
+	userHandler(responseRecorder, request, nil, nil, nil)
 
 	wantCode := http.StatusMethodNotAllowed
 	if responseRecorder.Code != wantCode {

+ 46 - 0
server/http/mock_user_manager_test.go

@@ -123,6 +123,52 @@ func (_c *mockUserManager_InsertUser_Call) RunAndReturn(run func(state.User) err
 	return _c
 }
 
+// SetUserPassword provides a mock function with given fields: u
+func (_m *mockUserManager) SetUserPassword(u state.User) error {
+	ret := _m.Called(u)
+
+	if len(ret) == 0 {
+		panic("no return value specified for SetUserPassword")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(state.User) error); ok {
+		r0 = rf(u)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// mockUserManager_SetUserPassword_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetUserPassword'
+type mockUserManager_SetUserPassword_Call struct {
+	*mock.Call
+}
+
+// SetUserPassword is a helper method to define mock.On call
+//   - u state.User
+func (_e *mockUserManager_Expecter) SetUserPassword(u interface{}) *mockUserManager_SetUserPassword_Call {
+	return &mockUserManager_SetUserPassword_Call{Call: _e.mock.On("SetUserPassword", u)}
+}
+
+func (_c *mockUserManager_SetUserPassword_Call) Run(run func(u state.User)) *mockUserManager_SetUserPassword_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(state.User))
+	})
+	return _c
+}
+
+func (_c *mockUserManager_SetUserPassword_Call) Return(_a0 error) *mockUserManager_SetUserPassword_Call {
+	_c.Call.Return(_a0)
+	return _c
+}
+
+func (_c *mockUserManager_SetUserPassword_Call) RunAndReturn(run func(state.User) error) *mockUserManager_SetUserPassword_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
 // newMockUserManager creates a new instance of mockUserManager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
 // The first argument is typically a *testing.T value.
 func newMockUserManager(t interface {

+ 74 - 5
state/user_store.go

@@ -27,6 +27,13 @@ type BlockedState int
 //go:embed migrations/*
 var migrations embed.FS
 
+var (
+	// ErrDupUser indicates that a user already exists.
+	ErrDupUser = errors.New("user already exists")
+	// ErrNoUser indicates that a user does not exist.
+	ErrNoUser = errors.New("user does not exist")
+)
+
 const (
 	// BlockedNo indicates that neither user blocks the other.
 	BlockedNo BlockedState = iota
@@ -44,6 +51,12 @@ type User struct {
 	WeakMD5Pass   []byte `json:"-"`
 }
 
+func NewUser() User {
+	return User{
+		AuthKey: uuid.New().String(),
+	}
+}
+
 // HashPassword creates a password hash using the MD5 digest algorithm. The
 // hash is stored in the User.StrongMD5Pass field.
 func (u *User) HashPassword(passwd string) error {
@@ -179,16 +192,72 @@ func (f SQLiteUserStore) User(screenName string) (*User, error) {
 	return u, err
 }
 
-// InsertUser inserts a user to the store. It does not overwrite any fields if
-// the user already exists.
+// InsertUser inserts a user to the store. Return ErrDupUser if a user with the
+// same screen name already exists.
 func (f SQLiteUserStore) InsertUser(u User) error {
 	q := `
 		INSERT INTO user (screenName, authKey, weakMD5Pass, strongMD5Pass)
 		VALUES (?, ?, ?, ?)
-		ON CONFLICT DO NOTHING
+		ON CONFLICT (screenName) DO NOTHING
 	`
-	_, err := f.db.Exec(q, u.ScreenName, u.AuthKey, u.WeakMD5Pass, u.StrongMD5Pass)
-	return err
+	result, err := f.db.Exec(q, u.ScreenName, u.AuthKey, u.WeakMD5Pass, u.StrongMD5Pass)
+	if err != nil {
+		return err
+	}
+
+	rowsAffected, err := result.RowsAffected()
+	if err != nil {
+		return err
+	}
+	if rowsAffected == 0 {
+		return ErrDupUser
+	}
+
+	return nil
+}
+
+// SetUserPassword sets the user's password hashes and auth key.
+func (f SQLiteUserStore) SetUserPassword(u User) error {
+	tx, err := f.db.Begin()
+	if err != nil {
+		return err
+	}
+
+	defer func() {
+		if err != nil {
+			tx.Rollback()
+		}
+	}()
+
+	q := `
+		UPDATE user
+		SET authKey = ?, weakMD5Pass = ?, strongMD5Pass = ?
+		WHERE screenName = ?
+	`
+	result, err := tx.Exec(q, u.AuthKey, u.WeakMD5Pass, u.StrongMD5Pass, u.ScreenName)
+	if err != nil {
+		return err
+	}
+
+	rowsAffected, err := result.RowsAffected()
+	if err != nil {
+		return err
+	}
+
+	if rowsAffected == 0 {
+		// it's possible the user didn't change OR the user doesn't exist.
+		// check if the user exists.
+		var exists int
+		err = tx.QueryRow("SELECT COUNT(*) FROM user WHERE screenName = ?", u.ScreenName).Scan(&exists)
+		if err != nil {
+			return err // Handle possible SQL errors during the select
+		}
+		if exists == 0 {
+			return ErrNoUser // User does not exist
+		}
+	}
+
+	return tx.Commit()
 }
 
 // Feedbag fetches the contents of a user's feedbag (buddy list).