[FIX] Missing sudo for technical models. Basic users not able to add attachments #289
Conversation
|
Hi @lmignon, |
| @@ -163,7 +163,7 @@ def write(self, vals): | |||
| @tools.ormcache() | |||
| def get_id_by_code_map(self): | |||
| """Return a dictionary with the code as key and the id as value.""" | |||
| return {rec.code: rec.id for rec in self.search([])} | |||
| return {rec.code: rec.id for rec in self.sudo().search([])} | |||
There was a problem hiding this comment.
@adrienpeiffer It's strange to have a sudo here but not in others methods. We should add tests to be sure that we cover all the cases.
There was a problem hiding this comment.
@adrienpeiffer Thank you for the fix. It would be nice to have unittest for such cases. Can you also add a file named 289.bugfix into the fs_attachment/readme/newsfragment and fs_storage/readme/newsfragment directories and describe the change and why this change. These files are used to maintain a human readable history of changes between versions.
| @@ -340,3 +340,40 @@ def test_storage_use_filename_obfuscation(self): | |||
| self.assertEqual(attachment.checksum, attachment.store_fname.split("/")[-1]) | |||
| self.assertEqual(attachment.checksum, attachment.fs_url.split("/")[-1]) | |||
| self.assertEqual(attachment.mimetype, "text/plain") | |||
|
|
|||
| def test_create_attachments_basic_user(self): | |||
There was a problem hiding this comment.
Thank you @adrienpeiffer for the fix and the test.
Can you squash the commit into one and rewrite the commit message. Since a lot of addons are managed into the same repo, it's a good habit to start the commit with the name of the impacted modules. Something like:
[FIX] fs_storage, fs_attachement: Missing sudo for technical models
Before this change, users with basic access rights was no more able to create an attachment. The problem was caused by the need to access system information to determine in which file system the attachment should be created, when this sensitive information is not accessible to everyone. The change consists of reading this information in sudo to bypass this security restriction in this specific context.
Before this change, users with basic access rights was no more able to create an attachment. The problem was caused by the need to access system information to determine in which file system the attachment should be created, when this sensitive information is not accessible to everyone. The change consists of reading this information in sudo to bypass this security restriction in this specific context.
adrienpeiffer
force-pushed
the
16.0-fix-sudo-fs-storage-ape
branch
from
71d3f0c
to
b866676
Compare
|
/ocabot merge patch |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at 0396c9a. Thanks a lot for contributing to OCA. ❤️ |
Basics users without settings access are not able to upload attachment