Merge pull request #120 from mittwald/fix/optional-old-user-password

do not require the old user password on updates
mittwald · Dec 2, 2021 · 1f6996d · 1f6996d
2 parents d4487de + 23d0e1b
commit 1f6996d
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 30 deletions.
diff --git a/apiv2/client.go b/apiv2/client.go
@@ -425,8 +425,8 @@ func (c *RESTClient) UpdateUserProfile(ctx context.Context, id int64, profile *m
 	return c.user.UpdateUserProfile(ctx, id, profile)
 }
 
-func (c *RESTClient) UpdateUserPassword(ctx context.Context, userID int64, old, new string) error {
-	return c.user.UpdateUserPassword(ctx, userID, old, new)
+func (c *RESTClient) UpdateUserPassword(ctx context.Context, userID int64, passwordRequest *modelv2.PasswordReq) error {
+	return c.user.UpdateUserPassword(ctx, userID, passwordRequest)
 }
 
 func (c *RESTClient) UserExists(ctx context.Context, idOrName intstr.IntOrString) (bool, error) {

diff --git a/apiv2/pkg/clients/user/user.go b/apiv2/pkg/clients/user/user.go
@@ -46,7 +46,7 @@ type Client interface {
 	SetUserSysAdmin(ctx context.Context, id int64, admin bool) error
 	DeleteUser(ctx context.Context, id int64) error
 	UpdateUserProfile(ctx context.Context, id int64, profile *modelv2.UserProfile) error
-	UpdateUserPassword(ctx context.Context, userID int64, old, new string) error
+	UpdateUserPassword(ctx context.Context, userID int64, passwordRequest *modelv2.PasswordReq) error
 	UserExists(ctx context.Context, idOrName intstr.IntOrString) (bool, error)
 }
 
@@ -265,10 +265,9 @@ func (c *RESTClient) UpdateUserProfile(ctx context.Context, id int64, profile *m
 }
 
 // UpdateUserPassword updates a user's password from 'old' to 'new'.
-func (c *RESTClient) UpdateUserPassword(ctx context.Context, userID int64, old, new string) error {
-	if old == "" {
-		return errors.New("no old password provided")
-	} else if new == "" {
+// 'old' is an optional parameter when called by an administrator.
+func (c *RESTClient) UpdateUserPassword(ctx context.Context, userID int64, passwordRequest *modelv2.PasswordReq) error {
+	if passwordRequest.NewPassword == "" {
 		return errors.New("no new password provided")
 	}
 
@@ -278,12 +277,9 @@ func (c *RESTClient) UpdateUserPassword(ctx context.Context, userID int64, old,
 	}
 
 	params := &user.UpdateUserPasswordParams{
-		Password: &modelv2.PasswordReq{
-			OldPassword: old,
-			NewPassword: new,
-		},
-		UserID:  userID,
-		Context: ctx,
+		Password: passwordRequest,
+		UserID:   userID,
+		Context:  ctx,
 	}
 
 	params.WithTimeout(c.Options.Timeout)

diff --git a/apiv2/pkg/clients/user/user_integration_test.go b/apiv2/pkg/clients/user/user_integration_test.go
@@ -158,7 +158,7 @@ func TestAPIUserDelete(t *testing.T) {
 	require.Nil(t, usr)
 }
 
-func TestAPIUserUpdate(t *testing.T) {
+func TestAPIUserUpdate_Profile(t *testing.T) {
 	ctx := context.Background()
 
 	c := NewClient(clienttesting.V2SwaggerClient, clienttesting.DefaultOpts, clienttesting.AuthInfo)
@@ -188,3 +188,26 @@ func TestAPIUserUpdate(t *testing.T) {
 	require.Equal(t, usr.Comment, "Some other comment")
 	require.Equal(t, usr.Realname, "Foo Baz")
 }
+
+func TestAPIUserUpdate_Password(t *testing.T) {
+	ctx := context.Background()
+
+	c := NewClient(clienttesting.V2SwaggerClient, clienttesting.DefaultOpts, clienttesting.AuthInfo)
+	err := c.NewUser(ctx, username, email, realname, password, comments)
+	require.NoError(t, err)
+
+	usr, err := c.GetUserByName(ctx, username)
+	require.NoError(t, err)
+
+	require.NotNil(t, usr)
+
+	defer func() {
+		_ = c.DeleteUser(ctx, usr.UserID)
+	}()
+
+	err = c.UpdateUserPassword(ctx, usr.UserID, &modelv2.PasswordReq{
+		NewPassword: password + "1",
+		OldPassword: "",
+	})
+	require.NoError(t, err)
+}
diff --git a/apiv2/pkg/clients/user/user_test.go b/apiv2/pkg/clients/user/user_test.go
@@ -253,24 +253,13 @@ func TestRESTClient_UpdateUserPassword(t *testing.T) {
 			mock.AnythingOfType("runtime.ClientAuthInfoWriterFunc")).
 			Return(&user.UpdateUserPasswordOK{}, nil)
 
-		err := apiClient.UpdateUserPassword(ctx, exampleUserID, "foo", "bar")
+		err := apiClient.UpdateUserPassword(ctx, exampleUserID, &modelv2.PasswordReq{
+			NewPassword: "bar",
+			OldPassword: "foo",
+		})
 		require.NoError(t, err)
 	})
 
-	t.Run("NoOldPassword", func(t *testing.T) {
-		mockClient.User.On("GetUser", getParams,
-			mock.AnythingOfType("runtime.ClientAuthInfoWriterFunc")).
-			Return(&user.GetUserOK{Payload: &modelv2.UserResp{UserID: exampleUserID}}, nil)
-
-		mockClient.User.On("UpdateUserPassword", updatePWParams,
-			mock.AnythingOfType("runtime.ClientAuthInfoWriterFunc")).
-			Return(&user.UpdateUserPasswordOK{}, nil)
-
-		err := apiClient.UpdateUserPassword(ctx, exampleUserID, "", "bar")
-		require.Error(t, err)
-		require.Contains(t, err.Error(), "no old password provided")
-	})
-
 	t.Run("NoNewPassword", func(t *testing.T) {
 		mockClient.User.On("GetUser", getParams,
 			mock.AnythingOfType("runtime.ClientAuthInfoWriterFunc")).
@@ -280,7 +269,10 @@ func TestRESTClient_UpdateUserPassword(t *testing.T) {
 			mock.AnythingOfType("runtime.ClientAuthInfoWriterFunc")).
 			Return(&user.UpdateUserPasswordOK{}, nil)
 
-		err := apiClient.UpdateUserPassword(ctx, exampleUserID, "foo", "")
+		err := apiClient.UpdateUserPassword(ctx, exampleUserID, &modelv2.PasswordReq{
+			NewPassword: "",
+			OldPassword: "foo",
+		})
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "no new password provided")
 	})