Merge pull request #13786 from appwrite/fix/membership-privacy-self · appwrite/appwrite@8aca509 · GitHub
Skip to content

Commit 8aca509

Browse files
HarshMN2345HarshMN2345
authored
Merge pull request #13786 from appwrite/fix/membership-privacy-self
fix(teams): exempt a member's own membership from the privacy policy
2 parents 85f75ba + 5141f67 commit 8aca509

4 files changed

Lines changed: 128 additions & 42 deletions

File tree

‎src/Appwrite/Platform/Modules/Teams/Http/Memberships/Get.php‎

Lines changed: 6 additions & 2 deletions

‎src/Appwrite/Platform/Modules/Teams/Http/Memberships/XList.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,14 @@ public function action(string $teamId, array $queries, string $search, bool $inc
141141
return $privacy || $isPrivilegedUser || $isAppUser;
142142
}, $membershipsPrivacy);
143143

144-
$memberships = array_map(function ($membership) use ($dbForProject, $team, $membershipsPrivacy) {
144+
// The policy only hides other members, a member always sees their own details
145+
$selfPrivacy = array_fill_keys(array_keys($membershipsPrivacy), true);
146+
$userSequence = $user->getSequence();
147+
148+
$memberships = array_map(function ($membership) use ($dbForProject, $team, $membershipsPrivacy, $selfPrivacy, $userSequence) {
149+
$isSelf = $userSequence !== null && $membership->getAttribute('userInternalId') === $userSequence;
150+
$membershipsPrivacy = $isSelf ? $selfPrivacy : $membershipsPrivacy;
151+
145152
$memberUser = !empty(array_filter($membershipsPrivacy))
146153
? $dbForProject->getDocument('users', $membership->getAttribute('userId'))
147154
: new Document();

‎tests/e2e/Services/Project/PoliciesMembershipPrivacyIntegrationTest.php‎

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -144,22 +144,32 @@ public function testMembershipPrivacyIntegration(): void
144144
'cookie' => 'a_session_' . $projectId . '=' . $user2Session,
145145
];
146146

147-
// Make a request as user2 to ensure accessedAt is updated
147+
// Make a request as each user to ensure accessedAt is updated
148148
$this->client->call(Client::METHOD_GET, '/account', $client2Headers);
149+
$this->client->call(Client::METHOD_GET, '/account', $clientHeaders);
149150

150151
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $teamId . '/memberships', $clientHeaders);
151152
$this->assertSame(200, $response['headers']['status-code']);
152153
$this->assertSame(2, $response['body']['total']);
153154
$this->assertCount(2, $response['body']['memberships']);
154155

155-
foreach ($response['body']['memberships'] as $membership) {
156-
$this->assertSame('', $membership['userName']);
157-
$this->assertSame('', $membership['userEmail']);
158-
$this->assertSame('', $membership['userPhone']);
159-
$this->assertSame('', $membership['userId']);
160-
$this->assertFalse($membership['mfa']);
161-
$this->assertSame('', $membership['userAccessedAt']);
162-
}
156+
// The policy hides user2 from user1
157+
$other = $this->findMembership($response['body']['memberships'], $membership2['body']['$id']);
158+
$this->assertSame('', $other['userName']);
159+
$this->assertSame('', $other['userEmail']);
160+
$this->assertSame('', $other['userPhone']);
161+
$this->assertSame('', $other['userId']);
162+
$this->assertFalse($other['mfa']);
163+
$this->assertSame('', $other['userAccessedAt']);
164+
165+
// The policy never hides user1 from themselves
166+
$own = $this->findMembership($response['body']['memberships'], $membership1['body']['$id']);
167+
$this->assertSame($user1Name, $own['userName']);
168+
$this->assertSame($user1Email, $own['userEmail']);
169+
$this->assertSame($user1Phone, $own['userPhone']);
170+
$this->assertSame($user1Id, $own['userId']);
171+
$this->assertFalse($own['mfa']);
172+
$this->assertNotEmpty($own['userAccessedAt']);
163173

164174
// Step 5: Update privacy to true
165175
$response = $this->client->call(Client::METHOD_PATCH, '/project/policies/membership-privacy', $serverHeaders, [
@@ -226,15 +236,22 @@ public function testMembershipUserAccessedAtDefault(): void
226236

227237
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships', $member['clientHeaders']);
228238
$this->assertSame(200, $response['headers']['status-code']);
229-
$this->assertCount(1, $response['body']['memberships']);
230-
$this->assertSame('', $response['body']['memberships'][0]['userAccessedAt']);
239+
$this->assertCount(2, $response['body']['memberships']);
231240

232-
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['membershipId'], $member['clientHeaders']);
241+
$this->assertSame('', $this->findMembership($response['body']['memberships'], $member['otherMembershipId'])['userAccessedAt']);
242+
// A member always sees their own last access time
243+
$this->assertNotEmpty($this->findMembership($response['body']['memberships'], $member['membershipId'])['userAccessedAt']);
244+
245+
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['otherMembershipId'], $member['clientHeaders']);
233246
$this->assertSame(200, $response['headers']['status-code']);
234247
$this->assertSame('', $response['body']['userAccessedAt']);
235248

249+
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['membershipId'], $member['clientHeaders']);
250+
$this->assertSame(200, $response['headers']['status-code']);
251+
$this->assertNotEmpty($response['body']['userAccessedAt']);
252+
236253
// The policy only applies to client requests, an API key still sees it
237-
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['membershipId'], $serverHeaders);
254+
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['otherMembershipId'], $serverHeaders);
238255
$this->assertSame(200, $response['headers']['status-code']);
239256
$this->assertNotEmpty($response['body']['userAccessedAt']);
240257
}
@@ -293,7 +310,7 @@ public function testMembershipUserAccessedAtPolicy(): void
293310
$policy = $readPolicy();
294311
$this->assertFalse($policy['userAccessedAt']);
295312

296-
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['membershipId'], $member['clientHeaders']);
313+
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['otherMembershipId'], $member['clientHeaders']);
297314
$this->assertSame(200, $response['headers']['status-code']);
298315
$this->assertSame('', $response['body']['userAccessedAt']);
299316

@@ -307,35 +324,73 @@ public function testMembershipUserAccessedAtPolicy(): void
307324
$this->assertTrue($policy['userName']);
308325
$this->assertTrue($policy['userMFA']);
309326

310-
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['membershipId'], $member['clientHeaders']);
327+
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships/' . $member['otherMembershipId'], $member['clientHeaders']);
311328
$this->assertSame(200, $response['headers']['status-code']);
312329
$this->assertNotEmpty($response['body']['userAccessedAt']);
313330
$this->assertNotFalse(\strtotime($response['body']['userAccessedAt']));
314331

315332
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships', $member['clientHeaders']);
316333
$this->assertSame(200, $response['headers']['status-code']);
317-
$this->assertCount(1, $response['body']['memberships']);
318-
$this->assertNotEmpty($response['body']['memberships'][0]['userAccessedAt']);
334+
$this->assertCount(2, $response['body']['memberships']);
335+
$this->assertNotEmpty($this->findMembership($response['body']['memberships'], $member['otherMembershipId'])['userAccessedAt']);
319336

320337
$setAccessedAt(false);
321338

322339
$this->assertFalse($readPolicy()['userAccessedAt']);
323340

324341
$response = $this->client->call(Client::METHOD_GET, '/teams/' . $member['teamId'] . '/memberships', $member['clientHeaders']);
325342
$this->assertSame(200, $response['headers']['status-code']);
326-
$this->assertSame('', $response['body']['memberships'][0]['userAccessedAt']);
343+
$this->assertSame('', $this->findMembership($response['body']['memberships'], $member['otherMembershipId'])['userAccessedAt']);
327344
}
328345

329346
/**
330-
* Create a team with a single member, signed in, whose accessedAt is populated.
347+
* @param array<int, array<string, mixed>> $memberships
348+
* @return array<string, mixed>
349+
*/
350+
private function findMembership(array $memberships, string $membershipId): array
351+
{
352+
$membershipsById = array_column($memberships, null, '$id');
353+
$this->assertArrayHasKey($membershipId, $membershipsById);
354+
355+
return $membershipsById[$membershipId];
356+
}
357+
358+
/**
359+
* Create a team with a signed-in viewer and one other member, both with accessedAt populated.
331360
*
332361
* @param array<string, string> $serverHeaders
333-
* @return array{teamId: string, membershipId: string, clientHeaders: array<string, string>}
362+
* @return array{teamId: string, membershipId: string, otherMembershipId: string, clientHeaders: array<string, string>}
334363
*/
335364
private function createTeamWithMember(string $projectId, array $serverHeaders): array
365+
{
366+
$team = $this->client->call(Client::METHOD_POST, '/teams', $serverHeaders, [
367+
'teamId' => ID::unique(),
368+
'name' => 'Access Team',
369+
'roles' => ['member'],
370+
]);
371+
$this->assertSame(201, $team['headers']['status-code']);
372+
$teamId = $team['body']['$id'];
373+
374+
$viewer = $this->addTeamMember($projectId, $serverHeaders, $teamId, 'Casey Carter');
375+
$other = $this->addTeamMember($projectId, $serverHeaders, $teamId, 'Dana Dean');
376+
377+
return [
378+
'teamId' => $teamId,
379+
'membershipId' => $viewer['membershipId'],
380+
'otherMembershipId' => $other['membershipId'],
381+
'clientHeaders' => $viewer['clientHeaders'],
382+
];
383+
}
384+
385+
/**
386+
* Create a user, add them to the team and sign them in so their accessedAt is populated.
387+
*
388+
* @param array<string, string> $serverHeaders
389+
* @return array{membershipId: string, clientHeaders: array<string, string>}
390+
*/
391+
private function addTeamMember(string $projectId, array $serverHeaders, string $teamId, string $name): array
336392
{
337393
$email = 'member_' . uniqid() . '@localhost.test';
338-
$name = 'Casey Carter';
339394
$password = 'password1234';
340395

341396
$user = $this->client->call(Client::METHOD_POST, '/users', $serverHeaders, [
@@ -346,14 +401,7 @@ private function createTeamWithMember(string $projectId, array $serverHeaders):
346401
]);
347402
$this->assertSame(201, $user['headers']['status-code']);
348403

349-
$team = $this->client->call(Client::METHOD_POST, '/teams', $serverHeaders, [
350-
'teamId' => ID::unique(),
351-
'name' => 'Access Team',
352-
'roles' => ['member'],
353-
]);
354-
$this->assertSame(201, $team['headers']['status-code']);
355-
356-
$membership = $this->client->call(Client::METHOD_POST, '/teams/' . $team['body']['$id'] . '/memberships', $serverHeaders, [
404+
$membership = $this->client->call(Client::METHOD_POST, '/teams/' . $teamId . '/memberships', $serverHeaders, [
357405
'userId' => $user['body']['$id'],
358406
'roles' => ['member'],
359407
]);
@@ -381,7 +429,6 @@ private function createTeamWithMember(string $projectId, array $serverHeaders):
381429
$this->assertSame(200, $response['headers']['status-code']);
382430

383431
return [
384-
'teamId' => $team['body']['$id'],
385432
'membershipId' => $membership['body']['$id'],
386433
'clientHeaders' => $clientHeaders,
387434
];

‎tests/e2e/Services/Teams/TeamsCustomClientTest.php‎

Lines changed: 37 additions & 9 deletions

0 commit comments

Comments
 (0)