diff --git a/app/Api/v1/Controllers/GroupController.php b/app/Api/v1/Controllers/GroupController.php index f9bb596f..d58994c6 100644 --- a/app/Api/v1/Controllers/GroupController.php +++ b/app/Api/v1/Controllers/GroupController.php @@ -9,6 +9,7 @@ use App\Facades\Groups; use App\Http\Controllers\Controller; use App\Models\Group; +use Illuminate\Http\Request; class GroupController extends Controller { @@ -17,9 +18,9 @@ class GroupController extends Controller * * @return \Illuminate\Http\Resources\Json\AnonymousResourceCollection */ - public function index() + public function index(Request $request) { - $groups = Groups::getAll(); + $groups = Groups::prependTheAllGroup($request->user()->groups, $request->user()->id); return GroupResource::collection($groups); } @@ -32,9 +33,11 @@ public function index() */ public function store(GroupStoreRequest $request) { + $this->authorize('create', Group::class); + $validated = $request->validated(); - $group = Groups::create($validated); + $group = $request->user()->groups()->create($validated); return (new GroupResource($group)) ->response() @@ -49,6 +52,8 @@ public function store(GroupStoreRequest $request) */ public function show(Group $group) { + $this->authorize('view', $group); + return new GroupResource($group); } @@ -61,6 +66,8 @@ public function show(Group $group) */ public function update(GroupStoreRequest $request, Group $group) { + $this->authorize('update', $group); + $validated = $request->validated(); Groups::update($group, $validated); @@ -77,6 +84,8 @@ public function update(GroupStoreRequest $request, Group $group) */ public function assignAccounts(GroupAssignRequest $request, Group $group) { + $this->authorize('update', $group); + $validated = $request->validated(); Groups::assign($validated['ids'], $group); @@ -85,16 +94,16 @@ public function assignAccounts(GroupAssignRequest $request, Group $group) } /** - * Get accounts assign to the group + * Get accounts assigned to the group * * @param \App\Models\Group $group * @return \App\Api\v1\Resources\TwoFAccountCollection */ public function accounts(Group $group) { - $twofaccounts = Groups::getAccounts($group); + $this->authorize('view', $group); - return new TwoFAccountCollection($twofaccounts); + return new TwoFAccountCollection($group->twofaccounts()); } /** @@ -105,6 +114,8 @@ public function accounts(Group $group) */ public function destroy(Group $group) { + $this->authorize('delete', $group); + Groups::delete($group->id); return response()->json(null, 204); diff --git a/app/Api/v1/Controllers/IconController.php b/app/Api/v1/Controllers/IconController.php index c53dcb80..4040c692 100644 --- a/app/Api/v1/Controllers/IconController.php +++ b/app/Api/v1/Controllers/IconController.php @@ -3,6 +3,7 @@ namespace App\Api\v1\Controllers; use App\Http\Controllers\Controller; +use App\Models\TwoFAccount; use App\Services\LogoService; use Illuminate\Http\Request; use Illuminate\Support\Facades\Storage; @@ -52,11 +53,17 @@ public function fetch(Request $request, LogoService $logoService) /** * delete an icon * + * @param \Illuminate\Http\Request $request * @param string $icon * @return \Illuminate\Http\JsonResponse */ - public function delete(string $icon) + public function delete(string $icon, Request $request) { + // An icon affected to someone else's twofaccount cannot be deleted + if ($icon && TwoFAccount::where('icon', $icon)->where('user_id', '<>', $request->user()->id)->count() > 0) { + abort(403, 'unauthorized'); + } + Storage::disk('icons')->delete($icon); return response()->json(null, 204); diff --git a/app/Api/v1/Controllers/QrCodeController.php b/app/Api/v1/Controllers/QrCodeController.php index aaecf011..c0b191dc 100644 --- a/app/Api/v1/Controllers/QrCodeController.php +++ b/app/Api/v1/Controllers/QrCodeController.php @@ -17,6 +17,8 @@ class QrCodeController extends Controller */ public function show(TwoFAccount $twofaccount) { + $this->authorize('view', $twofaccount); + $uri = $twofaccount->getURI(); return response()->json(['qrcode' => QrCode::encode($uri)], 200); diff --git a/app/Api/v1/Requests/GroupStoreRequest.php b/app/Api/v1/Requests/GroupStoreRequest.php index 14a6e3ae..4fe31dad 100644 --- a/app/Api/v1/Requests/GroupStoreRequest.php +++ b/app/Api/v1/Requests/GroupStoreRequest.php @@ -4,6 +4,7 @@ use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Auth; +use Illuminate\Validation\Rule; class GroupStoreRequest extends FormRequest { @@ -25,7 +26,12 @@ public function authorize() public function rules() { return [ - 'name' => 'required|string|max:32|unique:groups', + 'name' => [ + 'required', + 'string', + 'max:32', + Rule::unique('groups')->where(fn ($query) => $query->where('user_id', $this->user()->id)) + ] ]; } } diff --git a/app/Models/User.php b/app/Models/User.php index 9b32d9b1..6722051d 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -44,6 +44,7 @@ class User extends Authenticatable implements WebAuthnAuthenticatable 'email_verified_at' => 'datetime', 'is_admin' => 'boolean', 'twofaccounts_count' => 'integer', + 'groups_count' => 'integer', ]; /** @@ -107,4 +108,14 @@ public function twofaccounts() { return $this->hasMany(\App\Models\TwoFAccount::class); } + + /** + * Get the Groups of the user. + * + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function groups() + { + return $this->hasMany(\App\Models\Group::class); + } } diff --git a/app/Policies/GroupPolicy.php b/app/Policies/GroupPolicy.php new file mode 100644 index 00000000..4e3a1621 --- /dev/null +++ b/app/Policies/GroupPolicy.php @@ -0,0 +1,134 @@ +isOwnerOf($user, $group); + } + + /** + * Determine whether the user can view all provided models. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @param \Illuminate\Support\Collection $groups + * @return \Illuminate\Auth\Access\Response|bool + */ + public function viewEach(User $user, Group $group, $groups) + { + return $this->isOwnerOfEach($user, $groups); + } + + /** + * Determine whether the user can create models. + * + * @param \App\Models\User $user + * @return \Illuminate\Auth\Access\Response|bool + */ + public function create(User $user) + { + return true; + } + + /** + * Determine whether the user can update the model. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @return \Illuminate\Auth\Access\Response|bool + */ + public function update(User $user, Group $group) + { + return $this->isOwnerOf($user, $group); + } + + /** + * Determine whether the user can update all provided models. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @param \Illuminate\Support\Collection $groups + * @return \Illuminate\Auth\Access\Response|bool + */ + public function updateEach(User $user, Group $group, $groups) + { + return $this->isOwnerOfEach($user, $groups); + } + + /** + * Determine whether the user can delete the model. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @return \Illuminate\Auth\Access\Response|bool + */ + public function delete(User $user, Group $group) + { + return $this->isOwnerOf($user, $group); + } + + /** + * Determine whether the user can delete all provided models. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @param \Illuminate\Support\Collection $groups + * @return \Illuminate\Auth\Access\Response|bool + */ + public function deleteEach(User $user, Group $group, $groups) + { + return $this->isOwnerOfEach($user, $groups); + } + + /** + * Determine whether the user can restore the model. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @return \Illuminate\Auth\Access\Response|bool + */ + public function restore(User $user, Group $group) + { + return $this->isOwnerOf($user, $group); + } + + /** + * Determine whether the user can permanently delete the model. + * + * @param \App\Models\User $user + * @param \App\Models\Group $group + * @return \Illuminate\Auth\Access\Response|bool + */ + public function forceDelete(User $user, Group $group) + { + return $this->isOwnerOf($user, $group); + } +} diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 288d83fc..07dec275 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -6,7 +6,9 @@ use App\Extensions\WebauthnCredentialBroker; use App\Facades\Settings; use App\Models\TwoFAccount; +use App\Models\Group; use App\Policies\TwoFAccountPolicy; +use App\Policies\GroupPolicy; use App\Services\Auth\ReverseProxyGuard; use Illuminate\Auth\Passwords\DatabaseTokenRepository; use Illuminate\Foundation\Support\Providers\AuthServiceProvider as ServiceProvider; @@ -23,6 +25,7 @@ class AuthServiceProvider extends ServiceProvider */ protected $policies = [ TwoFAccount::class => TwoFAccountPolicy::class, + Group::class => GroupPolicy::class, ]; /** diff --git a/app/Services/GroupService.php b/app/Services/GroupService.php index 7a2980bb..a75704cc 100644 --- a/app/Services/GroupService.php +++ b/app/Services/GroupService.php @@ -11,30 +11,21 @@ class GroupService { /** - * Returns all existing groups + * Prepends the pseudo group named 'All' to a group collection * + * @param Collection $groups * @return Collection */ - public static function getAll() : Collection + public static function prependTheAllGroup(Collection $groups, int $userId) : Collection { - // We return the complete collection of groups - // stored in db plus a pseudo group corresponding to 'All' - // - // This pseudo group contains all twofaccounts regardless - // of the user created group they belong to. - - // Get the user created groups - $groups = Group::withCount('twofaccounts')->get(); - - // Create the pseudo group - $allGroup = new Group([ + $theAllGroup = new Group([ 'name' => __('commons.all'), ]); - $allGroup->id = 0; - $allGroup->twofaccounts_count = TwoFAccount::count(); + $theAllGroup->id = 0; + $theAllGroup->twofaccounts_count = TwoFAccount::where('user_id', $userId)->count(); - return $groups->prepend($allGroup); + return $groups->prepend($theAllGroup); } /** @@ -138,19 +129,6 @@ public static function assign($ids, Group $group = null) : void } } - /** - * Finds twofaccounts assigned to the group - * - * @param \App\Models\Group $group The group - * @return Collection The assigned accounts - */ - public static function getAccounts(Group $group) : Collection - { - $twofaccounts = $group->twofaccounts()->where('group_id', $group->id)->get(); - - return $twofaccounts; - } - /** * Determines the destination group * diff --git a/resources/js/views/Groups.vue b/resources/js/views/Groups.vue index 00d61a2e..0feaffe7 100644 --- a/resources/js/views/Groups.vue +++ b/resources/js/views/Groups.vue @@ -97,19 +97,19 @@ /** * Delete a group (after confirmation) */ - deleteGroup(id) { + async deleteGroup(id) { if(confirm(this.$t('groups.confirm.delete'))) { - this.axios.delete('/api/v1/groups/' + id) + await this.axios.delete('/api/v1/groups/' + id).then(response => { + // Remove the deleted group from the collection + this.groups = this.groups.filter(a => a.id !== id) + this.$notify({ type: 'is-success', text: this.$t('groups.group_successfully_deleted') }) - // Remove the deleted group from the collection - this.groups = this.groups.filter(a => a.id !== id) - this.$notify({ type: 'is-success', text: this.$t('groups.group_successfully_deleted') }) - - // Reset persisted group filter to 'All' (groupId=0) - // (backend will save to change automatically) - if( parseInt(this.$root.userPreferences.activeGroup) === id ) { - this.$root.userPreferences.activeGroup = 0 - } + // Reset persisted group filter to 'All' (groupId=0) + // (backend will save to change automatically) + if( parseInt(this.$root.userPreferences.activeGroup) === id ) { + this.$root.userPreferences.activeGroup = 0 + } + }) } }