-
Notifications
You must be signed in to change notification settings - Fork 297
Fix Broken Access Control (CWE-285) and update CLAUDE.md #163
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
Changes from 4 commits
c4ce745
ff0d467
4a7360d
9133e00
7485bb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| <?php | ||
|
|
||
| namespace App\Models; | ||
|
|
||
| use Illuminate\Database\Eloquent\Relations\BelongsTo; | ||
| use Spatie\MediaLibrary\MediaCollections\Models\Media as BaseMedia; | ||
|
|
||
| class Media extends BaseMedia | ||
| { | ||
| protected $fillable = [ | ||
| 'user_id', | ||
| 'model_type', | ||
| 'model_id', | ||
| 'uuid', | ||
| 'collection_name', | ||
| 'name', | ||
| 'file_name', | ||
| 'mime_type', | ||
| 'disk', | ||
| 'conversions_disk', | ||
| 'size', | ||
| 'manipulations', | ||
| 'custom_properties', | ||
| 'generated_conversions', | ||
| 'responsive_images', | ||
| 'order_column', | ||
| ]; | ||
|
|
||
| protected static function booted(): void | ||
| { | ||
| static::creating(function (Media $media) { | ||
| // Auto-assign current user ID when creating | ||
| if (empty($media->user_id) && auth()->check()) { | ||
| $media->user_id = auth()->id(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| public function user(): BelongsTo | ||
| { | ||
| return $this->belongsTo(User::class); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| <?php | ||
|
|
||
| namespace App\Policies; | ||
|
|
||
| use App\Models\Article; | ||
| use App\Models\User; | ||
|
|
||
| class ArticlePolicy | ||
| { | ||
| /** | ||
| * Determine whether the user can view any models. | ||
| */ | ||
| public function viewAny(User $user): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can view the model. | ||
| */ | ||
| public function view(User $user, Article $article): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can create models. | ||
| */ | ||
| public function create(User $user): bool | ||
| { | ||
| return $user->isAdmin() || $user->isEditor(); | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can update the model. | ||
| */ | ||
| public function update(User $user, Article $article): bool | ||
| { | ||
| // Admin can update anything, Editor can only update their own | ||
| return $user->isAdmin() || $article->user_id === $user->id; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can delete the model. | ||
| */ | ||
| public function delete(User $user, Article $article): bool | ||
| { | ||
| // Admin can delete anything, Editor can only delete their own | ||
| return $user->isAdmin() || $article->user_id === $user->id; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can restore the model. | ||
| */ | ||
| public function restore(User $user, Article $article): bool | ||
| { | ||
| return $user->isAdmin() || $article->user_id === $user->id; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can permanently delete the model. | ||
| */ | ||
| public function forceDelete(User $user, Article $article): bool | ||
| { | ||
| return $user->isAdmin(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| <?php | ||
|
|
||
| namespace App\Policies; | ||
|
|
||
| use App\Models\Media; | ||
| use App\Models\User; | ||
|
|
||
| class MediaPolicy | ||
| { | ||
| /** | ||
| * Determine whether the user can view any models. | ||
| */ | ||
| public function viewAny(User $user): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can view the model. | ||
| */ | ||
| public function view(User $user, Media $media): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can create models. | ||
| */ | ||
| public function create(User $user): bool | ||
| { | ||
| return $user->isAdmin() || $user->isEditor(); | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can update the model. | ||
| */ | ||
| public function update(User $user, Media $media): bool | ||
| { | ||
| // Admin can update anything, Editor can only update their own | ||
| return $user->isAdmin() || $media->user_id === $user->id; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can delete the model. | ||
| */ | ||
| public function delete(User $user, Media $media): bool | ||
| { | ||
| // Admin can delete anything, Editor can only delete their own | ||
| return $user->isAdmin() || $media->user_id === $user->id; | ||
| } | ||
|
Comment on lines
+37
to
+50
|
||
|
|
||
| /** | ||
| * Determine whether the user can restore the model. | ||
| */ | ||
| public function restore(User $user, Media $media): bool | ||
| { | ||
| return $user->isAdmin() || $media->user_id === $user->id; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether the user can permanently delete the model. | ||
| */ | ||
| public function forceDelete(User $user, Media $media): bool | ||
| { | ||
| return $user->isAdmin(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New ownership-based authorization is introduced here, but there are no corresponding feature tests asserting that an Editor cannot update/delete an Article owned by an Admin (and can still manage their own). Add coverage (e.g., in
tests/Feature/AdminCrudTest.phpor a dedicated security test) that exercises the Filament delete/edit actions and verifies 403/denial for non-owners.