From 6cfcbc2d107dfe37bed31cc6a8e9e77ecadf6663 Mon Sep 17 00:00:00 2001 From: "Edward Tirado Jr." Date: Sat, 18 Apr 2026 00:47:26 -0500 Subject: [PATCH 1/2] added tests for role updating --- tests/Feature/AuthTest.php | 22 +------ tests/Feature/UpdateCollaboratorRoleTest.php | 60 +++++++++++++------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/tests/Feature/AuthTest.php b/tests/Feature/AuthTest.php index 34b95ea..815f44f 100644 --- a/tests/Feature/AuthTest.php +++ b/tests/Feature/AuthTest.php @@ -16,8 +16,6 @@ class AuthTest extends TestCase ->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', - 'password' => 'password123', - 'password_confirmation' => 'password123', ]); $response->assertStatus(201) @@ -31,12 +29,10 @@ class AuthTest extends TestCase $response = $this->postJson('/api/register', [ 'username' => '', 'email' => 'not-an-email', - 'password' => 'short', - 'password_confirmation' => 'mismatch', ]); $response->assertStatus(422) - ->assertJsonValidationErrors(['username', 'email', 'password']); + ->assertJsonValidationErrors(['username', 'email']); } public function test_registration_fails_with_duplicate_email(): void @@ -46,8 +42,6 @@ class AuthTest extends TestCase $response = $this->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', - 'password' => 'password123', - 'password_confirmation' => 'password123', ]); $response->assertStatus(422) @@ -61,8 +55,6 @@ class AuthTest extends TestCase $response = $this->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', - 'password' => 'password123', - 'password_confirmation' => 'password123', ]); $response->assertStatus(422) @@ -111,18 +103,8 @@ class AuthTest extends TestCase public function test_unauthenticated_user_cannot_access_protected_routes(): void { - $response = $this->getJson('/api/user'); + $response = $this->getJson('/api/roles'); $response->assertStatus(401); } - - public function test_authenticated_user_can_access_user_endpoint(): void - { - $user = User::factory()->create(); - - $response = $this->actingAs($user)->getJson('/api/user'); - - $response->assertOk() - ->assertJsonFragment(['email' => $user->email]); - } } diff --git a/tests/Feature/UpdateCollaboratorRoleTest.php b/tests/Feature/UpdateCollaboratorRoleTest.php index f800019..6931615 100644 --- a/tests/Feature/UpdateCollaboratorRoleTest.php +++ b/tests/Feature/UpdateCollaboratorRoleTest.php @@ -14,28 +14,11 @@ class UpdateCollaboratorRoleTest extends TestCase use RefreshDatabase; private Role $adminRole; + private Role $editorRole; + private Role $viewerRole; - protected function setUp(): void - { - parent::setUp(); - $this->seed(DatabaseSeeder::class); - - $this->adminRole = Role::where('name', 'ADMIN')->first(); - $this->editorRole = Role::where('name', 'EDITOR')->first(); - $this->viewerRole = Role::where('name', 'VIEWER')->first(); - } - - private function makeList(User $owner): MovieList - { - return MovieList::create([ - 'name' => 'Test List', - 'owner' => $owner->getKey(), - 'slug' => 'test-list', - ]); - } - public function test_role_id_is_required(): void { $owner = User::factory()->create(); @@ -50,6 +33,15 @@ class UpdateCollaboratorRoleTest extends TestCase ->assertJsonValidationErrors(['role_id']); } + private function makeList(User $owner): MovieList + { + return MovieList::create([ + 'name' => 'Test List', + 'owner' => $owner->getKey(), + 'slug' => 'test-list', + ]); + } + public function test_role_id_must_exist_in_roles_table(): void { $owner = User::factory()->create(); @@ -125,6 +117,26 @@ class UpdateCollaboratorRoleTest extends TestCase $response->assertForbidden(); } + public function test_admin_collaborator_cannot_update_own_role(): void + { + $owner = User::factory()->create(); + $admin = User::factory()->create(); + $movieList = $this->makeList($owner); + $movieList->collaborators()->attach($admin, ['role_id' => $this->adminRole->getKey()]); + + $response = $this->actingAs($admin) + ->patchJson("/api/movielists/{$movieList->getKey()}/collaborators/{$admin->getKey()}", [ + 'role_id' => $this->editorRole->getKey(), + ]); + + $response->assertUnprocessable(); + $this->assertDatabaseHas('movie_list_user', [ + 'movie_list_id' => $movieList->getKey(), + 'user_id' => $admin->getKey(), + 'role_id' => $this->adminRole->getKey(), + ]); + } + public function test_unrelated_user_cannot_update_collaborator_role(): void { $owner = User::factory()->create(); @@ -140,4 +152,14 @@ class UpdateCollaboratorRoleTest extends TestCase $response->assertForbidden(); } + + protected function setUp(): void + { + parent::setUp(); + $this->seed(DatabaseSeeder::class); + + $this->adminRole = Role::where('name', 'ADMIN')->first(); + $this->editorRole = Role::where('name', 'EDITOR')->first(); + $this->viewerRole = Role::where('name', 'VIEWER')->first(); + } } From 30f0582214a6868041bd5069ffd508ccb923b455 Mon Sep 17 00:00:00 2001 From: "Edward Tirado Jr." Date: Sat, 18 Apr 2026 00:48:03 -0500 Subject: [PATCH 2/2] cleaned up movie list policy --- app/Http/Controllers/MovieListController.php | 20 +++++------ app/Models/MovieList.php | 2 +- app/Models/User.php | 38 ++++++++++++++++++-- app/Policies/MovieListPolicy.php | 32 +++++++---------- routes/api.php | 2 +- 5 files changed, 61 insertions(+), 33 deletions(-) diff --git a/app/Http/Controllers/MovieListController.php b/app/Http/Controllers/MovieListController.php index 1ede17b..5da35d7 100644 --- a/app/Http/Controllers/MovieListController.php +++ b/app/Http/Controllers/MovieListController.php @@ -8,7 +8,6 @@ use App\Http\Resources\MovieListResource; use App\Interfaces\MovieDbInterface; use App\Models\Movie; use App\Models\MovieList; -use App\Models\Role; use App\Models\User; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -34,18 +33,18 @@ class MovieListController extends Controller /** * Store a newly created resource in storage. */ - public function store(CreateMovieListRequest $request) + public function store(CreateMovieListRequest $request): MovieListResource { $this->authorize('create', MovieList::class); $validated = $request->validated(); $movieList = MovieList::create([ ...$validated, - 'owner' => auth()->id(), + 'owner' => Auth::user()->id, 'slug' => Str::slug($validated['name']), ]); - return response()->json($movieList, 201); + return MovieListResource::make($movieList); } /** @@ -77,12 +76,13 @@ class MovieListController extends Controller $this->authorize('delete', $movieList); $movieList->delete(); - return response()->json(['message', 'Movie list deleted successfully'], 204); + return response()->json(['message' => 'Movie list deleted successfully'], 204); } public function addMovie(MovieDbInterface $movieDb, Request $request, MovieList $movieList): MovieListResource { - $this->authorize('update', $movieList); + $this->authorize('editMovies', $movieList); + $movieResult = $movieDb->find($request->input('movie')['imdbId'], ['type' => 'imdb']); $movie = Movie::where('imdb_id', $movieResult->imdbId)->first(); @@ -94,7 +94,7 @@ class MovieListController extends Controller public function removeMovie(MovieList $movieList, Movie $movie): MovieListResource { - $this->authorize('update', $movieList); + $this->authorize('editMovies', $movieList); $movieList->movies()->detach($movie); $movieList->load('movies'); @@ -104,13 +104,13 @@ class MovieListController extends Controller public function updateCollaboratorRole(Request $request, MovieList $movieList, User $collaborator): MovieListResource|JsonResponse { + $this->authorize('update', $movieList); $request->validate([ 'role_id' => 'required|exists:roles,id', ]); - $adminRole = Role::query()->where('name', 'ADMIN')->first()?->id; - if (Auth::id() !== $movieList->owner && ! Auth::user()->hasRole($movieList, $adminRole)) { - return response()->json(['message' => 'Unauthorized'], 403); + if (Auth::id() === $collaborator->getKey()) { + return response()->json(['message' => 'Cannot edit own role'], 422); } $movieList->collaborators()->updateExistingPivot($collaborator->getKey(), [ diff --git a/app/Models/MovieList.php b/app/Models/MovieList.php index d663eda..5ed423e 100644 --- a/app/Models/MovieList.php +++ b/app/Models/MovieList.php @@ -27,7 +27,7 @@ class MovieList extends Model return $this->belongsToMany(Movie::class); } - public function getUserRole($userId): string + public function getUserRole($userId): ?string { $roleId = $this->collaborators() ->where('user_id', $userId) diff --git a/app/Models/User.php b/app/Models/User.php index fc76c80..8fac25a 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -15,6 +15,10 @@ class User extends Authenticatable /** @use HasFactory<\Database\Factories\UserFactory> */ use HasFactory, Notifiable; + private static $adminRoleId = null; + + private static $editorRoleId = null; + /** * The attributes that are mass assignable. * @@ -45,10 +49,33 @@ class User extends Authenticatable return $this->hasMany(MovieList::class, 'owner'); } - public function hasRole(MovieList $movieList, int $role): bool + public function isListEditor(MovieList $movieList): bool + { + self::$editorRoleId = Role::query() + ->where('name', 'EDITOR') + ->value('id'); + + return $this->isListAdmin($movieList) || $this->hasRole($movieList->getKey(), self::$editorRoleId); + } + + public function isListAdmin(MovieList $movieList): bool + { + self::$adminRoleId = Role::query() + ->where('name', 'ADMIN') + ->value('id'); + + return $this->isListOwner($movieList) || $this->hasRole($movieList->getKey(), self::$adminRoleId); + } + + public function isListOwner(MovieList $movieList): bool + { + return $this->getKey() === $movieList->owner; + } + + public function hasRole(int $movieListId, int $role): bool { return $this->sharedLists() - ->wherePivot('movie_list_id', $movieList->id) + ->wherePivot('movie_list_id', $movieListId) ->wherePivot('role_id', $role) ->exists(); } @@ -60,6 +87,13 @@ class User extends Authenticatable ->withTimestamps(); } + public function roles(): BelongsToMany + { + return $this->belongsToMany(Role::class, 'movie_list_user') + ->withPivot('role_id') + ->withTimestamps(); + } + /** * Get the attributes that should be cast. * diff --git a/app/Policies/MovieListPolicy.php b/app/Policies/MovieListPolicy.php index 91cc5cb..132f366 100644 --- a/app/Policies/MovieListPolicy.php +++ b/app/Policies/MovieListPolicy.php @@ -22,29 +22,23 @@ class MovieListPolicy public function view(User $user, MovieList $movieList): bool { - if ($movieList->owner === $user->getKey() || $movieList->isPublic || $user->sharedLists->contains($movieList)) { - return true; - } - - return false; - } - - public function update(User $user, MovieList $movieList): bool - { - - if ($movieList->owner === $user->getKey()) { - return true; - } - - return false; + return $movieList->is_public + || $user->isListOwner($movieList) + || $user->sharedLists->contains($movieList); } public function delete(User $user, MovieList $movieList): bool { - if ($movieList->owner === $user->getKey()) { - return true; - } + return $user->isListOwner($movieList); + } - return false; + public function editMovies(User $user, MovieList $movieList): bool + { + return $user->isListEditor($movieList); + } + + public function update(User $user, MovieList $movieList): bool + { + return $user->isListAdmin($movieList); } } diff --git a/routes/api.php b/routes/api.php index 28a88f3..b84e7de 100644 --- a/routes/api.php +++ b/routes/api.php @@ -10,7 +10,6 @@ use Illuminate\Support\Facades\Route; // Public auth routes Route::post('/register', [AuthController::class, 'register'])->name('auth.register'); Route::post('/login', [AuthController::class, 'login'])->name('auth.login'); -Route::post('/reset-password', [AuthController::class, 'resetPassword'])->name('auth.reset-password'); Route::post('/forgot-password', [AuthController::class, 'forgotPassword'])->name('auth.forgot-password'); Route::get('/invitations/{token}/accept', [InvitationController::class, 'accept'])->name('invitations.accept'); Route::get('/invitations/{token}/decline', [InvitationController::class, 'decline'])->name('invitations.decline'); @@ -18,6 +17,7 @@ Route::get('/invitations/{token}/decline', [InvitationController::class, 'declin // Authenticated routes Route::middleware('auth:sanctum')->group(function () { Route::post('/logout', [AuthController::class, 'logout'])->name('auth.logout'); + Route::post('/reset-password', [AuthController::class, 'resetPassword'])->name('auth.reset-password'); // Invitations Route::post('/invitations', [InvitationController::class, 'store'])->name('invitations.store');