Benoît Viguier

Results 246 comments of Benoît Viguier

> Has anybody already checked whether the exception which are thrown by policies, guards and authenticator interact nicely with the exception handler such that the correct HTTP codes (i.e. 401...

> * I would therefore advocate to rename `AlbumAuthorizationProvider` into `AlbumQueryPolicy` and move the file into `app/Policies` as well (same for photos). I know that this is not a policy...

> Why not simply `throw new UnauthenticatedException();` and use the default message `'User is not authenticated'`? I am not sure if I like this new message. "Id cannot be null"...

> I would call this change in behavior surprising, because I assume users always expect to be able to do everything with photos (and album) owned by themselves. agreed. I...

I still need to rename the Policies method and I hope it should be good. :)

> However, I wonder if the check for ownership can simply be done in `PhotoPolicy::before` together with the admin check. I cannot think of a possible use case where an...

While I tried to refactor the `authorizeAlbumWrite`, I realize that it was a lot of copy paste. I am wondering if it would not be better to create multiple authorization...

> I only wonder what it would save us? At the moment you only call a single function authorizeAlbumWrite() and you want to replace it by using a trait which...

@nagmat84 See https://github.com/LycheeOrg/Lychee/pull/1435 for my proposition. :) It uses traits to defines the different most used _authorize_ cases: - Album (change properties) - AlbumAlbums (merge, move) - Photo (change properties)...

@nagmat84 I will have to trouble you with another round, because you requested some changes. :D