Skip to content

Add user-group permissions to query #3425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/Constants/UsersUserGroupsConstants.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* SPDX-License-Identifier: MIT
* Copyright (c) 2017-2018 Tobias Reich
* Copyright (c) 2018-2025 LycheeOrg.
*/

namespace App\Constants;

class UsersUserGroupsConstants
{
// computed table name
public const USERS_USER_GROUPS = 'users_user_groups';

// Id names
public const USER_ID = 'user_id';
public const USER_GROUP_ID = 'user_group_id';

// Attributes name
public const ROLE = 'role';
}
3 changes: 2 additions & 1 deletion app/Http/Resources/Rights/SettingsRightsResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use App\Policies\SettingsPolicy;
use App\Policies\UserGroupPolicy;
use Illuminate\Support\Facades\Gate;
use LycheeVerify\Verify;
use Spatie\LaravelData\Data;
use Spatie\TypeScriptTransformer\Attributes\TypeScript;

Expand All @@ -33,6 +34,6 @@ public function __construct()
$this->can_see_diagnostics = Gate::check(SettingsPolicy::CAN_SEE_DIAGNOSTICS, [Configs::class]);
$this->can_update = Gate::check(SettingsPolicy::CAN_UPDATE, [Configs::class]);
$this->can_access_dev_tools = Gate::check(SettingsPolicy::CAN_ACCESS_DEV_TOOLS, [Configs::class]);
$this->can_acess_user_groups = Gate::check(UserGroupPolicy::CAN_LIST, [UserGroup::class]) && config('features.user-groups') === true;
$this->can_acess_user_groups = resolve(Verify::class)->check() && Gate::check(UserGroupPolicy::CAN_LIST, [UserGroup::class]) && config('features.user-groups') === true;
}
}
11 changes: 11 additions & 0 deletions app/Models/AccessPermission.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*
* @property int $id
* @property int|null $user_id
* @property int|null $user_group_id
* @property string|null $base_album_id
* @property bool $is_link_required
* @property string|null $password
Expand Down Expand Up @@ -120,6 +121,16 @@ public function user(): BelongsTo
return $this->belongsTo(User::class, 'user_id', 'id');
}

/**
* Return the relationship between an AccessPermission and its associated UserGroup.
*
* @return BelongsTo<UserGroup,$this>
*/
public function user_group(): BelongsTo
{
return $this->belongsTo(UserGroup::class, 'user_group_id', 'id');
}

/**
* Given an AccessPermission, duplicate its reccord.
* - Password is NOT transfered
Expand Down
11 changes: 9 additions & 2 deletions app/Models/BaseAlbumImpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,22 @@ public function access_permissions(): hasMany
*/
public function current_user_permissions(): AccessPermission|null
{
return $this->access_permissions->first(fn (AccessPermission $p) => $p->user_id !== null && $p->user_id === Auth::id());
if (Auth::guest()) {
return null; // No permissions for guests
}

$user = Auth::user();

return $this->access_permissions->first(fn (AccessPermission $p) => $p->user_id === $user->id)
?? $this->access_permissions->first(fn (AccessPermission $p) => in_array($p->user_group_id, $user->user_groups->map(fn ($g) => $g->id)->all(), true));
}

/**
* Returns the relationship between an album and its associated public permissions.
*/
public function public_permissions(): AccessPermission|null
{
return $this->access_permissions->first(fn (AccessPermission $p) => $p->user_id === null);
return $this->access_permissions->first(fn (AccessPermission $p) => $p->user_id === null && $p->user_group_id === null);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions app/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ class User extends Authenticatable implements WebAuthnAuthenticatable

protected $hidden = [];

protected $with = [
'user_groups',
];

/**
* Create a new Eloquent query builder for the model.
*
Expand Down
58 changes: 41 additions & 17 deletions app/Policies/AlbumQueryPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use App\Models\Builders\AlbumBuilder;
use App\Models\Builders\TagAlbumBuilder;
use App\Models\TagAlbum;
use App\Models\User;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Query\Builder as BaseBuilder;
use Illuminate\Support\Facades\Auth;
Expand Down Expand Up @@ -519,26 +520,49 @@ private function getComputedAccessPermissionSubQuery(bool $full = false): BaseBu
$select[] = APC::GRANTS_UPLOAD;
$select[] = APC::USER_ID;
}
$user_id = Auth::id();
if (Auth::guest()) {
return DB::table('access_permissions', APC::COMPUTED_ACCESS_PERMISSIONS)->select($select)->whereNull(APC::USER_ID)->whereNull(APC::USER_GROUP_ID);
}

return DB::table('access_permissions', APC::COMPUTED_ACCESS_PERMISSIONS)->select($select)
->when(
Auth::check(),
fn ($q1) => $q1
->where(APC::USER_ID, '=', $user_id)
->orWhere(
fn ($q2) => $q2->whereNull(APC::USER_ID)
->whereNotIn(
APC::COMPUTED_ACCESS_PERMISSIONS . '.' . APC::BASE_ALBUM_ID,
fn ($q3) => $q3->select('acc_per.' . APC::BASE_ALBUM_ID)
->from('access_permissions', 'acc_per')
->where(APC::USER_ID, '=', $user_id)
)
/** @var User $user */
$user = Auth::user();
// Collect the user groups of the current user.
/** @var int[] $user_groups */
$user_groups = $user->user_groups->map(fn ($g) => $g->id)->all();

return DB::table('access_permissions', APC::COMPUTED_ACCESS_PERMISSIONS)
->select($select)
// First select the permissions based on the user.
->where(APC::USER_ID, '=', $user->id)
// Then select the permissions based on the user groups.
->orWhere(
fn ($q2) => $q2->whereIn(
APC::COMPUTED_ACCESS_PERMISSIONS . '.' . APC::USER_GROUP_ID,
$user_groups
)
// and ensure that we already have not selected the user permissions.
// This is important to avoid selecting the user permissions twice.
->whereNotIn(
APC::COMPUTED_ACCESS_PERMISSIONS . '.' . APC::BASE_ALBUM_ID,
fn ($q3) => $q3->select('acc_per.' . APC::BASE_ALBUM_ID)
->from('access_permissions', 'acc_per')
->where(APC::USER_ID, '=', $user->id)
)
)
->when(
!Auth::check(),
fn ($q1) => $q1->whereNull(APC::USER_ID)
// Then select the public permissions.
->orWhere(
fn ($q2) => $q2->whereNull(APC::USER_ID)->whereNull(APC::USER_GROUP_ID)
->whereNotIn(
APC::COMPUTED_ACCESS_PERMISSIONS . '.' . APC::BASE_ALBUM_ID,
// Ensure that we already have not selected the user or group permissions.
fn ($q3) => $q3->select('acc_per.' . APC::BASE_ALBUM_ID)
->from('access_permissions', 'acc_per')
->where(APC::USER_ID, '=', $user->id)
->orWhereIn(
APC::COMPUTED_ACCESS_PERMISSIONS . '.' . APC::USER_GROUP_ID,
$user_groups
)
)
);
}

Expand Down
19 changes: 17 additions & 2 deletions database/factories/AccessPermissionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use App\Models\AccessPermission;
use App\Models\Album;
use App\Models\User;
use App\Models\UserGroup;
use Illuminate\Database\Eloquent\Factories\Factory;
use Illuminate\Support\Facades\Hash;

Expand Down Expand Up @@ -49,6 +50,7 @@ public function public()
return $this->state(function (array $attributes) {
return [
'user_id' => null,
'user_group_id' => null,
];
});
}
Expand All @@ -67,9 +69,22 @@ public function for_user(User $user)
return $this->state(function (array $attributes) use ($user) {
return [
'user_id' => $user->id,
'user_group_id' => null,
];
})->afterCreating(function (AccessPermission $perm) {
$perm->load('album', 'user');
$perm->load('album', 'user', 'user_group');
});
}

public function for_user_group(UserGroup $userGroup)
{
return $this->state(function (array $attributes) use ($userGroup) {
return [
'user_id' => null,
'user_group_id' => $userGroup->id,
];
})->afterCreating(function (AccessPermission $perm) {
$perm->load('album', 'user', 'user_group');
});
}

Expand Down Expand Up @@ -134,7 +149,7 @@ public function for_album(Album $album)
'base_album_id' => $album->id,
];
})->afterCreating(function (AccessPermission $perm) {
$perm->load('album', 'user');
$perm->load('album', 'user', 'user_group');
});
}
}
31 changes: 31 additions & 0 deletions tests/Feature_v2/Album/AlbumTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,37 @@ public function testGetAnon(): void
]);
}

public function testGetAsGroup(): void
{
$response = $this->actingAs($this->userWithGroup1)->getJsonWithData('Album', ['album_id' => $this->album1->id]);
$this->assertOk($response);
$response->assertJson([
'config' => [
'is_base_album' => true,
'is_model_album' => true,
'is_accessible' => true,
'is_password_protected' => false,
'is_search_accessible' => true,
],
'resource' => [
'id' => $this->album1->id,
'title' => $this->album1->title,
'albums' => [],
'photos' => [
[
'id' => $this->photo1->id,
],
[
'id' => $this->photo1b->id,
],
],
],
]);

$response->assertJsonCount(0, 'resource.albums');
$response->assertJsonCount(2, 'resource.photos');
}

public function testGetAsOwner(): void
{
$response = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album', ['album_id' => $this->tagAlbum1->id]);
Expand Down
2 changes: 1 addition & 1 deletion tests/Feature_v2/Album/SharingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public function testOverride(): void
'grants_upload' => true,
]);
$this->assertOk($response);
self::assertEquals(2, AccessPermission::where(APC::BASE_ALBUM_ID, '=', $this->album1->id)->count());
self::assertEquals(3, AccessPermission::where(APC::BASE_ALBUM_ID, '=', $this->album1->id)->count());

// Update sub album permission.
$response = $this->actingAs($this->userMayUpload1)->putJson('Sharing', [
Expand Down
12 changes: 12 additions & 0 deletions tests/Feature_v2/Base/BaseApiWithDataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ abstract class BaseApiWithDataTest extends BaseApiTest
protected AccessPermission $perm1;
protected AccessPermission $perm4;
protected AccessPermission $perm44;
protected AccessPermission $perm11;

protected UserGroup $group1;
protected UserGroup $group2;
Expand Down Expand Up @@ -147,6 +148,17 @@ public function setUp(): void
->grants_full_photo()
->create();

$this->perm11 = AccessPermission::factory()
->for_user_group($this->group1)
->for_album($this->album1)
->visible()
->grants_edit()
->grants_delete()
->grants_upload()
->grants_download()
->grants_full_photo()
->create();

$this->album5 = Album::factory()->as_root()->owned_by($this->admin)->create();

$this->withoutVite();
Expand Down
3 changes: 2 additions & 1 deletion tests/Traits/CatchFailures.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ protected function assertStatus(TestResponse $response, int|array $expectedStatu
}
$this->trimException($exception);
dump($exception);
// We remove 204 as it does not have content
// We remove 302 because it does not have json data.
} elseif (!in_array($response->getStatusCode(), [204, 302, ...$expectedStatusCodeArray], true)) {
$exception = $response->json();
$this->trimException($exception);
dump($exception);
}
PHPUnit::assertContains($response->getStatusCode(), $expectedStatusCodeArray);
}
Expand Down