diff --git a/app/Http/Controllers/MovieListController.php b/app/Http/Controllers/MovieListController.php index 5da35d7..1ede17b 100644 --- a/app/Http/Controllers/MovieListController.php +++ b/app/Http/Controllers/MovieListController.php @@ -8,6 +8,7 @@ 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; @@ -33,18 +34,18 @@ class MovieListController extends Controller /** * Store a newly created resource in storage. */ - public function store(CreateMovieListRequest $request): MovieListResource + public function store(CreateMovieListRequest $request) { $this->authorize('create', MovieList::class); $validated = $request->validated(); $movieList = MovieList::create([ ...$validated, - 'owner' => Auth::user()->id, + 'owner' => auth()->id(), 'slug' => Str::slug($validated['name']), ]); - return MovieListResource::make($movieList); + return response()->json($movieList, 201); } /** @@ -76,13 +77,12 @@ 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('editMovies', $movieList); - + $this->authorize('update', $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('editMovies', $movieList); + $this->authorize('update', $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', ]); - if (Auth::id() === $collaborator->getKey()) { - return response()->json(['message' => 'Cannot edit own role'], 422); + $adminRole = Role::query()->where('name', 'ADMIN')->first()?->id; + if (Auth::id() !== $movieList->owner && ! Auth::user()->hasRole($movieList, $adminRole)) { + return response()->json(['message' => 'Unauthorized'], 403); } $movieList->collaborators()->updateExistingPivot($collaborator->getKey(), [ diff --git a/app/Models/MovieList.php b/app/Models/MovieList.php index 5ed423e..d663eda 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 8fac25a..fc76c80 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -15,10 +15,6 @@ 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. * @@ -49,33 +45,10 @@ class User extends Authenticatable return $this->hasMany(MovieList::class, 'owner'); } - 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 + public function hasRole(MovieList $movieList, int $role): bool { return $this->sharedLists() - ->wherePivot('movie_list_id', $movieListId) + ->wherePivot('movie_list_id', $movieList->id) ->wherePivot('role_id', $role) ->exists(); } @@ -87,13 +60,6 @@ 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 132f366..91cc5cb 100644 --- a/app/Policies/MovieListPolicy.php +++ b/app/Policies/MovieListPolicy.php @@ -22,23 +22,29 @@ class MovieListPolicy public function view(User $user, MovieList $movieList): bool { - return $movieList->is_public - || $user->isListOwner($movieList) - || $user->sharedLists->contains($movieList); - } + if ($movieList->owner === $user->getKey() || $movieList->isPublic || $user->sharedLists->contains($movieList)) { + return true; + } - public function delete(User $user, MovieList $movieList): bool - { - return $user->isListOwner($movieList); - } - - public function editMovies(User $user, MovieList $movieList): bool - { - return $user->isListEditor($movieList); + return false; } public function update(User $user, MovieList $movieList): bool { - return $user->isListAdmin($movieList); + + if ($movieList->owner === $user->getKey()) { + return true; + } + + return false; + } + + public function delete(User $user, MovieList $movieList): bool + { + if ($movieList->owner === $user->getKey()) { + return true; + } + + return false; } } diff --git a/routes/api.php b/routes/api.php index b84e7de..28a88f3 100644 --- a/routes/api.php +++ b/routes/api.php @@ -10,6 +10,7 @@ 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'); @@ -17,7 +18,6 @@ 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'); diff --git a/tests/Feature/AuthTest.php b/tests/Feature/AuthTest.php index 815f44f..34b95ea 100644 --- a/tests/Feature/AuthTest.php +++ b/tests/Feature/AuthTest.php @@ -16,6 +16,8 @@ class AuthTest extends TestCase ->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', + 'password' => 'password123', + 'password_confirmation' => 'password123', ]); $response->assertStatus(201) @@ -29,10 +31,12 @@ 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']); + ->assertJsonValidationErrors(['username', 'email', 'password']); } public function test_registration_fails_with_duplicate_email(): void @@ -42,6 +46,8 @@ class AuthTest extends TestCase $response = $this->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', + 'password' => 'password123', + 'password_confirmation' => 'password123', ]); $response->assertStatus(422) @@ -55,6 +61,8 @@ class AuthTest extends TestCase $response = $this->postJson('/api/register', [ 'username' => 'johndoe', 'email' => 'john@example.com', + 'password' => 'password123', + 'password_confirmation' => 'password123', ]); $response->assertStatus(422) @@ -103,8 +111,18 @@ class AuthTest extends TestCase public function test_unauthenticated_user_cannot_access_protected_routes(): void { - $response = $this->getJson('/api/roles'); + $response = $this->getJson('/api/user'); $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 6931615..f800019 100644 --- a/tests/Feature/UpdateCollaboratorRoleTest.php +++ b/tests/Feature/UpdateCollaboratorRoleTest.php @@ -14,11 +14,28 @@ 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(); @@ -33,15 +50,6 @@ 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(); @@ -117,26 +125,6 @@ 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(); @@ -152,14 +140,4 @@ 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(); - } }