From 961fd7e8491e7df29dabddd2c7cdeabbc11fae1b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 17 Dec 2018 11:17:03 -0800 Subject: [PATCH] In Legalpad, prompt for MFA at the end of the workflow instead of the beginning Summary: Depends on D19895. Ref T13222. This is a simple behavioral improvement for the current MFA implementation in Legalpad: don't MFA the user and //then// realize that they forgot to actually check the box. Test Plan: - Submitted form without the box checked, got an error saying "check the box" instead of MFA. - Submitted the form with the box checked, got an MFA prompt. - Passed the MFA gate, got a signed form. - Tried to sign another form, hit MFA timed lockout. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D19896 --- .../LegalpadDocumentSignController.php | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignController.php b/src/applications/legalpad/controller/LegalpadDocumentSignController.php index 27769432c2..ab98c0bb78 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentSignController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php @@ -151,21 +151,6 @@ final class LegalpadDocumentSignController extends LegalpadController { $errors = array(); $hisec_token = null; if ($request->isFormOrHisecPost() && !$has_signed) { - - // Require two-factor auth to sign legal documents. - if ($viewer->isLoggedIn()) { - $workflow_key = sprintf( - 'legalpad.sign(%s)', - $document->getPHID()); - - $hisec_token = id(new PhabricatorAuthSessionEngine()) - ->setWorkflowKey($workflow_key) - ->requireHighSecurityToken( - $viewer, - $request, - $document->getURI()); - } - list($form_data, $errors, $field_errors) = $this->readSignatureForm( $document, $request); @@ -192,6 +177,20 @@ final class LegalpadDocumentSignController extends LegalpadController { $signature->setVerified($verified); if (!$errors) { + // Require MFA to sign legal documents. + if ($viewer->isLoggedIn()) { + $workflow_key = sprintf( + 'legalpad.sign(%s)', + $document->getPHID()); + + $hisec_token = id(new PhabricatorAuthSessionEngine()) + ->setWorkflowKey($workflow_key) + ->requireHighSecurityToken( + $viewer, + $request, + $document->getURI()); + } + $signature->save(); // If the viewer is logged in, signing for themselves, send them to