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.

Sign up for GitHub

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

Jump to bottom

[FIX] edi_webservice_oca: add internal user webservice backend read a… #809

Merged
merged 1 commit into from
Dec 22, 2023
Merged

[FIX] edi_webservice_oca: add internal user webservice backend read a… #809

merged 1 commit into from
Dec 22, 2023

Conversation

petrus-v
Copy link
Contributor

@petrus-v petrus-v commented Sep 5, 2023

…ccess

queue.job task are running in the context with the user that create the edi exchage record as those user are able to create exchange they should be able to read webservice backend in order to etablish the connexion to send payloads to the related webserivce

@OCA-git-bot
Copy link
Contributor

Hi @simahawk, @etobella,
some modules you are maintaining are being modified, check this out!

simahawk

This comment was marked as duplicate.

Sign in to view

simahawk
simahawk requested changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't like this. It makes too easy to spoil sensitive data.
I'd rather use sudo in webservice adapters only to read what's needed.

@petrus-v petrus-v force-pushed the 14.0-fix-access-write-to-webservice-edi branch from 26e42f6 to 376f211 Compare September 7, 2023 13:03
@petrus-v petrus-v requested a review from simahawk September 7, 2023 13:05
simahawk
simahawk requested changes Sep 7, 2023
View reviewed changes
edi_webservice_oca/components/send.py Outdated Show resolved Hide resolved
edi_webservice_oca/tests/test_send.py Outdated Show resolved Hide resolved
simahawk
simahawk reviewed Sep 8, 2023
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last change if you don't mind... Plus, please squash when done and we are ready to merge 👍

edi_webservice_oca/tests/test_send.py Outdated Show resolved Hide resolved
@petrus-v petrus-v force-pushed the 14.0-fix-access-write-to-webservice-edi branch from bf7387f to 4df67d0 Compare September 8, 2023 12:06
@simahawk
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-809-by-simahawk-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 18, 2023
@OCA-git-bot
Merge PR #809 into 14.0
964a0b1 
Signed-off-by simahawk
@OCA-git-bot
Copy link
Contributor

@simahawk your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-809-by-simahawk-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@petrus-v petrus-v force-pushed the 14.0-fix-access-write-to-webservice-edi branch from 4df67d0 to 370ad00 Compare November 6, 2023 19:27
@petrus-v
Copy link
Contributor Author

petrus-v commented Nov 6, 2023

@simahawk I've just rebase this branch, every things looks okay ! not sure why oca-git-bot failed at some point (I've deep into)

@petrus-v @simahawk
[FIX] edi_webservice_oca: use sudo to get webserivce backend info
f01bde0 
queue.job task are running in the context with the user that create
the edi exchage record as those user are able to create exchange
they should be able to read webservice backend while sending data
in order to etablish the connexion to send payloads
to the related webserivce.

We don't want to give explicit read access using access model
record to avoid user to retreives third party service credentials.

Co-authored-by: Simone Orsi <[email protected]>
@petrus-v petrus-v force-pushed the 14.0-fix-access-write-to-webservice-edi branch from 370ad00 to f01bde0 Compare December 19, 2023 07:41
@petrus-v
Copy link
Contributor Author

I've just rebase this branch, all CI seems ok, @simahawk could you re-try to merge this one ?

@etobella
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-809-by-etobella-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a878f75 into OCA:14.0 Dec 22, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 629125c. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damdam-s damdam-s damdam-s approved these changes

@simahawk simahawk simahawk approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@petrus-v @OCA-git-bot @simahawk @etobella @damdam-s