Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the implementation of showing the old QR code #6847

Closed
wants to merge 3 commits into from
+61 −88
Closed

Remove the implementation of showing the old QR code #6847

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27b16ef
remove the implementation of showing the old qr code
Sachin-Mamoru Sep 1, 2024
2834fb6
add changeset
Sachin-Mamoru Sep 1, 2024
ac880b5
Update apps/myaccount/src/components/multi-factor-authentication/auth…
Sachin-Mamoru Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chatty-fishes-peel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@wso2is/myaccount": minor
---

Remove the implementation of showing the old qr code
145 changes: 57 additions & 88 deletions ...yaccount/src/components/multi-factor-authentication/authenticators/totp-authenticator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
* under the License.
*/

import { TestableComponentInterface } from "@wso2is/core/models";

Check warning on line 19 in apps/myaccount/src/components/multi-factor-authentication/authenticators/totp-authenticator.tsx

View workflow job for this annotation

GitHub Actions / ⬣ ESLint (STATIC ANALYSIS) (lts/*, 8.7.4) 

incorrect header
import { GenericIcon, LinkButton, Popup } from "@wso2is/react-components";
import QRCode from "qrcode.react";
import React, { FormEvent, PropsWithChildren, SyntheticEvent, useEffect, useRef, useState } from "react";
import React, { PropsWithChildren, ReactElement, SyntheticEvent, useEffect, useRef, useState } from "react";
import { Trans, useTranslation } from "react-i18next";
import { useSelector } from "react-redux";
import {
Expand All @@ -41,11 +41,9 @@
initTOTPCode,
refreshTOTPCode,
updateEnabledAuthenticators,
validateTOTPCode,
viewTOTPCode
validateTOTPCode
} from "../../../api";
import { getMFAIcons } from "../../../configs";
import { commonConfig } from "../../../extensions";
import {
AlertInterface,
AlertLevels,
Expand Down Expand Up @@ -103,8 +101,8 @@
const [ isTOTPError, setIsTOTPError ] = useState<boolean>(false);
const [ isRevokeTOTPModalOpen, setIsRevokeTOTPModalOpen ] = useState<boolean>(false);
const [ isViewTOTPModalOpen, setIsViewTOTPModalOpen ] = useState<boolean>(false);
const [ isConfirmRegenerationModalOpen, setIsConfirmRegenerationModalOpen ] = useState<boolean>(false);
const [ viewTOTPModalCurrentStep, setViewTOTPModalCurrentStep ] = useState<number>(0);
const [ isConfirmRegenerate, setIsConfirmRegenerate ] = useState<boolean>(false);

const pinCode1: React.MutableRefObject<HTMLInputElement> = useRef<HTMLInputElement>();
const pinCode2: React.MutableRefObject<HTMLInputElement> = useRef<HTMLInputElement>();
Expand Down Expand Up @@ -654,39 +652,6 @@
);
};

/**
* Handle view TOTP event.
*/
const handleViewTOTP = (): void => {
setIsLoading(true);
viewTOTPCode()
.then((response: any) => {
const qrCodeUrl: string = window.atob(response?.data?.qrCodeUrl);

setQrCode(qrCodeUrl);
setIsViewTOTPModalOpen(true);
})
.catch((error: any) => {
onAlertFired({
description: t(translateKey + "notifications.initError.error.description", {
error
}),
level: AlertLevels.ERROR,
message: t(translateKey + "notifications.initError.error.message")
});
})
.finally(() => {
setIsLoading(false);
});
};

/**
* Handle confirm QR code regeneration checkbox event.
*/
const handleConfirmRegenerateQRCode = (_: FormEvent<HTMLInputElement>, data: CheckboxProps): void => {
setIsConfirmRegenerate(data.checked ?? false);
};

/**
* Handle regenerate QR code event.
*/
Expand All @@ -697,6 +662,8 @@
const qrCodeUrl: string = window.atob(response?.data?.qrCodeUrl);

setQrCode(qrCodeUrl);
setIsViewTOTPModalOpen(true);
setIsConfirmRegenerationModalOpen(false);
setViewTOTPModalCurrentStep(1);
})
.catch((errorMessage: any) => {
Expand All @@ -713,6 +680,49 @@
});
};

/**
* Render the totp QR code regenerate confirmation modal
*/
const renderConfirmRegenerateModal = (): ReactElement => {
return (
Sachin-Mamoru marked this conversation as resolved.
Show resolved Hide resolved
<Modal
data-testid={ `${testId}-regenerate-confirm-modal` }
size="small"
open={ isConfirmRegenerationModalOpen }
closeOnDimmerClick={ false }
dimmer="blurring"
>
<Modal.Content data-testid={ `${testId}-regenerate-confirm-modal-content` }>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refrain from using data-testid use data-componentid instead.

Copy link
Contributor

@Achintha444 Achintha444 Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check other places and remove data-testid

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a component we had earlier, not a new one. So it has already used testids and I think if we change them, a lot of e2e might break.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to add a data-componentid param then, keeping the data-testid, and create an issue to check the E2E on this

<Container>
<h3>{ t(translateKey + "modals.scan.regenerateConfirmLabel") }</h3>
</Container>
<Message className="display-flex" size="small" warning>
<Icon name="info" color="orange" corner />
<Message.Content className="tiny">
{ t(translateKey + "modals.scan.regenerateWarning.generic") }
</Message.Content>
</Message>
</Modal.Content>
<Modal.Actions data-testid={ `${testId}-regenerate-confirm-modal-actions` }>
<LinkButton
color="red"
onClick={ () => setIsConfirmRegenerationModalOpen(false) }
data-testid={ `${testId}-regenerate-confirm-modal-actions-cancel-button` }
>
{ t("common:cancel") }
</LinkButton>
<Button
color="red"
onClick={ handleRegenerateQRCode }
data-testid={ `${testId}-regenerate-confirm-modal-actions-confirm-button` }
>
{ t(translateKey + "regenerate") }
</Button>
</Modal.Actions>
</Modal>
);
};

/**
* Renders modal content.
*
Expand All @@ -724,59 +734,15 @@
return (
<Segment basic >
<h5 className=" text-center"> { t(translateKey + "modals.scan.heading") }</h5>
<Segment textAlign="center" basic className="qr-code">
{ qrCode
? <QRCode value={ qrCode } data-testid={ `${ testId }-view-modals-scan-qrcode` }/>
: null
}
</Segment>
<div className="ml-3 mr-3">
<Message className="display-flex" size="small" warning>
<Icon name="warning sign" color="orange" corner />
<Message.Content className="tiny">
{ t(commonConfig.accountSecurityPage.mfa.totp.regenerateWarning) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the possibility of removing regenerateWarning config as well. Seems the usage has been removed this PR.

</Message.Content>
</Message>
{
commonConfig.accountSecurityPage.mfa.totp.showRegenerateConfirmation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the showRegenerateConfirmation from config files as well. With the improved UI this config won't be used.

identity-apps/apps/myaccount/src/extensions/configs/common.ts

Line 39 in f5b6df6

showRegenerateConfirmation: false

After the unification the config is always set to false. Therefore no point of keeping it at all.

? (
<Checkbox
className="mb-2"
label={ t(translateKey + "modals.scan.regenerateConfirmLabel") }
checked={ isConfirmRegenerate }
onChange={ handleConfirmRegenerateQRCode }
data-componentid={ `${ testId }-regenerate-assertion-checkbox` }
/>
) : null
}
<div className = "totp-verify-step-btn">
<Grid.Row columns={ 1 }>
<Grid.Column mobile={ 16 } tablet={ 16 } computer={ 16 }>
<Button
primary
type="button"
className=" totp-verify-action-button"
onClick={ handleRegenerateQRCode }
disabled= {
isLoading
|| (commonConfig.accountSecurityPage
.mfa.totp.showRegenerateConfirmation && !isConfirmRegenerate)
}
data-testid={ `${ testId }-view-modal-actions-primary-button` }
>
{ t(translateKey + "regenerate") }
</Button>
</Grid.Column>
</Grid.Row>
</div>
<div className = "totp-verify-step-btn">
<Grid.Row columns={ 1 }>
<Grid.Column mobile={ 16 } tablet={ 16 } computer={ 16 }>
<Button
type="button"
onClick={ () => {
setIsConfirmRegenerate(false);
setIsViewTOTPModalOpen(false);
setIsConfirmRegenerationModalOpen(false);
} }
className="link-button totp-verify-action-button"
data-testid={ `${ testId }-view-modal-actions-cancel-button` }>
Expand Down Expand Up @@ -863,8 +829,8 @@
className = "totp-verify-done-button"
data-testid={ `${ testId }-modal-actions-primary-button` }
onClick= { () => {
setIsConfirmRegenerate(false);
setIsViewTOTPModalOpen(false);
setIsConfirmRegenerationModalOpen(false);
setViewTOTPModalCurrentStep(0);
handleSessionTerminationModalVisibility(true);
} }
Expand Down Expand Up @@ -1028,6 +994,7 @@
) : (
<>
{ renderViewTOTPWizard() }
{ renderConfirmRegenerateModal() }
{ renderRevokeTOTPModal() }
<Grid padded={ true } data-testid={ testId }>
<Grid.Row columns={ 3 }>
Expand Down Expand Up @@ -1090,16 +1057,18 @@
(
<Icon
link={ true }
onClick={ handleViewTOTP }
onClick={ () => {
setIsConfirmRegenerationModalOpen(true);
} }
className="list-icon padded-icon"
size="small"
color="grey"
name="eye"
data-testid={ `${testId}-view-button` }
name="refresh"
data-testid={ `${testId}-regenerate-button` }
/>
)
}
content={ t(translateKey + "hint") }
content={ t(translateKey + "regenerate") }
position="top right"
inverted
/>
Expand Down
Loading