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

Warn if the permission does not exist #515

Closed
gforcada opened this issue Oct 11, 2023 · 3 comments · Fixed by #526
Closed

Warn if the permission does not exist #515

gforcada opened this issue Oct 11, 2023 · 3 comments · Fixed by #526

Comments

@gforcada
Copy link
Sponsor Member

I was doing some tests using api.user.has_permission and the test was failing 💥 but of course I should be right 🤣

Turns out that I have a permission:

  <permission
    id="der.freitag.my-permission"
    title="der.freitag: Cool stuff"
    />

And my api call was:

api.user.has_permission(
    'der.freitag.my-permission',
    obj=self.portal.autoren[user1['USER_ID']])
)

But, actually this is indeed wrong, the right call is:

api.user.has_permission(
    'der.freitag: Cool stuff',
    obj=self.portal.autoren[user1['USER_ID']])
)

i.e. one has to use the title of the permission, not the id! 🤦🏾

It would be nice if either plone.api could use either of them, or otherwise warn the user if the permission does not exist at all.

I can hardly think of any valid/reasonable use case where you don't want the permission that you are checking to not exist.

As a first step, mentioning it on the documentation it would be helpful ✨

@davisagli
Copy link
Sponsor Member

It would be nice if either plone.api could use either of them

This doesn't sound nice to me; there could be some edge case where the id of one permission matches the title of a different one, and then you are checking the wrong one and have a security bug.

otherwise warn the user if the permission does not exist at all.

This does sound helpful

@ale-rt
Copy link
Member

ale-rt commented Apr 29, 2024

It would be nice if either plone.api could use either of them

This doesn't sound nice to me; there could be some edge case where the id of one permission matches the title of a different one, and then you are checking the wrong one and have a security bug.

I agree.

Maybe that could be achieved by having a function signature that would allow making that possible, something like:

@mutually_exclusive_parameters("username", "user")
@mutually_exclusive("permission", "permission_id")
def has_permission(permission=None, username=None, user=None, obj=None, permission_id=None):
     ...

I am slightly -1 on that because so far the need did not surface.

@gforcada
Copy link
Sponsor Member Author

I did not go that route, as you say, if it's asked later we can implement it.

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

Successfully merging a pull request may close this issue.

3 participants