Legalpad - make it work for not logged in users

Summary: Adds "verified" and "secretKey" to Legalpad document signatures. For logged in users using an email address they own, things are verified right away. Otherwise, the email is sent a verification letter. When the user clicks the link the signature is marked verified.

Test Plan: signed the document with a bogus email address not logged in. verified the email that would be sent looked good from command line. followed link and successfully verified bogus email address

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran, asherkin

Maniphest Tasks: T4283

Differential Revision: https://secure.phabricator.com/D7930

resources/sql/autopatches/20140113.legalpadsig.1.sql

@@ -0,0 +1,8 @@
+ALTER TABLE {$NAMESPACE}_legalpad.legalpad_documentsignature
+  ADD secretKey VARCHAR(20) NOT NULL COLLATE utf8_bin;
+
+ALTER TABLE {$NAMESPACE}_legalpad.legalpad_documentsignature
+  ADD verified TINYINT(1) DEFAULT 0;
+
+ALTER TABLE {$NAMESPACE}_legalpad.legalpad_documentsignature
+  ADD KEY `secretKey` (secretKey);

resources/sql/autopatches/20140113.legalpadsig.2.php

@@ -0,0 +1,23 @@
+<?php
+
+echo "Adding secretkeys to legalpad document signatures.\n";
+
+$table = new LegalpadDocumentSignature();
+$conn_w = $table->establishConnection('w');
+$iterator = new LiskMigrationIterator($table);
+foreach ($iterator as $sig) {
+  $id = $sig->getID();
+
+  echo "Populating signature {$id}...\n";
+
+  if (!$sig->getSecretKey()) {
+    queryfx(
+      $conn_w,
+      'UPDATE %T SET secretKey = %s WHERE id = %d',
+      $table->getTableName(),
+      Filesystem::readRandomCharacters(20),
+      $id);
+  }
+}
+
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -838,6 +838,7 @@ phutil_register_library_map(array(
     'LegalpadDocumentSearchEngine' => 'applications/legalpad/query/LegalpadDocumentSearchEngine.php',
     'LegalpadDocumentSignController' => 'applications/legalpad/controller/LegalpadDocumentSignController.php',
     'LegalpadDocumentSignature' => 'applications/legalpad/storage/LegalpadDocumentSignature.php',
+    'LegalpadDocumentSignatureVerificationController' => 'applications/legalpad/controller/LegalpadDocumentSignatureVerificationController.php',
     'LegalpadDocumentViewController' => 'applications/legalpad/controller/LegalpadDocumentViewController.php',
     'LegalpadMockMailReceiver' => 'applications/legalpad/mail/LegalpadMockMailReceiver.php',
     'LegalpadReplyHandler' => 'applications/legalpad/mail/LegalpadReplyHandler.php',
@@ -3357,6 +3358,7 @@ phutil_register_library_map(array(
     'LegalpadDocumentSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'LegalpadDocumentSignController' => 'LegalpadController',
     'LegalpadDocumentSignature' => 'LegalpadDAO',
+    'LegalpadDocumentSignatureVerificationController' => 'LegalpadController',
     'LegalpadDocumentViewController' => 'LegalpadController',
     'LegalpadMockMailReceiver' => 'PhabricatorObjectMailReceiver',
     'LegalpadReplyHandler' => 'PhabricatorMailReplyHandler',

src/applications/auth/query/PhabricatorExternalAccountQuery.php

@@ -167,4 +167,35 @@ final class PhabricatorExternalAccountQuery
     return 'PhabricatorApplicationPeople';
   }
 
+  /**
+   * Attempts to find an external account and if none exists creates a new
+   * external account with a shiny new ID and PHID.
+   *
+   * NOTE: This function assumes the first item in various query parameters is
+   * the correct value to use in creating a new external account.
+   */
+  public function loadOneOrCreate() {
+    $account = $this->executeOne();
+    if (!$account) {
+      $account = new PhabricatorExternalAccount();
+      if ($this->accountIDs) {
+        $account->setAccountID(reset($this->accountIDs));
+      }
+      if ($this->accountTypes) {
+        $account->setAccountType(reset($this->accountTypes));
+      }
+      if ($this->accountDomains) {
+        $account->setAccountDomain(reset($this->accountDomains));
+      }
+      if ($this->accountSecrets) {
+        $account->setAccountSecret(reset($this->accountSecrets));
+      }
+      if ($this->userPHIDs) {
+        $account->setUserPHID(reset($this->userPHIDs));
+      }
+      $account->save();
+    }
+    return $account;
+  }
+
 }

src/applications/legalpad/application/PhabricatorApplicationLegalpad.php

@@ -48,6 +48,8 @@ final class PhabricatorApplicationLegalpad extends PhabricatorApplication {
         'edit/(?P<id>\d+)/' => 'LegalpadDocumentEditController',
         'comment/(?P<id>\d+)/' => 'LegalpadDocumentCommentController',
         'view/(?P<id>\d+)/' => 'LegalpadDocumentViewController',
+        'verify/(?P<code>[^/]+)/' =>
+          'LegalpadDocumentSignatureVerificationController',
         'document/' => array(
           'preview/' => 'PhabricatorMarkupPreviewController'),
       ));

src/applications/legalpad/controller/LegalpadDocumentSignController.php

@@ -7,6 +7,10 @@ final class LegalpadDocumentSignController extends LegalpadController {
 
   private $id;
 
+  public function shouldRequireLogin() {
+    return false;
+  }
+
   public function willProcessRequest(array $data) {
     $this->id = $data['id'];
   }
@@ -25,38 +29,70 @@ final class LegalpadDocumentSignController extends LegalpadController {
       return new Aphront404Response();
     }
 
-    $signature = id(new LegalpadDocumentSignature())
-      ->loadOneWhere(
-        'documentPHID = %s AND documentVersion = %d AND signerPHID = %s',
-        $document->getPHID(),
-        $document->getVersions(),
-        $user->getPHID());
+    $signer_phid = null;
+    $signature = null;
+    $signature_data = array();
+    if ($user->isLoggedIn()) {
+      $signer_phid = $user->getPHID();
+      $signature_data = array(
+        'email' => $user->loadPrimaryEmailAddress());
+    } else if ($request->isFormPost()) {
+      $email = new PhutilEmailAddress($request->getStr('email'));
+      $email_obj = id(new PhabricatorUserEmail())
+        ->loadOneWhere('address = %s', $email->getAddress());
+      if ($email_obj) {
+        return $this->signInResponse();
+      }
+      $external_account = id(new PhabricatorExternalAccountQuery())
+        ->setViewer($user)
+        ->withAccountTypes(array('email'))
+        ->withAccountDomains(array($email->getDomainName()))
+        ->withAccountIDs(array($email->getAddress()))
+        ->loadOneOrCreate();
+      if ($external_account->getUserPHID()) {
+        return $this->signInResponse();
+      }
+      $signer_phid = $external_account->getPHID();
+    }
+
+    if ($signer_phid) {
+      $signature = id(new LegalpadDocumentSignature())
+        ->loadOneWhere(
+          'documentPHID = %s AND documentVersion = %d AND signerPHID = %s',
+          $document->getPHID(),
+          $document->getVersions(),
+          $signer_phid);
+    }
 
     if (!$signature) {
       $has_signed = false;
       $error_view = null;
       $signature = id(new LegalpadDocumentSignature())
-        ->setSignerPHID($user->getPHID())
+        ->setSignerPHID($signer_phid)
         ->setDocumentPHID($document->getPHID())
-        ->setDocumentVersion($document->getVersions());
-      $data = array(
-        'name' => $user->getRealName(),
-        'email' => $user->loadPrimaryEmailAddress());
-      $signature->setSignatureData($data);
+        ->setDocumentVersion($document->getVersions())
+        ->setSignatureData($signature_data);
     } else {
       $has_signed = true;
+      if ($signature->isVerified()) {
+        $title = pht('Already Signed');
+        $body = $this->getVerifiedSignatureBlurb();
+      } else {
+        $title = pht('Already Signed but...');
+        $body = $this->getUnverifiedSignatureBlurb();
+      }
       $error_view = id(new AphrontErrorView())
         ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
-        ->setTitle(pht('Already Signed'))
-        ->appendChild(pht('Thank you for signing and agreeing'));
-      $data = $signature->getSignatureData();
+        ->setTitle($title)
+        ->appendChild($body);
+      $signature_data = $signature->getSignatureData();
     }
 
     $e_name = true;
     $e_email = true;
     $e_address_1 = true;
     $errors = array();
-    if ($request->isFormPost()) {
+    if ($request->isFormPost() && !$has_signed) {
       $name = $request->getStr('name');
       $email = $request->getStr('email');
       $address_1 = $request->getStr('address_1');
@@ -68,8 +104,9 @@ final class LegalpadDocumentSignController extends LegalpadController {
         $e_name = pht('Required');
         $errors[] = pht('Name field is required.');
       }
-      $data['name'] = $name;
+      $signature_data['name'] = $name;
 
+      $addr_obj = null;
       if (!$email) {
         $e_email = pht('Required');
         $errors[] = pht('Email field is required.');
@@ -81,29 +118,47 @@ final class LegalpadDocumentSignController extends LegalpadController {
           $errors[] = pht('A valid email is required.');
         }
       }
-      $data['email'] = $email;
+      $signature_data['email'] = $email;
 
       if (!$address_1) {
         $e_address_1 = pht('Required');
         $errors[] = pht('Address line 1 field is required.');
       }
-      $data['address_1'] = $address_1;
-      $data['address_2'] = $address_2;
-      $data['phone'] = $phone;
-      $signature->setSignatureData($data);
+      $signature_data['address_1'] = $address_1;
+      $signature_data['address_2'] = $address_2;
+      $signature_data['phone'] = $phone;
+      $signature->setSignatureData($signature_data);
 
       if (!$agree) {
         $errors[] = pht(
           'You must check "I agree to the terms laid forth above."');
       }
 
+      $verified = LegalpadDocumentSignature::UNVERIFIED;
+      if ($user->isLoggedIn() && $addr_obj) {
+        $email_obj = id(new PhabricatorUserEmail())
+          ->loadOneWhere('address = %s', $addr_obj->getAddress());
+        if ($email_obj && $email_obj->getUserPHID() == $user->getPHID()) {
+          $verified = LegalpadDocumentSignature::VERIFIED;
+        }
+      }
+      $signature->setVerified($verified);
+
       if (!$errors) {
         $signature->save();
         $has_signed = true;
+        if ($signature->isVerified()) {
+          $body = $this->getVerifiedSignatureBlurb();
+        } else {
+          $body = $this->getUnverifiedSignatureBlurb();
+          $this->sendVerifySignatureEmail(
+            $document,
+            $signature);
+        }
         $error_view = id(new AphrontErrorView())
           ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
-          ->setTitle(pht('Signature successful'))
-          ->appendChild(pht('Thank you for signing and agreeing'));
+          ->setTitle(pht('Signature Successful'))
+          ->appendChild($body);
       } else {
         $error_view = id(new AphrontErrorView())
           ->setTitle(pht('Error in submission.'))
@@ -218,10 +273,61 @@ final class LegalpadDocumentSignController extends LegalpadController {
         ->setValue(pht('Sign and Agree'))
         ->setDisabled($has_signed));
 
-    return id(new PHUIObjectBoxView())
+    $view = id(new PHUIObjectBoxView())
       ->setHeaderText(pht('Sign and Agree'))
-      ->setErrorView($error_view)
       ->setForm($form);
+    if ($error_view) {
+      $view->setErrorView($error_view);
+    }
+    return $view;
+  }
+
+  private function getVerifiedSignatureBlurb() {
+    return pht('Thank you for signing and agreeing.');
+  }
+
+  private function getUnverifiedSignatureBlurb() {
+    return pht('Thank you for signing and agreeing. However, you must '.
+               'verify your email address. Please check your email '.
+               'and follow the instructions.');
+  }
+
+  private function sendVerifySignatureEmail(
+    LegalpadDocument $doc,
+    LegalpadDocumentSignature $signature) {
+
+    $signature_data = $signature->getSignatureData();
+    $email = new PhutilEmailAddress($signature_data['email']);
+    $doc_link = PhabricatorEnv::getProductionURI($doc->getMonogram());
+    $path = $this->getApplicationURI(sprintf(
+      '/verify/%s/',
+      $signature->getSecretKey()));
+    $link = PhabricatorEnv::getProductionURI($path);
+
+    $body = <<<EOBODY
+Hi {$signature_data['name']},
+
+This email address was used to sign a Legalpad document ({$doc_link}).
+Please verify you own this email address by clicking this link:
+
+  {$link}
+
+Your signature is invalid until you verify you own the email.
+EOBODY;
+
+    id(new PhabricatorMetaMTAMail())
+      ->addRawTos(array($email->getAddress()))
+      ->setSubject(pht('[Legalpad] Signature Verification'))
+      ->setBody($body)
+      ->setRelatedPHID($signature->getDocumentPHID())
+      ->saveAndSend();
+  }
+
+  private function signInResponse() {
+    return id(new Aphront403Response())
+      ->setForbiddenText(pht(
+        'The email address specified is associated with an account. '.
+        'Please login to that account and sign this document again.'));
   }
 
 }

src/applications/legalpad/controller/LegalpadDocumentSignatureVerificationController.php

@@ -0,0 +1,84 @@
+<?php
+
+final class LegalpadDocumentSignatureVerificationController
+extends LegalpadController {
+
+  private $code;
+
+  public function willProcessRequest(array $data) {
+    $this->code = $data['code'];
+  }
+
+  public function shouldRequireEmailVerification() {
+    return false;
+  }
+
+  public function shouldRequireLogin() {
+    return false;
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $signature = id(new LegalpadDocumentSignature())
+      ->loadOneWhere('secretKey = %s', $this->code);
+
+    if (!$signature) {
+      $title = pht('Unable to Verify Signature');
+      $content = pht(
+        'The verification code you provided is incorrect or the signature '.
+        'has been removed. '.
+        'Make sure you followed the link in the email correctly.');
+      $uri = $this->getApplicationURI();
+      $continue = pht('Rats!');
+    } else {
+      $document = id(new LegalpadDocumentQuery())
+        ->setViewer($user)
+        ->withPHIDs(array($signature->getDocumentPHID()))
+        ->executeOne();
+      // the document could be deleted or have its permissions changed
+      // 4oh4 time
+      if (!$document) {
+        return new Aphront404Response();
+      }
+      $uri = '/'.$document->getMonogram();
+      if ($signature->isVerified()) {
+        $title = pht('Signature Already Verified');
+        $content = pht(
+          'This signature has already been verified.');
+        $continue = pht('Continue to Legalpad Document');
+      } else {
+        $guard = AphrontWriteGuard::beginScopedUnguardedWrites();
+          $signature
+            ->setVerified(LegalpadDocumentSignature::VERIFIED)
+            ->save();
+        unset($guard);
+        $title = pht('Signature Verified');
+        $content = pht('The signature is now verified.');
+        $continue = pht('Continue to Legalpad Document');
+      }
+    }
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($user)
+      ->setTitle($title)
+      ->setMethod('GET')
+      ->addCancelButton($uri, $continue)
+      ->appendChild($content);
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addTextCrumb(pht('Verify Signature'));
+
+    return $this->buildApplicationPage(
+      array(
+        $crumbs,
+        $dialog,
+      ),
+      array(
+        'title' => pht('Verify Signature'),
+        'device' => true,
+      ));
+  }
+
+}

src/applications/legalpad/storage/LegalpadDocument.php

@@ -61,6 +61,10 @@ final class LegalpadDocument extends LegalpadDAO
     return parent::save();
   }
 
+  public function getMonogram() {
+    return 'L'.$this->getID();
+  }
+
 /* -(  PhabricatorSubscribableInterface Implementation  )-------------------- */
 
   public function isAutomaticallySubscribed($phid) {

src/applications/legalpad/storage/LegalpadDocumentSignature.php

@@ -5,10 +5,15 @@
  */
 final class LegalpadDocumentSignature extends LegalpadDAO {
 
+  const VERIFIED = 0;
+  const UNVERIFIED = 1;
+
   protected $documentPHID;
   protected $documentVersion;
   protected $signerPHID;
   protected $signatureData = array();
+  protected $verified;
+  protected $secretKey;
 
   public function getConfiguration() {
     return array(
@@ -18,6 +23,15 @@ final class LegalpadDocumentSignature extends LegalpadDAO {
     ) + parent::getConfiguration();
   }
 
+  public function save() {
+    if (!$this->getSecretKey()) {
+      $this->setSecretKey(Filesystem::readRandomCharacters(20));
+    }
+    return parent::save();
+  }
 
+  public function isVerified() {
+    return $this->getVerified() != self::UNVERIFIED;
+  }
 
 }

src/applications/metamta/receiver/PhabricatorMailReceiver.php

@@ -80,20 +80,13 @@ abstract class PhabricatorMailReceiver {
     $email_key = 'phabricator.allow-email-users';
     $allow_email_users = PhabricatorEnv::getEnvConfig($email_key);
     if ($allow_email_users) {
-      $xuser = id(new PhabricatorExternalAccount())->loadOneWhere(
-        'accountType = %s AND accountDomain = %s and accountID = %s',
-        'email',
-        'self',
-        $from);
-      if (!$xuser) {
-        $xuser = id(new PhabricatorExternalAccount())
-          ->setAccountID($from)
-          ->setAccountType('email')
-          ->setAccountDomain('self')
-          ->setDisplayName($from)
-          ->setEmail($from)
-          ->save();
-      }
+      $from_obj = new PhutilEmailAddress($from);
+      $xuser = id(new PhabricatorExternalAccountQuery())
+        ->setViewer($user)
+        ->withAccountTypes(array('email'))
+        ->withAccountDomains(array($from_obj->getDomainName(), 'self'))
+        ->withAccountIDs(array($from_obj->getAddress()))
+        ->loadOneOrCreate();
       return $xuser->getPhabricatorUser();
     } else {
       $reasons[] = pht(