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

[FW][FIX] mail: allow bypassing message attachments check #165173

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented May 12, 2024

We only care about data fields when it comes to restricting access to message attachments.
As some modules may use these attachments directly, it may be desirable for them to use
the attachment name or similar metadata fields.

"write" is only restricted if writing on a data field, as it's effectively the same as unlinking
for our purposes. Other fields have the same access rights as prior to [1]

Additional changes:

  • Admins can delete attachments from message in the UI again
  • Check returns True instead of None when it succeeds, to match its parent
  • AccessError is raised with a cause so that we can detect if it was raised because of the message check

1: 4c4e63f

task-3519815

Forward-Port-Of: #165166
Forward-Port-Of: #164894

@robodoo
Copy link
Contributor

robodoo commented May 12, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented May 12, 2024

@reth-odoo @awa-odoo cherrypicking of pull request #164894 failed.

stdout:

Auto-merging addons/mail/i18n/mail.pot
CONFLICT (content): Merge conflict in addons/mail/i18n/mail.pot
Auto-merging addons/mail/models/ir_attachment.py
CONFLICT (content): Merge conflict in addons/mail/models/ir_attachment.py
CONFLICT (modify/delete): addons/test_mail/tests/test_ir_attachment.py deleted in HEAD and modified in 78bd49b312f6 ([FIX] mail: add exceptions to message attachment checks).  Version 78bd49b312f6 ([FIX] mail: add exceptions to message attachment checks) of addons/test_mail/tests/test_ir_attachment.py left in tree.

stderr:

21:55:11.645862 git.c:463               trace: built-in: git cherry-pick 78bd49b312f6921152b92cda359b11a6b8ea49c3
error: could not apply 78bd49b312f6... [FIX] mail: add exceptions to message attachment checks
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@C3POdoo C3POdoo added the RD research & development, internal work label May 12, 2024
@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels May 12, 2024
Some modules may use attachments from mail messages directly.

In that case it may be desirable to at least be able to write
over the name and other non-critical information even if the
attachment is linked to a document.

The restrictions on writing on message attachments is reduced
to only apply to data fields, as those are the only ones that
we really don't want people to change.

Also return True instead of None in the override of `check`
to match the behavior of the parent.

Also reword the error message to convey writing is also forbidden.

Complementary to 4c4e63f

task-3519815
@reth-odoo reth-odoo force-pushed the master-15.0-documents-fix-mail-attachment-write-access-reth-oQui-fw branch from bbd026b to 82bef7d Compare May 13, 2024 06:38
@reth-odoo
Copy link
Contributor

@robodoo r+

@C3POdoo C3POdoo requested review from a team May 13, 2024 06:40
robodoo pushed a commit that referenced this pull request May 13, 2024
Some modules may use attachments from mail messages directly.

In that case it may be desirable to at least be able to write
over the name and other non-critical information even if the
attachment is linked to a document.

The restrictions on writing on message attachments is reduced
to only apply to data fields, as those are the only ones that
we really don't want people to change.

Also return True instead of None in the override of `check`
to match the behavior of the parent.

Also reword the error message to convey writing is also forbidden.

Complementary to 4c4e63f

task-3519815

closes #165173

Related: odoo/enterprise#62343
Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
Signed-off-by: Renaud Thiry (reth) <reth@odoo.com>
@robodoo robodoo closed this May 13, 2024
@robodoo robodoo added the 17.3 label May 13, 2024
zel-odoo pushed a commit to odoo-dev/odoo that referenced this pull request May 24, 2024
Some modules may use attachments from mail messages directly.

In that case it may be desirable to at least be able to write
over the name and other non-critical information even if the
attachment is linked to a document.

The restrictions on writing on message attachments is reduced
to only apply to data fields, as those are the only ones that
we really don't want people to change.

Also return True instead of None in the override of `check`
to match the behavior of the parent.

Also reword the error message to convey writing is also forbidden.

Complementary to 4c4e63f

task-3519815

closes odoo#165173

Related: odoo/enterprise#62343
Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
Signed-off-by: Renaud Thiry (reth) <reth@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
17.3 conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants