From 2a672f5a633b98f1bcecc73d3addbfc78eecc4b5 Mon Sep 17 00:00:00 2001 From: Darshan Date: Sat, 5 Apr 2025 19:20:24 +0530 Subject: [PATCH 1/4] fix: don't allow the only owner in an org to change their role to anything else. --- app/controllers/api/teams.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index dfdccf5e99..d4db6a4a52 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -1034,7 +1034,6 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId') ->param('membershipId', '', new UID(), 'Membership ID.') ->param('roles', [], function (Document $project) { if ($project->getId() === 'console') { - ; $roles = array_keys(Config::getParam('roles', [])); array_filter($roles, function ($role) { return !in_array($role, [Auth::USER_ROLE_APPS, Auth::USER_ROLE_GUESTS, Auth::USER_ROLE_USERS]); @@ -1046,9 +1045,10 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId') ->inject('request') ->inject('response') ->inject('user') + ->inject('project') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $teamId, string $membershipId, array $roles, Request $request, Response $response, Document $user, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $teamId, string $membershipId, array $roles, Request $request, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents) { $team = $dbForProject->getDocument('teams', $teamId); if ($team->isEmpty()) { @@ -1069,6 +1069,21 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId') $isAppUser = Auth::isAppUser(Authorization::getRoles()); $isOwner = Authorization::isRole('team:' . $team->getId() . '/owner'); + if ($project->getId() === 'console') { + // Quick check: fetch up to 2 owners to determine if only one exists + $ownersCount = $dbForProject->count( + collection: 'memberships', + queries: [Query::contains('roles', ['owner'])], + max: 2 + ); + + // If there's only one owner, + // and the requester is that owner, prevent role change + if ($ownersCount === 1 && $isOwner) { + throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'There must be at least one owner in the organization.'); + } + } + if (!$isOwner && !$isPrivilegedUser && !$isAppUser) { // Not owner, not admin, not app (server) throw new Exception(Exception::USER_UNAUTHORIZED, 'User is not allowed to modify roles'); } From 7297188c1bf0dd67229afc2ef30ad527cc76fecc Mon Sep 17 00:00:00 2001 From: Darshan Date: Sat, 5 Apr 2025 19:42:20 +0530 Subject: [PATCH 2/4] update: roles check. --- app/controllers/api/teams.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index d4db6a4a52..3e0e366b6b 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -1077,9 +1077,9 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId') max: 2 ); - // If there's only one owner, - // and the requester is that owner, prevent role change - if ($ownersCount === 1 && $isOwner) { + // Prevent role change if there's only one owner left, + // the requester is that owner, and the new `$roles` no longer include 'owner'! + if ($ownersCount === 1 && $isOwner && !\in_array('owner', $roles)) { throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'There must be at least one owner in the organization.'); } } From c3c568653fd9f4a5693e5b06a8c4d6c342e30fc6 Mon Sep 17 00:00:00 2001 From: Darshan Date: Sun, 6 Apr 2025 10:21:55 +0530 Subject: [PATCH 3/4] add: roles check test. --- tests/e2e/Services/Teams/TeamsBase.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/e2e/Services/Teams/TeamsBase.php b/tests/e2e/Services/Teams/TeamsBase.php index 2328e4cdbf..87486696d2 100644 --- a/tests/e2e/Services/Teams/TeamsBase.php +++ b/tests/e2e/Services/Teams/TeamsBase.php @@ -37,6 +37,31 @@ trait TeamsBase $teamUid = $response1['body']['$id']; $teamName = $response1['body']['name']; + /** + * Test: Attempt to downgrade the only OWNER in a team (should fail) + */ + // Step 1: Fetch all team memberships — only one exists at this point + $response = $this->client->call(Client::METHOD_GET, '/teams/' . $teamUid . '/memberships', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + // Step 2: Extract the membership ID of the only member (also the only OWNER) + $membershipID = $response['body']['memberships'][0]['$id']; + + // Step 3: Attempt to downgrade the member's role to 'developer' + $response = $this->client->call(Client::METHOD_PATCH, '/teams/' . $teamUid . '/memberships/' . $membershipID, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'roles' => ['developer'] + ]); + + // Step 4: Assert failure — cannot remove the only OWNER from a team + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('general_argument_invalid', $response['body']['type']); + $this->assertEquals('There must be at least one owner in the organization.', $response['body']['message']); + $teamId = ID::unique(); $response2 = $this->client->call(Client::METHOD_POST, '/teams', array_merge([ 'content-type' => 'application/json', From 2243f0c72e45e5a8abfe716bb2b8470375a07f6b Mon Sep 17 00:00:00 2001 From: Darshan Date: Sun, 6 Apr 2025 10:29:18 +0530 Subject: [PATCH 4/4] fix: test for only console db <> orgs. --- tests/e2e/Services/Teams/TeamsBase.php | 44 +++++++++++++++----------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/tests/e2e/Services/Teams/TeamsBase.php b/tests/e2e/Services/Teams/TeamsBase.php index 87486696d2..80ac1621ee 100644 --- a/tests/e2e/Services/Teams/TeamsBase.php +++ b/tests/e2e/Services/Teams/TeamsBase.php @@ -38,29 +38,35 @@ trait TeamsBase $teamName = $response1['body']['name']; /** - * Test: Attempt to downgrade the only OWNER in a team (should fail) + * Test: Attempt to downgrade the only OWNER in an organization (should fail) */ - // Step 1: Fetch all team memberships — only one exists at this point - $response = $this->client->call(Client::METHOD_GET, '/teams/' . $teamUid . '/memberships', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders())); + if ($this->getProject()['$id'] === 'console') { + // Step 1: Fetch all team memberships — only one exists at this point + $response = $this->client->call(Client::METHOD_GET, '/teams/' . $teamUid . '/memberships', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'queries' => [ + Query::limit(1)->toString(), + ], + ]); - // Step 2: Extract the membership ID of the only member (also the only OWNER) - $membershipID = $response['body']['memberships'][0]['$id']; + // Step 2: Extract the membership ID of the only member (also the only OWNER) + $membershipID = $response['body']['memberships'][0]['$id']; - // Step 3: Attempt to downgrade the member's role to 'developer' - $response = $this->client->call(Client::METHOD_PATCH, '/teams/' . $teamUid . '/memberships/' . $membershipID, array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'roles' => ['developer'] - ]); + // Step 3: Attempt to downgrade the member's role to 'developer' + $response = $this->client->call(Client::METHOD_PATCH, '/teams/' . $teamUid . '/memberships/' . $membershipID, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'roles' => ['developer'] + ]); - // Step 4: Assert failure — cannot remove the only OWNER from a team - $this->assertEquals(400, $response['headers']['status-code']); - $this->assertEquals('general_argument_invalid', $response['body']['type']); - $this->assertEquals('There must be at least one owner in the organization.', $response['body']['message']); + // Step 4: Assert failure — cannot remove the only OWNER from a team + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('general_argument_invalid', $response['body']['type']); + $this->assertEquals('There must be at least one owner in the organization.', $response['body']['message']); + } $teamId = ID::unique(); $response2 = $this->client->call(Client::METHOD_POST, '/teams', array_merge([