Skip to content

Commit

Permalink
Fix for relying on entity queries to check access (#854)
Browse files Browse the repository at this point in the history
  • Loading branch information
divya-intelli authored Jun 15, 2023
1 parent 760e99e commit 386c83f
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ public function teamAppKeys($team, $app): JsonResponse {
$payload = [];
if ($team) {
$app_storage = $this->entityTypeManager->getStorage('team_app');
// Lists all the team apps ids.
// Team app is accessible to all the team members.
$app_ids = $app_storage->getQuery()
->accessCheck(FALSE)
->condition('companyName', $team->id())
->condition('name', $app->getName())
->execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ protected function invokeHook($hook, EntityInterface $entity) {
* {@inheritdoc}
*/
public function loadByRecipient(string $email, ?string $team_id = NULL): array {
$query = $this->getQuery()->condition('recipient', $email);
$query = $this->getQuery()->accessCheck(TRUE)->condition('recipient', $email);

if ($team_id) {
$query->condition('team', $team_id);
Expand All @@ -161,7 +161,8 @@ public function loadByRecipient(string $email, ?string $team_id = NULL): array {
* {@inheritdoc}
*/
public function getInvitationsToExpire(): array {
$query = $this->getQuery()->condition('expiry', $this->time->getCurrentTime(), '<')
// Team invitation is accessable as we need to update status in cron run.
$query = $this->getQuery()->accessCheck(FALSE)->condition('expiry', $this->time->getCurrentTime(), '<')
->condition('status', TeamInvitationInterface::STATUS_PENDING);

$ids = $query->execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ public function convert($value, $definition, $name, array $defaults) {
$team = is_object($defaults['team']) ? $defaults['team'] : $this->entityTypeManager->getStorage('team')->load($defaults['team']);
if ($team) {
$app_storage = $this->entityTypeManager->getStorage('team_app');
// Lists all the team apps ids.
// Team app is accessible to all the team members.
$app_ids = $app_storage->getQuery()
->accessCheck(FALSE)
->condition('companyName', $team->id())
->condition('name', $value)
->execute();
Expand Down
3 changes: 3 additions & 0 deletions src/Controller/DeveloperAppKeysController.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ public function developerAppKeys($user, $app): JsonResponse {
if ($user) {
if ($developer_id = $user->get('apigee_edge_developer_id')->value) {
$app_storage = $this->entityTypeManager->getStorage('developer_app');
// Lists all the developer apps ids for a particular
// developer email id and app name.
$app_ids = $app_storage->getQuery()
->accessCheck(FALSE)
->condition('developerId', $developer_id)
->condition('name', $app->getName())
->execute();
Expand Down
4 changes: 3 additions & 1 deletion src/Entity/Storage/DeveloperAppStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ protected function entityController(): EdgeEntityControllerInterface {
* {@inheritdoc}
*/
public function loadByDeveloper(string $developer_id): array {
$query = $this->getQuery();
// Lists all the developer apps ids for a particular
// developer email id and app name.
$query = $this->getQuery()->accessCheck(FALSE);
// We have to figure out whether this is an email or a UUID to call the
// best API endpoint that is possible.
if ($this->emailValidator->isValid($developer_id)) {
Expand Down
3 changes: 3 additions & 0 deletions src/ParamConverter/DeveloperAppNameConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ public function convert($value, $definition, $name, array $defaults) {
$developer_id = $user->get('apigee_edge_developer_id')->value;
if ($developer_id) {
$app_storage = $this->entityTypeManager->getStorage('developer_app');
// Lists all the developer apps ids for a particular
// developer email id and app name.
$app_ids = $app_storage->getQuery()
->accessCheck(FALSE)
->condition('developerId', $developer_id)
->condition('name', $value)
->execute();
Expand Down
2 changes: 2 additions & 0 deletions tests/src/Functional/DeveloperAppUITestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ protected function assertAppCrud(?callable $beforeCreate = NULL, ?callable $afte
$storage = \Drupal::entityTypeManager()->getStorage('developer_app');
/** @var \Drupal\apigee_edge\Entity\DeveloperApp $app */
$app = $storage->load(array_values($storage->getQuery()
->accessCheck(TRUE)
->condition('developerId', $developer->uuid())
->condition('name', $name)
->execute())[0]);
Expand Down Expand Up @@ -348,6 +349,7 @@ protected function loadDeveloperApp(string $name, Developer $developer = NULL):
$storage = \Drupal::entityTypeManager()->getStorage('developer_app');
$results_ids = $storage
->getQuery()
->accessCheck(FALSE)
->condition('developerId', $developer->uuid())
->condition('name', $name)
->execute();
Expand Down
10 changes: 10 additions & 0 deletions tests/src/Functional/QueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public function testQueries() {
*/
protected function developerQueryTest() {
$result = $this->developerStorage->getQuery()
->accessCheck(FALSE)
->condition('email', "{$this->prefix}.test", 'STARTS_WITH')
->condition('email', '@example.com', 'ENDS_WITH')
->sort('lastName')
Expand All @@ -145,6 +146,7 @@ protected function developerQueryTest() {
]), array_values($result));

$result = $this->developerStorage->getQuery()
->accessCheck(FALSE)
->condition('email', "{$this->prefix}.test", 'STARTS_WITH')
->condition('email', '@example.com', 'ENDS_WITH')
->sort('email')
Expand All @@ -153,6 +155,7 @@ protected function developerQueryTest() {
$this->assertEquals(array_values(["{$this->prefix}[email protected]"]), array_values($result));

$result = $this->developerStorage->getQuery()
->accessCheck(FALSE)
->condition('email', "{$this->prefix}.test", 'STARTS_WITH')
->condition('email', '@example.com', 'ENDS_WITH')
->count()
Expand All @@ -177,19 +180,22 @@ protected function smartQueryTest() {
// When primary id(s) of entities is set to something empty we should
// get back an empty result.
$result = $this->developerStorage->getQuery()
->accessCheck(FALSE)
->condition('email', NULL)
->count()
->execute();
$this->assertEquals(0, $result);

$result = $this->developerStorage->getQuery()
->accessCheck(FALSE)
->condition('developerId', NULL)
->count()
->execute();
$this->assertEquals(0, $result);

$developer = reset($this->edgeDevelopers);
$result = $this->developerAppStorage->getQuery()
->accessCheck(FALSE)
->condition('developerId', $developer->getDeveloperId())
->count()
->execute();
Expand All @@ -199,12 +205,14 @@ protected function smartQueryTest() {
// Edge by calling the proper API endpoint - is set to something empty
// we should get back an empty result.
$result = $this->developerAppStorage->getQuery()
->accessCheck(FALSE)
->condition('developerId', NULL)
->count()
->execute();
$this->assertEquals(0, $result);

$result = $this->developerAppStorage->getQuery()
->accessCheck(FALSE)
->condition('email', $developer->getEmail())
->count()
->execute();
Expand All @@ -214,6 +222,7 @@ protected function smartQueryTest() {
// Edge by calling the proper API endpoint - is set to something empty
// we should get back an empty result.
$result = $this->developerAppStorage->getQuery()
->accessCheck(FALSE)
->condition('email', NULL)
->count()
->execute();
Expand All @@ -222,6 +231,7 @@ protected function smartQueryTest() {
// If app name is set to something empty then query should not fail and
// we should get back an empty list even if the developer has apps.
$result = $this->developerAppStorage->getQuery()
->accessCheck(FALSE)
->condition('email', $developer->getEmail())
->condition('name', NULL)
->count()
Expand Down

0 comments on commit 386c83f

Please sign in to comment.