From 7213eb01e0307095faae57b016d6754558843fd7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 5 Feb 2015 10:57:17 -0800 Subject: [PATCH] Only let users log in to an OAuth server if they can see it Summary: Fixes T7169. We just weren't doing a policy-aware query. Basic idea here is that if you set an app to be visible only to specific users, those specific users are the only ones who should be able to authorize it. In the Phacility cluster, this allows us to prevent users who haven't been invited from logging in to an instance. Test Plan: - Tried to log into an instance I was not a member of. - Logged into an instance I am a member of. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7169 Differential Revision: https://secure.phabricator.com/D11696 --- .../PhabricatorOAuthServerAuthController.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php index 48fe6ae98c..e38d276b93 100644 --- a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php +++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php @@ -39,8 +39,23 @@ final class PhabricatorOAuthServerAuthController // one giant try / catch around all the exciting database stuff so we // can return a 'server_error' response if something goes wrong! try { - $client = id(new PhabricatorOAuthServerClient()) - ->loadOneWhere('phid = %s', $client_phid); + try { + $client = id(new PhabricatorOAuthServerClientQuery()) + ->setViewer($viewer) + ->withPHIDs(array($client_phid)) + ->executeOne(); + } catch (PhabricatorPolicyException $ex) { + // We require that users must be able to see an OAuth application + // in order to authorize it. This allows an application's visibility + // policy to be used to restrict authorized users. + + // None of the OAuth error responses are a perfect fit for this, but + // 'invalid_client' seems closest. + return $this->buildErrorResponse( + 'invalid_client', + pht('Not Authorized'), + pht('You are not authorized to authenticate.')); + } if (!$client) { return $this->buildErrorResponse(