feat: check permission based on logged in user guard #1384
Conversation
|
@Daniyal-Javani @musapinar |
|
I'm not sure the update to the docs is correct. I think the correct new explanation is more along the lines of the following (which also feels too verbose for the docs):
Keep in mind that sometimes there won't be an actual match for the logged-in user:
It's this last case (create) that worries me. What about an Administrator, where your app has used an Admin login guard, and you're creating permissions without specifying a guard, and now it's going to default to the admin guard instead of the web guard which non-admins use? |
Daniyal-Javani
force-pushed
the
feature/feature/check-permission-based-on-logged-in-users-guard
branch
from
003e264
to
645a372
Compare
Updated docblocks to add clarity regarding the lookup flow
drbyte
force-pushed
the
feature/feature/check-permission-based-on-logged-in-users-guard
branch
from
232a0e9
to
d4128e0
Compare
|
I think this solution is way too deep into the logic. This can (maybe) conflict with admin panels where I want to create auth user independent roles and permission. I would prefer to pass the auth user guard name during the gate check to |
I'd be interested in seeing an implementation of that where the current tests pass. I recall exploring that sort of approach but finding several tests failing (where refactoring those tests wasn't relevant to the code change). |
|
Dear contributor, because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it. |
…logged-in-users-guard
|
Dear contributor, because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it. |
Historically, since v2.0.0, the "detected guard_name" that was applied during lookups was detected from the "first possible defined guard" for that model, even if the "current" guard wasn't "first in the list". This has led to some confusion when expecting that the current user's guard would actually be used. This PR changes things to **use the current user's guard** first, assuming it's in the list of potential guards acceptable for the current User model. After that it will fallback to the old behavior of selecting the first available matchable guard for the current User model. THIS IS A BREAKING CHANGE, so be sure to test all authorization aspects of your application to ensure desired behavior. You MAY also be able to remove some old complex monkey-patching that you might have done to work around this issue in prior versions. Fixes #1682 Fixes #1608 Fixes #1384 Fixes #1515 Fixes #1516
…d during lookups was detected from the "first possible defined guard" for that model, even if the "current" guard wasn't "first in the list". This has led to some confusion when expecting that the current user's guard would actually be used. This PR changes things to **use the current user's guard** first, assuming it's in the list of potential guards acceptable for the current User model. After that it will fallback to the old behavior of selecting the first available matchable guard for the current User model. THIS IS A BREAKING CHANGE, so be sure to test all authorization aspects of your application to ensure desired behavior. You MAY also be able to remove some old complex monkey-patching that you might have done to work around this issue in prior versions. Fixes spatie#1682 Fixes spatie#1608 Fixes spatie#1384 Fixes spatie#1515 Fixes spatie#1516
@musapinar idea to fix #1313