Skip to content
This repository has been archived by the owner on Jul 8, 2023. It is now read-only.

HasObjPerm directive on a update mutation fails updating the cache, due to unhashable type OperationInfo #158

Closed
gersmann opened this issue Dec 27, 2022 · 7 comments

Comments

@gersmann
Copy link
Contributor

gersmann commented Dec 27, 2022

Specifying a HasObjPerm directive on a update mutation fails updating the cache, due to unhashable type OperationInfo. This is due to the resolver returning OperationInfo instead of FieldValueType, if the user does not have the expected permissions (in this casereporting.change_fieldvalue).

The directive works for the 'good' case, where the permission is granted.

I am wondering if I am missing something, or what the correct way of using object permissions on mutations would be. Should there be an exception on return value permission checking for OperationInfo?

Mutation Setup:

update_field_value: FieldValueType = gql.django.update_mutation(
        FieldValueInput,
        directives=[HasObjPerm("reporting.change_fieldvalue")],
    )

Stack:

  File ".../app-QPiINJSB-py3.11/lib/python3.11/site-packages/strawberry_django_plus/utils/aio.py", line 156, in resolve
    ret = resolver(value)
          ^^^^^^^^^^^^^^^
  File ".../app-QPiINJSB-py3.11/lib/python3.11/site-packages/strawberry_django_plus/permissions.py", line 634, in resolve_for_user
    return self._resolve_obj_perms(helper, root, info, user, ret)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../app-QPiINJSB-py3.11/lib/python3.11/site-packages/strawberry_django_plus/permissions.py", line 695, in _resolve_obj_perms
    has_perm = cache.get((self, obj))
               ^^^^^^^^^^^^^^^^^^^^^^
TypeError: unhashable type: 'OperationInfo'
@bellini666
Copy link
Member

Hey @gersmann . This seems to be indeed a bug. I think OperationInfo should be hashable.

I'll try to fix it here right now

@gersmann
Copy link
Contributor Author

Ah, nice, thank you @bellini666.

I am wondering if OperationInfo will still be checked for permissions, I think so from the current control flow? Should the RetVal permission check maybe get skipped for it, bc I don't think that the Django permission backends will be able to handle it?

@bellini666
Copy link
Member

I am wondering if OperationInfo will still be checked for permissions, I think so from the current control flow? Should the RetVal permission check maybe get skipped for it, bc I don't think that the Django permission backends will be able to handle it?

What do you mean?

@gersmann
Copy link
Contributor Author

Nevermind, misunderstanding on my side. Works like a charm now.

@gersmann
Copy link
Contributor Author

Actually, not really, that is what I meant:

image

I'd expect has_perm to only get called with Django model objects?

@bellini666
Copy link
Member

@gersmann oh, I see what you mean. Please check if https://github.com/blb-ventures/strawberry-django-plus/releases/tag/v1.33.1 fixes that

@gersmann
Copy link
Contributor Author

@bellini666 yes, all good. Thank you.

frleb pushed a commit to frleb/strawberry-django-plus that referenced this issue Feb 28, 2023
…-class

Documentation for overriding the field class
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants