Skip to content
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

Authorizer contract needs separate methods for show-related and show-relationship actions #6

Closed
Mina-R-Meshriky opened this issue Jun 10, 2021 · 3 comments

Comments

@Mina-R-Meshriky
Copy link

public function showRelationship(Request $request, object $model, string $fieldName): bool;

lets assume we have a companySchema and branchSchema and they have a m-t-m relationship
lets also assume that a user has permission to 'index', 'show', 'showRelationship' of the branchSchema but does not have any permissions to the companySchema using the Authorizer.

if that user used the url: domain.com/api/v1/branches/{branch_id}/companies he will be able to see the data of each company ?

@lindyhopchris
Copy link
Contributor

"No" is the answer. The two are answering different questions.

The index action for the Branch model is answering the question: Can the current user view all branches?

The show branch relationship action on the Company model is answering the question: Can the current user view this specific company's branches?

As you can see, the questions are different, which is why they need separate authorisation methods.

@Mina-R-Meshriky
Copy link
Author

Mina-R-Meshriky commented Jun 10, 2021

No, maybe the index was a poor choice but the problem is
the same authorizer is used in both showRelated and showRelationship which are two separate routes
now the showRelated will show data of the related schema, which the user might not have permission to see (if he can't use index or show on the related schema I mean) but he will be able to get the data from 'showRelated', which is strange since he can't see the main resource.
but at the same time, I need to show him that his branch is related to several companies
So, maybe the authorizer can have two separate methods for showRelated and showRelationship

Edit: Ok maybe I am overthinking it

@lindyhopchris
Copy link
Contributor

Ok, so the title of this issue and the description were a bit misleading! Effectively you're asking for show related and show relationship to have separate authorization methods on the interface.

I'd be happy to make that change - however I would keep our default implementation doing the same thing i.e. using the same method on the policy to authorize both. However, by adding the additional method to the authorizer interface, you would have the opportunity to do things different by writing your own authorizer (or extending ours, and just overloading the single method you needed to change).

The reason for not wanting to change our default implementation is I would not want to force people to implement two methods on their policy for something that is done as one method at the moment. Particularly because in most apps, the logic for reading the relationship (whether it is the relates resources or the relationship identifiers) is identical.

I'm very limited on time at the moment, and this change won't be a priority for me as I won't need it in any of the apps I work on. Can you submit a PR?

@lindyhopchris lindyhopchris reopened this Jun 11, 2021
@lindyhopchris lindyhopchris changed the title shouldn't the show related route be authorized by the related index authorizer ? Authorizer contract needs separate methods for show-related and show-relationship actions Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants