fix old password match (#14273)

* fix old password match * return bad request * added test cases & code refactor * comments addressed * code refactor --------- Co-authored-by: mohitdeuex <mohit.y@deuexsolutions.com>
open-metadata · Dec 13, 2023 · d3b5a1a · d3b5a1a
1 parent bc4d048
commit d3b5a1a
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 13 deletions.
diff --git a/...data-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java b/...data-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java
@@ -16,7 +16,6 @@
 import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
 import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR;
 import static javax.ws.rs.core.Response.Status.NOT_IMPLEMENTED;
-import static javax.ws.rs.core.Response.Status.UNAUTHORIZED;
 import static org.openmetadata.schema.api.teams.CreateUser.CreatePasswordType.ADMIN_CREATE;
 import static org.openmetadata.schema.auth.ChangePasswordRequest.RequestType.SELF;
 import static org.openmetadata.schema.auth.ChangePasswordRequest.RequestType.USER;
@@ -271,7 +270,7 @@ public void changeUserPwdWithOldPwd(UriInfo uriInfo, String userName, ChangePass
 
     if (request.getRequestType() == SELF
         && !BCrypt.verifyer().verify(request.getOldPassword().toCharArray(), storedHashPassword).verified) {
-      throw new CustomExceptionMessage(UNAUTHORIZED, "Old Password is not correct");
+      throw new CustomExceptionMessage(BAD_REQUEST, "Old Password is not correct");
     }
 
     storedBasicAuthMechanism.setPassword(newHashedPassword);

diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Users/ChangePasswordForm.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Users/ChangePasswordForm.test.tsx
@@ -0,0 +1,144 @@
+/*
+ *  Copyright 2023 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+import { act, fireEvent, render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+import ChangePasswordForm from './ChangePasswordForm';
+
+const mockSave = jest.fn();
+
+const mockCancel = jest.fn();
+
+const MOCK_PROPS = {
+  visible: true,
+  onCancel: mockCancel,
+  isLoggedInUser: true,
+  isLoading: false,
+  onSave: mockSave,
+};
+
+describe('ChangePasswordForm', () => {
+  it('should render correctly', async () => {
+    render(<ChangePasswordForm {...MOCK_PROPS} />);
+    const modal = await screen.findByTestId('modal-container');
+
+    expect(modal).toBeInTheDocument();
+  });
+
+  it('should handle form submission correctly for logged in user', async () => {
+    render(<ChangePasswordForm {...MOCK_PROPS} />);
+    const cancelButton = await screen.findByText('Cancel');
+    const submitButton = await screen.findByText('label.update-entity');
+
+    expect(cancelButton).toBeInTheDocument();
+    expect(submitButton).toBeInTheDocument();
+
+    userEvent.type(
+      await screen.findByTestId('input-oldPassword'),
+      'oldPassword'
+    );
+
+    userEvent.type(await screen.findByTestId('input-newPassword'), 'Test@123');
+    userEvent.type(
+      await screen.findByTestId('input-confirm-newPassword'),
+      'Test@123'
+    );
+
+    await act(async () => {
+      fireEvent.click(submitButton);
+    });
+
+    expect(mockSave).toHaveBeenCalledTimes(1);
+  });
+
+  it('handles form submission correctly for admin', async () => {
+    render(
+      <ChangePasswordForm
+        visible
+        isLoading={false}
+        isLoggedInUser={false}
+        onCancel={mockCancel}
+        onSave={mockSave}
+      />
+    );
+
+    const cancelButton = await screen.findByText('Cancel');
+    const submitButton = await screen.findByText('label.update-entity');
+
+    expect(cancelButton).toBeInTheDocument();
+    expect(submitButton).toBeInTheDocument();
+
+    userEvent.type(await screen.findByTestId('input-newPassword'), 'Test@123');
+    userEvent.type(
+      await screen.findByTestId('input-confirm-newPassword'),
+      'Test@123'
+    );
+
+    await act(async () => {
+      fireEvent.click(submitButton);
+    });
+
+    expect(mockSave).toHaveBeenCalledWith({
+      newPassword: 'Test@123',
+      confirmPassword: 'Test@123',
+    });
+  });
+
+  it('should invoke onCancel when Cancel button is clicked', async () => {
+    render(
+      <ChangePasswordForm
+        visible
+        isLoading={false}
+        isLoggedInUser={false}
+        onCancel={mockCancel}
+        onSave={mockSave}
+      />
+    );
+
+    const cancelButton = await screen.findByText('Cancel');
+    const submitButton = await screen.findByText('label.update-entity');
+
+    expect(cancelButton).toBeInTheDocument();
+    expect(submitButton).toBeInTheDocument();
+
+    await act(async () => {
+      fireEvent.click(cancelButton);
+    });
+
+    expect(mockCancel).toHaveBeenCalledTimes(1);
+  });
+
+  it('displays loading state during submission', async () => {
+    render(<ChangePasswordForm {...MOCK_PROPS} isLoading />);
+    const submitButton = await screen.findByText('label.update-entity');
+
+    userEvent.type(
+      await screen.findByTestId('input-oldPassword'),
+      'oldPassword'
+    );
+
+    userEvent.type(await screen.findByTestId('input-newPassword'), 'Test@123');
+    userEvent.type(
+      await screen.findByTestId('input-confirm-newPassword'),
+      'Test@123'
+    );
+
+    await act(async () => {
+      fireEvent.click(submitButton);
+    });
+
+    expect(
+      await screen.findByRole('img', { name: 'loading' })
+    ).toBeInTheDocument();
+  });
+});
diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Users/ChangePasswordForm.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Users/ChangePasswordForm.tsx
@@ -42,6 +42,7 @@ const ChangePasswordForm: React.FC<ChangePasswordForm> = ({
       centered
       closable={false}
       confirmLoading={isLoading}
+      data-testid="modal-container"
       maskClosable={false}
       okButtonProps={{
         form: 'change-password-form',
@@ -67,6 +68,7 @@ const ChangePasswordForm: React.FC<ChangePasswordForm> = ({
         onFinish={onSave}>
         {isLoggedInUser && (
           <Form.Item
+            data-testid="oldPassword"
             label={t('label.old-password')}
             name="oldPassword"
             rules={[
@@ -76,7 +78,7 @@ const ChangePasswordForm: React.FC<ChangePasswordForm> = ({
             ]}>
             <Input.Password
               autoComplete="off"
-              data-testid="name"
+              data-testid="input-oldPassword"
               placeholder={t('label.enter-type-password', {
                 type: t('label.old'),
               })}
@@ -97,6 +99,7 @@ const ChangePasswordForm: React.FC<ChangePasswordForm> = ({
           ]}>
           <Input.Password
             autoComplete="off"
+            data-testid="input-newPassword"
             placeholder={t('label.enter-type-password', {
               type: t('label.new'),
             })}
@@ -118,6 +121,7 @@ const ChangePasswordForm: React.FC<ChangePasswordForm> = ({
           ]}>
           <Input.Password
             autoComplete="off"
+            data-testid="input-confirm-newPassword"
             placeholder={t('label.confirm-new-password')}
           />
         </Form.Item>

diff --git a/.../ui/src/components/Users/UsersProfile/UserProfileDetails/UserProfileDetails.component.tsx b/.../ui/src/components/Users/UsersProfile/UserProfileDetails/UserProfileDetails.component.tsx
@@ -171,27 +171,31 @@ const UserProfileDetails = ({
       ),
     [showChangePasswordComponent]
   );
-
   const handleChangePassword = async (data: ChangePasswordRequest) => {
     try {
       setIsLoading(true);
+
+      const newData = {
+        username: userData.name,
+        requestType: isLoggedInUser ? RequestType.Self : RequestType.User,
+      };
+
       const sendData = {
         ...data,
-        ...(isAdminUser &&
-          !isLoggedInUser && {
-            username: userData.name,
-            requestType: RequestType.User,
-          }),
+        ...newData,
       };
+
       await changePassword(sendData);
-      setIsChangePassword(false);
+
       showSuccessToast(
         t('server.update-entity-success', { entity: t('label.password') })
       );
-    } catch (err) {
-      showErrorToast(err as AxiosError);
+
+      setIsChangePassword(false);
+    } catch (error) {
+      showErrorToast(error as AxiosError);
     } finally {
-      setIsLoading(true);
+      setIsLoading(false);
     }
   };