[IMP] account_peppol: Send & Receive onboarding revamp

[clbr-odoo]

Remove the Sender/Receiver checkbox. This checkbox is useless, it can be computed by checking the Peppol network.

There are two possible flows:

• Either the company is already receiver on Peppol through another access point than Odoo -> we need to register him as a sender.
• Either the company is not registered anywhere -> we must register him as a receiver on Odoo.

task-4394408

[robodoo]

[clbr-odoo]

@aboo-odoo Regarding the security warning on the call above moved in a separate method, I'm afraid we can't avoid it...
The href contains the URL. We retrieve it from the call done to
smp_url = f"http://B-{hash_participant}.iso6523-actorid-upis.{sml_zone}.tech.ec.europa.eu/{endpoint_participant}" above. This domain is handled by Peppol who certifies Access Points that are registered on the network. The href content is built by those Access Points so I think it can be considered safe-ish by design.

Here is what those urls could look like:
First call that list services accepted by the user (which we can't know in advance):
http://b-b7a1e8b32951d519bbef26d9ac24804c.iso6523-actorid-upis.edelivery.tech.ec.europa.eu/iso6523-actorid-upis::0208:0246697724
Second (problematic) call done on the first href found above (in this case the Access Point is Odoo, but could be any Access Point validated by Open Peppol):
https://iap-services.odoo.com/iso6523-actorid-upis%3A%3A0208%3A0246697724/services/busdox-docid-qns%3A%3Aurn%3Aoasis%3Anames%3Aspecification%3Aubl%3Aschema%3Axsd%3AInvoice-2%3A%3AInvoice%23%23urn%3Acen.eu%3Aen16931%3A2017%23compliant%23urn%3Afdc%3Apeppol.eu%3A2017%3Apoacc%3Abilling%3A3.0%3A%3A2.1

[aboo-odoo]

Hello @odoo/rd-security ,

Could we have your opinion on this ?

The method requests a URL that is given through an LXML etree element. The problem is see is if a user manages to find a way to generate any etree element elsewhere in Odoo, he can create a demo db on Odoo (he'll be admin), create a server action and use both the etree generator and this method to call any service through us.

What would be the best way to deal with that ? URLs in the etree element have no common part and can be anything returned to us by the PEPPOL service, so there is no way to make sure the URLs has a certain format. We could always make the method standalone. What do you think ?

[xmo-odoo]

The method requests a URL that is given through an LXML etree element. The problem is see is if a user manages to find a way to generate any etree element elsewhere in Odoo, he can create a demo db on Odoo (he'll be admin), create a server action and use both the etree generator and this method to call any service through us.

What would be the best way to deal with that ? URLs in the etree element have no common part and can be anything returned to us by the PEPPOL service, so there is no way to make sure the URLs has a certain format. We could always make the method standalone. What do you think ?

Why is it a separate method on a separate model in a separate file? It's called from just one location, and doesn't even use self. If you restrict (or remove) its access scope, it becomes hard or impossible to misuse.

Even better would be to have a separate object which is never returned from any method, and mediates all peppol interactions.

[clbr-odoo]

Hello, thank you for the review @xmo-odoo

Well in theory we could want to get that information for any partner, not just the company but you're right, for the moment we only use it in the context of the company, so why not encapsulate it.
We'll see later if the needs come.

Even better would be to have a separate object which is never returned from any method, and mediates all peppol interactions.

I don't get a clear idea of what you mean here. Do you suggest to move this kind of call in a separate Python (not Odoo model) class ? We could but that would only work for this call and the precedent call, which are the only two calls made to Peppol Network directly. The rest of the Peppol traffic happens trough the account_edi_proxy_user model + IAP so I don't think it will work that well 🤔 We're refactoring a lot of things around Peppol so I'm really open to discussion on a better design security-wise.

Can you please check again if the change is correct ? This task should be in the freeze 🙂
https://github.com/odoo/odoo/pull/197353/files#diff-eb787f70e3eec203d4a99f336cf054ee98ccf59de6c8e6b10a7ad722afec8ad2R281

[xmo-odoo]

@robodoo override=ci/security

[xmo-odoo]

Do you suggest to move this kind of call in a separate Python (not Odoo model) class ?

Yes. IIRC the payment modules do that sort of things.

The rest of the Peppol traffic happens trough the account_edi_proxy_user model + IAP so I don't think it will work that well 🤔

Maybe it should not do that, and while that model can be used to store configuration it should not mediate the interactions themselves? Generally speaking, the issue is that models can be called directly from server actions or even over RPC, so there's not much separating the untrusted user from calls through. Free functions or non-model objects still require significant care in their use, but they can encapsulate and abstract the calls and provide less opportunity to leak out capabilities.

[clbr-odoo]

I see, thank you for the insight. I was considering removing/decoupling that model for other good reasons, that's one more "pro" 👍

[smetl]

@robodoo r+