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 #165166

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: #165158
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 a8e88749f18d ([FIX] mail: add exceptions to message attachment checks).  Version a8e88749f18d ([FIX] mail: add exceptions to message attachment checks) of addons/test_mail/tests/test_ir_attachment.py left in tree.

stderr:

15:29:59.329317 git.c:463               trace: built-in: git cherry-pick a8e88749f18d00a7f2321b5d083befcbe7265d20
error: could not apply a8e88749f18d... [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

@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
@C3POdoo C3POdoo added the RD research & development, internal work label 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 saas-17.2-15.0-documents-fix-mail-attachment-write-access-reth-Dqzt-fw branch from 64a971f to 78bd49b Compare May 12, 2024 16:50
@reth-odoo
Copy link
Contributor

@robodoo r+

@C3POdoo C3POdoo requested review from a team May 12, 2024 16:52
robodoo pushed a commit that referenced this pull request 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

closes #165166

Related: odoo/enterprise#62338
Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
Signed-off-by: Renaud Thiry (reth) <reth@odoo.com>
@robodoo robodoo closed this May 12, 2024
@fw-bot fw-bot deleted the saas-17.2-15.0-documents-fix-mail-attachment-write-access-reth-Dqzt-fw branch May 26, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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