Add "Mailing List" users

epriestley · epriestley · commit 992c19957760 · 2015-06-03T18:42:33.000-07:00
Summary: Ref T8387. Adds new mailing list users. This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address. Test Plan: - Created and edited a mailing list user. - Viewed profile. - Viewed People list. - Searched for lists / nonlists. - Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors. - Hit the web/api/ssh session blocks. Reviewers: btrahan Reviewed By: btrahan Subscribers: eadler, tycho.tatitscheff, epriestley Maniphest Tasks: T8387 Differential Revision: https://secure.phabricator.com/D13123
diff --git a/resources/sql/autopatches/20150602.mlist.1.sql b/resources/sql/autopatches/20150602.mlist.1.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_user.user
+  ADD isMailingList BOOL NOT NULL;
diff --git a/scripts/ssh/ssh-exec.php b/scripts/ssh/ssh-exec.php
@@ -182,11 +182,11 @@
       'P' => $user->getPHID(),
     ));
 
-  if (!$user->isUserActivated()) {
+  if (!$user->canEstablishSSHSessions()) {
     throw new Exception(
       pht(
-        'Your account ("%s") is not activated. Visit the web interface '.
-        'for more information.',
+        'Your account ("%s") does not have permission to establish SSH '.
+        'sessions. Visit the web interface for more information.',
         $user->getUsername()));
   }
 
diff --git a/scripts/user/account_admin.php b/scripts/user/account_admin.php
@@ -125,7 +125,7 @@
 
 $is_system_agent = $user->getIsSystemAgent();
 $set_system_agent = phutil_console_confirm(
-  pht('Is this user a bot/script?'),
+  pht('Is this user a bot?'),
   $default_no = !$is_system_agent);
 
 $verify_email = null;
@@ -165,7 +165,7 @@
 
 printf(
   $tpl,
-  pht('Bot/Script'),
+  pht('Bot'),
   $original->getIsSystemAgent() ? 'Y' : 'N',
   $set_system_agent ? 'Y' : 'N');
 
diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@@ -158,6 +158,21 @@ public function loadUserForSession($session_type, $session_token) {
         $session_dict[substr($key, 2)] = $value;
       }
     }
+
+    $user = $user_table->loadFromArray($info);
+    switch ($session_type) {
+      case PhabricatorAuthSession::TYPE_WEB:
+        // Explicitly prevent bots and mailing lists from establishing web
+        // sessions. It's normally impossible to attach authentication to these
+        // accounts, and likewise impossible to generate sessions, but it's
+        // technically possible that a session could exist in the database. If
+        // one does somehow, refuse to load it.
+        if (!$user->canEstablishWebSessions()) {
+          return null;
+        }
+        break;
+    }
+
     $session = id(new PhabricatorAuthSession())->loadFromArray($session_dict);
 
     $ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type);
@@ -181,7 +196,6 @@ public function loadUserForSession($session_type, $session_token) {
       unset($unguarded);
     }
 
-    $user = $user_table->loadFromArray($info);
     $user->attachSession($session);
     return $user;
   }
diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php
@@ -475,10 +475,10 @@ private function validateAuthenticatedUser(
     ConduitAPIRequest $request,
     PhabricatorUser $user) {
 
-    if (!$user->isUserActivated()) {
+    if (!$user->canEstablishAPISessions()) {
       return array(
-        'ERR-USER-DISABLED',
-        pht('User account is not activated.'),
+        'ERR-INVALID-AUTH',
+        pht('User account is not permitted to use the API.'),
       );
     }
 
diff --git a/src/applications/conduit/settings/PhabricatorConduitTokensSettingsPanel.php b/src/applications/conduit/settings/PhabricatorConduitTokensSettingsPanel.php
@@ -20,6 +20,10 @@ public function getPanelGroup() {
   }
 
   public function isEnabled() {
+    if ($this->getUser()->getIsMailingList()) {
+      return false;
+    }
+
     return true;
   }
 
diff --git a/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php b/src/applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php
@@ -19,6 +19,10 @@ public function getPanelGroup() {
   }
 
   public function isEnabled() {
+    if ($this->getUser()->getIsMailingList()) {
+      return false;
+    }
+
     return PhabricatorEnv::getEnvConfig('diffusion.allow-http-auth');
   }
 
diff --git a/src/applications/people/conduit/UserConduitAPIMethod.php b/src/applications/people/conduit/UserConduitAPIMethod.php
@@ -18,6 +18,9 @@ protected function buildUserInformationDictionary(
     if ($user->getIsSystemAgent()) {
       $roles[] = 'agent';
     }
+    if ($user->getIsMailingList()) {
+      $roles[] = 'list';
+    }
     if ($user->getIsAdmin()) {
       $roles[] = 'admin';
     }
diff --git a/src/applications/people/controller/PhabricatorPeopleCreateController.php b/src/applications/people/controller/PhabricatorPeopleCreateController.php
@@ -4,8 +4,6 @@ final class PhabricatorPeopleCreateController
   extends PhabricatorPeopleController {
 
   public function handleRequest(AphrontRequest $request) {
-    $this->requireApplicationCapability(
-      PeopleCreateUsersCapability::CAPABILITY);
     $admin = $request->getUser();
 
     id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession(
@@ -17,7 +15,7 @@ public function handleRequest(AphrontRequest $request) {
     if ($request->isFormPost()) {
       $v_type = $request->getStr('type');
 
-      if ($v_type == 'standard' || $v_type == 'bot') {
+      if ($v_type == 'standard' || $v_type == 'bot' || $v_type == 'list') {
         return id(new AphrontRedirectResponse())->setURI(
           $this->getApplicationURI('new/'.$v_type.'/'));
       }
@@ -41,6 +39,41 @@ public function handleRequest(AphrontRequest $request) {
     $bot_admin = pht(
       'Administrators have greater access to edit these accounts.');
 
+    $types = array();
+
+    $can_create = $this->hasApplicationCapability(
+      PeopleCreateUsersCapability::CAPABILITY);
+    if ($can_create) {
+      $types[] = array(
+        'type' => 'standard',
+        'name' => pht('Create Standard User'),
+        'help' => pht('Create a standard user account.'),
+      );
+    }
+
+    $types[] = array(
+      'type' => 'bot',
+      'name' => pht('Create Bot User'),
+      'help' => pht('Create a new user for use with automated scripts.'),
+    );
+
+    $types[] = array(
+      'type' => 'list',
+      'name' => pht('Create Mailing List User'),
+      'help' => pht(
+        'Create a mailing list user to represent an existing, external '.
+        'mailing list like a Google Group or a Mailman list.'),
+    );
+
+    $buttons = id(new AphrontFormRadioButtonControl())
+      ->setLabel(pht('Account Type'))
+      ->setName('type')
+      ->setValue($v_type);
+
+    foreach ($types as $type) {
+      $buttons->addButton($type['type'], $type['name'], $type['help']);
+    }
+
     $form = id(new AphrontFormView())
       ->setUser($admin)
       ->appendRemarkupInstructions(
@@ -49,19 +82,7 @@ public function handleRequest(AphrontRequest $request) {
           'explanation of user account types, see [[ %s | User Guide: '.
           'Account Roles ]].',
           PhabricatorEnv::getDoclink('User Guide: Account Roles')))
-      ->appendChild(
-        id(new AphrontFormRadioButtonControl())
-          ->setLabel(pht('Account Type'))
-          ->setName('type')
-          ->setValue($v_type)
-          ->addButton(
-            'standard',
-            pht('Create Standard User'),
-            hsprintf('%s<br /><br />%s', $standard_caption, $standard_admin))
-          ->addButton(
-            'bot',
-            pht('Create Bot/Script User'),
-            hsprintf('%s<br /><br />%s', $bot_caption, $bot_admin)))
+      ->appendChild($buttons)
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->addCancelButton($this->getApplicationURI())
diff --git a/src/applications/people/controller/PhabricatorPeopleListController.php b/src/applications/people/controller/PhabricatorPeopleListController.php
@@ -33,20 +33,12 @@ protected function buildApplicationCrumbs() {
     $crumbs = parent::buildApplicationCrumbs();
     $viewer = $this->getRequest()->getUser();
 
-    $can_create = $this->hasApplicationCapability(
-      PeopleCreateUsersCapability::CAPABILITY);
-    if ($can_create) {
+    if ($viewer->getIsAdmin()) {
       $crumbs->addAction(
         id(new PHUIListItemView())
         ->setName(pht('Create New User'))
         ->setHref($this->getApplicationURI('create/'))
         ->setIcon('fa-plus-square'));
-    } else if ($viewer->getIsAdmin()) {
-      $crumbs->addAction(
-        id(new PHUIListItemView())
-        ->setName(pht('Create New Bot'))
-        ->setHref($this->getApplicationURI('new/bot/'))
-        ->setIcon('fa-plus-square'));
     }
 
     return $crumbs;
diff --git a/src/applications/people/controller/PhabricatorPeopleNewController.php b/src/applications/people/controller/PhabricatorPeopleNewController.php
@@ -7,15 +7,19 @@ public function handleRequest(AphrontRequest $request) {
     $type = $request->getURIData('type');
     $admin = $request->getUser();
 
+    $is_bot = false;
+    $is_list = false;
     switch ($type) {
       case 'standard':
         $this->requireApplicationCapability(
           PeopleCreateUsersCapability::CAPABILITY);
-        $is_bot = false;
         break;
       case 'bot':
         $is_bot = true;
         break;
+      case 'list':
+        $is_list = true;
+        break;
       default:
         return new Aphront404Response();
     }
@@ -77,8 +81,8 @@ public function handleRequest(AphrontRequest $request) {
           // Automatically approve the user, since an admin is creating them.
           $user->setIsApproved(1);
 
-          // If the user is a bot, approve their email too.
-          if ($is_bot) {
+          // If the user is a bot or list, approve their email too.
+          if ($is_bot || $is_list) {
             $email->setIsVerified(1);
           }
 
@@ -92,7 +96,13 @@ public function handleRequest(AphrontRequest $request) {
               ->makeSystemAgentUser($user, true);
           }
 
-          if ($welcome_checked && !$is_bot) {
+          if ($is_list) {
+            id(new PhabricatorUserEditor())
+              ->setActor($admin)
+              ->makeMailingListUser($user, true);
+          }
+
+          if ($welcome_checked && !$is_bot && !$is_list) {
             $user->sendWelcomeEmail($admin);
           }
 
@@ -123,7 +133,10 @@ public function handleRequest(AphrontRequest $request) {
 
     if ($is_bot) {
       $form->appendRemarkupInstructions(
-        pht('You are creating a new **bot/script** user account.'));
+        pht('You are creating a new **bot** user account.'));
+    } else if ($is_list) {
+      $form->appendRemarkupInstructions(
+        pht('You are creating a new **mailing list** user account.'));
     } else {
       $form->appendRemarkupInstructions(
         pht('You are creating a new **standard** user account.'));
@@ -150,7 +163,7 @@ public function handleRequest(AphrontRequest $request) {
           ->setCaption(PhabricatorUserEmail::describeAllowedAddresses())
           ->setError($e_email));
 
-    if (!$is_bot) {
+    if (!$is_bot && !$is_list) {
       $form->appendChild(
         id(new AphrontFormCheckboxControl())
           ->addCheckbox(
@@ -171,7 +184,7 @@ public function handleRequest(AphrontRequest $request) {
         ->appendChild(id(new AphrontFormDividerControl()))
         ->appendRemarkupInstructions(
           pht(
-            '**Why do bot/script accounts need an email address?**'.
+            '**Why do bot accounts need an email address?**'.
             "\n\n".
             'Although bots do not normally receive email from Phabricator, '.
             'they can interact with other systems which require an email '.
diff --git a/src/applications/people/customfield/PhabricatorUserRolesField.php b/src/applications/people/customfield/PhabricatorUserRolesField.php
@@ -37,6 +37,9 @@ public function renderPropertyViewValue(array $handles) {
     if ($user->getIsSystemAgent()) {
       $roles[] = pht('Bot');
     }
+    if ($user->getIsMailingList()) {
+      $roles[] = pht('Mailing List');
+    }
 
     if ($roles) {
       return implode(', ', $roles);
diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php
@@ -278,6 +278,43 @@ public function makeSystemAgentUser(PhabricatorUser $user, $system_agent) {
     return $this;
   }
 
+  /**
+   * @task role
+   */
+  public function makeMailingListUser(PhabricatorUser $user, $mailing_list) {
+    $actor = $this->requireActor();
+
+    if (!$user->getID()) {
+      throw new Exception(pht('User has not been created yet!'));
+    }
+
+    $user->openTransaction();
+      $user->beginWriteLocking();
+
+        $user->reload();
+        if ($user->getIsMailingList() == $mailing_list) {
+          $user->endWriteLocking();
+          $user->killTransaction();
+          return $this;
+        }
+
+        $log = PhabricatorUserLog::initializeNewLog(
+          $actor,
+          $user->getPHID(),
+          PhabricatorUserLog::ACTION_MAILING_LIST);
+        $log->setOldValue($user->getIsMailingList());
+        $log->setNewValue($mailing_list);
+
+        $user->setIsMailingList((int)$mailing_list);
+        $user->save();
+
+        $log->save();
+
+      $user->endWriteLocking();
+    $user->saveTransaction();
+
+    return $this;
+  }
 
   /**
    * @task role
diff --git a/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php b/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php
@@ -44,6 +44,10 @@ public function loadHandles(
       $handle->setFullName($user->getFullName());
       $handle->setImageURI($user->getProfileImageURI());
 
+      if ($user->getIsMailingList()) {
+        $handle->setIcon('fa-envelope-o');
+      }
+
       $availability = null;
       if (!$user->isUserActivated()) {
         $availability = PhabricatorObjectHandle::AVAILABILITY_DISABLED;
diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php
@@ -12,6 +12,7 @@ final class PhabricatorPeopleQuery
   private $dateCreatedBefore;
   private $isAdmin;
   private $isSystemAgent;
+  private $isMailingList;
   private $isDisabled;
   private $isApproved;
   private $nameLike;
@@ -67,6 +68,11 @@ public function withIsSystemAgent($system_agent) {
     return $this;
   }
 
+  public function withIsMailingList($mailing_list) {
+    $this->isMailingList = $mailing_list;
+    return $this;
+  }
+
   public function withIsDisabled($disabled) {
     $this->isDisabled = $disabled;
     return $this;
@@ -340,6 +346,13 @@ protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
         (int)$this->isSystemAgent);
     }
 
+    if ($this->isMailingList !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'user.isMailingList = %d',
+        (int)$this->isMailingList);
+    }
+
     if (strlen($this->nameLike)) {
       $where[] = qsprintf(
         $conn_r,
diff --git a/src/applications/people/query/PhabricatorPeopleSearchEngine.php b/src/applications/people/query/PhabricatorPeopleSearchEngine.php
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php
diff --git a/src/applications/people/typeahead/PhabricatorPeopleDatasource.php b/src/applications/people/typeahead/PhabricatorPeopleDatasource.php
diff --git a/src/applications/settings/controller/PhabricatorSettingsMainController.php b/src/applications/settings/controller/PhabricatorSettingsMainController.php
diff --git a/src/applications/settings/panel/PhabricatorConduitCertificateSettingsPanel.php b/src/applications/settings/panel/PhabricatorConduitCertificateSettingsPanel.php
diff --git a/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php b/src/applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+ALTER TABLE {$NAMESPACE}_user.user`
	`2`	`+ ADD isMailingList BOOL NOT NULL;`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,10 @@ public function getPanelGroup() {`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`public function isEnabled() {`
	`23`	`+ if ($this->getUser()->getIsMailingList()) {`
	`24`	`+ return false;`
	`25`	`+ }`
	`26`	`+`
`23`	`27`	`return true;`
`24`	`28`	`}`
`25`	`29`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,10 @@ public function getPanelGroup() {`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`public function isEnabled() {`
	`22`	`+ if ($this->getUser()->getIsMailingList()) {`
	`23`	`+ return false;`
	`24`	`+ }`
	`25`	`+`
`22`	`26`	`return PhabricatorEnv::getEnvConfig('diffusion.allow-http-auth');`
`23`	`27`	`}`
`24`	`28`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,9 @@ protected function buildUserInformationDictionary(`
`18`	`18`	`if ($user->getIsSystemAgent()) {`
`19`	`19`	`$roles[] = 'agent';`
`20`	`20`	`}`
	`21`	`+ if ($user->getIsMailingList()) {`
	`22`	`+ $roles[] = 'list';`
	`23`	`+ }`
`21`	`24`	`if ($user->getIsAdmin()) {`
`22`	`25`	`$roles[] = 'admin';`
`23`	`26`	`}`