From 789dc6cb5e4d0c232b5131a5e47ab114b8d06639 Mon Sep 17 00:00:00 2001 From: Marek Sapota Date: Mon, 24 Oct 2011 12:27:16 -0700 Subject: [PATCH] Allow anonymus access to Differential. Summary: Add possibility for not logged in users to browse and see Differential revisions. Test Plan: Set 'differential.anonymous-access' config option to true, log out, you should be able to browse Differential without logging back in. Reviewers: epriestley, jungejason Reviewed By: epriestley CC: aran, epriestley, mareksapota Differential Revision: 1044 --- conf/default.conf.php | 5 + .../base/DifferentialController.php | 28 ++-- .../differential/controller/base/__init__.php | 1 + .../DifferentialChangesetViewController.php | 4 + .../DifferentialRevisionListController.php | 126 ++++++++------- .../DifferentialRevisionViewController.php | 151 ++++++++++-------- .../people/storage/user/PhabricatorUser.php | 4 + .../standard/PhabricatorStandardPageView.php | 12 ++ 8 files changed, 197 insertions(+), 134 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index e719e03ab1..892e395046 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -570,6 +570,11 @@ return array( // not actually be receiving thorough review. 'differential.enable-email-accept' => false, + // If you set this to true, users won't need to login to view differential + // revisions. Anonymous users will have read-only access and won't be able to + // interact with the revisions. + 'differential.anonymous-access' => false, + // -- Maniphest ------------------------------------------------------------- // diff --git a/src/applications/differential/controller/base/DifferentialController.php b/src/applications/differential/controller/base/DifferentialController.php index 50300b0210..12dabd77d2 100644 --- a/src/applications/differential/controller/base/DifferentialController.php +++ b/src/applications/differential/controller/base/DifferentialController.php @@ -18,10 +18,16 @@ abstract class DifferentialController extends PhabricatorController { + protected function allowsAnonymousAccess() { + return PhabricatorEnv::getEnvConfig('differential.anonymous-access'); + } + public function buildStandardPageResponse($view, array $data) { require_celerity_resource('differential-core-view-css'); + $viewer_is_anonymous = !$this->getRequest()->getUser()->isLoggedIn(); + $page = $this->buildStandardPageView(); $page->setApplicationName('Differential'); @@ -29,18 +35,22 @@ abstract class DifferentialController extends PhabricatorController { $page->setTitle(idx($data, 'title')); $page->setGlyph("\xE2\x9A\x99"); $page->appendChild($view); - $page->setTabs( - array( - 'revisions' => array( - 'name' => 'Revisions', - 'href' => '/differential/', - ), + $tabs = array( + 'revisions' => array( + 'name' => 'Revisions', + 'href' => '/differential/', + ) + ); + if (!$viewer_is_anonymous) { + $tabs = array_merge($tabs, array( 'create' => array( 'name' => 'Create Diff', 'href' => '/differential/diff/create/', - ), - ), - idx($data, 'tab')); + ) + )); + } + $page->setTabs($tabs, idx($data, 'tab')); + $page->setIsLoggedOut($viewer_is_anonymous); $response = new AphrontWebpageResponse(); return $response->setContent($page->render()); diff --git a/src/applications/differential/controller/base/__init__.php b/src/applications/differential/controller/base/__init__.php index 015845d45e..00b02d92ec 100644 --- a/src/applications/differential/controller/base/__init__.php +++ b/src/applications/differential/controller/base/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'aphront/response/webpage'); phutil_require_module('phabricator', 'applications/base/controller/base'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php index 176b4f505e..8d55996ff0 100644 --- a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php @@ -19,6 +19,10 @@ class DifferentialChangesetViewController extends DifferentialController { + public function shouldRequireLogin() { + return !$this->allowsAnonymousAccess(); + } + public function processRequest() { $request = $this->getRequest(); diff --git a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php index 2ec76b63f5..4d9f144bc4 100644 --- a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php @@ -20,6 +20,10 @@ class DifferentialRevisionListController extends DifferentialController { private $filter; + public function shouldRequireLogin() { + return !$this->allowsAnonymousAccess(); + } + public function willProcessRequest(array $data) { $this->filter = idx($data, 'filter'); } @@ -27,6 +31,7 @@ class DifferentialRevisionListController extends DifferentialController { public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); + $viewer_is_anonymous = !$user->isLoggedIn(); if ($request->isFormPost()) { $phid_arr = $request->getArr('view_user'); @@ -35,72 +40,77 @@ class DifferentialRevisionListController extends DifferentialController { ->setURI($request->getRequestURI()->alter('phid', $view_target)); } - $filters = array( - 'User Revisions', - 'active' => array( - 'name' => 'Active Revisions', - 'queries' => array( - array( - 'query' - => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_SELF, - 'header' => 'Action Required', - 'nodata' => 'You have no revisions requiring action.', - ), - array( - 'query' - => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_OTHERS, - 'header' => 'Waiting on Others', - 'nodata' => 'You have no revisions waiting on others', + $filters = array(); + if (!$viewer_is_anonymous) { + $filters = array( + 'User Revisions', + 'active' => array( + 'name' => 'Active Revisions', + 'queries' => array( + array( + 'query' + => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_SELF, + 'header' => 'Action Required', + 'nodata' => 'You have no revisions requiring action.', + ), + array( + 'query' + => DifferentialRevisionListData::QUERY_NEED_ACTION_FROM_OTHERS, + 'header' => 'Waiting on Others', + 'nodata' => 'You have no revisions waiting on others', + ), ), ), - ), - 'open' => array( - 'name' => 'Open Revisions', - 'queries' => array( - array( - 'query' => DifferentialRevisionListData::QUERY_OPEN_OWNED, - 'header' => 'Your Open Revisions', + 'open' => array( + 'name' => 'Open Revisions', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OPEN_OWNED, + 'header' => 'Your Open Revisions', + ), ), ), - ), - 'reviews' => array( - 'name' => 'Open Reviews', - 'queries' => array( - array( - 'query' => DifferentialRevisionListData::QUERY_OPEN_REVIEWER, - 'header' => 'Your Open Reviews', + 'reviews' => array( + 'name' => 'Open Reviews', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OPEN_REVIEWER, + 'header' => 'Your Open Reviews', + ), ), ), - ), - 'all' => array( - 'name' => 'All Revisions', - 'queries' => array( - array( - 'query' => DifferentialRevisionListData::QUERY_OWNED, - 'header' => 'Your Revisions', + 'all' => array( + 'name' => 'All Revisions', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OWNED, + 'header' => 'Your Revisions', + ), ), ), - ), - 'related' => array( - 'name' => 'All Revisions and Reviews', - 'queries' => array( - array( - 'query' => DifferentialRevisionListData::QUERY_OWNED_OR_REVIEWER, - 'header' => 'Your Revisions and Reviews', + 'related' => array( + 'name' => 'All Revisions and Reviews', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_OWNED_OR_REVIEWER, + 'header' => 'Your Revisions and Reviews', + ), ), ), - ), - 'updates' => array( - 'name' => 'Updates', - 'queries' => array( - array( - 'query' => DifferentialRevisionListData::QUERY_UPDATED_SINCE, - 'header' => - 'Diffs that have been updated since you\'ve last viewed them', + 'updates' => array( + 'name' => 'Updates', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_UPDATED_SINCE, + 'header' => + 'Diffs that have been updated since you\'ve last viewed them', + ), ), ), - ), - '
', + '
' + ); + } + $filters = array_merge($filters, array( 'All Revisions', 'allopen' => array( 'name' => 'Open', @@ -112,10 +122,14 @@ class DifferentialRevisionListController extends DifferentialController { ), ), ), - ); + )); if (empty($filters[$this->filter])) { - $this->filter = 'active'; + if (!$viewer_is_anonymous) { + $this->filter = 'active'; + } else { + $this->filter = 'allopen'; + } } $view_phid = nonempty($request->getStr('phid'), $user->getPHID()); diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index e752acc52d..aa6b215d1e 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -20,6 +20,10 @@ class DifferentialRevisionViewController extends DifferentialController { private $revisionID; + public function shouldRequireLogin() { + return !$this->allowsAnonymousAccess(); + } + public function willProcessRequest(array $data) { $this->revisionID = $data['id']; } @@ -28,6 +32,7 @@ class DifferentialRevisionViewController extends DifferentialController { $request = $this->getRequest(); $user = $request->getUser(); + $viewer_is_anonymous = !$user->isLoggedIn(); $revision = id(new DifferentialRevision())->load($this->revisionID); if (!$revision) { @@ -197,7 +202,7 @@ class DifferentialRevisionViewController extends DifferentialController { $changeset_view = new DifferentialChangesetListView(); $changeset_view->setChangesets($visible_changesets); - $changeset_view->setEditable(true); + $changeset_view->setEditable(!$viewer_is_anonymous); $changeset_view->setStandaloneViews(true); $changeset_view->setRevision($revision); $changeset_view->setRenderingReferences($rendering_references); @@ -221,25 +226,27 @@ class DifferentialRevisionViewController extends DifferentialController { $toc_view->setRevisionID($revision->getID()); $toc_view->setWhitespace($whitespace); - $draft = id(new PhabricatorDraft())->loadOneWhere( - 'authorPHID = %s AND draftKey = %s', - $user->getPHID(), - 'differential-comment-'.$revision->getID()); - if ($draft) { - $draft = $draft->getDraft(); - } else { - $draft = null; + if (!$viewer_is_anonymous) { + $draft = id(new PhabricatorDraft())->loadOneWhere( + 'authorPHID = %s AND draftKey = %s', + $user->getPHID(), + 'differential-comment-'.$revision->getID()); + if ($draft) { + $draft = $draft->getDraft(); + } else { + $draft = null; + } + + $comment_form = new DifferentialAddCommentView(); + $comment_form->setRevision($revision); + $comment_form->setActions($this->getRevisionCommentActions($revision)); + $comment_form->setActionURI('/differential/comment/save/'); + $comment_form->setUser($user); + $comment_form->setDraft($draft); + + $this->updateViewTime($user->getPHID(), $revision->getPHID()); } - $comment_form = new DifferentialAddCommentView(); - $comment_form->setRevision($revision); - $comment_form->setActions($this->getRevisionCommentActions($revision)); - $comment_form->setActionURI('/differential/comment/save/'); - $comment_form->setUser($user); - $comment_form->setDraft($draft); - - $this->updateViewTime($user->getPHID(), $revision->getPHID()); - $pane_id = celerity_generate_unique_node_id(); Javelin::initBehavior( 'differential-keyboard-navigation', @@ -247,19 +254,22 @@ class DifferentialRevisionViewController extends DifferentialController { 'haunt' => $pane_id, )); + $page_pane = id(new DifferentialPrimaryPaneView()) + ->setLineWidthFromChangesets($changesets) + ->setID($pane_id) + ->appendChild( + $revision_detail->render(). + $comment_view->render(). + $diff_history->render(). + $warning. + $local_view->render(). + $toc_view->render(). + $changeset_view->render()); + if ($comment_form) { + $page_pane->appendChild($comment_form->render()); + } return $this->buildStandardPageResponse( - id(new DifferentialPrimaryPaneView()) - ->setLineWidthFromChangesets($changesets) - ->setID($pane_id) - ->appendChild( - $revision_detail->render(). - $comment_view->render(). - $diff_history->render(). - $warning. - $local_view->render(). - $toc_view->render(). - $changeset_view->render(). - $comment_form->render()), + $page_pane, array( 'title' => 'D'.$revision->getID().' '.$revision->getTitle(), )); @@ -296,6 +306,7 @@ class DifferentialRevisionViewController extends DifferentialController { $viewer_is_owner = ($revision->getAuthorPHID() == $viewer_phid); $viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers()); $viewer_is_cc = in_array($viewer_phid, $revision->getCCPHIDs()); + $viewer_is_anonymous = !$this->getRequest()->getUser()->isLoggedIn(); $status = $revision->getStatus(); $revision_id = $revision->getID(); $revision_phid = $revision->getPHID(); @@ -310,52 +321,54 @@ class DifferentialRevisionViewController extends DifferentialController { ); } - if (!$viewer_is_owner && !$viewer_is_reviewer) { - $action = $viewer_is_cc ? 'rem' : 'add'; - $links[] = array( - 'class' => $viewer_is_cc ? 'subscribe-rem' : 'subscribe-add', - 'href' => "/differential/subscribe/{$action}/{$revision_id}/", - 'name' => $viewer_is_cc ? 'Unsubscribe' : 'Subscribe', - 'instant' => true, - ); - } else { - $links[] = array( - 'class' => 'subscribe-rem unavailable', - 'name' => 'Automatically Subscribed', - ); - } + if (!$viewer_is_anonymous) { + if (!$viewer_is_owner && !$viewer_is_reviewer) { + $action = $viewer_is_cc ? 'rem' : 'add'; + $links[] = array( + 'class' => $viewer_is_cc ? 'subscribe-rem' : 'subscribe-add', + 'href' => "/differential/subscribe/{$action}/{$revision_id}/", + 'name' => $viewer_is_cc ? 'Unsubscribe' : 'Subscribe', + 'instant' => true, + ); + } else { + $links[] = array( + 'class' => 'subscribe-rem unavailable', + 'name' => 'Automatically Subscribed', + ); + } - require_celerity_resource('phabricator-object-selector-css'); - require_celerity_resource('javelin-behavior-phabricator-object-selector'); + require_celerity_resource('phabricator-object-selector-css'); + require_celerity_resource('javelin-behavior-phabricator-object-selector'); - $links[] = array( - 'class' => 'action-dependencies', - 'name' => 'Edit Dependencies', - 'href' => "/search/attach/{$revision_phid}/DREV/dependencies/", - 'sigil' => 'workflow', - ); - - if (PhabricatorEnv::getEnvConfig('maniphest.enabled')) { $links[] = array( - 'class' => 'attach-maniphest', - 'name' => 'Edit Maniphest Tasks', - 'href' => "/search/attach/{$revision_phid}/TASK/", + 'class' => 'action-dependencies', + 'name' => 'Edit Dependencies', + 'href' => "/search/attach/{$revision_phid}/DREV/dependencies/", 'sigil' => 'workflow', ); + + if (PhabricatorEnv::getEnvConfig('maniphest.enabled')) { + $links[] = array( + 'class' => 'attach-maniphest', + 'name' => 'Edit Maniphest Tasks', + 'href' => "/search/attach/{$revision_phid}/TASK/", + 'sigil' => 'workflow', + ); + } + + $links[] = array( + 'class' => 'transcripts-metamta', + 'name' => 'MetaMTA Transcripts', + 'href' => "/mail/?phid={$revision_phid}", + ); + + $links[] = array( + 'class' => 'transcripts-herald', + 'name' => 'Herald Transcripts', + 'href' => "/herald/transcript/?phid={$revision_phid}", + ); } - $links[] = array( - 'class' => 'transcripts-metamta', - 'name' => 'MetaMTA Transcripts', - 'href' => "/mail/?phid={$revision_phid}", - ); - - $links[] = array( - 'class' => 'transcripts-herald', - 'name' => 'Herald Transcripts', - 'href' => "/herald/transcript/?phid={$revision_phid}", - ); - return $links; } diff --git a/src/applications/people/storage/user/PhabricatorUser.php b/src/applications/people/storage/user/PhabricatorUser.php index b7313ca68a..433cfd2c17 100644 --- a/src/applications/people/storage/user/PhabricatorUser.php +++ b/src/applications/people/storage/user/PhabricatorUser.php @@ -86,6 +86,10 @@ class PhabricatorUser extends PhabricatorUserDAO { return $this; } + public function isLoggedIn() { + return !($this->getPHID() === null); + } + public function save() { if (!$this->getConduitCertificate()) { $this->setConduitCertificate($this->generateConduitCertificate()); diff --git a/src/view/page/standard/PhabricatorStandardPageView.php b/src/view/page/standard/PhabricatorStandardPageView.php index 9ac48deaba..a68baf7d92 100644 --- a/src/view/page/standard/PhabricatorStandardPageView.php +++ b/src/view/page/standard/PhabricatorStandardPageView.php @@ -35,6 +35,18 @@ class PhabricatorStandardPageView extends AphrontPageView { return $this; } + public function setIsLoggedOut($is_logged_out) { + if ($is_logged_out) { + $this->tabs = array_merge($this->tabs, array( + 'login' => array( + 'name' => 'Login', + 'href' => '/login/' + ) + )); + } + return $this; + } + public function getIsAdminInterface() { return $this->isAdminInterface; }