From 90b83d7a92c56aee95bb2e1eead74ef13980c68b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 22 Oct 2013 13:47:52 -0700
Subject: [PATCH] Fix logged-out Diffusion calls to Conduit

Summary:
Conduit doesn't currently have an analog to "shouldAllowPublic", so the recent policy checks added here caught legitimate Conduit calls when viewing Diffusion as a logged-out user.

Add `shouldAllowPublic()` and set it for all the Diffusion queries.

(More calls probably need this, but we can add it when we hit them.)

Test Plan: Looked at Diffusion as a logged-out user with public access enabled.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7380
---
 src/applications/conduit/call/ConduitCall.php     | 15 +++++++++++----
 .../conduit/method/ConduitAPIMethod.php           |  4 ++++
 .../ConduitAPI_diffusion_abstractquery_Method.php |  5 +++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/applications/conduit/call/ConduitCall.php b/src/applications/conduit/call/ConduitCall.php
index b9e5a1491a..0b819bd093 100644
--- a/src/applications/conduit/call/ConduitCall.php
+++ b/src/applications/conduit/call/ConduitCall.php
@@ -86,10 +86,17 @@ final class ConduitCall {
 
     $this->request->setUser($user);
 
-    if ($this->shouldRequireAuthentication()) {
-      // TODO: As per below, this should get centralized and cleaned up.
-      if (!$user->isLoggedIn() && !$user->isOmnipotent()) {
-        throw new ConduitException("ERR-INVALID-AUTH");
+    if (!$this->shouldRequireAuthentication()) {
+      // No auth requirement here.
+    } else {
+
+      $allow_public = $this->handler->shouldAllowPublic() &&
+                      PhabricatorEnv::getEnvConfig('policy.allow-public');
+      if (!$allow_public) {
+        if (!$user->isLoggedIn() && !$user->isOmnipotent()) {
+          // TODO: As per below, this should get centralized and cleaned up.
+          throw new ConduitException("ERR-INVALID-AUTH");
+        }
       }
 
       // TODO: This would be slightly cleaner by just using a Query, but the
diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php
index 3a9f389b5f..e549f5755b 100644
--- a/src/applications/conduit/method/ConduitAPIMethod.php
+++ b/src/applications/conduit/method/ConduitAPIMethod.php
@@ -104,6 +104,10 @@ abstract class ConduitAPIMethod
     return true;
   }
 
+  public function shouldAllowPublic() {
+    return false;
+  }
+
   public function shouldAllowUnguardedWrites() {
     return false;
   }
diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php
index 3b84520d86..c4e48779b7 100644
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php
@@ -6,9 +6,14 @@
 abstract class ConduitAPI_diffusion_abstractquery_Method
   extends ConduitAPI_diffusion_Method {
 
+  public function shouldAllowPublic() {
+    return true;
+  }
+
   public function getMethodStatus() {
     return self::METHOD_STATUS_UNSTABLE;
   }
+
   public function getMethodStatusDescription() {
     return pht(
       'See T2784 - migrating diffusion working copy calls to conduit methods. '.