From 9a28db5f86db0e0a5ca03446e96bdd1f9572cf32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Wi=C5=9Bniewski?= Date: Wed, 20 Sep 2023 13:21:07 +0200 Subject: [PATCH 1/3] Self update roles --- app/Dtos/RoleCreateDto.php | 7 ++ app/Dtos/RoleSearchDto.php | 11 ++ app/Dtos/RoleUpdateDto.php | 7 ++ app/Dtos/SelfUpdateRoles.php | 25 +++++ app/Enums/ExceptionsEnums/Exceptions.php | 1 + app/Http/Controllers/AuthController.php | 6 ++ app/Http/Requests/RoleIndexRequest.php | 1 + app/Http/Requests/RoleStoreRequest.php | 1 + app/Http/Requests/RoleUpdateRequest.php | 1 + app/Http/Resources/RoleResource.php | 1 + app/Models/Role.php | 5 + app/Services/AuthService.php | 13 +++ .../Contracts/AuthServiceContract.php | 3 + .../Contracts/UserServiceContract.php | 3 + app/Services/UserService.php | 25 +++++ ...add_is_joinable_columnt_to_roles_table.php | 21 ++++ docs/paths/Auth.yml | 20 ++++ docs/paths/Roles.yml | 5 + docs/requests/Auth.yml | 12 +++ docs/requests/Roles.yml | 8 ++ docs/schemas/Roles.yml | 3 + routes/auth.php | 2 + tests/Feature/AuthTest.php | 101 ++++++++++++++++++ tests/Feature/RoleTest.php | 14 +++ tests/Feature/UserTest.php | 6 ++ 25 files changed, 302 insertions(+) create mode 100644 app/Dtos/SelfUpdateRoles.php create mode 100644 database/migrations/2023_09_14_102544_add_is_joinable_columnt_to_roles_table.php diff --git a/app/Dtos/RoleCreateDto.php b/app/Dtos/RoleCreateDto.php index 1290778fc..fc6875587 100644 --- a/app/Dtos/RoleCreateDto.php +++ b/app/Dtos/RoleCreateDto.php @@ -17,6 +17,7 @@ class RoleCreateDto extends Dto implements InstantiateFromRequest private bool|Missing $is_registration_role; private array $permissions; private array|Missing $metadata; + private bool|Missing $is_joinable; public static function instantiateFromRequest(FormRequest $request): self { @@ -26,6 +27,7 @@ public static function instantiateFromRequest(FormRequest $request): self is_registration_role: $request->input('is_registration_role', new Missing()), permissions: $request->input('permissions', []), metadata: self::mapMetadata($request), + is_joinable: $request->input('is_joinable', new Missing()), ); } @@ -48,4 +50,9 @@ public function getPermissions(): array { return $this->permissions; } + + public function getIsJoinable(): bool|Missing + { + return $this->is_joinable; + } } diff --git a/app/Dtos/RoleSearchDto.php b/app/Dtos/RoleSearchDto.php index af84c86f9..118ea6200 100644 --- a/app/Dtos/RoleSearchDto.php +++ b/app/Dtos/RoleSearchDto.php @@ -16,6 +16,7 @@ public function __construct( private ?array $metadata, private ?array $metadata_private, private ?array $ids, + private ?bool $is_joinable, ) {} public function toArray(): array @@ -50,6 +51,10 @@ public function toArray(): array $data['ids'] = $this->getIds(); } + if ($this->getIsJoinable() !== null) { + $data['is_joinable'] = $this->getIsJoinable(); + } + return $data; } @@ -63,6 +68,7 @@ public static function instantiateFromRequest(FormRequest $request): self $request->input('metadata', null), $request->input('metadata_private', null), $request->input('ids', null), + $request->input('is_joinable', null), ); } @@ -100,4 +106,9 @@ public function getIds(): ?array { return $this->ids; } + + public function getIsJoinable(): ?bool + { + return $this->is_joinable; + } } diff --git a/app/Dtos/RoleUpdateDto.php b/app/Dtos/RoleUpdateDto.php index 26dabe510..47ef814c0 100644 --- a/app/Dtos/RoleUpdateDto.php +++ b/app/Dtos/RoleUpdateDto.php @@ -14,6 +14,7 @@ class RoleUpdateDto extends Dto implements InstantiateFromRequest private Missing|string|null $description; private bool|Missing $is_registration_role; private array|Missing $permissions; + private bool|Missing $is_joinable; public static function instantiateFromRequest(FormRequest|RoleUpdateRequest $request): self { @@ -22,6 +23,7 @@ public static function instantiateFromRequest(FormRequest|RoleUpdateRequest $req description: $request->input('description', new Missing()), is_registration_role: $request->input('is_registration_role', new Missing()), permissions: $request->input('permissions', new Missing()), + is_joinable: $request->input('is_joinable', new Missing()), ); } @@ -44,4 +46,9 @@ public function getPermissions(): array|Missing { return $this->permissions; } + + public function getIsJoinable(): bool|Missing + { + return $this->is_joinable; + } } diff --git a/app/Dtos/SelfUpdateRoles.php b/app/Dtos/SelfUpdateRoles.php new file mode 100644 index 000000000..de1c0bcee --- /dev/null +++ b/app/Dtos/SelfUpdateRoles.php @@ -0,0 +1,25 @@ + [ + 'uuid', + Rule::exists('roles', 'id')->where(fn ($query) => $query->whereNotIn('type', [RoleType::AUTHENTICATED, RoleType::UNAUTHENTICATED])), + ], + ]; + } +} diff --git a/app/Enums/ExceptionsEnums/Exceptions.php b/app/Enums/ExceptionsEnums/Exceptions.php index e7fe1359d..deea0f00f 100644 --- a/app/Enums/ExceptionsEnums/Exceptions.php +++ b/app/Enums/ExceptionsEnums/Exceptions.php @@ -127,6 +127,7 @@ final class Exceptions extends Enum public const CLIENT_PROVIDER_MERGE_TOKEN_INVALID = 'Provider merge token is invalid'; public const CLIENT_PROVIDER_MERGE_TOKEN_MISMATCH = 'Provider merge token is for an account with different email address'; + public const CLIENT_JOINING_NON_JOINABLE_ROLE = 'Can\'t join to a non joinable role'; public static function getCode(string $value): int { diff --git a/app/Http/Controllers/AuthController.php b/app/Http/Controllers/AuthController.php index e94c45b96..3f362be4b 100644 --- a/app/Http/Controllers/AuthController.php +++ b/app/Http/Controllers/AuthController.php @@ -4,6 +4,7 @@ use App\Dtos\RegisterDto; use App\Dtos\SavedAddressDto; +use App\Dtos\SelfUpdateRoles; use App\Dtos\TFAConfirmDto; use App\Dtos\TFAPasswordDto; use App\Dtos\TFASetupDto; @@ -243,4 +244,9 @@ public function updateProfile(ProfileUpdateRequest $request): JsonResource $this->authService->updateProfile(UpdateProfileDto::instantiateFromRequest($request)), ); } + + public function selfUpdateRoles(SelfUpdateRoles $dto): JsonResource + { + return UserResource::make($this->authService->selfUpdateRoles($dto)); + } } diff --git a/app/Http/Requests/RoleIndexRequest.php b/app/Http/Requests/RoleIndexRequest.php index 8cca43e5e..e0a264a6b 100644 --- a/app/Http/Requests/RoleIndexRequest.php +++ b/app/Http/Requests/RoleIndexRequest.php @@ -17,6 +17,7 @@ public function rules(): array 'metadata_private' => ['nullable', 'array'], 'ids' => ['array'], 'ids.*' => ['uuid'], + 'is_joinable' => ['boolean'], ]; } } diff --git a/app/Http/Requests/RoleStoreRequest.php b/app/Http/Requests/RoleStoreRequest.php index 86f59b681..df8e54cdf 100644 --- a/app/Http/Requests/RoleStoreRequest.php +++ b/app/Http/Requests/RoleStoreRequest.php @@ -19,6 +19,7 @@ public function rules(): array 'is_registration_role' => ['boolean'], 'permissions' => ['array'], 'permissions.*' => ['string'], + 'is_joinable' => ['boolean'], ] ); } diff --git a/app/Http/Requests/RoleUpdateRequest.php b/app/Http/Requests/RoleUpdateRequest.php index 2740a058e..fd2a6437c 100644 --- a/app/Http/Requests/RoleUpdateRequest.php +++ b/app/Http/Requests/RoleUpdateRequest.php @@ -19,6 +19,7 @@ public function rules(): array 'is_registration_role' => ['boolean'], 'permissions' => ['array'], 'permissions.*' => ['string'], + 'is_joinable' => ['boolean'], ]; } } diff --git a/app/Http/Resources/RoleResource.php b/app/Http/Resources/RoleResource.php index 11d6e8718..ede1db1ae 100644 --- a/app/Http/Resources/RoleResource.php +++ b/app/Http/Resources/RoleResource.php @@ -20,6 +20,7 @@ public function base(Request $request): array 'assignable' => $this->resource->isAssignable(), 'deletable' => $this->resource->type->is(RoleType::REGULAR), 'users_count' => $this->resource->users_count, + 'is_joinable' => $this->resource->is_joinable, ], $this->metadataResource('roles.show_metadata_private')); } diff --git a/app/Models/Role.php b/app/Models/Role.php index c85cc6400..cddc4cc98 100644 --- a/app/Models/Role.php +++ b/app/Models/Role.php @@ -11,6 +11,7 @@ use App\Traits\HasDiscountConditions; use App\Traits\HasMetadata; use App\Traits\HasUuid; +use Heseya\Searchable\Criteria\Equals; use Heseya\Searchable\Criteria\Like; use Heseya\Searchable\Traits\HasCriteria; use Illuminate\Database\Eloquent\Builder; @@ -26,6 +27,7 @@ * @property RoleType $type * @property string $name * @property bool $is_registration_role + * @property bool $is_joinable * * @mixin IdeHelperRole */ @@ -43,11 +45,13 @@ class Role extends SpatieRole implements AuditableContract 'description', 'guard_name', 'is_registration_role', + 'is_joinable', ]; protected $casts = [ 'type' => RoleType::class, 'is_registration_role' => 'bool', + 'is_joinable' => 'bool', ]; protected array $criteria = [ @@ -58,6 +62,7 @@ class Role extends SpatieRole implements AuditableContract 'metadata' => MetadataSearch::class, 'metadata_private' => MetadataPrivateSearch::class, 'ids' => WhereInIds::class, + 'is_joinable' => Equals::class, ]; public function users(): BelongsToMany diff --git a/app/Services/AuthService.php b/app/Services/AuthService.php index 964eb82ee..3b5f61341 100644 --- a/app/Services/AuthService.php +++ b/app/Services/AuthService.php @@ -3,6 +3,7 @@ namespace App\Services; use App\Dtos\RegisterDto; +use App\Dtos\SelfUpdateRoles; use App\Dtos\TFAConfirmDto; use App\Dtos\TFAPasswordDto; use App\Dtos\TFASetupDto; @@ -369,6 +370,18 @@ public function selfRemove(string $password): void $this->userService->destroy($user); } + public function selfUpdateRoles(SelfUpdateRoles $dto): User + { + if ($this->isAppAuthenticated()) { + throw new ClientException(Exceptions::CLIENT_APPS_NO_ACCESS); + } + + /** @var User $user */ + $user = Auth::user(); + + return $this->userService->selfUpdateRoles($user, $dto); + } + private function verifyTFA(?string $code): void { if (!Auth::user()?->is_tfa_active && $code !== null) { diff --git a/app/Services/Contracts/AuthServiceContract.php b/app/Services/Contracts/AuthServiceContract.php index 4577be9c5..e013182ea 100644 --- a/app/Services/Contracts/AuthServiceContract.php +++ b/app/Services/Contracts/AuthServiceContract.php @@ -3,6 +3,7 @@ namespace App\Services\Contracts; use App\Dtos\RegisterDto; +use App\Dtos\SelfUpdateRoles; use App\Dtos\TFAConfirmDto; use App\Dtos\TFAPasswordDto; use App\Dtos\TFASetupDto; @@ -51,4 +52,6 @@ public function register(RegisterDto $dto): User; public function updateProfile(UpdateProfileDto $dto): User; public function selfRemove(string $password): void; + + public function selfUpdateRoles(SelfUpdateRoles $dto): User; } diff --git a/app/Services/Contracts/UserServiceContract.php b/app/Services/Contracts/UserServiceContract.php index 08571c104..759e9fb75 100644 --- a/app/Services/Contracts/UserServiceContract.php +++ b/app/Services/Contracts/UserServiceContract.php @@ -2,6 +2,7 @@ namespace App\Services\Contracts; +use App\Dtos\SelfUpdateRoles; use App\Dtos\UserCreateDto; use App\Dtos\UserDto; use App\Models\User; @@ -16,4 +17,6 @@ public function create(UserCreateDto $dto): User; public function update(User $user, UserDto $dto): User; public function destroy(User $user): void; + + public function selfUpdateRoles(User $user, SelfUpdateRoles $dto): User; } diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 54a4d407c..8dd7e031f 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\Dtos\SelfUpdateRoles; use App\Dtos\UserCreateDto; use App\Dtos\UserDto; use App\Enums\ExceptionsEnums\Exceptions; @@ -26,6 +27,7 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; +use Spatie\LaravelData\Optional; readonly class UserService implements UserServiceContract { @@ -267,4 +269,27 @@ public function destroy(User $user): void } }); } + + public function selfUpdateRoles(User $user, SelfUpdateRoles $dto): User + { + $roleModels = collect(); + if (!($dto->roles instanceof Optional)) { + /** @var Collection $roleModels */ + $roleModels = Role::query() + ->whereIn('id', $dto->roles) + ->where('is_joinable', '=', true) + ->get(); + + if ($roleModels->count() < count($dto->roles)) { + throw new ClientException(Exceptions::CLIENT_JOINING_NON_JOINABLE_ROLE); + } + } + + // @phpstan-ignore-next-line + $roleModels = $roleModels->merge($user->roles->filter(fn (Role $role): bool => !$role->is_joinable)); + + $user->syncRoles($roleModels); + + return $user; + } } diff --git a/database/migrations/2023_09_14_102544_add_is_joinable_columnt_to_roles_table.php b/database/migrations/2023_09_14_102544_add_is_joinable_columnt_to_roles_table.php new file mode 100644 index 000000000..b259baae6 --- /dev/null +++ b/database/migrations/2023_09_14_102544_add_is_joinable_columnt_to_roles_table.php @@ -0,0 +1,21 @@ +boolean('is_joinable')->default(false); + }); + } + + public function down(): void + { + Schema::table('roles', function (Blueprint $table): void { + $table->dropColumn('is_joinable'); + }); + } +}; diff --git a/docs/paths/Auth.yml b/docs/paths/Auth.yml index f1abae56b..795059ee9 100644 --- a/docs/paths/Auth.yml +++ b/docs/paths/Auth.yml @@ -585,3 +585,23 @@ BillingAddressesParams: type: object security: - BearerAuth: [ ] + +SelfUpdateRoles: + patch: + tags: + - Auth + summary: 'edit your own roles' + requestBody: + $ref: './../requests/Auth.yml#/SelfUpdateRoles' + responses: + 200: + description: Success + content: + application/json: + schema: + properties: + data: + $ref: './../schemas/Users.yml#/UserView' + type: object + security: + - BearerAuth: [ ] diff --git a/docs/paths/Roles.yml b/docs/paths/Roles.yml index 260016820..7138be711 100644 --- a/docs/paths/Roles.yml +++ b/docs/paths/Roles.yml @@ -48,6 +48,11 @@ Roles: items: type: string format: uuid + - name: is_joinable + in: query + description: 'Is the user able to self-assign the role' + schema: + type: boolean responses: 200: description: Success diff --git a/docs/requests/Auth.yml b/docs/requests/Auth.yml index d2e548344..9cc4a5be9 100644 --- a/docs/requests/Auth.yml +++ b/docs/requests/Auth.yml @@ -86,3 +86,15 @@ UpdateProfile: preferences: $ref: '././../schemas/Auth.yml#/Preferences' type: object + +SelfUpdateRoles: + content: + application/json: + schema: + properties: + roles: + type: array + items: + type: string + example: '119c0a63-1ea1-4769-8d5f-169f68de5598' + type: object diff --git a/docs/requests/Roles.yml b/docs/requests/Roles.yml index 1fed24292..ab415c751 100644 --- a/docs/requests/Roles.yml +++ b/docs/requests/Roles.yml @@ -28,6 +28,10 @@ RoleStore: $ref: './../schemas/Metadata.yml#/Metadata' metadata_private: $ref: './../schemas/Metadata.yml#/Metadata' + is_joinable: + description: 'Whether the role can be assigned to user by himself.' + type: boolean + example: false RoleUpdate: content: @@ -52,4 +56,8 @@ RoleUpdate: items: type: string example: roles.add + is_joinable: + description: 'Whether the role can be assigned to user by himself.' + type: boolean + example: false type: object diff --git a/docs/schemas/Roles.yml b/docs/schemas/Roles.yml index 978b5d2b9..3a9c52750 100644 --- a/docs/schemas/Roles.yml +++ b/docs/schemas/Roles.yml @@ -29,6 +29,9 @@ Role: $ref: './../schemas/Metadata.yml#/Metadata' metadata_private: $ref: './../schemas/Metadata.yml#/Metadata' + is_joinable: + description: 'Whether the role can be assigned to user by himself.' + type: boolean RoleView: type: object diff --git a/routes/auth.php b/routes/auth.php index e92fac958..f10ac11c6 100644 --- a/routes/auth.php +++ b/routes/auth.php @@ -13,6 +13,8 @@ Route::get('profile', [AuthController::class, 'profile']); Route::patch('profile', [AuthController::class, 'updateProfile']) ->middleware('can:authenticated'); + Route::patch('profile/roles', [AuthController::class, 'selfUpdateRoles']) + ->middleware('can:authenticated'); Route::patch('profile/metadata-personal', [MetadataController::class, 'updateOrCreateLoggedMyPersonal']) ->middleware('can:authenticated'); Route::prefix('profile') diff --git a/tests/Feature/AuthTest.php b/tests/Feature/AuthTest.php index 564a427b5..528b63a3a 100644 --- a/tests/Feature/AuthTest.php +++ b/tests/Feature/AuthTest.php @@ -1265,6 +1265,107 @@ public function testUpdateProfile(): void ]); } + public function testSelfUpdateRolesUnauthorized(): void + { + $role = Role::factory()->create([ + 'is_joinable' => true, + ]); + + $this->json('PATCH', '/auth/profile/roles', [ + 'roles' => [ + $role->getKey(), + ], + ]) + ->assertForbidden(); + } + + public function testSelfUpdateRoles(): void + { + $role = Role::factory()->create([ + 'name' => 'New joinable role', + 'type' => RoleType::REGULAR, + 'is_joinable' => true, + ]); + + $noJoinable = Role::factory()->create([ + 'name' => 'No joinable role', + 'type' => RoleType::REGULAR, + 'is_joinable' => false, + ]); + + $this->user->roles()->attach($noJoinable->getKey()); + + $this->actingAs($this->user)->json('PATCH', '/auth/profile/roles', [ + 'roles' => [ + $role->getKey(), + ], + ]) + ->assertOk() + ->assertJsonFragment([ + 'id' => $noJoinable->getKey(), + 'name' => 'No joinable role', + 'is_joinable' => false, + ]) + ->assertJsonFragment([ + 'id' => $role->getKey(), + 'name' => 'New joinable role', + 'is_joinable' => true, + ]); + } + + public function testSelfUpdateRolesNoJoinable(): void + { + $role = Role::factory()->create([ + 'name' => 'New joinable role', + 'type' => RoleType::REGULAR, + 'is_joinable' => false, + ]); + + $this->actingAs($this->user)->json('PATCH', '/auth/profile/roles', [ + 'roles' => [ + $role->getKey(), + ], + ]) + ->assertUnprocessable() + ->assertJsonFragment([ + 'key' => Exceptions::coerce(Exceptions::CLIENT_JOINING_NON_JOINABLE_ROLE)->key, + ])->assertJsonFragment([ + 'message' => Exceptions::CLIENT_JOINING_NON_JOINABLE_ROLE, + ]); + } + + public function testSelfUpdateRolesRemove(): void + { + $role = Role::factory()->create([ + 'name' => 'New joinable role', + 'type' => RoleType::REGULAR, + 'is_joinable' => true, + ]); + + $noJoinable = Role::factory()->create([ + 'name' => 'No joinable role', + 'type' => RoleType::REGULAR, + 'is_joinable' => false, + ]); + + $this->user->roles()->saveMany([$role, $noJoinable]); + + $this->actingAs($this->user)->json('PATCH', '/auth/profile/roles', [ + 'roles' => [], + ]) + ->assertOk() + ->assertJsonFragment([ + 'id' => $noJoinable->getKey(), + 'name' => 'No joinable role', + 'is_joinable' => false, + ]) + ->assertJsonMissing([ + 'id' => $role->getKey(), + 'name' => 'New joinable role', + 'is_joinable' => true, + ]); + } + public function testCheckIdentityUnauthorized(): void { $user = User::factory()->create(); diff --git a/tests/Feature/RoleTest.php b/tests/Feature/RoleTest.php index 8190d3fa4..734887fbd 100644 --- a/tests/Feature/RoleTest.php +++ b/tests/Feature/RoleTest.php @@ -60,6 +60,7 @@ public function testIndex($user): void 'deletable' => true, 'metadata' => [], 'users_count' => 1, + 'is_joinable' => false, ], ]) ->assertJsonFragment([[ @@ -71,6 +72,7 @@ public function testIndex($user): void 'deletable' => true, 'metadata' => [], 'users_count' => 0, + 'is_joinable' => false, ], ]); } @@ -309,6 +311,7 @@ public function testIndexSearchByUnassignable($user): void 'deletable' => true, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]) ->assertJsonFragment([[ @@ -320,6 +323,7 @@ public function testIndexSearchByUnassignable($user): void 'deletable' => true, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]) ->assertJsonFragment([[ @@ -331,6 +335,7 @@ public function testIndexSearchByUnassignable($user): void 'deletable' => false, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]) ->assertJsonFragment([[ @@ -342,6 +347,7 @@ public function testIndexSearchByUnassignable($user): void 'deletable' => false, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]); } @@ -388,6 +394,7 @@ public function testIndexSearch($user): void 'deletable' => true, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]) ->assertJsonFragment([[ @@ -399,6 +406,7 @@ public function testIndexSearch($user): void 'deletable' => true, 'users_count' => 0, 'metadata' => [], + 'is_joinable' => false, ], ]); } @@ -638,6 +646,7 @@ public function testCreate($user): void 'test.custom1', 'test.custom2', ], + 'is_joinable' => true, ]); $response @@ -653,6 +662,7 @@ public function testCreate($user): void 'test.custom2', ], 'metadata' => [], + 'is_joinable' => true, ], ]); @@ -972,6 +982,7 @@ public function testUpdate($user): void $role = Role::create([ 'name' => 'role1', 'description' => 'Role 1', + 'is_joinable' => false, ]); $permission1 = Permission::create(['name' => 'test.custom1']); @@ -987,6 +998,7 @@ public function testUpdate($user): void 'test.custom2', 'test.custom3', ], + 'is_joinable' => true, ]); $response @@ -1002,6 +1014,7 @@ public function testUpdate($user): void 'test.custom3', ], 'metadata' => [], + 'is_joinable' => true, ], ]); @@ -1009,6 +1022,7 @@ public function testUpdate($user): void $role->getKeyName() => $role->getKey(), 'name' => 'test_role', 'description' => 'Test role', + 'is_joinable' => true, ]); $role = Role::findByName('test_role'); diff --git a/tests/Feature/UserTest.php b/tests/Feature/UserTest.php index c3e15d777..2d477353e 100644 --- a/tests/Feature/UserTest.php +++ b/tests/Feature/UserTest.php @@ -900,6 +900,7 @@ public function testCreateRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonFragment([[ 'id' => $role2->getKey(), @@ -910,6 +911,7 @@ public function testCreateRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonFragment([[ 'id' => $role3->getKey(), @@ -920,6 +922,7 @@ public function testCreateRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonPath('data.permissions', $permissions); @@ -1249,6 +1252,7 @@ public function testUpdateAddRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonFragment([[ 'id' => $role2->getKey(), @@ -1259,6 +1263,7 @@ public function testUpdateAddRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonFragment([[ 'id' => $role3->getKey(), @@ -1269,6 +1274,7 @@ public function testUpdateAddRoles($user): void 'deletable' => true, 'users_count' => null, 'metadata' => [], + 'is_joinable' => false, ], ])->assertJsonPath('data.permissions', $permissions); From 9441aaa50c9725530d7360d38063ab5c25adcf5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Wi=C5=9Bniewski?= Date: Wed, 20 Sep 2023 13:44:23 +0200 Subject: [PATCH 2/3] Fix style --- app/Policies/WebHookPolicy.php | 1 + app/Services/UserService.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Policies/WebHookPolicy.php b/app/Policies/WebHookPolicy.php index 5b818ef61..c024303b2 100644 --- a/app/Policies/WebHookPolicy.php +++ b/app/Policies/WebHookPolicy.php @@ -52,6 +52,7 @@ private function canChange(App|User $user, WebHook $webHook, string $method): Re ? Response::allow() : throw new ClientException(Exceptions::CLIENT_WEBHOOK_APP_ACTION, errorArray: ['method' => $method]); } + // Webhook stworzony przez użytkownika return $user instanceof User ? Response::allow() diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 8dd7e031f..e1b050a08 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -285,7 +285,7 @@ public function selfUpdateRoles(User $user, SelfUpdateRoles $dto): User } } - // @phpstan-ignore-next-line + /** @phpstan-ignore-next-line */ $roleModels = $roleModels->merge($user->roles->filter(fn (Role $role): bool => !$role->is_joinable)); $user->syncRoles($roleModels); From d56f8734e11ffeb8fdb6d8aaf5a054298f195400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Wi=C5=9Bniewski?= Date: Thu, 21 Sep 2023 12:02:29 +0200 Subject: [PATCH 3/3] Only regular roles joinable --- app/Enums/ExceptionsEnums/Exceptions.php | 1 + app/Services/RoleService.php | 8 +++++++ tests/Feature/RoleTest.php | 30 ++++++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/app/Enums/ExceptionsEnums/Exceptions.php b/app/Enums/ExceptionsEnums/Exceptions.php index 2cf1b634f..e8f73f99c 100644 --- a/app/Enums/ExceptionsEnums/Exceptions.php +++ b/app/Enums/ExceptionsEnums/Exceptions.php @@ -128,6 +128,7 @@ final class Exceptions extends Enum public const CLIENT_PROVIDER_MERGE_TOKEN_MISMATCH = 'Provider merge token is for an account with different email address'; public const CLIENT_JOINING_NON_JOINABLE_ROLE = 'Can\'t join to a non joinable role'; + public const CLIENT_UPDATE_NOT_REGULAR_JOINABLE = 'Can\'t update is_joinable field in role types other than regular'; public static function getCode(string $value): int { diff --git a/app/Services/RoleService.php b/app/Services/RoleService.php index 6c9676734..4116e3427 100644 --- a/app/Services/RoleService.php +++ b/app/Services/RoleService.php @@ -59,6 +59,14 @@ public function update(Role $role, RoleUpdateDto $dto): Role { $user = Auth::user(); + if ( + !($dto->getIsJoinable() instanceof Missing) + && $role->type->isNot(RoleType::REGULAR) + && $dto->getIsJoinable() !== $role->is_joinable + ) { + throw new ClientException(Exceptions::CLIENT_UPDATE_NOT_REGULAR_JOINABLE); + } + if (!$user?->hasAllPermissions($role->getAllPermissions())) { throw new ClientException(Exceptions::CLIENT_UPDATE_ROLE_WITHOUT_PERMISSION); } diff --git a/tests/Feature/RoleTest.php b/tests/Feature/RoleTest.php index 734887fbd..c41e15fdb 100644 --- a/tests/Feature/RoleTest.php +++ b/tests/Feature/RoleTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature; +use App\Enums\ExceptionsEnums\Exceptions; use App\Enums\RoleType; use App\Models\Permission; use App\Models\Role; @@ -1073,6 +1074,35 @@ public function testUpdateToRegistrationRole($user): void ]); } + /** + * @dataProvider authProvider + */ + public function testUpdateOwnerIsJoinable($user): void + { + $this->{$user}->givePermissionTo('roles.edit'); + + $role = Role::create([ + 'name' => 'role1', + 'description' => 'Role 1', + 'is_joinable' => false, + ]); + $role->type = RoleType::OWNER; + $role->save(); + + $this + ->actingAs($this->{$user}) + ->patchJson('/roles/id:' . $role->getKey(), [ + 'name' => 'test_role', + 'is_joinable' => true, + ]) + ->assertUnprocessable() + ->assertJsonFragment([ + 'key' => Exceptions::coerce(Exceptions::CLIENT_UPDATE_NOT_REGULAR_JOINABLE)->key, + ])->assertJsonFragment([ + 'message' => Exceptions::CLIENT_UPDATE_NOT_REGULAR_JOINABLE, + ]); + } + /** * @dataProvider authProvider */