From c26bd6c52fb6d72e68475532592638160015b3ee Mon Sep 17 00:00:00 2001
From: Tharun Rajendran <rajendrantharun@live.com>
Date: Tue, 13 Jul 2021 12:02:03 +0530
Subject: [PATCH] Password Field Improvements (#36160)

* Password: added show password functionality
added autcomplete props
created password component

Fixes #28721

* addressed review changes and added unit tests

* wrapped passwordField component in forwardRef

* fix validation and tests
---
 .../ForgottenPassword/ChangePassword.tsx      | 19 ++++----
 .../ChangePasswordPage.test.tsx               |  4 +-
 .../app/core/components/Login/LoginForm.tsx   |  9 ++--
 .../PasswordField/PasswordField.test.tsx      | 22 ++++++++++
 .../PasswordField/PasswordField.tsx           | 44 +++++++++++++++++++
 .../app/core/components/Signup/SignupPage.tsx | 13 +++---
 .../features/profile/ChangePasswordForm.tsx   | 17 +++----
 7 files changed, 97 insertions(+), 31 deletions(-)
 create mode 100644 public/app/core/components/PasswordField/PasswordField.test.tsx
 create mode 100644 public/app/core/components/PasswordField/PasswordField.tsx

diff --git a/public/app/core/components/ForgottenPassword/ChangePassword.tsx b/public/app/core/components/ForgottenPassword/ChangePassword.tsx
index c85ca19f0f6..69c813b4a50 100644
--- a/public/app/core/components/ForgottenPassword/ChangePassword.tsx
+++ b/public/app/core/components/ForgottenPassword/ChangePassword.tsx
@@ -1,7 +1,8 @@
 import React, { FC, SyntheticEvent } from 'react';
-import { Tooltip, Form, Field, Input, VerticalGroup, Button } from '@grafana/ui';
+import { Tooltip, Form, Field, VerticalGroup, Button } from '@grafana/ui';
 import { selectors } from '@grafana/e2e-selectors';
 import { submitButton } from '../Login/LoginForm';
+import { PasswordField } from '../PasswordField/PasswordField';
 interface Props {
   onSubmit: (pw: string) => void;
   onSkip?: (event?: SyntheticEvent) => void;
@@ -21,21 +22,19 @@ export const ChangePassword: FC<Props> = ({ onSubmit, onSkip }) => {
       {({ errors, register, getValues }) => (
         <>
           <Field label="New password" invalid={!!errors.newPassword} error={errors?.newPassword?.message}>
-            <Input
-              autoFocus
+            <PasswordField
               id="new-password"
-              type="password"
-              {...register('newPassword', {
-                required: 'New password is required',
-              })}
+              autoFocus
+              autoComplete="new-password"
+              {...register('newPassword', { required: 'New Password is required' })}
             />
           </Field>
           <Field label="Confirm new password" invalid={!!errors.confirmNew} error={errors?.confirmNew?.message}>
-            <Input
+            <PasswordField
               id="confirm-new-password"
-              type="password"
+              autoComplete="new-password"
               {...register('confirmNew', {
-                required: 'Confirmed password is required',
+                required: 'Confirmed Password is required',
                 validate: (v: string) => v === getValues().newPassword || 'Passwords must match!',
               })}
             />
diff --git a/public/app/core/components/ForgottenPassword/ChangePasswordPage.test.tsx b/public/app/core/components/ForgottenPassword/ChangePasswordPage.test.tsx
index 57f6ff8ce62..8ef3dfde83a 100644
--- a/public/app/core/components/ForgottenPassword/ChangePasswordPage.test.tsx
+++ b/public/app/core/components/ForgottenPassword/ChangePasswordPage.test.tsx
@@ -48,8 +48,8 @@ describe('ChangePassword Page', () => {
     render(<ChangePasswordPage {...props} />);
 
     fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
-    expect(await screen.findByText('New password is required')).toBeInTheDocument();
-    expect(screen.getByText('Confirmed password is required')).toBeInTheDocument();
+    expect(await screen.findByText('New Password is required')).toBeInTheDocument();
+    expect(screen.getByText('Confirmed Password is required')).toBeInTheDocument();
 
     await act(async () => {
       await userEvent.type(screen.getByLabelText('New password'), 'admin');
diff --git a/public/app/core/components/Login/LoginForm.tsx b/public/app/core/components/Login/LoginForm.tsx
index 2a840e0cdf5..e7dc1d4d589 100644
--- a/public/app/core/components/Login/LoginForm.tsx
+++ b/public/app/core/components/Login/LoginForm.tsx
@@ -4,6 +4,7 @@ import { selectors } from '@grafana/e2e-selectors';
 import { FormModel } from './LoginCtrl';
 import { Button, Form, Input, Field } from '@grafana/ui';
 import { css } from '@emotion/css';
+import { PasswordField } from '../PasswordField/PasswordField';
 
 interface Props {
   children: ReactElement;
@@ -39,11 +40,11 @@ export const LoginForm: FC<Props> = ({ children, onSubmit, isLoggingIn, password
               />
             </Field>
             <Field label="Password" invalid={!!errors.password} error={errors.password?.message}>
-              <Input
+              <PasswordField
+                id="current-password"
+                autoComplete="current-password"
+                passwordHint={passwordHint}
                 {...register('password', { required: 'Password is required' })}
-                type="password"
-                placeholder={passwordHint}
-                aria-label={selectors.pages.Login.password}
               />
             </Field>
             <Button aria-label={selectors.pages.Login.submit} className={submitButton} disabled={isLoggingIn}>
diff --git a/public/app/core/components/PasswordField/PasswordField.test.tsx b/public/app/core/components/PasswordField/PasswordField.test.tsx
new file mode 100644
index 00000000000..0f85ae02901
--- /dev/null
+++ b/public/app/core/components/PasswordField/PasswordField.test.tsx
@@ -0,0 +1,22 @@
+import React from 'react';
+import { fireEvent, render, screen } from '@testing-library/react';
+import { PasswordField } from './PasswordField';
+
+describe('PasswordField', () => {
+  const props = {
+    id: 'password',
+    placeholder: 'enter password',
+    'data-testid': 'password-field',
+  };
+  it('should renders correctly', () => {
+    render(<PasswordField {...props} />);
+    expect(screen.getByTestId('password-field')).toBeInTheDocument();
+    expect(screen.getByRole('button')).toBeInTheDocument();
+  });
+  it('should able to show password value if clicked on password-reveal icon', () => {
+    render(<PasswordField {...props} />);
+    expect(screen.getByTestId('password-field')).toHaveProperty('type', 'password');
+    fireEvent.click(screen.getByRole('button'));
+    expect(screen.getByTestId('password-field')).toHaveProperty('type', 'text');
+  });
+});
diff --git a/public/app/core/components/PasswordField/PasswordField.tsx b/public/app/core/components/PasswordField/PasswordField.tsx
new file mode 100644
index 00000000000..fcf8dad7729
--- /dev/null
+++ b/public/app/core/components/PasswordField/PasswordField.tsx
@@ -0,0 +1,44 @@
+import React, { FC, useState } from 'react';
+import { selectors } from '@grafana/e2e-selectors';
+
+import { Input, IconButton } from '@grafana/ui';
+
+export interface Props {
+  autoFocus?: boolean;
+  autoComplete?: string;
+  id?: string;
+  passwordHint?: string;
+}
+
+export const PasswordField: FC<Props> = React.forwardRef<HTMLInputElement, Props>(
+  ({ autoComplete, autoFocus, id, passwordHint, ...props }, ref) => {
+    const [showPassword, setShowPassword] = useState(false);
+
+    return (
+      <Input
+        id={id}
+        autoFocus={autoFocus}
+        autoComplete={autoComplete}
+        {...props}
+        type={showPassword ? 'text' : 'password'}
+        placeholder={passwordHint}
+        aria-label={selectors.pages.Login.password}
+        ref={ref}
+        suffix={
+          <IconButton
+            name={showPassword ? 'eye-slash' : 'eye'}
+            surface="header"
+            aria-controls={id}
+            aria-expanded={showPassword}
+            onClick={(e) => {
+              e.preventDefault();
+              setShowPassword(!showPassword);
+            }}
+          />
+        }
+      />
+    );
+  }
+);
+
+PasswordField.displayName = 'PasswordField';
diff --git a/public/app/core/components/Signup/SignupPage.tsx b/public/app/core/components/Signup/SignupPage.tsx
index 6eac404d1c6..833b4533e56 100644
--- a/public/app/core/components/Signup/SignupPage.tsx
+++ b/public/app/core/components/Signup/SignupPage.tsx
@@ -6,6 +6,7 @@ import appEvents from 'app/core/app_events';
 import { AppEvents } from '@grafana/data';
 import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
 import { InnerBox, LoginLayout } from '../Login/LoginLayout';
+import { PasswordField } from '../PasswordField/PasswordField';
 
 interface SignupDTO {
   name?: string;
@@ -90,23 +91,21 @@ export const SignupPage: FC<Props> = (props) => {
                 </Field>
               )}
               <Field label="Password" invalid={!!errors.password} error={errors?.password?.message}>
-                <Input
+                <PasswordField
                   id="new-password"
-                  {...register('password', {
-                    required: 'Password is required',
-                  })}
                   autoFocus
-                  type="password"
+                  autoComplete="new-password"
+                  {...register('password', { required: 'Password is required' })}
                 />
               </Field>
               <Field label="Confirm password" invalid={!!errors.confirm} error={errors?.confirm?.message}>
-                <Input
+                <PasswordField
                   id="confirm-new-password"
+                  autoComplete="new-password"
                   {...register('confirm', {
                     required: 'Confirmed password is required',
                     validate: (v) => v === getValues().password || 'Passwords must match!',
                   })}
-                  type="password"
                 />
               </Field>
 
diff --git a/public/app/features/profile/ChangePasswordForm.tsx b/public/app/features/profile/ChangePasswordForm.tsx
index 9afe5d3d1f5..05b6be12a2b 100644
--- a/public/app/features/profile/ChangePasswordForm.tsx
+++ b/public/app/features/profile/ChangePasswordForm.tsx
@@ -1,10 +1,11 @@
 import React, { FC } from 'react';
 import { css } from '@emotion/css';
-import { Button, Field, Form, HorizontalGroup, Input, LinkButton } from '@grafana/ui';
+import { Button, Field, Form, HorizontalGroup, LinkButton } from '@grafana/ui';
 
 import config from 'app/core/config';
 import { UserDTO } from 'app/types';
 import { ChangePasswordFields } from './types';
+import { PasswordField } from '../../core/components/PasswordField/PasswordField';
 
 export interface Props {
   user: UserDTO;
@@ -34,17 +35,17 @@ export const ChangePasswordForm: FC<Props> = ({ user, onChangePassword, isSaving
           return (
             <>
               <Field label="Old password" invalid={!!errors.oldPassword} error={errors?.oldPassword?.message}>
-                <Input
-                  id="old-password"
-                  type="password"
+                <PasswordField
+                  id="current-password"
+                  autoComplete="current-password"
                   {...register('oldPassword', { required: 'Old password is required' })}
                 />
               </Field>
 
               <Field label="New password" invalid={!!errors.newPassword} error={errors?.newPassword?.message}>
-                <Input
+                <PasswordField
                   id="new-password"
-                  type="password"
+                  autoComplete="new-password"
                   {...register('newPassword', {
                     required: 'New password is required',
                     validate: {
@@ -56,9 +57,9 @@ export const ChangePasswordForm: FC<Props> = ({ user, onChangePassword, isSaving
               </Field>
 
               <Field label="Confirm password" invalid={!!errors.confirmNew} error={errors?.confirmNew?.message}>
-                <Input
+                <PasswordField
                   id="confirm-new-password"
-                  type="password"
+                  autoComplete="new-password"
                   {...register('confirmNew', {
                     required: 'New password confirmation is required',
                     validate: (v) => v === getValues().newPassword || 'Passwords must match',