[feature] Allowed read only access of shared objects to non-superusers #238 #444
Conversation
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
3 times, most recently
from
0929c13 to
80511df
Compare
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
2 times, most recently
from
e72b20e to
c1da85c
Compare
github-project-automation
bot
moved this to In progress
in OpenWISP Priorities for next releases
There was a problem hiding this comment.
I haven't performed manual testing yet, I should be able to do this tomorrow.
In the meantime I have looked at the code and left some comments below.
I also noticed two more important points:
- Docs: the documentation has a section dedicated to shared objects, I think it's worth to update the text to reflect the functionality achieved in this PR.
- DRF 1.6: did you address this point which is listed in the issue description?
tests/testapp/tests/test_selenium.py
Outdated
| def test_organization_autocomplete_filter(self): | ||
| """ | ||
| The autocomplete_filter should show option to filter | ||
| shared objects to non-superuser. |
There was a problem hiding this comment.
I am not sure I fully understand this docstring/comment: should the option to filter shared objects appear only to users who have the is_superuser flag set to False? Or what does this text mean exactly? Let's make it unambiguous.
| django-phonenumber-field~=8.1.0 | ||
| phonenumbers~=9.0.4 | ||
| openwisp-utils[rest,celery] @ https://github.com/openwisp/openwisp-utils/tarball/1.2 | ||
| openwisp-utils[rest,celery] @ https://github.com/openwisp/openwisp-utils/tarball/bump-drf |
There was a problem hiding this comment.
TODO: Revert this before merging
There was a problem hiding this comment.
This is what I found during testing in openwisp-controller:
openwisp/openwisp-controller#1024 (review)
We'll also need a way to protect some sensitive fields like credentials parameters, can you create a subissue for this and link it to the parent issue please? We should work on that as soon as this PR is ready.
pandafy
changed the title
pandafy
moved this from To do (general)
to Needs review
in OpenWISP Contributor's Board
pandafy
moved this from In progress
to Ready for review/testing
in OpenWISP Priorities for next releases
docs/user/basic-concepts.rst
Outdated
| objects. However, they can use these shared objects when creating related | ||
| organization-specific resources. E.g., an organization manager can use a | ||
| shared VPN server to create a configuration template for their | ||
| organization. |
There was a problem hiding this comment.
Let's mention this works both in admin and REST API, let's mention that non superusers may be prevented to viewing sensitive details of certain shared objects which would allow them to gain unauthorized access to resources used by other organizations.
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
from
fd88b21 to
4c181e0
Compare
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
from
4c181e0 to
65c6b17
Compare
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
from
b1a8dc9 to
e81b36f
Compare
pandafy
force-pushed
the
issues/238-view-shared-objects
branch
from
e81b36f to
f348ee4
Compare
openwisp_users/api/mixins.py
Outdated
| and not request.user.is_superuser | ||
| and "organization" in obj | ||
| and obj["organization"] is None |
There was a problem hiding this comment.
WIP: The organization field will be made configurable.
Checklist
Reference to Existing Issue
Closes #238
TODOS
-[x] DRF integration
-[x] Tests