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
This commit is contained in:
@@ -86,10 +86,17 @@ final class ConduitCall {
|
|||||||
|
|
||||||
$this->request->setUser($user);
|
$this->request->setUser($user);
|
||||||
|
|
||||||
if ($this->shouldRequireAuthentication()) {
|
if (!$this->shouldRequireAuthentication()) {
|
||||||
// TODO: As per below, this should get centralized and cleaned up.
|
// No auth requirement here.
|
||||||
if (!$user->isLoggedIn() && !$user->isOmnipotent()) {
|
} else {
|
||||||
throw new ConduitException("ERR-INVALID-AUTH");
|
|
||||||
|
$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
|
// TODO: This would be slightly cleaner by just using a Query, but the
|
||||||
|
|||||||
@@ -104,6 +104,10 @@ abstract class ConduitAPIMethod
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldAllowPublic() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
public function shouldAllowUnguardedWrites() {
|
public function shouldAllowUnguardedWrites() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,9 +6,14 @@
|
|||||||
abstract class ConduitAPI_diffusion_abstractquery_Method
|
abstract class ConduitAPI_diffusion_abstractquery_Method
|
||||||
extends ConduitAPI_diffusion_Method {
|
extends ConduitAPI_diffusion_Method {
|
||||||
|
|
||||||
|
public function shouldAllowPublic() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
public function getMethodStatus() {
|
public function getMethodStatus() {
|
||||||
return self::METHOD_STATUS_UNSTABLE;
|
return self::METHOD_STATUS_UNSTABLE;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getMethodStatusDescription() {
|
public function getMethodStatusDescription() {
|
||||||
return pht(
|
return pht(
|
||||||
'See T2784 - migrating diffusion working copy calls to conduit methods. '.
|
'See T2784 - migrating diffusion working copy calls to conduit methods. '.
|
||||||
|
|||||||
Reference in New Issue
Block a user