Legalpad - allow for legalpad documents to be required to be signed for using Phabricator

Summary: Fixes T7159. Test Plan: Created a legalpad document that needed a signature and I was required to sign it no matter what page I hit. Signed it and things worked! Added a new legalpad document and I had to sign again! Ran unit tests and they passed! Logged out as a user who was roadblocked into signing a bunch of stuff and it worked! Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7159 Differential Revision: https://secure.phabricator.com/D11759
2015-02-12 15:22:56 -08:00
parent d598edc5f3
commit d39da529ca
19 changed files with 219 additions and 28 deletions
--- a/resources/sql/autopatches/20150212.legalpad.session.1.sql
+++ b/resources/sql/autopatches/20150212.legalpad.session.1.sql
@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_user.phabricator_session
+  ADD signedLegalpadDocuments BOOL NOT NULL DEFAULT 0;
+
+ALTER TABLE {$NAMESPACE}_legalpad.legalpad_document
+  ADD requireSignature BOOL NOT NULL DEFAULT 0;
--- a/resources/sql/autopatches/20150212.legalpad.session.2.sql
+++ b/resources/sql/autopatches/20150212.legalpad.session.2.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_legalpad.legalpad_document
+  ADD KEY `key_required` (requireSignature, dateModified);
--- a/src/applications/auth/controller/PhabricatorAuthFinishController.php
+++ b/src/applications/auth/controller/PhabricatorAuthFinishController.php
@@ -11,6 +11,10 @@ final class PhabricatorAuthFinishController
    return true;
  }

+  public function shouldAllowLegallyNonCompliantUsers() {
+    return true;
+  }
+
  public function processRequest() {
    $request = $this->getRequest();
    $viewer = $request->getUser();
--- a/src/applications/auth/controller/PhabricatorAuthValidateController.php
+++ b/src/applications/auth/controller/PhabricatorAuthValidateController.php
@@ -11,6 +11,10 @@ final class PhabricatorAuthValidateController
    return true;
  }

+  public function shouldAllowLegallyNonCompliantUsers() {
+    return true;
+  }
+
  public function processRequest() {
    $request = $this->getRequest();
    $viewer = $request->getUser();
--- a/src/applications/auth/controller/PhabricatorLogoutController.php
+++ b/src/applications/auth/controller/PhabricatorLogoutController.php
@@ -21,6 +21,10 @@ final class PhabricatorLogoutController
    return true;
  }

+  public function shouldAllowLegallyNonCompliantUsers() {
+    return true;
+  }
+
  public function handleRequest(AphrontRequest $request) {
    $request = $this->getRequest();
    $user = $request->getUser();
--- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
+++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@@ -134,6 +134,7 @@ final class PhabricatorAuthSessionEngine extends Phobject {
          s.sessionStart AS s_sessionStart,
          s.highSecurityUntil AS s_highSecurityUntil,
          s.isPartial AS s_isPartial,
+          s.signedLegalpadDocuments as s_signedLegalpadDocuments,
          u.*
        FROM %T u JOIN %T s ON u.phid = s.userPHID
        AND s.type = %s AND s.sessionKey = %s',
@@ -232,6 +233,7 @@ final class PhabricatorAuthSessionEngine extends Phobject {
        ->setSessionStart(time())
        ->setSessionExpires(time() + $session_ttl)
        ->setIsPartial($partial ? 1 : 0)
+        ->setSignedLegalpadDocuments(0)
        ->save();

      $log = PhabricatorUserLog::initializeNewLog(
@@ -553,6 +555,52 @@ final class PhabricatorAuthSessionEngine extends Phobject {
  }


+/* -(  Legalpad Documents )-------------------------------------------------- */
+
+
+  /**
+   * Upgrade a session to have all legalpad documents signed.
+   *
+   * @param PhabricatorUser User whose session should upgrade.
+   * @param array LegalpadDocument objects
+   * @return void
+   * @task partial
+   */
+  public function signLegalpadDocuments(PhabricatorUser $viewer, array $docs) {
+
+    if (!$viewer->hasSession()) {
+      throw new Exception(
+        pht('Signing session legalpad documents of user with no session!'));
+    }
+
+    $session = $viewer->getSession();
+
+    if ($session->getSignedLegalpadDocuments()) {
+      throw new Exception(pht(
+        'Session has already signed required legalpad documents!'));
+    }
+
+    $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+      $session->setSignedLegalpadDocuments(1);
+
+      queryfx(
+        $session->establishConnection('w'),
+        'UPDATE %T SET signedLegalpadDocuments = %d WHERE id = %d',
+        $session->getTableName(),
+        1,
+        $session->getID());
+
+      if (!empty($docs)) {
+        $log = PhabricatorUserLog::initializeNewLog(
+          $viewer,
+          $viewer->getPHID(),
+          PhabricatorUserLog::ACTION_LOGIN_LEGALPAD);
+        $log->save();
+      }
+    unset($unguarded);
+  }
+
+
 /* -(  One Time Login URIs  )------------------------------------------------ */


--- a/src/applications/auth/storage/PhabricatorAuthSession.php
+++ b/src/applications/auth/storage/PhabricatorAuthSession.php
@@ -13,6 +13,7 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO
  protected $sessionExpires;
  protected $highSecurityUntil;
  protected $isPartial;
+  protected $signedLegalpadDocuments;

  private $identityObject = self::ATTACHABLE;

@@ -26,6 +27,7 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO
        'sessionExpires' => 'epoch',
        'highSecurityUntil' => 'epoch?',
        'isPartial' => 'bool',
+        'signedLegalpadDocuments' => 'bool',
      ),
      self::CONFIG_KEY_SCHEMA => array(
        'sessionKey' => array(
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -53,6 +53,10 @@ abstract class PhabricatorController extends AphrontController {
    return PhabricatorEnv::getEnvConfig('security.require-multi-factor-auth');
  }

+  public function shouldAllowLegallyNonCompliantUsers() {
+    return false;
+  }
+
  public function willBeginExecution() {
    $request = $this->getRequest();

@@ -221,6 +225,47 @@ abstract class PhabricatorController extends AphrontController {
      }
    }

+
+    if (!$this->shouldAllowLegallyNonCompliantUsers()) {
+      $legalpad_class = 'PhabricatorLegalpadApplication';
+      $legalpad = id(new PhabricatorApplicationQuery())
+        ->setViewer($user)
+        ->withClasses(array($legalpad_class))
+        ->withInstalled(true)
+        ->execute();
+      $legalpad = head($legalpad);
+
+      $doc_query = id(new LegalpadDocumentQuery())
+        ->setViewer($user)
+        ->withSignatureRequired(1)
+        ->needViewerSignatures(true);
+
+      if ($user->hasSession() &&
+          !$user->getSession()->getIsPartial() &&
+          !$user->getSession()->getSignedLegalpadDocuments() &&
+          $user->isLoggedIn() &&
+          $legalpad) {
+
+        $sign_docs = $doc_query->execute();
+        $must_sign_docs = array();
+        foreach ($sign_docs as $sign_doc) {
+          if (!$sign_doc->getUserSignature($user->getPHID())) {
+            $must_sign_docs[] = $sign_doc;
+          }
+        }
+        if ($must_sign_docs) {
+          $controller = new LegalpadDocumentSignController();
+          $this->getRequest()->setURIMap(array(
+            'id' => head($must_sign_docs)->getID(),));
+          $this->setCurrentApplication($legalpad);
+          return $this->delegateToController($controller);
+        } else {
+          $engine = id(new PhabricatorAuthSessionEngine())
+            ->signLegalpadDocuments($user, $sign_docs);
+        }
+      }
+    }
+
    // NOTE: We do this last so that users get a login page instead of a 403
    // if they need to login.
    if ($this->shouldRequireAdmin() && !$user->getIsAdmin()) {
--- a/src/applications/base/controller/tests/PhabricatorAccessControlTestCase.php
+++ b/src/applications/base/controller/tests/PhabricatorAccessControlTestCase.php
@@ -170,7 +170,7 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase {
    // Test public access.

    $this->checkAccess(
-      'No Login Required',
+      'Public Access',
      id(clone $controller)->setConfig('public', true),
      $request,
      array(
--- a/src/applications/celerity/controller/CelerityResourceController.php
+++ b/src/applications/celerity/controller/CelerityResourceController.php
@@ -18,6 +18,10 @@ abstract class CelerityResourceController extends PhabricatorController {
    return true;
  }

+  public function shouldAllowLegallyNonCompliantUsers() {
+    return true;
+  }
+
  abstract public function getCelerityResourceMap();

  protected function serveResource($path, $package_hash = null) {
--- a/src/applications/legalpad/constants/LegalpadTransactionType.php
+++ b/src/applications/legalpad/constants/LegalpadTransactionType.php
@@ -6,5 +6,6 @@ final class LegalpadTransactionType extends LegalpadConstants {
  const TYPE_TEXT = 'text';
  const TYPE_SIGNATURE_TYPE = 'legalpad:signature-type';
  const TYPE_PREAMBLE = 'legalpad:premable';
+  const TYPE_REQUIRE_SIGNATURE = 'legalpad:require-signature';

 }
--- a/src/applications/legalpad/controller/LegalpadDocumentEditController.php
+++ b/src/applications/legalpad/controller/LegalpadDocumentEditController.php
@@ -2,17 +2,11 @@

 final class LegalpadDocumentEditController extends LegalpadController {

-  private $id;
-
-  public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
+  public function handleRequest(AphrontRequest $request) {
    $user = $request->getUser();

-    if (!$this->id) {
+    $id = $request->getURIData('id');
+    if (!$id) {
      $is_create = true;

      $this->requireApplicationCapability(
@@ -34,7 +28,7 @@ final class LegalpadDocumentEditController extends LegalpadController {
            PhabricatorPolicyCapability::CAN_VIEW,
            PhabricatorPolicyCapability::CAN_EDIT,
          ))
-        ->withIDs(array($this->id))
+        ->withIDs(array($id))
        ->executeOne();
      if (!$document) {
        return new Aphront404Response();
@@ -48,6 +42,7 @@ final class LegalpadDocumentEditController extends LegalpadController {
    $text = $document->getDocumentBody()->getText();
    $v_signature_type = $document->getSignatureType();
    $v_preamble = $document->getPreamble();
+    $v_require_signature = $document->getRequireSignature();

    $errors = array();
    $can_view = null;
@@ -97,6 +92,24 @@ final class LegalpadDocumentEditController extends LegalpadController {
        ->setTransactionType(LegalpadTransactionType::TYPE_PREAMBLE)
        ->setNewValue($v_preamble);

+      $v_require_signature = $request->getBool('requireSignature', 0);
+      if ($v_require_signature) {
+        if (!$user->getIsAdmin()) {
+          $errors[] = pht('Only admins may require signature.');
+        }
+        $corp = LegalpadDocument::SIGNATURE_TYPE_CORPORATION;
+        if ($v_signature_type == $corp) {
+          $errors[] = pht(
+            'Only documents with signature type "individual" may require '.
+            'signing to use Phabricator.');
+        }
+      }
+      if ($user->getIsAdmin()) {
+        $xactions[] = id(new LegalpadTransaction())
+          ->setTransactionType(LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE)
+          ->setNewValue($v_require_signature);
+      }
+
      if (!$errors) {
        $editor = id(new LegalpadDocumentEditor())
          ->setContentSourceFromRequest($request)
@@ -133,11 +146,29 @@ final class LegalpadDocumentEditController extends LegalpadController {
          ->setName(pht('signatureType'))
          ->setValue($v_signature_type)
          ->setOptions(LegalpadDocument::getSignatureTypeMap()));
+      $show_require = true;
    } else {
      $form->appendChild(
        id(new AphrontFormMarkupControl())
          ->setLabel(pht('Who Should Sign?'))
          ->setValue($document->getSignatureTypeName()));
+      $individual = LegalpadDocument::SIGNATURE_TYPE_INDIVIDUAL;
+      $show_require = $document->getSignatureType() == $individual;
+    }
+
+    if ($show_require) {
+      $form
+        ->appendChild(
+          id(new AphrontFormCheckboxControl())
+          ->setDisabled(!$user->getIsAdmin())
+          ->setLabel(pht('Require Signature'))
+          ->addCheckbox(
+            'requireSignature',
+            'requireSignature',
+            pht(
+              'Should signing this document be required to use Phabricator? '.
+              'Applies to invidivuals only.'),
+            $v_require_signature));
    }

    $form
--- a/src/applications/legalpad/controller/LegalpadDocumentSignController.php
+++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php
@@ -2,23 +2,16 @@

 final class LegalpadDocumentSignController extends LegalpadController {

-  private $id;
-
  public function shouldAllowPublic() {
    return true;
  }

-  public function willProcessRequest(array $data) {
-    $this->id = $data['id'];
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
+  public function handleRequest(AphrontRequest $request) {
    $viewer = $request->getUser();

    $document = id(new LegalpadDocumentQuery())
      ->setViewer($viewer)
-      ->withIDs(array($this->id))
+      ->withIDs(array($request->getURIData('id')))
      ->needDocumentBodies(true)
      ->executeOne();
    if (!$document) {
--- a/src/applications/legalpad/controller/LegalpadDocumentSignatureAddController.php
+++ b/src/applications/legalpad/controller/LegalpadDocumentSignatureAddController.php
@@ -2,13 +2,7 @@

 final class LegalpadDocumentSignatureAddController extends LegalpadController {

-  private $id;
-
-  public function willProcessRequest(array $data) {
-    $this->id = $data['id'];
-  }
-
-  public function processRequest() {
+  public function handleRequest(AphrontRequest $request) {
    $request = $this->getRequest();
    $viewer = $request->getUser();

@@ -20,7 +14,7 @@ final class LegalpadDocumentSignatureAddController extends LegalpadController {
          PhabricatorPolicyCapability::CAN_VIEW,
          PhabricatorPolicyCapability::CAN_EDIT,
        ))
-      ->withIDs(array($this->id))
+      ->withIDs(array($request->getURIData('id')))
      ->executeOne();
    if (!$document) {
      return new Aphront404Response();
--- a/src/applications/legalpad/editor/LegalpadDocumentEditor.php
+++ b/src/applications/legalpad/editor/LegalpadDocumentEditor.php
@@ -32,6 +32,7 @@ final class LegalpadDocumentEditor
    $types[] = LegalpadTransactionType::TYPE_TEXT;
    $types[] = LegalpadTransactionType::TYPE_SIGNATURE_TYPE;
    $types[] = LegalpadTransactionType::TYPE_PREAMBLE;
+    $types[] = LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE;

    return $types;
  }
@@ -49,6 +50,8 @@ final class LegalpadDocumentEditor
        return $object->getSignatureType();
      case LegalpadTransactionType::TYPE_PREAMBLE:
        return $object->getPreamble();
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
+        return $object->getRequireSignature();
    }
  }

@@ -61,6 +64,7 @@ final class LegalpadDocumentEditor
      case LegalpadTransactionType::TYPE_TEXT:
      case LegalpadTransactionType::TYPE_SIGNATURE_TYPE:
      case LegalpadTransactionType::TYPE_PREAMBLE:
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
        return $xaction->getNewValue();
    }
  }
@@ -87,12 +91,27 @@ final class LegalpadDocumentEditor
      case LegalpadTransactionType::TYPE_PREAMBLE:
        $object->setPreamble($xaction->getNewValue());
        break;
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
+        $object->setRequireSignature($xaction->getNewValue());
+        break;
    }
  }

  protected function applyCustomExternalTransaction(
    PhabricatorLiskDAO $object,
    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
+        if ($xaction->getNewValue()) {
+          $session = new PhabricatorAuthSession();
+          queryfx(
+            $session->establishConnection('w'),
+            'UPDATE %T SET signedLegalpadDocuments = 0',
+            $session->getTableName());
+        }
+        break;
+    }
    return;
  }

@@ -138,6 +157,7 @@ final class LegalpadDocumentEditor
      case LegalpadTransactionType::TYPE_TEXT:
      case LegalpadTransactionType::TYPE_SIGNATURE_TYPE:
      case LegalpadTransactionType::TYPE_PREAMBLE:
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
        return $v;
    }

@@ -182,6 +202,7 @@ final class LegalpadDocumentEditor
      case LegalpadTransactionType::TYPE_TEXT:
      case LegalpadTransactionType::TYPE_TITLE:
      case LegalpadTransactionType::TYPE_PREAMBLE:
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
        return true;
    }

--- a/src/applications/legalpad/query/LegalpadDocumentQuery.php
+++ b/src/applications/legalpad/query/LegalpadDocumentQuery.php
@@ -10,6 +10,7 @@ final class LegalpadDocumentQuery
  private $signerPHIDs;
  private $dateCreatedAfter;
  private $dateCreatedBefore;
+  private $signatureRequired;

  private $needDocumentBodies;
  private $needContributors;
@@ -41,6 +42,11 @@ final class LegalpadDocumentQuery
    return $this;
  }

+  public function withSignatureRequired($bool) {
+    $this->signatureRequired = $bool;
+    return $this;
+  }
+
  public function needDocumentBodies($need_bodies) {
    $this->needDocumentBodies = $need_bodies;
    return $this;
@@ -204,6 +210,13 @@ final class LegalpadDocumentQuery
        $this->contributorPHIDs);
    }

+    if ($this->signatureRequired !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'd.requireSignature = %d',
+        $this->signatureRequired);
+    }
+
    $where[] = $this->buildPagingClause($conn_r);

    return $this->formatWhereClause($where);
--- a/src/applications/legalpad/storage/LegalpadDocument.php
+++ b/src/applications/legalpad/storage/LegalpadDocument.php
@@ -18,6 +18,7 @@ final class LegalpadDocument extends LegalpadDAO
  protected $mailKey;
  protected $signatureType;
  protected $preamble;
+  protected $requireSignature;

  const SIGNATURE_TYPE_INDIVIDUAL = 'user';
  const SIGNATURE_TYPE_CORPORATION = 'corp';
@@ -44,6 +45,7 @@ final class LegalpadDocument extends LegalpadDAO
      ->attachSignatures(array())
      ->setSignatureType(self::SIGNATURE_TYPE_INDIVIDUAL)
      ->setPreamble('')
+      ->setRequireSignature(0)
      ->setViewPolicy($view_policy)
      ->setEditPolicy($edit_policy);
  }
@@ -61,11 +63,15 @@ final class LegalpadDocument extends LegalpadDAO
        'mailKey' => 'bytes20',
        'signatureType' => 'text4',
        'preamble' => 'text',
+        'requireSignature' => 'bool',
      ),
      self::CONFIG_KEY_SCHEMA => array(
        'key_creator' => array(
          'columns' => array('creatorPHID', 'dateModified'),
        ),
+        'key_required' => array(
+          'columns' => array('requireSignature', 'dateModified'),
+        ),
      ),
    ) + parent::getConfiguration();
  }
--- a/src/applications/legalpad/storage/LegalpadTransaction.php
+++ b/src/applications/legalpad/storage/LegalpadTransaction.php
@@ -54,6 +54,17 @@ final class LegalpadTransaction extends PhabricatorApplicationTransaction {
        return pht(
          '%s updated the preamble.',
          $this->renderHandleLink($author_phid));
+      case LegalpadTransactionType::TYPE_REQUIRE_SIGNATURE:
+        if ($new) {
+          $text = pht(
+            '%s set the document to require signatures.',
+            $this->renderHandleLink($author_phid));
+        } else {
+          $text = pht(
+            '%s set the document to not require signatures.',
+            $this->renderHandleLink($author_phid));
+        }
+        return $text;
    }

    return parent::getTitle();
--- a/src/applications/people/storage/PhabricatorUserLog.php
+++ b/src/applications/people/storage/PhabricatorUserLog.php
@@ -8,6 +8,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO
  const ACTION_LOGIN_FULL     = 'login-full';
  const ACTION_LOGOUT         = 'logout';
  const ACTION_LOGIN_FAILURE  = 'login-fail';
+  const ACTION_LOGIN_LEGALPAD = 'login-legalpad';
  const ACTION_RESET_PASSWORD = 'reset-pass';

  const ACTION_CREATE         = 'create';
@@ -53,6 +54,8 @@ final class PhabricatorUserLog extends PhabricatorUserDAO
      self::ACTION_LOGIN_PARTIAL => pht('Login: Partial Login'),
      self::ACTION_LOGIN_FULL => pht('Login: Upgrade to Full'),
      self::ACTION_LOGIN_FAILURE => pht('Login: Failure'),
+      self::ACTION_LOGIN_LEGALPAD =>
+        pht('Login: Signed Required Legalpad Documents'),
      self::ACTION_LOGOUT => pht('Logout'),
      self::ACTION_RESET_PASSWORD => pht('Reset Password'),
      self::ACTION_CREATE => pht('Create Account'),