From e8773d5e9a096febbe3fd525487a1e8d96ed916f Mon Sep 17 00:00:00 2001 From: Thiritin Date: Mon, 27 May 2024 13:45:16 +0200 Subject: [PATCH] HOTFIX: Fix Ids in GroupUser Response aswell as restrict adding users to groups twice --- .../Api/v1/GroupUserController.php | 13 ++++++ app/Http/Resources/V1/GroupUserResource.php | 6 +-- app/Services/NextcloudService.php | 11 +++++ tests/Feature/Api/v1/GroupsTest.php | 41 +++++++++++++++++-- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Api/v1/GroupUserController.php b/app/Http/Controllers/Api/v1/GroupUserController.php index ae92f977..9ecb3e9d 100644 --- a/app/Http/Controllers/Api/v1/GroupUserController.php +++ b/app/Http/Controllers/Api/v1/GroupUserController.php @@ -9,6 +9,7 @@ use App\Models\Group; use App\Models\User; use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; use Spatie\QueryBuilder\AllowedFilter; use Spatie\QueryBuilder\QueryBuilder; @@ -27,10 +28,22 @@ public function store(GroupUserStoreRequest $request, Group $group) $this->authorize('create', [$group->users()->find($request->user()->id)->pivot]); $useField = isset($request->validationData()['email']) ? 'email' : 'id'; + $user = match ($useField) { 'email' => User::where('email', $request->validationData()['email'])->firstOrFail(), 'id' => User::findByHashidOrFail($request->validationData()['id']), }; + + // validated email + if (!$user->hasVerifiedEmail()) { + throw ValidationException::withMessages(['email' => 'User has not verified their email']); + } + + // ensure user does not already exist, if he does, throw validation error + if ($group->users->contains($user)) { + throw ValidationException::withMessages([$useField => 'User is already in the group']); + } + $group->users()->attach($user, ['level' => $request->validationData()['level']]); return new GroupUserResource($group->users()->find($user->id)); } diff --git a/app/Http/Resources/V1/GroupUserResource.php b/app/Http/Resources/V1/GroupUserResource.php index d19aea62..0fc1f122 100644 --- a/app/Http/Resources/V1/GroupUserResource.php +++ b/app/Http/Resources/V1/GroupUserResource.php @@ -10,14 +10,14 @@ class GroupUserResource extends JsonResource { /** - * @param Request $request + * @param Request $request * @return array */ public function toArray($request) { return [ - 'group_id' => $this->pivot->group_id, - 'user_id' => $this->pivot->user_id, + 'group_id' => $this->pivot->group->hashid, + 'user_id' => $this->pivot->user->hashid, 'level' => $this->pivot->level, ]; } diff --git a/app/Services/NextcloudService.php b/app/Services/NextcloudService.php index 4703523c..76af5484 100644 --- a/app/Services/NextcloudService.php +++ b/app/Services/NextcloudService.php @@ -65,6 +65,17 @@ public static function setManageAcl(Group $group, User $user, bool $allow): void ])->throwIfServerError(); } + public static function getUsers() + { + $res = Http::nextcloud()->get("ocs/v2.php/cloud/users", [ + 'offset' => 0, + 'limit' => 1000, + ])->throw(); + // Parse xml + $xml = simplexml_load_string($res->body()); + return (array) $xml->data->users->element; + } + public static function createUser(User $user) { Http::nextcloud()->post("ocs/v2.php/cloud/users", [ diff --git a/tests/Feature/Api/v1/GroupsTest.php b/tests/Feature/Api/v1/GroupsTest.php index b7f8c03a..af0c5d0d 100644 --- a/tests/Feature/Api/v1/GroupsTest.php +++ b/tests/Feature/Api/v1/GroupsTest.php @@ -255,15 +255,48 @@ $request->assertSuccessful(); // verify response contains user_id, group_id and level $request->assertJsonFragment([ + "user_id" => $userToBeInvited->hashid, + "group_id" => $group->hashid, + "level" => "member", + ]); + assertDatabaseHas('group_user', [ "user_id" => $userToBeInvited->id, "group_id" => $group->id, "level" => "member", ]); +}); + +// Add member twice should cause error via email +test('Adding the same email twice should cause an error', function () { + $group = Group::factory()->create(); + + $user = Sanctum::actingAs( + User::factory()->create(), + ['groups.read', 'groups.update', 'groups.delete'] + ); + $userToBeInvited = User::factory()->create(); + $data = [ + "email" => $userToBeInvited->email, + "level" => "member", + ]; + $group->users()->sync([$user->id => ['level' => GroupUserLevel::Admin]]); + + $request = post(route('api.v1.groups.users.store', $group), $data); + $request->assertSuccessful(); + // verify response contains user_id, group_id and level + $request->assertJsonFragment([ + "user_id" => $userToBeInvited->hashid, + "group_id" => $group->hashid, + "level" => "member", + ]); assertDatabaseHas('group_user', [ "user_id" => $userToBeInvited->id, "group_id" => $group->id, "level" => "member", ]); + + $request = postJson(route('api.v1.groups.users.store', $group), $data, ['Accept' => 'application/json']); + $request->assertJsonValidationErrors(['email']); }); test('Add member to group as admin via id', function () { @@ -285,8 +318,8 @@ $request->assertSuccessful(); // verify response contains user_id, group_id and level $request->assertJsonFragment([ - "user_id" => $userToBeInvited->id, - "group_id" => $group->id, + "user_id" => $userToBeInvited->hashid, + "group_id" => $group->hashid, "level" => "member", ]); assertDatabaseHas('group_user', [ @@ -395,8 +428,8 @@ $request = get(route('api.v1.groups.users.index', ["group" => $group])); $request->assertSuccessful(); $request->assertJsonFragment([ - "user_id" => $user->id, - "group_id" => $group->id, + "user_id" => $user->hashid, + "group_id" => $group->hashid, "level" => GroupUserLevel::Admin, ]); });