mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-27 01:02:42 +01:00
If a user can't see an application, prevent them from using its controllers
Summary: Ref T603. Broadly, this allows you to implement a policy like "Only users in Engineering can use Differential." This isn't complete, and there will be a long tail of special cases to deal with. Some examples: - If you can't use Differential, should you still be able to attach/detach revisions from tasks? - You currently will be able to. - This actually seems pretty reasonable. - But in other cases it might not be: the "send user a message" action should probably require access to Conpherence. - If you can't use Differential, should you still be able to see feed stories about it? - You currently will be able to, if you can see the revisions. - This seems not-so-reasonable and we should probably lock it down. - If you can't use Differential, can users CC you on revisions? - Currently, they can, and you can't do anything about it. - Probably they shouldn't be able to? This seems challenging to explain in the UI. - If you can't use Differential, can you write a Herald rule against it? - You currently will be able to. - Seems like you obviously shouldn't be able to. - I think this is a general issue right now (you can still write Differential herald rules even if you uninstall the application, I believe). There are probably a few more things I haven't thought of. However, there are a finite number of these things and I suspect there aren't //too/ many more than this -- I can't come up with like 100 of them, and half of the ones above have easy fixes. Despite the rough edges, I think this accomplishes 95% of what installs expect from it. Test Plan: Restricted Differential and saw it vanish from the home page. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7203
This commit is contained in:
parent
516116e229
commit
0d83e1d66f
2 changed files with 15 additions and 1 deletions
|
@ -122,6 +122,17 @@ abstract class PhabricatorController extends AphrontController {
|
|||
return new Aphront403Response();
|
||||
}
|
||||
|
||||
// If the user doesn't have access to the application, don't let them use
|
||||
// any of its controllers. We query the application in order to generate
|
||||
// a policy exception if the viewer doesn't have permission.
|
||||
$application = $this->getCurrentApplication();
|
||||
if ($application) {
|
||||
id(new PhabricatorApplicationQuery())
|
||||
->setViewer($user)
|
||||
->withPHIDs(array($application->getPHID()))
|
||||
->executeOne();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
public function buildStandardPageView() {
|
||||
|
|
|
@ -21,7 +21,10 @@ abstract class PhabricatorDirectoryController extends PhabricatorController {
|
|||
$nav = new AphrontSideNavFilterView();
|
||||
$nav->setBaseURI(new PhutilURI('/'));
|
||||
|
||||
$applications = PhabricatorApplication::getAllInstalledApplications();
|
||||
$applications = id(new PhabricatorApplicationQuery())
|
||||
->setViewer($user)
|
||||
->withInstalled(true)
|
||||
->execute();
|
||||
|
||||
foreach ($applications as $key => $application) {
|
||||
if (!$application->shouldAppearInLaunchView()) {
|
||||
|
|
Loading…
Reference in a new issue