From 4858d43d1601fd93f3af0e9c32fa7930cb1cee58 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Oct 2018 10:49:44 -0700 Subject: [PATCH] Add 'autocomplete="off"' to MFA TOTP inputs Summary: Ref T13202. See . If browsers are autofilling this, I think browser behavior here is bad, but behavior is probably better on the balance if we hint this as `autocomplete="off"` and this is a minor concesssion. Test Plan: - I couldn't immediately get any browser to try to autofill this field (perhaps I've disabled autofill, or just not enabled it aggressively?), but this change didn't break anything. - After the change, answered a TOTP prompt normally. - After the change, inspected page content and saw `autocomplete="off"` on the `` node. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13202 Differential Revision: https://secure.phabricator.com/D19722 --- .../auth/factor/PhabricatorTOTPAuthFactor.php | 1 + .../form/control/PHUIFormNumberControl.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 10c44aaec0..373adfbb54 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -154,6 +154,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { id(new PHUIFormNumberControl()) ->setName($this->getParameterName($config, 'totpcode')) ->setLabel(pht('App Code')) + ->setDisableAutocomplete(true) ->setCaption(pht('Factor Name: %s', $config->getFactorName())) ->setValue(idx($validation_result, 'value')) ->setError(idx($validation_result, 'error', true))); diff --git a/src/view/form/control/PHUIFormNumberControl.php b/src/view/form/control/PHUIFormNumberControl.php index d65e590746..26e7e03955 100644 --- a/src/view/form/control/PHUIFormNumberControl.php +++ b/src/view/form/control/PHUIFormNumberControl.php @@ -2,11 +2,28 @@ final class PHUIFormNumberControl extends AphrontFormControl { + private $disableAutocomplete; + + public function setDisableAutocomplete($disable_autocomplete) { + $this->disableAutocomplete = $disable_autocomplete; + return $this; + } + + public function getDisableAutocomplete() { + return $this->disableAutocomplete; + } + protected function getCustomControlClass() { return 'phui-form-number'; } protected function renderInput() { + if ($this->getDisableAutocomplete()) { + $autocomplete = 'off'; + } else { + $autocomplete = null; + } + return javelin_tag( 'input', array( @@ -15,6 +32,7 @@ final class PHUIFormNumberControl extends AphrontFormControl { 'name' => $this->getName(), 'value' => $this->getValue(), 'disabled' => $this->getDisabled() ? 'disabled' : null, + 'autocomplete' => $autocomplete, 'id' => $this->getID(), )); }