From 0b11bc2d70a14395b5251d851f46de629f39a6f1 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 8 Jun 2022 11:17:40 -0400 Subject: [PATCH] Oauth - Move CUD permissions to the BAO This ensures permissions are checked regardless of the api version or action being performed. It also consolidates the code used to check access so that the API checkAccess action will stay consistently and always perform the same access checks as the other api actions. --- .../CRM/OAuth/BAO/OAuthContactToken.php | 94 +++++++++++++++++++ .../Api4/Action/OAuthContactToken/Create.php | 62 ------------ .../Api4/Action/OAuthContactToken/Delete.php | 10 -- .../OnlyModifyOwnTokensTrait.php | 31 ------ .../Api4/Action/OAuthContactToken/Update.php | 10 -- .../Civi/Api4/OAuthContactToken.php | 15 --- .../phpunit/Civi/OAuth/AuthCodeFlowTest.php | 4 +- .../phpunit/api/v4/OAuthContactTokenTest.php | 41 ++++---- 8 files changed, 114 insertions(+), 153 deletions(-) delete mode 100644 ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Create.php delete mode 100644 ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Delete.php delete mode 100644 ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php delete mode 100644 ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Update.php diff --git a/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php b/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php index c6b2add4d3..69228fb622 100644 --- a/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php +++ b/ext/oauth-client/CRM/OAuth/BAO/OAuthContactToken.php @@ -2,6 +2,100 @@ class CRM_OAuth_BAO_OAuthContactToken extends CRM_OAuth_DAO_OAuthContactToken { + /** + * Create or update OAuthContactToken based on array-data + * + * @param array $record + * @return CRM_OAuth_DAO_OAuthContactToken + */ + public static function create($record) { + self::fillAndValidate($record, CRM_Core_Session::getLoggedInContactID()); + return static::writeRecord($record); + } + + /** + * @param $id + * @return CRM_OAuth_BAO_OAuthContactToken + * @throws CRM_Core_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + public static function del($id) { + $record = ['id' => $id]; + self::fillAndValidate($record, CRM_Core_Session::getLoggedInContactID()); + return static::deleteRecord($record); + } + + /** + * @param string $entityName + * @param string $action + * @param array $record + * @param $userId + * @return bool + * @see CRM_Core_DAO::checkAccess + */ + public static function _checkAccess(string $entityName, string $action, array $record, $userId): bool { + try { + $record['check_permissions'] = TRUE; + self::fillAndValidate($record, $userId); + return TRUE; + } + catch (\Civi\API\Exception\UnauthorizedException $e) { + return FALSE; + } + } + + /** + * @param $record + * @param $userId + * @throws \Civi\API\Exception\UnauthorizedException + */ + private static function fillAndValidate(&$record, $userId) { + if (!empty($record['id']) && empty($record['contact_id'])) { + $record['contact_id'] = CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'contact_id'); + } + self::fillContactIdFromTag($record); + if (!empty($record['check_permissions'])) { + $cid = $record['contact_id']; + if (!CRM_Contact_BAO_Contact_Permission::allow($cid, CRM_Core_Permission::EDIT, $userId)) { + throw new \Civi\API\Exception\UnauthorizedException('Access denied to contact'); + } + if (!CRM_Core_Permission::check([['manage all OAuth contact tokens', 'manage my OAuth contact tokens']], $userId)) { + throw new \Civi\API\Exception\UnauthorizedException('Access denied to OAuthContactToken'); + } + if ( + !CRM_Core_Permission::check(['manage all OAuth contact tokens'], $userId) && + $cid != $userId + ) { + throw new \Civi\API\Exception\UnauthorizedException('Access denied to OAuthContactToken for contact'); + } + } + } + + /** + * @param array $record + */ + private static function fillContactIdFromTag(&$record): void { + if (isset($record['contact_id'])) { + return; + } + + $tag = $record['tag'] ?? NULL; + + if ('linkContact:' === substr($tag, 0, 12)) { + $record['contact_id'] = substr($tag, 12); + } + elseif ('nullContactId' === $tag) { + $record['contact_id'] = NULL; + } + elseif ('createContact' === $tag) { + $contact = CRM_OAuth_ContactFromToken::createContact($record); + $record['contact_id'] = $contact['id']; + } + else { + $record['contact_id'] = CRM_Core_Session::getLoggedInContactID(); + } + } + /** * @inheritDoc */ diff --git a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Create.php b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Create.php deleted file mode 100644 index ddb03ce3b7..0000000000 --- a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Create.php +++ /dev/null @@ -1,62 +0,0 @@ -fillContactIdFromTag(); - $this->assertPermissionForTokenContact(); - parent::_run($result); - } - - private function fillContactIdFromTag(): void { - if (isset($this->values['contact_id'])) { - return; - } - - $tag = $this->values['tag'] ?? ''; - - if ('linkContact:' === substr($tag, 0, 12)) { - $this->values['contact_id'] = substr($tag, 12); - } - elseif ('nullContactId' === $tag) { - $this->values['contact_id'] = NULL; - } - elseif ('createContact' === $tag) { - $contact = \CRM_OAuth_ContactFromToken::createContact($this->values); - $this->values['contact_id'] = $contact['id']; - } - else { - $this->values['contact_id'] = \CRM_Core_Session::singleton() - ->getLoggedInContactID(); - } - } - - /** - * @throws \Civi\API\Exception\UnauthorizedException - */ - private function assertPermissionForTokenContact(): void { - if (!$this->getCheckPermissions()) { - return; - } - if (\CRM_Core_Permission::check('manage all OAuth contact tokens')) { - return; - } - if (\CRM_Core_Permission::check('manage my OAuth contact tokens')) { - $loggedInContactID = \CRM_Core_Session::singleton() - ->getLoggedInContactID(); - $tokenContactID = $this->values['contact_id'] ?? NULL; - if ($loggedInContactID == $tokenContactID) { - return; - } - } - throw new \Civi\API\Exception\UnauthorizedException(ts( - "You do not have permission to create OAuth tokens for contact id %1", - [1 => $tokenContactID])); - } - -} diff --git a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Delete.php b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Delete.php deleted file mode 100644 index d0d02bc1ae..0000000000 --- a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Delete.php +++ /dev/null @@ -1,10 +0,0 @@ -getLoggedInContactID(); - foreach ($this->where as $clause) { - [$field, $op, $val] = $clause; - if ($field !== 'contact_id') { - continue; - } - if (($op === '=' || $op === 'LIKE') && $val != $loggedInContactID) { - return FALSE; - } - if ($op === 'IN' && $val != [$loggedInContactID]) { - return FALSE; - } - } - $this->addWhere('contact_id', '=', $loggedInContactID); - return TRUE; - } - -} diff --git a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Update.php b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Update.php deleted file mode 100644 index 954566a652..0000000000 --- a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/Update.php +++ /dev/null @@ -1,10 +0,0 @@ -setCheckPermissions($checkPermissions); - } - - public static function update($checkPermissions = TRUE) { - $action = new Action\OAuthContactToken\Update(static::class, __FUNCTION__); - return $action->setCheckPermissions($checkPermissions); - } - - public static function delete($checkPermissions = TRUE) { - $action = new Action\OAuthContactToken\Delete(static::class, __FUNCTION__); - return $action->setCheckPermissions($checkPermissions); - } - public static function permissions(): array { return [ 'meta' => ['access CiviCRM'], diff --git a/ext/oauth-client/tests/phpunit/Civi/OAuth/AuthCodeFlowTest.php b/ext/oauth-client/tests/phpunit/Civi/OAuth/AuthCodeFlowTest.php index 94579c9c25..5dd61ce826 100644 --- a/ext/oauth-client/tests/phpunit/Civi/OAuth/AuthCodeFlowTest.php +++ b/ext/oauth-client/tests/phpunit/Civi/OAuth/AuthCodeFlowTest.php @@ -218,7 +218,7 @@ class AuthCodeFlowTest extends \PHPUnit\Framework\TestCase implements $this->assertEquals(['foo'], $tokenRecord['scopes']); $this->assertEquals('example-access-token-value', $tokenRecord['access_token']); $this->assertEquals('example-refresh-token-value', $tokenRecord['refresh_token']); - $this->assertNull($tokenRecord['contact_id']); + $this->assertTrue(!isset($tokenRecord['contact_id'])); } public function testContactToken_AnonymousUser_CreateContact() { @@ -385,7 +385,7 @@ class AuthCodeFlowTest extends \PHPUnit\Framework\TestCase implements $this->assertEquals(['foo'], $tokenRecord['scopes']); $this->assertEquals('example-access-token-value', $tokenRecord['access_token']); $this->assertEquals('example-refresh-token-value', $tokenRecord['refresh_token']); - $this->assertNull($tokenRecord['contact_id']); + $this->assertTrue(!isset($tokenRecord['contact_id'])); } } diff --git a/ext/oauth-client/tests/phpunit/api/v4/OAuthContactTokenTest.php b/ext/oauth-client/tests/phpunit/api/v4/OAuthContactTokenTest.php index ed19b09222..360df66235 100644 --- a/ext/oauth-client/tests/phpunit/api/v4/OAuthContactTokenTest.php +++ b/ext/oauth-client/tests/phpunit/api/v4/OAuthContactTokenTest.php @@ -137,7 +137,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $strangerTokenCreateVals = $this->getTestTokenCreateValues( $client, $notLoggedInContactID, 'other'); - $this->usePerms(['manage all OAuth contact tokens']); + $this->usePerms(['manage all OAuth contact tokens', 'edit all contacts']); $createOtherContactToken = Civi\Api4\OAuthContactToken::create() ->setValues($strangerTokenCreateVals) ->execute(); @@ -149,7 +149,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $this->assertEquals($strangerTokenCreateVals['access_token'], $token['access_token']); $this->assertEquals($strangerTokenCreateVals['refresh_token'], $token['refresh_token']); - $this->usePerms(['manage my OAuth contact tokens']); + $this->usePerms(['manage my OAuth contact tokens', 'edit my contact']); $createOwnToken = Civi\Api4\OAuthContactToken::create() ->setValues($ownTokenCreateVals) ->execute(); @@ -161,7 +161,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $this->assertEquals($ownTokenCreateVals['access_token'], $token['access_token']); $this->assertEquals($ownTokenCreateVals['refresh_token'], $token['refresh_token']); - $this->usePerms(['manage my OAuth contact tokens']); + $this->usePerms(['manage my OAuth contact tokens', 'edit all contacts']); try { Civi\Api4\OAuthContactToken::create() ->setValues($strangerTokenCreateVals) @@ -217,7 +217,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $notLoggedInContactID ); - $this->usePerms(['manage all OAuth contact tokens', 'view all contacts']); + $this->usePerms(['manage all OAuth contact tokens', 'edit all contacts']); $updateTokensWithFullAccess = Civi\Api4\OAuthContactToken::update() ->addWhere('contact_id', '=', $notLoggedInContactID) ->setValues(['access_token' => 'stranger-token-revised']) @@ -226,7 +226,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $token = $updateTokensWithFullAccess->first(); $this->assertEquals($strangerContactToken['id'], $token['id']); - $this->usePerms(['manage my OAuth contact tokens', 'view my contact']); + $this->usePerms(['manage my OAuth contact tokens', 'edit my contact']); $updateTokensWithLimitedAccess = Civi\Api4\OAuthContactToken::update() ->addWhere('client_id.guid', '=', $client['guid']) ->setValues(['access_token' => 'own-token-revised']) @@ -235,7 +235,7 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $token = $updateTokensWithLimitedAccess->first(); $this->assertEquals($ownContactToken['id'], $token['id']); - $this->usePerms(['manage my OAuth contact tokens', 'view my contact']); + $this->usePerms(['manage my OAuth contact tokens', 'edit my contact']); $getUpdatedTokensWithLimitedAccess = Civi\Api4\OAuthContactToken::get() ->execute(); $this->assertCount(1, $getUpdatedTokensWithLimitedAccess); @@ -243,17 +243,12 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement $this->assertEquals($loggedInContactID, $token['contact_id']); $this->assertEquals("own-token-revised", $token['access_token']); - $this->usePerms(['manage my OAuth contact tokens', 'view my contact']); - try { - Civi\Api4\OAuthContactToken::update() - ->addWhere('contact_id', '=', $notLoggedInContactID) - ->setValues(['access_token' => "stranger-token-revised"]) - ->execute(); - $this->fail('Expected \Civi\API\Exception\UnauthorizedException but none was thrown'); - } - catch (\Civi\API\Exception\UnauthorizedException $e) { - // exception successfully thrown - } + $this->usePerms(['manage my OAuth contact tokens', 'view all contacts']); + $updates = Civi\Api4\OAuthContactToken::update() + ->addWhere('contact_id', '=', $notLoggedInContactID) + ->setValues(['access_token' => "stranger-token-revised"]) + ->execute(); + $this->assertCount(0, $updates, 'User should not have access to update'); $this->usePerms(['manage my OAuth contact tokens', 'view my contact']); $updateTokensForWrongContact = Civi\Api4\OAuthContactToken::update() @@ -269,30 +264,30 @@ class api_v4_OAuthContactTokenTest extends \PHPUnit\Framework\TestCase implement [$loggedInContactID, $notLoggedInContactID] = $this->createTestContactIDs(); $this->createOwnAndStrangerTokens($client, $loggedInContactID, $notLoggedInContactID); - $this->usePerms(['manage my OAuth contact tokens', 'view all contacts']); + $this->usePerms(['manage my OAuth contact tokens', 'edit all contacts']); $deleteTokensWithLimitedAccess = Civi\Api4\OAuthContactToken::delete() ->setWhere([['client_id.guid', '=', $client['guid']]]) ->execute(); - $this->usePerms(['manage my OAuth contact tokens', 'view all contacts']); + $this->usePerms(['manage my OAuth contact tokens', 'edit all contacts']); $getTokensWithLimitedAccess = Civi\Api4\OAuthContactToken::get()->execute(); $this->assertCount(0, $getTokensWithLimitedAccess); - $this->usePerms(['manage all OAuth contact tokens', 'view all contacts']); + $this->usePerms(['manage all OAuth contact tokens', 'edit all contacts']); $getTokensWithFullAccess = Civi\Api4\OAuthContactToken::get()->execute(); $this->assertCount(1, $getTokensWithFullAccess); $this->usePerms(['manage my OAuth contact tokens', 'view all contacts']); - $this->expectException(\Civi\API\Exception\UnauthorizedException::class); - Civi\Api4\OAuthContactToken::delete() + $deleted = Civi\Api4\OAuthContactToken::delete() ->addWhere('contact_id', '=', $notLoggedInContactID) ->execute(); + $this->assertCount(0, $deleted); } public function testGetByScope() { $client = $this->createClient(); - $this->usePerms(['manage all OAuth contact tokens', 'view all contacts']); + $this->usePerms(['manage all OAuth contact tokens', 'edit all contacts']); $tokenCreationVals = [ 'client_id' => $client['id'], 'contact_id' => 1, -- 2.25.1