From 0d60e826667f53a0033449df6b78b28b7a150ad9 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 10 Dec 2021 11:56:11 +0100 Subject: [PATCH 1/3] fix(database): permissions using an admin user --- app/controllers/api/database.php | 38 ++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/database.php b/app/controllers/api/database.php index d60d055912..8c4f2d20be 100644 --- a/app/controllers/api/database.php +++ b/app/controllers/api/database.php @@ -1640,16 +1640,19 @@ App::post('/v1/database/collections/:collectionId/documents') $data['$read'] = (is_null($read) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $read ?? []; // By default set read permissions for user $data['$write'] = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; // By default set write permissions for user - // Users can only add their roles to documents, API keys can add any + // Users can only add their roles to documents, API keys and Admin users can add any $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array - foreach ($data['$read'] as $read) { - if (!Auth::isAppUser($roles) && !Authorization::isRole($read)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); + + if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { + foreach ($data['$read'] as $read) { + if (!Authorization::isRole($read)) { + throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } } - } - foreach ($data['$write'] as $write) { - if (!Auth::isAppUser($roles) && !Authorization::isRole($write)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); + foreach ($data['$write'] as $write) { + if (!Authorization::isRole($write)) { + throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } } } @@ -1998,16 +2001,19 @@ App::patch('/v1/database/collections/:collectionId/documents/:documentId') $data['$read'] = (is_null($read)) ? ($document->getRead() ?? []) : $read; // By default inherit read permissions $data['$write'] = (is_null($write)) ? ($document->getWrite() ?? []) : $write; // By default inherit write permissions - // Users can only add their roles to documents, API keys can add any + // Users can only add their roles to documents, API keys and Admin users can add any $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array - foreach ($data['$read'] as $read) { - if (!Auth::isAppUser($roles) && !Authorization::isRole($read)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); + + if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { + foreach ($data['$read'] as $read) { + if (!Authorization::isRole($read)) { + throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } } - } - foreach ($data['$write'] as $write) { - if (!Auth::isAppUser($roles) && !Authorization::isRole($write)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); + foreach ($data['$write'] as $write) { + if (!Authorization::isRole($write)) { + throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } } } From b530fd66cd4f91ef04b8df4df43789077783a6d2 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 10 Dec 2021 17:48:54 +0100 Subject: [PATCH 2/3] fix(storage): add same permissions check to files uploaded --- app/controllers/api/storage.php | 43 ++++++- .../Storage/StorageCustomClientTest.php | 111 +++++++++++++++++- 2 files changed, 152 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 732e061c3f..0522826af4 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -1,5 +1,6 @@ isEmpty()) ? ['user:'.$user->getId()] : $read ?? []; // By default set read permissions for user + $write = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; + + // Users can only add their roles to files, API keys and Admin users can add any + $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + + if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { + foreach ($read as $role) { + if (!Authorization::isRole($role)) { + throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } + } + foreach ($write as $role) { + if (!Authorization::isRole($role)) { + throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } + } + } + $file = $request->getFiles('file'); /* @@ -563,13 +583,34 @@ App::put('/v1/storage/files/:fileId') ->param('write', [], new ArrayList(new Text(64)), 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->inject('response') ->inject('dbForInternal') + ->inject('user') ->inject('audits') ->inject('usage') - ->action(function ($fileId, $read, $write, $response, $dbForInternal, $audits, $usage) { + ->action(function ($fileId, $read, $write, $response, $dbForInternal, $user, $audits, $usage) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Database $dbForInternal */ + /** @var Utopia\Database\Document $user */ /** @var Appwrite\Event\Event $audits */ + $read = (is_null($read) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $read ?? []; // By default set read permissions for user + $write = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; + + // Users can only add their roles to files, API keys and Admin users can add any + $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + + if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { + foreach ($read as $role) { + if (!Authorization::isRole($role)) { + throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } + } + foreach ($write as $role) { + if (!Authorization::isRole($role)) { + throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + } + } + } + $file = $dbForInternal->getDocument('files', $fileId); if (empty($file->getId())) { diff --git a/tests/e2e/Services/Storage/StorageCustomClientTest.php b/tests/e2e/Services/Storage/StorageCustomClientTest.php index 2a0ead59f6..a0dee63156 100644 --- a/tests/e2e/Services/Storage/StorageCustomClientTest.php +++ b/tests/e2e/Services/Storage/StorageCustomClientTest.php @@ -3,6 +3,9 @@ namespace Tests\E2E\Services\Storage; use CURLFile; +use Exception; +use SebastianBergmann\RecursionContext\InvalidArgumentException; +use PHPUnit\Framework\ExpectationFailedException; use Tests\E2E\Client; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\ProjectCustom; @@ -14,7 +17,7 @@ class StorageCustomClientTest extends Scope use ProjectCustom; use SideClient; - public function testCreateFileDefaultPermissions():void + public function testCreateFileDefaultPermissions(): array { /** * Test for SUCCESS @@ -36,5 +39,111 @@ class StorageCustomClientTest extends Scope $this->assertEquals('permissions.png', $file['body']['name']); $this->assertEquals('image/png', $file['body']['mimeType']); $this->assertEquals(47218, $file['body']['sizeOriginal']); + + return $file['body']; + } + + public function testCreateFileAbusePermissions(): void + { + /** + * Test for FAILURE + */ + $file = $this->client->call(Client::METHOD_POST, '/storage/files', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'fileId' => 'unique()', + 'file' => new CURLFile(realpath(__DIR__ . '/../../../resources/logo.png'), 'image/png', 'permissions.png'), + 'folderId' => 'xyz', + 'read' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Read permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); + + $file = $this->client->call(Client::METHOD_POST, '/storage/files', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'fileId' => 'unique()', + 'file' => new CURLFile(realpath(__DIR__ . '/../../../resources/logo.png'), 'image/png', 'permissions.png'), + 'folderId' => 'xyz', + 'write' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Write permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); + + $file = $this->client->call(Client::METHOD_POST, '/storage/files', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'fileId' => 'unique()', + 'file' => new CURLFile(realpath(__DIR__ . '/../../../resources/logo.png'), 'image/png', 'permissions.png'), + 'folderId' => 'xyz', + 'read' => ['user:notme'], + 'write' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Read permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); + } + + /** + * @depends testCreateFileDefaultPermissions + */ + public function testUpdateFileAbusePermissions(array $data): void + { + /** + * Test for FAILURE + */ + $file = $this->client->call(Client::METHOD_PUT, '/storage/files/' . $data['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'read' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Read permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); + + $file = $this->client->call(Client::METHOD_PUT, '/storage/files/' . $data['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'write' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Write permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); + + $file = $this->client->call(Client::METHOD_PUT, '/storage/files/' . $data['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'read' => ['user:notme'], + 'write' => ['user:notme'] + ]); + + $this->assertEquals($file['headers']['status-code'], 400); + $this->assertStringStartsWith('Read permissions must be one of:', $file['body']['message']); + $this->assertStringContainsString('role:all', $file['body']['message']); + $this->assertStringContainsString('role:member', $file['body']['message']); + $this->assertStringContainsString('user:'.$this->getUser()['$id'], $file['body']['message']); } } \ No newline at end of file From aef6c113700e22d321267bbaf1c135e8237b933f Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 10 Dec 2021 18:52:33 +0100 Subject: [PATCH 3/3] fix(auth): use getRoles instead of static property --- app/controllers/api/account.php | 17 +++++++----- app/controllers/api/database.php | 12 ++++----- app/controllers/api/storage.php | 12 ++++----- app/controllers/api/teams.php | 12 ++++----- app/controllers/general.php | 2 +- app/controllers/shared/api.php | 9 ++++--- src/Appwrite/Auth/Auth.php | 10 ++++---- tests/unit/Auth/AuthTest.php | 44 ++++++++++++++++---------------- 8 files changed, 61 insertions(+), 57 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 5c86755813..d2e68b1144 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -648,8 +648,9 @@ App::post('/v1/account/sessions/magic-url') throw new Exception('SMTP Disabled', 503); } - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); $user = $dbForInternal->findOne('users', [new Query('email', Query::TYPE_EQUAL, [$email])]); @@ -1780,8 +1781,9 @@ App::post('/v1/account/recovery') throw new Exception('SMTP Disabled', 503); } - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); $email = \strtolower($email); $profile = $dbForInternal->findOne('users', [new Query('deleted', Query::TYPE_EQUAL, [false]), new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address @@ -1971,9 +1973,10 @@ App::post('/v1/account/verification') if(empty(App::getEnv('_APP_SMTP_HOST'))) { throw new Exception('SMTP Disabled', 503); } - - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); $verificationSecret = Auth::tokenGenerator(); diff --git a/app/controllers/api/database.php b/app/controllers/api/database.php index 8c4f2d20be..3759c63ad2 100644 --- a/app/controllers/api/database.php +++ b/app/controllers/api/database.php @@ -1641,17 +1641,17 @@ App::post('/v1/database/collections/:collectionId/documents') $data['$write'] = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; // By default set write permissions for user // Users can only add their roles to documents, API keys and Admin users can add any - $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { foreach ($data['$read'] as $read) { if (!Authorization::isRole($read)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); } } foreach ($data['$write'] as $write) { if (!Authorization::isRole($write)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); } } } @@ -2002,17 +2002,17 @@ App::patch('/v1/database/collections/:collectionId/documents/:documentId') $data['$write'] = (is_null($write)) ? ($document->getWrite() ?? []) : $write; // By default inherit write permissions // Users can only add their roles to documents, API keys and Admin users can add any - $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { foreach ($data['$read'] as $read) { if (!Authorization::isRole($read)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); } } foreach ($data['$write'] as $write) { if (!Authorization::isRole($write)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); } } } diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index 0522826af4..3e095f4d01 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -63,17 +63,17 @@ App::post('/v1/storage/files') $write = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; // Users can only add their roles to files, API keys and Admin users can add any - $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { foreach ($read as $role) { if (!Authorization::isRole($role)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); } } foreach ($write as $role) { if (!Authorization::isRole($role)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); } } } @@ -596,17 +596,17 @@ App::put('/v1/storage/files/:fileId') $write = (is_null($write) && !$user->isEmpty()) ? ['user:'.$user->getId()] : $write ?? []; // Users can only add their roles to files, API keys and Admin users can add any - $roles = \array_fill_keys(Authorization::getRoles(), true); // Auth::isAppUser expects roles to be keys, not values of assoc array + $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { foreach ($read as $role) { if (!Authorization::isRole($role)) { - throw new Exception('Read permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Read permissions must be one of: ('.\implode(', ', $roles).')', 400); } } foreach ($write as $role) { if (!Authorization::isRole($role)) { - throw new Exception('Write permissions must be one of: ('.\implode(', ', array_keys($roles)).')', 400); + throw new Exception('Write permissions must be one of: ('.\implode(', ', $roles).')', 400); } } } diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 5d8e1781b7..5b4476fb09 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -49,8 +49,8 @@ App::post('/v1/teams') Authorization::disable(); - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); + $isAppUser = Auth::isAppUser(Authorization::getRoles()); $teamId = $teamId == 'unique()' ? $dbForInternal->getId() : $teamId; $team = $dbForInternal->createDocument('teams', new Document([ @@ -293,8 +293,8 @@ App::post('/v1/teams/:teamId/memberships') throw new Exception('SMTP Disabled', 503); } - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); + $isAppUser = Auth::isAppUser(Authorization::getRoles()); $email = \strtolower($email); $name = (empty($name)) ? $email : $name; @@ -566,8 +566,8 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId') throw new Exception('User not found', 404); } - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); + $isAppUser = Auth::isAppUser(Authorization::getRoles()); $isOwner = Authorization::isRole('team:'.$team->getId().'/owner');; if (!$isOwner && !$isPrivilegedUser && !$isAppUser) { // Not owner, not admin, not app (server) diff --git a/app/controllers/general.php b/app/controllers/general.php index 4d2c6b7135..e75b2f2164 100644 --- a/app/controllers/general.php +++ b/app/controllers/general.php @@ -254,7 +254,7 @@ App::init(function ($utopia, $request, $response, $console, $project, $dbForCons if(!empty($service)) { if(array_key_exists($service, $project->getAttribute('services',[])) && !$project->getAttribute('services',[])[$service] - && !Auth::isPrivilegedUser(Authorization::$roles)) { + && !Auth::isPrivilegedUser(Authorization::getRoles())) { throw new Exception('Service is disabled', 503); } } diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 210cb46912..79f476c133 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -64,8 +64,9 @@ App::init(function ($utopia, $request, $response, $project, $user, $events, $aud ; } - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $roles = Authorization::getRoles(); + $isPrivilegedUser = Auth::isPrivilegedUser($roles); + $isAppUser = Auth::isAppUser($roles); if (($abuse->check() // Route is rate-limited && App::getEnv('_APP_OPTIONS_ABUSE', 'enabled') !== 'disabled') // Abuse is not disabled @@ -128,8 +129,8 @@ App::init(function ($utopia, $request, $project) { $route = $utopia->match($request); - $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::$roles); - $isAppUser = Auth::isAppUser(Authorization::$roles); + $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); + $isAppUser = Auth::isAppUser(Authorization::getRoles()); if($isAppUser || $isPrivilegedUser) { // Skip limits for app and console devs return; diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 2895b3db11..8ac5839ee0 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -242,9 +242,9 @@ class Auth public static function isPrivilegedUser(array $roles): bool { if ( - array_key_exists('role:'.self::USER_ROLE_OWNER, $roles) || - array_key_exists('role:'.self::USER_ROLE_DEVELOPER, $roles) || - array_key_exists('role:'.self::USER_ROLE_ADMIN, $roles) + in_array('role:'.self::USER_ROLE_OWNER, $roles) || + in_array('role:'.self::USER_ROLE_DEVELOPER, $roles) || + in_array('role:'.self::USER_ROLE_ADMIN, $roles) ) { return true; } @@ -261,7 +261,7 @@ class Auth */ public static function isAppUser(array $roles): bool { - if (array_key_exists('role:'.self::USER_ROLE_APP, $roles)) { + if (in_array('role:'.self::USER_ROLE_APP, $roles)) { return true; } @@ -278,7 +278,7 @@ class Auth { $roles = []; - if (!self::isPrivilegedUser(Authorization::$roles) && !self::isAppUser(Authorization::$roles)) { + if (!self::isPrivilegedUser(Authorization::getRoles()) && !self::isAppUser(Authorization::getRoles())) { if ($user->getId()) { $roles[] = 'user:'.$user->getId(); $roles[] = 'role:'.Auth::USER_ROLE_MEMBER; diff --git a/tests/unit/Auth/AuthTest.php b/tests/unit/Auth/AuthTest.php index c3eb711155..c3b9d00758 100644 --- a/tests/unit/Auth/AuthTest.php +++ b/tests/unit/Auth/AuthTest.php @@ -172,35 +172,35 @@ class AuthTest extends TestCase public function testIsPrivilegedUser() { $this->assertEquals(false, Auth::isPrivilegedUser([])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_MEMBER => true])); - $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_ADMIN => true])); - $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_DEVELOPER => true])); - $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER => true])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP => true])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_SYSTEM => true])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_MEMBER])); + $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_ADMIN])); + $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_DEVELOPER])); + $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_SYSTEM])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP => true, 'role:'.Auth::USER_ROLE_APP => true])); - $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP => true, 'role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER => true, 'role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER => true, 'role:'.Auth::USER_ROLE_ADMIN => true, 'role:'.Auth::USER_ROLE_DEVELOPER => true])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP, 'role:'.Auth::USER_ROLE_APP])); + $this->assertEquals(false, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_APP, 'role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER, 'role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(true, Auth::isPrivilegedUser(['role:'.Auth::USER_ROLE_OWNER, 'role:'.Auth::USER_ROLE_ADMIN, 'role:'.Auth::USER_ROLE_DEVELOPER])); } public function testIsAppUser() { $this->assertEquals(false, Auth::isAppUser([])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_MEMBER => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_ADMIN => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_DEVELOPER => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER => true])); - $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_SYSTEM => true])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_MEMBER])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_ADMIN])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_DEVELOPER])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER])); + $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_SYSTEM])); - $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP => true, 'role:'.Auth::USER_ROLE_APP => true])); - $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP => true, 'role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER => true, 'role:'.Auth::USER_ROLE_GUEST => true])); - $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER => true, 'role:'.Auth::USER_ROLE_ADMIN => true, 'role:'.Auth::USER_ROLE_DEVELOPER => true])); + $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP, 'role:'.Auth::USER_ROLE_APP])); + $this->assertEquals(true, Auth::isAppUser(['role:'.Auth::USER_ROLE_APP, 'role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER, 'role:'.Auth::USER_ROLE_GUEST])); + $this->assertEquals(false, Auth::isAppUser(['role:'.Auth::USER_ROLE_OWNER, 'role:'.Auth::USER_ROLE_ADMIN, 'role:'.Auth::USER_ROLE_DEVELOPER])); } public function testGuestRoles()