From 4f8d07594e2af46dfa228a9b7177c7f5b0b47a0b Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 30 Jun 2016 08:22:20 -0700 Subject: [PATCH] Fix a CSRF issue with adding new email addresses Summary: The first dialog was being given the wrong user (`$user`, should be `$viewer`), leading to a CSRF issue. (The CSRF token it generated was invalid in all validation contexts, so this wasn't a security problem or a way to capture CSRF tokens for other users.) Use `newDialog()` instead. (This seems completely unrelated to the vaguely-similar-looking issues we saw earlier this week.) Test Plan: - Added a new email address. - Clicked "Done" on the last step. - Completed workflow instead of getting a CSRF error. Reviewers: chad, tide Reviewed By: tide Differential Revision: https://secure.phabricator.com/D16200 --- .../panel/PhabricatorEmailAddressesSettingsPanel.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php index ac02fae00e..b5d8ea8617 100644 --- a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php @@ -227,8 +227,7 @@ final class PhabricatorEmailAddressesSettingsPanel $object->sendVerificationEmail($user); - $dialog = id(new AphrontDialogView()) - ->setUser($user) + $dialog = $this->newDialog() ->addHiddenInput('new', 'verify') ->setTitle(pht('Verification Email Sent')) ->appendChild(phutil_tag('p', array(), pht( @@ -259,8 +258,7 @@ final class PhabricatorEmailAddressesSettingsPanel ->setCaption(PhabricatorUserEmail::describeAllowedAddresses()) ->setError($e_email)); - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) + $dialog = $this->newDialog() ->addHiddenInput('new', 'true') ->setTitle(pht('New Address')) ->appendChild($errors)