From e435e457dec903fc6842690d40fbe331451d1c2e Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Wed, 12 Feb 2025 09:50:13 +0000 Subject: [PATCH 1/3] chore: added check for team membership exists --- app/config/errors.php | 7 ++++++- app/controllers/api/teams.php | 8 +++++--- src/Appwrite/Extend/Exception.php | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index 461521f5e0..f552b8898d 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -356,9 +356,14 @@ return [ ], Exception::TEAM_INVALID_SECRET => [ 'name' => Exception::TEAM_INVALID_SECRET, - 'description' => 'The team invitation secret is invalid. Please request a new invitation and try again.', + 'description' => 'The team invitation secret is invalid. Please request a new invitation and try again.', 'code' => 401, ], + Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS => [ + 'name' => Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS, + 'description' => 'Team membership already exists. Please check your existing memberships and try again.', + 'code' => 409, + ], Exception::TEAM_MEMBERSHIP_MISMATCH => [ 'name' => Exception::TEAM_MEMBERSHIP_MISMATCH, 'description' => 'The membership ID does not belong to the team ID.', diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 18faaeceeb..597424f861 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -588,9 +588,8 @@ App::post('/v1/teams/:teamId/memberships') Query::equal('teamInternalId', [$team->getInternalId()]), ]); + $secret = Auth::tokenGenerator(); if ($membership->isEmpty()) { - $secret = Auth::tokenGenerator(); - $membershipId = ID::unique(); $membership = new Document([ '$id' => $membershipId, @@ -618,7 +617,8 @@ App::post('/v1/teams/:teamId/memberships') $dbForProject->createDocument('memberships', $membership); Authorization::skip(fn () => $dbForProject->increaseDocumentAttribute('teams', $team->getId(), 'total', 1)); - } else { + } elseif ($membership->getAttribute('joined') === null) { + $membership->setAttribute('secret', Auth::hash($secret)); $membership->setAttribute('invited', DateTime::now()); if ($isPrivilegedUser || $isAppUser) { @@ -629,6 +629,8 @@ App::post('/v1/teams/:teamId/memberships') $membership = ($isPrivilegedUser || $isAppUser) ? Authorization::skip(fn () => $dbForProject->updateDocument('memberships', $membership->getId(), $membership)) : $dbForProject->updateDocument('memberships', $membership->getId(), $membership); + } else { + throw new Exception(Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS); } diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index d4f47ca177..7f75d8e97b 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -114,6 +114,7 @@ class Exception extends \Exception public const TEAM_NOT_FOUND = 'team_not_found'; public const TEAM_INVITE_NOT_FOUND = 'team_invite_not_found'; public const TEAM_INVALID_SECRET = 'team_invalid_secret'; + public const TEAM_MEMBERSHIP_ALREADY_EXISTS = 'team_membership_already_exists'; public const TEAM_MEMBERSHIP_MISMATCH = 'team_membership_mismatch'; public const TEAM_INVITE_MISMATCH = 'team_invite_mismatch'; public const TEAM_ALREADY_EXISTS = 'team_already_exists'; From d259f21c652a04f34cb849c2033056f2792f974c Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Wed, 12 Feb 2025 10:48:32 +0000 Subject: [PATCH 2/3] refactor: tests to match the new membership logic --- tests/e2e/Services/Teams/TeamsBaseClient.php | 9 +++++---- tests/e2e/Services/Teams/TeamsBaseServer.php | 19 ++++--------------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/tests/e2e/Services/Teams/TeamsBaseClient.php b/tests/e2e/Services/Teams/TeamsBaseClient.php index 3aad86c670..3fcd9c043d 100644 --- a/tests/e2e/Services/Teams/TeamsBaseClient.php +++ b/tests/e2e/Services/Teams/TeamsBaseClient.php @@ -226,10 +226,6 @@ trait TeamsBaseClient $this->assertEquals($response['body']['teamId'], substr($lastEmail['text'], strpos($lastEmail['text'], '&teamId=', 0) + 8, 20)); $this->assertEquals($teamName, substr($lastEmail['text'], strpos($lastEmail['text'], '&teamName=', 0) + 10, 7)); - $secret = substr($lastEmail['text'], strpos($lastEmail['text'], '&secret=', 0) + 8, 256); - $membershipUid = substr($lastEmail['text'], strpos($lastEmail['text'], '?membershipId=', 0) + 14, 20); - $userUid = substr($lastEmail['text'], strpos($lastEmail['text'], '&userId=', 0) + 8, 20); - /** * Test with UserId * Create user @@ -308,6 +304,11 @@ trait TeamsBaseClient $this->assertEquals(201, $response['headers']['status-code']); + $lastEmail = $this->getLastEmail(); + $membershipUid = substr($lastEmail['text'], strpos($lastEmail['text'], '?membershipId=', 0) + 14, 20); + $userUid = substr($lastEmail['text'], strpos($lastEmail['text'], '&userId=', 0) + 8, 20); + $secret = substr($lastEmail['text'], strpos($lastEmail['text'], '&secret=', 0) + 8, 256); + /** * Test for FAILURE */ diff --git a/tests/e2e/Services/Teams/TeamsBaseServer.php b/tests/e2e/Services/Teams/TeamsBaseServer.php index bade16cf2f..0c6d85e276 100644 --- a/tests/e2e/Services/Teams/TeamsBaseServer.php +++ b/tests/e2e/Services/Teams/TeamsBaseServer.php @@ -175,17 +175,10 @@ trait TeamsBaseServer $userUid = $response['body']['userId']; $membershipUid = $response['body']['$id']; - // $response = $this->client->call(Client::METHOD_GET, '/users/'.$userUid, array_merge([ - // 'content-type' => 'application/json', - // 'x-appwrite-project' => $this->getProject()['$id'], - // ], $this->getHeaders()), []); + /** + * Test for FAILURE + */ - // $this->assertEquals($userUid, $response['body']['$id']); - // $this->assertContains('team:'.$teamUid, $response['body']['roles']); - // $this->assertContains('team:'.$teamUid.'/admin', $response['body']['roles']); - // $this->assertContains('team:'.$teamUid.'/editor', $response['body']['roles']); - - // test for resending invitation $response = $this->client->call(Client::METHOD_POST, '/teams/' . $teamUid . '/memberships', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], @@ -196,11 +189,7 @@ trait TeamsBaseServer 'url' => 'http://localhost:5000/join-us#title' ]); - $this->assertEquals(201, $response['headers']['status-code']); - - /** - * Test for FAILURE - */ + $this->assertEquals(409, $response['headers']['status-code']); // membership already created $response = $this->client->call(Client::METHOD_POST, '/teams/' . $teamUid . '/memberships', array_merge([ 'content-type' => 'application/json', From 2457485230975c8b6e39097ff86f0a1e0513cf34 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Thu, 13 Feb 2025 13:16:29 +0000 Subject: [PATCH 3/3] chore: removed redundant exception --- app/config/errors.php | 5 ----- app/controllers/api/teams.php | 5 ++--- src/Appwrite/Extend/Exception.php | 1 - 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index f552b8898d..7c7f6dc9ec 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -359,11 +359,6 @@ return [ 'description' => 'The team invitation secret is invalid. Please request a new invitation and try again.', 'code' => 401, ], - Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS => [ - 'name' => Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS, - 'description' => 'Team membership already exists. Please check your existing memberships and try again.', - 'code' => 409, - ], Exception::TEAM_MEMBERSHIP_MISMATCH => [ 'name' => Exception::TEAM_MEMBERSHIP_MISMATCH, 'description' => 'The membership ID does not belong to the team ID.', diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 597424f861..06e653c105 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -617,7 +617,7 @@ App::post('/v1/teams/:teamId/memberships') $dbForProject->createDocument('memberships', $membership); Authorization::skip(fn () => $dbForProject->increaseDocumentAttribute('teams', $team->getId(), 'total', 1)); - } elseif ($membership->getAttribute('joined') === null) { + } elseif ($membership->getAttribute('confirm') === false) { $membership->setAttribute('secret', Auth::hash($secret)); $membership->setAttribute('invited', DateTime::now()); @@ -630,10 +630,9 @@ App::post('/v1/teams/:teamId/memberships') Authorization::skip(fn () => $dbForProject->updateDocument('memberships', $membership->getId(), $membership)) : $dbForProject->updateDocument('memberships', $membership->getId(), $membership); } else { - throw new Exception(Exception::TEAM_MEMBERSHIP_ALREADY_EXISTS); + throw new Exception(Exception::MEMBERSHIP_ALREADY_CONFIRMED); } - if ($isPrivilegedUser || $isAppUser) { $dbForProject->purgeCachedDocument('users', $invitee->getId()); } else { diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 7f75d8e97b..d4f47ca177 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -114,7 +114,6 @@ class Exception extends \Exception public const TEAM_NOT_FOUND = 'team_not_found'; public const TEAM_INVITE_NOT_FOUND = 'team_invite_not_found'; public const TEAM_INVALID_SECRET = 'team_invalid_secret'; - public const TEAM_MEMBERSHIP_ALREADY_EXISTS = 'team_membership_already_exists'; public const TEAM_MEMBERSHIP_MISMATCH = 'team_membership_mismatch'; public const TEAM_INVITE_MISMATCH = 'team_invite_mismatch'; public const TEAM_ALREADY_EXISTS = 'team_already_exists';