[ADD] base_external_system: Implement interface/adapter

[lasley]

This is a unified interface/adapter mechanism as we somewhat planned in #985 (comment)

@bealdav @florian-dacosta - Any thoughts on this before I proceed in an implementation on the connector_sftp? I added a tiny bit of logic for a unified client interface, but most everything else is up to the developer. Let me know if anything is unclear.

Readme:

Base - External System

This module provides an interface/adapter mechanism for the definition of remote
systems.

Implementation

The credentials for systems are stored in the external.system model, and are to
be configured by the user. This model is the unified interface for the underlying
adapters.

Using the Interface

Given an external.system singleton called external_system, you would do the
following to get the underlying system client:

with external_system.client() as client:
    client.do_something()

The client will be destroyed once the context has completed. Destruction takes place
in the adapter's external_destroy_client method.

The only unified aspect of this interface is the client connection itself. Other more
opinionated interface/adapter mechanisms can be implemented in other modules, such as
the file system interface in OCA/server-tools/external_file_location.

Creating an Adapter

Modules looking to add an external system adapter should inherit the
external.system.adapter model and override the following methods:

• external_get_client: Returns a usable client for the system
• external_destroy_client: Destroy the connection, if applicable. Does not need
  to be defined if the connection destroys itself.

Configuration

Configure external systems in Settings => Technical => External Systems

[lasley]

Ok I've implemented this system in #985 (1851e6b) and made some updates to the way things work here in 85b6c0c in response to actual usage vs. theory.

Still tests to go here, but I think we're good in terms of the way things work. Will be testing the SFTP connector (in prod 😆 ) over the next few weeks.

[bealdav]

@lasley Thanks for this huge work.

@hparfr don't you think than keychain could interesting there ?

[hparfr]

Hi,
The interface seams pleasant and easy to consume. It's a nice improvement.

But for the implementation, from the security perspective, I think you will gain a lot to use Keychain module from server tools. Having private keys and password in clear is dangerous.

You may also have a look to storage_backend and storage_file in https://github.com/akretion/storage

[lasley]

Unfortunately I do not believe in the security provided by keychain and will not use apps that require it as a dependency. As I mentioned in the PR when I noted my objections on it, there is a very real risk of data loss, no key rotation method, and no additional security beyond standard physical encryption at rest in the event of application exploit - which is what we're trying to guard against with encrypting in the DB. These risks for no gain mean that I am hardline against the module.

We have a module, red_october, that we use for encrypting confidential data. I do not implement red_october in my modules directly because I feel this is a security decision that should be left granular and implemented in glue modules.

While I would love to have a secure by default Odoo, an external system will always be required as a middle-man in order for Odoo to actually be secure. Anything else is a facade and is more dangerous than nothing IMO. This standpoint is due to multiple things, all of which are simply design decisions in Odoo core and not actually "bugs".

[lasley]

Alright this is implemented in a few systems now and confirmed to work well functionally. This last commit should make us go 🍏 with 100% coverage.

[hparfr]

At least, state clearly in the readme that the password and the private key are stored in plain text.

[lasley]

@hparfr - sure, done

[hparfr]

Precision :
We do have a similare module backend_storage, currently on akretion/storage that we plan to put on OCA after some refactorings. Our implementation will allow more complex uses cases, less protocol dependant and is based on keychain.

I did approve your PR, because I think your implementation can be usefull in lof of cases and it seams easy to consume.

I hope, approving your PR now, will not put storage_backend module at risk to be intergated in OCA in the future. Both are related to access external systems but implementation are not based on the same principles, and features goal are very differents.

[lasley]

I wonder if we need some sort of abstract menu for external systems, related or otherwise. I see where you're coming from in terms of implementation/use difference between our modules, and I also see us getting proliferated menus all over the place (I'm also thinking backends that are related to say, connectors).

[lasley]

An alternative would be that I could make this module more low level, providing merely the model attributes in the system and the relation to the other system (external.system.interface). Everything else could be implemented in another module.

How compatible are your backend modules to the data structure laid out here? Something we can work with/adapt easily?

[lasley]

I'm making a dependent module at the moment, so merging seeing as we have the approvals