Refactor 2fa modal to prevent implementation details from leaking

src/shared/components/common/totp-modal.tsx

@@ -1,12 +1,21 @@
-import { Component, linkEvent } from "inferno";
+import {
+  Component,
+  MouseEventHandler,
+  RefObject,
+  createRef,
+  linkEvent,
+} from "inferno";
 import { I18NextService } from "../../services";
 import { toast } from "../../toast";
+import type { Modal } from "bootstrap";
 
 interface TotpModalProps {
   /**Takes totp as param, returns whether submit was successful*/
   onSubmit: (totp: string) => Promise<boolean>;
+  onClose: MouseEventHandler;
   type: "login" | "remove" | "generate";
   secretUrl?: string;
+  show?: boolean;
 }
 
 interface TotpModalState {
@@ -17,13 +26,11 @@ interface TotpModalState {
 const TOTP_LENGTH = 6;
 
 async function handleSubmit(modal: TotpModal, totp: string) {
-  const succeeded = await modal.props.onSubmit(totp);
+  const successful = await modal.props.onSubmit(totp);
 
+  if (!successful) {
     modal.setState({ totp: "" });
-  if (succeeded) {
-    document.getElementById("totp-close-button")?.click();
-  } else {
-    document.getElementById(`totp-input-0`)?.focus();
+    modal.inputRefs[0]?.focus();
   }
 }
 
@@ -37,7 +44,7 @@ function handleInput(
   }
 
   modal.setState(prev => ({ ...prev, totp: prev.totp + event.target.value }));
-  document.getElementById(`totp-input-${i + 1}`)?.focus();
+  modal.inputRefs[i + 1]?.focus();
 
   const { totp } = modal.state;
   if (totp.length >= TOTP_LENGTH) {
@@ -56,7 +63,7 @@ function handleKeyUp(
       ...prev,
       totp: prev.totp.slice(0, prev.totp.length - 1),
     }));
-    document.getElementById(`totp-input-${i - 1}`)?.focus();
+    modal.inputRefs[i - 1]?.focus();
   }
 }
 
@@ -77,6 +84,9 @@ export default class TotpModal extends Component<
   TotpModalProps,
   TotpModalState
 > {
+  private readonly modalDivRef: RefObject<HTMLDivElement>;
+  inputRefs: (HTMLInputElement | null)[] = [];
+  modal: Modal;
   state: TotpModalState = {
     totp: "",
   };
@@ -84,32 +94,57 @@ export default class TotpModal extends Component<
   constructor(props: TotpModalProps, context: any) {
     super(props, context);
 
+    this.modalDivRef = createRef();
+
     this.clearTotp = this.clearTotp.bind(this);
     this.handleShow = this.handleShow.bind(this);
   }
 
   async componentDidMount() {
-    document
-      .getElementById("totpModal")
-      ?.addEventListener("shown.bs.modal", this.handleShow);
+    this.modalDivRef.current?.addEventListener(
+      "shown.bs.modal",
+      this.handleShow,
+    );
 
-    document
-      .getElementById("totpModal")
-      ?.addEventListener("hidden.bs.modal", this.clearTotp);
+    this.modalDivRef.current?.addEventListener(
+      "hidden.bs.modal",
+      this.clearTotp,
+    );
+
+    const Modal = (await import("bootstrap/js/dist/modal")).default;
+    this.modal = new Modal(this.modalDivRef.current!);
+
+    if (this.props.show) {
+      this.modal.show();
+    }
   }
 
   componentWillUnmount() {
-    document
-      .getElementById("totpModal")
-      ?.removeEventListener("shown.bs.modal", this.handleShow);
+    this.modalDivRef.current?.removeEventListener(
+      "shown.bs.modal",
+      this.handleShow,
+    );
 
-    document
-      .getElementById("totpModal")
-      ?.removeEventListener("hidden.bs.modal", this.clearTotp);
+    this.modalDivRef.current?.removeEventListener(
+      "hidden.bs.modal",
+      this.clearTotp,
+    );
+
+    this.modal.dispose();
+  }
+
+  componentDidUpdate({ show: prevShow }: TotpModalProps) {
+    if (!!prevShow !== !!this.props.show) {
+      if (this.props.show) {
+        this.modal.show();
+      } else {
+        this.modal.hide();
+      }
+    }
   }
 
   render() {
-    const { type, secretUrl } = this.props;
+    const { type, secretUrl, onClose } = this.props;
     const { totp } = this.state;
 
     return (
@@ -120,6 +155,7 @@ export default class TotpModal extends Component<
         aria-hidden
         aria-labelledby="#totpModalTitle"
         data-bs-backdrop="static"
+        ref={this.modalDivRef}
       >
         <div className="modal-dialog modal-fullscreen-sm-down">
           <div className="modal-content">
@@ -134,9 +170,8 @@ export default class TotpModal extends Component<
               <button
                 type="button"
                 className="btn-close"
-                data-bs-dismiss="modal"
                 aria-label="Close"
-                id="totp-close-button"
+                onClick={onClose}
               />
             </header>
             <div className="modal-body d-flex flex-column  align-items-center justify-content-center">
@@ -186,6 +221,9 @@ export default class TotpModal extends Component<
                       onInput={linkEvent({ modal: this, i }, handleInput)}
                       onKeyUp={linkEvent({ modal: this, i }, handleKeyUp)}
                       onPaste={linkEvent(this, handlePaste)}
+                      ref={element => {
+                        this.inputRefs[i] = element;
+                      }}
                     />
                   ))}
                 </div>
@@ -195,7 +233,7 @@ export default class TotpModal extends Component<
               <button
                 type="button"
                 className="btn btn-danger"
-                data-bs-dismiss="modal"
+                onClick={onClose}
               >
                 {I18NextService.i18n.t("cancel")}
               </button>
@@ -211,7 +249,7 @@ export default class TotpModal extends Component<
   }
 
   async handleShow() {
-    document.getElementById("totp-input-0")?.focus();
+    this.inputRefs[0]?.focus();
 
     if (this.props.type === "generate") {
       const { getSVG } = await import("@shortcm/qr-image/lib/svg");

src/shared/components/home/login.tsx

@@ -30,6 +30,7 @@ interface State {
     password: string;
   };
   siteRes: GetSiteResponse;
+  show2faModal: boolean;
 }
 
 async function handleLoginSuccess(i: Login, loginRes: LoginResponse) {
@@ -65,9 +66,7 @@ async function handleLoginSubmit(i: Login, event: any) {
     switch (loginRes.state) {
       case "failed": {
         if (loginRes.msg === "missing_totp_token") {
-          const Modal = (await import("bootstrap/js/dist/modal")).default;
-          const modal = new Modal(document.getElementById("totpModal")!);
-          modal.show();
+          i.setState({ show2faModal: true });
         } else {
           toast(I18NextService.i18n.t(loginRes.msg), "danger");
         }
@@ -94,6 +93,10 @@ function handleLoginPasswordChange(i: Login, event: any) {
   i.setState(prevState => (prevState.form.password = event.target.value));
 }
 
+function handleClose2faModal(i: Login) {
+  i.setState({ show2faModal: false });
+}
+
 export class Login extends Component<
   RouteComponentProps<Record<string, never>>,
   State
@@ -107,6 +110,7 @@ export class Login extends Component<
       password: "",
     },
     siteRes: this.isoData.site_res,
+    show2faModal: false,
   };
 
   constructor(props: any, context: any) {
@@ -139,7 +143,12 @@ export class Login extends Component<
           title={this.documentTitle}
           path={this.context.router.route.match.url}
         />
-        <TotpModal type="login" onSubmit={this.handleSubmitTotp} />
+        <TotpModal
+          type="login"
+          onSubmit={this.handleSubmitTotp}
+          show={this.state.show2faModal}
+          onClose={linkEvent(this, handleClose2faModal)}
+        />
         <div className="row">
           <div className="col-12 col-lg-6 offset-lg-3">{this.loginForm()}</div>
         </div>
@@ -154,14 +163,15 @@ export class Login extends Component<
       totp_2fa_token: totp,
     });
 
-    const succeeded = loginRes.state === "success";
-    if (succeeded) {
+    const successful = loginRes.state === "success";
+    if (successful) {
+      this.setState({ show2faModal: false });
       handleLoginSuccess(this, loginRes.data);
     } else {
       toast("Invalid 2FA Token", "danger");
     }
 
-    return succeeded;
+    return successful;
   }
 
   loginForm() {

src/shared/components/person/settings.tsx

@@ -118,6 +118,7 @@ interface SettingsState {
   searchPersonOptions: Choice[];
   searchInstanceOptions: Choice[];
   isIsomorphic: boolean;
+  show2faModal: boolean;
 }
 
 type FilterType = "user" | "community" | "instance";
@@ -156,14 +157,15 @@ const Filter = ({
   </div>
 );
 
-async function handleGenerateTotp(i: Settings, event: any) {
+async function handleGenerateTotp(i: Settings) {
   i.setState({ generateTotpRes: LOADING_REQUEST });
 
   const generateTotpRes = await HttpService.client.generateTotpSecret();
 
   if (generateTotpRes.state === "failed") {
-    event.stopPropagation();
     toast(generateTotpRes.msg, "danger");
+  } else {
+    i.setState({ show2faModal: true });
   }
 
   i.setState({
@@ -171,6 +173,14 @@ async function handleGenerateTotp(i: Settings, event: any) {
   });
 }
 
+function handleShowTotpModal(i: Settings) {
+  i.setState({ show2faModal: true });
+}
+
+function handleClose2faModal(i: Settings) {
+  i.setState({ show2faModal: false });
+}
+
 export class Settings extends Component<any, SettingsState> {
   private isoData = setIsoData<SettingsData>(this.context);
   state: SettingsState = {
@@ -196,6 +206,7 @@ export class Settings extends Component<any, SettingsState> {
     isIsomorphic: false,
     generateTotpRes: EMPTY_REQUEST,
     updateTotpRes: EMPTY_REQUEST,
+    show2faModal: false,
   };
 
   constructor(props: any, context: any) {
@@ -1041,16 +1052,20 @@ export class Settings extends Component<any, SettingsState> {
         <button
           type="button"
           className="btn btn-secondary my-2"
-          onClick={
-            !totpEnabled ? linkEvent(this, handleGenerateTotp) : undefined
-          }
-          data-bs-toggle="modal"
-          data-bs-target="#totpModal"
+          onClick={linkEvent(
+            this,
+            totpEnabled ? handleShowTotpModal : handleGenerateTotp,
+          )}
         >{`${
           totpEnabled ? "Disable" : "Enable"
         } 2 factor authentication`}</button>
         {totpEnabled ? (
-          <TotpModal type="remove" onSubmit={this.handleDisable2fa} />
+          <TotpModal
+            type="remove"
+            onSubmit={this.handleDisable2fa}
+            show={this.state.show2faModal}
+            onClose={linkEvent(this, handleClose2faModal)}
+          />
         ) : (
           <TotpModal
             type="generate"
@@ -1060,6 +1075,8 @@ export class Settings extends Component<any, SettingsState> {
                 ? generateTotpRes.data.totp_secret_url
                 : undefined
             }
+            show={this.state.show2faModal}
+            onClose={linkEvent(this, handleClose2faModal)}
           />
         )}
       </>
@@ -1074,14 +1091,12 @@ export class Settings extends Component<any, SettingsState> {
       totp_token: totp,
     });
 
-    if (updateTotpRes.state === "failed") {
-      toast("Invalid TOTP", "danger");
-    }
-
     this.setState({ updateTotpRes });
 
-    const succeeded = updateTotpRes.state === "success";
-    if (succeeded) {
+    const successful = updateTotpRes.state === "success";
+    if (successful) {
+      this.setState({ show2faModal: false });
+
       const siteRes = await HttpService.client.getSite();
       UserService.Instance.myUserInfo!.local_user_view.local_user.totp_2fa_enabled =
         enabled;
@@ -1096,9 +1111,11 @@ export class Settings extends Component<any, SettingsState> {
         } 2 factor authentication`,
         "success",
       );
+    } else {
+      toast("Invalid TOTP", "danger");
     }
 
-    return succeeded;
+    return successful;
   }
 
   handleEnable2fa(totp: string) {