From 4991c0691ab7ab5950ebccdd951e6e4e9b308b64 Mon Sep 17 00:00:00 2001 From: Hpar Date: Fri, 10 Feb 2017 16:33:48 +0100 Subject: [PATCH] Keychain: account manager for external systems (#644) * Add keychain module --- keychain/README.rst | 240 ++++++++++++++++++++++++++ keychain/__init__.py | 1 + keychain/__openerp__.py | 25 +++ keychain/models/__init__.py | 1 + keychain/models/keychain.py | 200 +++++++++++++++++++++ keychain/security/ir.model.access.csv | 2 + keychain/tests/__init__.py | 1 + keychain/tests/test_keychain.py | 220 +++++++++++++++++++++++ keychain/views/keychain_view.xml | 50 ++++++ 9 files changed, 740 insertions(+) create mode 100644 keychain/README.rst create mode 100644 keychain/__init__.py create mode 100644 keychain/__openerp__.py create mode 100644 keychain/models/__init__.py create mode 100644 keychain/models/keychain.py create mode 100644 keychain/security/ir.model.access.csv create mode 100644 keychain/tests/__init__.py create mode 100644 keychain/tests/test_keychain.py create mode 100644 keychain/views/keychain_view.xml diff --git a/keychain/README.rst b/keychain/README.rst new file mode 100644 index 000000000..bd25adcfe --- /dev/null +++ b/keychain/README.rst @@ -0,0 +1,240 @@ +.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg + :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html + :alt: License: AGPL-3 + +================ +Keychain Account +================ + +This module allows you to store credentials of external systems. + +* All the crendentials are stored in one place: easier to manage and to audit. +* Multi-account made possible without effort. +* Store additionnal data for each account. +* Validation rules for additional data. +* Have different account for different environments (prod / test / env / etc). + + +By default, passwords are encrypted with a key stored in Odoo config. +It's far from an ideal password storage setup, but it's way better +than password in clear text in the database. +It can be easily replaced by another system. See "Security" chapter below. + +Accounts may be: market places (Amazon, Cdiscount, ...), carriers (Laposte, UPS, ...) +or any third party system called from Odoo. + +This module is aimed for developers. +The logic to choose between accounts will be achieved in dependent modules. + + +========== +Uses cases +========== + +Possible use case for deliveries: you need multiple accounts for the same carrier. +It can be for instance due to carrier restrictions (immutable sender address), +or business rules (each warehouse use a different account). + + +Configuration +============= + +After the installation of this module, you need to add some entries +in Odoo's config file: (etc/openerp.cfg) + +> keychain_key = fyeMIx9XVPBBky5XZeLDxVc9dFKy7Uzas3AoyMarHPA= + +You can generate keys with `python keychain/bin/generate_key.py`. + +This key is used to encrypt account passwords. + +If you plan to use environments, you should add a key per environment: + +> keychain_key_dev = 8H_qFvwhxv6EeO9bZ8ww7BUymNt3xtQKYEq9rjAPtrc= + +> keychain_key_prod = y5z-ETtXkVI_ADoFEZ5CHLvrNjwOPxsx-htSVbDbmRc= + +keychain_key is used for encryption when no environment is set. + + +Usage (for module dev) +====================== + + +* Add this keychain as a dependency in __openerp__.py +* Subclass `keychain.account` and add your module in namespaces: `(see after for the name of namespace )` + +.. code:: python + + class LaposteAccount(models.Model): + _inherit = 'keychain.account' + + namespace = fields.Selection( + selection_add=[('roulier_laposte', 'Laposte')]) + +* Add the default data (as dict): + +.. code:: python + + class LaposteAccount(models.Model): + # ... + def _roulier_laposte_init_data(self): + return { + "agencyCode": "", + "recommandationLevel": "R1" + } + +* Implement validation of user entered data: + +.. code:: python + + class LaposteAccount(models.Model): + # ... + def _roulier_laposte_validate_data(self, data): + return len(data.get("agencyCode") > 3) + +* In your code, fetch the account: + +.. code:: python + + import random + + def get_auth(self): + keychain = self.env['keychain.account'] + if self.env.user.has_group('stock.group_stock_user'): + retrieve = keychain.suspend_security().retrieve + else: + retrieve = keychain.retrieve + + accounts = retrieve( + [['namespace', '=', 'roulier_laposte']]) + account = random.choice(accounts) + return { + 'login': account.login, + 'password': account.get_password() + } + + +In this example, an account is randomly picked. Usually this is set according +to rules specific for each client. + +You have to restrict user access of your methods with suspend_security(). + +Warning: _init_data and _validate_data should be prefixed with your namespace! +Choose python naming function compatible name. + +Switching from prod to dev +========================== + +You may adopt one of the following strategies: + +* store your dev accounts in production db using the dev key +* import your dev accounts with Odoo builtin methods like a data.xml (in a dedicated module). +* import your dev accounts with your own migration/cleanup script +* etc. + +Note: only the password field is unreadable without the proper key, login and data fields +are available on all environments. + +You may also use a same `technical_name` and different `environment` for choosing at runtime +between accounts. + +Usage (for user) +================ + +Go to *settings / keychain*, create a record with the following + +* Namespace: type of account (ie: Laposte) +* Name: human readable label "Warehouse 1" +* Technical Name: name used by a consumer module (like "warehouse_1") +* Login: login of the account +* Password_clear: For entering the password in clear text (not stored unencrypted) +* Password: password encrypted, unreadable without the key (in config) +* Data: a JSON string for additionnal values (additionnal config for the account, like: `{"agencyCode": "Lyon", "insuranceLevel": "R1"})` +* Environment: usually prod or dev or blank (for all) + + + +.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas + :alt: Try me on Runbot + :target: https://runbot.odoo-community.org/runbot/server-tools/9.0 + + +Known issues / Roadmap +====================== +- Account inheritence is not supported out-of-the-box (like defining common settings for all environments) +- Adapted to work with `server_environnement` modules +- Key expiration or rotation should be done manually +- Import passwords from data.xml + +Security +======== + +This discussion: https://github.com/OCA/server-tools/pull/644 may help you decide if this module is suitable for your needs or not. + +Common sense: Odoo is not a safe place for storing sensitive data. +But sometimes you don't have any other possibilities. +This module is designed to store credentials of data like carrier account, smtp, api keys... +but definitively not for credits cards number, medical records, etc. + + +By default, passwords are stored encrypted in the db using +symetric encryption `Fernet `_. +The encryption key is stored in openerp.tools.config. + +Threats even with this module installed: + +- unauthorized Odoo user want to access data: access is rejected by Odoo security rules +- authorized Odoo user try to access data with rpc api: he gets the passwords encrypted, he can't recover because the key and the decrypted password are not exposed through rpc +- db is stolen: without the key it's currently pretty hard to recover the passwords +- Odoo is compromised (malicious module or vulnerability): hacker has access to python and can do what he wants with Odoo: passwords of the current env can be easily decrypted +- server is compromised: idem + +If your dev server is compromised, hacker can't decrypt your prod passwords +since you have different keys between dev and prod. + +If you want something more secure: don't store any sensitive data in Odoo, +use an external system as a proxy, you can still use this module +for storing all other data related to your accounts. + + +Bug Tracker +=========== + +Bugs are tracked on `GitHub Issues +`_. In case of trouble, please +check there if your issue has already been reported. If you spotted it first, +help us smashing it by providing a detailed and welcomed feedback. + +Credits +======= + +`Akretion `_ + + +Contributors +------------ + +* Raphaël Reverdy + +Funders +------- + +The development of this module has been financially supported by: + +* `Akretion `_ + +Maintainer +---------- + +.. image:: https://odoo-community.org/logo.png + :alt: Odoo Community Association + :target: https://odoo-community.org + +This module is maintained by the OCA. + +OCA, or the Odoo Community Association, is a nonprofit organization whose +mission is to support the collaborative development of Odoo features and +promote its widespread use. + +To contribute to this module, please visit https://odoo-community.org. diff --git a/keychain/__init__.py b/keychain/__init__.py new file mode 100644 index 000000000..0650744f6 --- /dev/null +++ b/keychain/__init__.py @@ -0,0 +1 @@ +from . import models diff --git a/keychain/__openerp__.py b/keychain/__openerp__.py new file mode 100644 index 000000000..a43cbe292 --- /dev/null +++ b/keychain/__openerp__.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Copyright <2016> Akretion +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +{ + "name": "Keychain", + "summary": "Store accounts and credentials", + "version": "9.0.1.0.0", + "category": "Uncategorized", + "website": "https://akretion.com/", + "author": "Akretion, Odoo Community Association (OCA)", + "license": "AGPL-3", + "application": False, + "installable": True, + "external_dependencies": { + "python": [ + 'cryptography'], + }, + "depends": [ + "base", + ], + "data": [ + "security/ir.model.access.csv", + 'views/keychain_view.xml' + ], +} diff --git a/keychain/models/__init__.py b/keychain/models/__init__.py new file mode 100644 index 000000000..b84bb9112 --- /dev/null +++ b/keychain/models/__init__.py @@ -0,0 +1 @@ +from . import keychain \ No newline at end of file diff --git a/keychain/models/keychain.py b/keychain/models/keychain.py new file mode 100644 index 000000000..0008adb3c --- /dev/null +++ b/keychain/models/keychain.py @@ -0,0 +1,200 @@ +# -*- coding: utf-8 -*- +# © 2016 Akretion Mourad EL HADJ MIMOUNE, David BEAL, Raphaël REVERDY +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from functools import wraps + +import logging +import json + +from openerp import models, fields, api +from openerp.exceptions import ValidationError +from openerp.tools.config import config +from openerp.tools.translate import _ + +_logger = logging.getLogger(__name__) + +try: + from cryptography.fernet import Fernet, MultiFernet, InvalidToken +except ImportError as err: + _logger.debug(err) + + +def implemented_by_keychain(func): + """Call a prefixed function based on 'namespace'.""" + @wraps(func) + def wrapper(cls, *args, **kwargs): + fun_name = func.__name__ + fun = '_%s%s' % (cls.namespace, fun_name) + if not hasattr(cls, fun): + fun = '_default%s' % (fun_name) + return getattr(cls, fun)(*args, **kwargs) + return wrapper + + +class KeychainAccount(models.Model): + """Manage all accounts of external systems in one place.""" + + _name = 'keychain.account' + + name = fields.Char(required=True, help="Humain readable label") + technical_name = fields.Char( + required=True, + help="Technical name. Must be unique") + namespace = fields.Selection([], help="Type of account", required=True) + environment = fields.Char( + required=False, + help="'prod', 'dev', etc. or empty (for all)" + ) + login = fields.Char(help="Login") + clear_password = fields.Char( + help="Password. Leave empty if no changes", + inverse='_inverse_set_password', + compute='_compute_password', + store=False) + password = fields.Char( + help="Password is derived from clear_password", + readonly=True) + data = fields.Text(help="Additionnal data as json") + + def _compute_password(self): + # Only needed in v8 for _description_searchable issues + return True + + def get_password(self): + """Password in clear text.""" + try: + return self._decode_password(self.password) + except Warning as warn: + raise Warning(_( + "%s \n" + "Account: %s %s %s " % ( + warn, + self.login, self.name, self.technical_name + ) + )) + + def get_data(self): + """Data in dict form.""" + return self._parse_data(self.data) + + @api.constrains('data') + def _check_data(self): + """Ensure valid input in data field.""" + for account in self: + if account.data: + parsed = account._parse_data(account.data) + if not account._validate_data(parsed): + raise ValidationError(_("Data not valid")) + + def _inverse_set_password(self): + """Encode password from clear text.""" + # inverse function + for rec in self: + rec.password = rec._encode_password( + rec.clear_password, rec.environment) + + @api.model + def retrieve(self, domain): + """Search accounts for a given domain. + + Environment is added by this function. + Use this instead of search() to benefit from environment filtering. + Use user.has_group() and suspend_security() before + calling this method. + """ + domain.append(['environment', 'in', self._retrieve_env()]) + return self.search(domain) + + @api.multi + def write(self, vals): + """At this time there is no namespace set.""" + if not vals.get('data') and not self.data: + vals['data'] = self._serialize_data(self._init_data()) + return super(KeychainAccount, self).write(vals) + + @implemented_by_keychain + def _validate_data(self, data): + pass + + @implemented_by_keychain + def _init_data(self): + pass + + @staticmethod + def _retrieve_env(): + """Return the current environments. + + You may override this function to fit your needs. + + returns: a tuple like: + ('dev', 'test', False) + Which means accounts for dev, test and blank (not set) + Order is important: the first one is used for encryption. + """ + current = config.get('running_env') or False + envs = [current] + if False not in envs: + envs.append(False) + return envs + + @staticmethod + def _serialize_data(data): + return json.dumps(data) + + @staticmethod + def _parse_data(data): + try: + return json.loads(data) + except ValueError: + raise ValidationError(_("Data should be a valid JSON")) + + @classmethod + def _encode_password(cls, data, env): + cipher = cls._get_cipher(env) + return cipher.encrypt(str((data or '').encode('UTF-8'))) + + @classmethod + def _decode_password(cls, data): + cipher = cls._get_cipher() + try: + return unicode(cipher.decrypt(str(data)), 'UTF-8') + except InvalidToken: + raise Warning(_( + "Password has been encrypted with a different " + "key. Unless you can recover the previous key, " + "this password is unreadable." + )) + + @classmethod + def _get_cipher(cls, force_env=None): + """Return a cipher using the keys of environments. + + force_env = name of the env key. + Useful for encoding against one precise env + """ + def _get_keys(envs): + suffixes = [ + '_%s' % env if env else '' + for env in envs] # ('_dev', '') + keys_name = [ + 'keychain_key%s' % suf + for suf in suffixes] # prefix it + keys_str = [ + config.get(key) + for key in keys_name] # fetch from config + return [ + Fernet(key) for key in keys_str # build Fernet object + if key and len(key) > 0 # remove False values + ] + + if force_env: + envs = [force_env] + else: + envs = cls._retrieve_env() # ex: ('dev', False) + keys = _get_keys(envs) + if len(keys) == 0: + raise Warning(_( + "No 'keychain_key_%s' entries found in config file. " + "Use a key similar to: %s" % (envs[0], Fernet.generate_key()) + )) + return MultiFernet(keys) diff --git a/keychain/security/ir.model.access.csv b/keychain/security/ir.model.access.csv new file mode 100644 index 000000000..5b3de013f --- /dev/null +++ b/keychain/security/ir.model.access.csv @@ -0,0 +1,2 @@ +id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink +access_keychain_account,access_keychain_account,model_keychain_account,,0,0,0,0 diff --git a/keychain/tests/__init__.py b/keychain/tests/__init__.py new file mode 100644 index 000000000..779cc403e --- /dev/null +++ b/keychain/tests/__init__.py @@ -0,0 +1 @@ +from . import test_keychain \ No newline at end of file diff --git a/keychain/tests/test_keychain.py b/keychain/tests/test_keychain.py new file mode 100644 index 000000000..ef82d6db9 --- /dev/null +++ b/keychain/tests/test_keychain.py @@ -0,0 +1,220 @@ +# -*- coding: utf-8 -*- +from openerp.tests.common import TransactionCase +from openerp.tools.config import config +from openerp.exceptions import ValidationError + + +import logging + +_logger = logging.getLogger(__name__) + +try: + from cryptography.fernet import Fernet +except ImportError as err: + _logger.debug(err) + + +class TestKeychain(TransactionCase): + + def setUp(self): + super(TestKeychain, self).setUp() + + self.keychain = self.env['keychain.account'] + config['keychain_key'] = Fernet.generate_key() + + self.old_running_env = config['running_env'] + config['running_env'] = None + + def _init_data(self): + return { + "c": True, + "a": "b", + "d": "", + } + + def _validate_data(self, data): + return 'c' in data + + keychain_clss = self.keychain.__class__ + keychain_clss._keychain_test_init_data = _init_data + keychain_clss._keychain_test_validate_data = _validate_data + + self.keychain._fields['namespace'].selection.append( + ('keychain_test', 'test') + ) + + def tearDown(self): + config['running_env'] = self.old_running_env + return super(TestKeychain, self).tearDown() + + def _create_account(self): + vals = { + "name": "test", + "namespace": "keychain_test", + "login": "test", + "technical_name": "keychain.test" + } + return self.keychain.create(vals) + + def test_password(self): + """It should encrypt passwords.""" + account = self._create_account() + passwords = ('', '12345', 'djkqfljfqm', u"""&é"'(§è!ç""") + + for password in passwords: + account.clear_password = password + account._inverse_set_password() + self.assertTrue(account.clear_password != account.password) + self.assertEqual(account.get_password(), password) + + def test_wrong_key(self): + """It should raise an exception when encoded key != decoded.""" + account = self._create_account() + password = 'urieapocq' + account.clear_password = password + account._inverse_set_password() + config['keychain_key'] = Fernet.generate_key() + try: + account.get_password() + self.assertTrue(False, 'It should not work with another key') + except Warning as err: + self.assertTrue(True, 'It should raise a Warning') + self.assertTrue( + 'has been encrypted with a diff' in str(err), + 'It should display the right msg') + else: + self.assertTrue(False, 'It should raise a Warning') + + def test_no_key(self): + """It should raise an exception when no key is set.""" + account = self._create_account() + del config.options['keychain_key'] + + with self.assertRaises(Warning) as err: + account.clear_password = 'aiuepr' + account._inverse_set_password() + self.assertTrue(False, 'It should not work without key') + self.assertTrue( + 'Use a key similar to' in str(err.exception), + 'It should display the right msg') + + def test_badly_formatted_key(self): + """It should raise an exception when key is not acceptable format.""" + account = self._create_account() + + config['keychain_key'] = "" + with self.assertRaises(Warning): + account.clear_password = 'aiuepr' + account._inverse_set_password() + self.assertTrue(False, 'It should not work missing formated key') + + self.assertTrue(True, 'It shoud raise a ValueError') + + def test_retrieve_env(self): + """Retrieve env should always return False at the end""" + config['running_env'] = False + self.assertListEqual(self.keychain._retrieve_env(), [False]) + + config['running_env'] = 'dev' + self.assertListEqual(self.keychain._retrieve_env(), ['dev', False]) + + config['running_env'] = 'prod' + self.assertListEqual(self.keychain._retrieve_env(), ['prod', False]) + + def test_multienv(self): + """Encrypt with dev, decrypt with dev.""" + account = self._create_account() + config['keychain_key_dev'] = Fernet.generate_key() + config['keychain_key_prod'] = Fernet.generate_key() + config['running_env'] = 'dev' + + account.clear_password = 'abc' + account._inverse_set_password() + self.assertEqual( + account.get_password(), + 'abc', 'Should work with dev') + + config['running_env'] = 'prod' + with self.assertRaises(Warning): + self.assertEqual( + account.get_password(), + 'abc', 'Should not work with prod key') + + def test_multienv_blank(self): + """Encrypt with blank, decrypt for all.""" + account = self._create_account() + config['keychain_key'] = Fernet.generate_key() + config['keychain_key_dev'] = Fernet.generate_key() + config['keychain_key_prod'] = Fernet.generate_key() + config['running_env'] = '' + + account.clear_password = 'abc' + account._inverse_set_password() + self.assertEqual( + account.get_password(), + 'abc', 'Should work with dev') + + config['running_env'] = 'prod' + self.assertEqual( + account.get_password(), + 'abc', 'Should work with prod') + + def test_multienv_force(self): + """Set the env on the record""" + + account = self._create_account() + account.environment = 'prod' + + config['keychain_key'] = Fernet.generate_key() + config['keychain_key_dev'] = Fernet.generate_key() + config['keychain_key_prod'] = Fernet.generate_key() + config['running_env'] = '' + + account.clear_password = 'abc' + account._inverse_set_password() + + with self.assertRaises(Warning): + self.assertEqual( + account.get_password(), + 'abc', 'Should not work with dev') + + config['running_env'] = 'prod' + self.assertEqual( + account.get_password(), + 'abc', 'Should work with prod') + + def test_wrong_json(self): + """It should raise an exception when data is not valid json.""" + account = self._create_account() + wrong_jsons = ("{'hi':'o'}", "{'oq", '[>}') + for json in wrong_jsons: + with self.assertRaises(ValidationError) as err: + account.write({"data": json}) + self.assertTrue( + False, + 'Should not validate baddly formatted json') + self.assertTrue( + 'Data should be a valid JSON' in str(err.exception), + 'It should raise a ValidationError') + + def test_invalid_json(self): + """It should raise an exception when data don't pass _validate_data.""" + account = self._create_account() + invalid_jsons = ('{}', '{"hi": 1}') + for json in invalid_jsons: + with self.assertRaises(ValidationError) as err: + account.write({"data": json}) + self.assertTrue( + 'Data not valid' in str(err.exception), + 'It should raise a ValidationError') + + def test_valid_json(self): + """It should work with valid data.""" + account = self._create_account() + valid_jsons = ('{"c": true}', '{"c": 1}', '{"a": "o", "c": "b"}') + for json in valid_jsons: + try: + account.write({"data": json}) + self.assertTrue(True, 'Should validate json') + except: + self.assertTrue(False, 'It should validate a good json') diff --git a/keychain/views/keychain_view.xml b/keychain/views/keychain_view.xml new file mode 100644 index 000000000..223f84899 --- /dev/null +++ b/keychain/views/keychain_view.xml @@ -0,0 +1,50 @@ + + + + + keychain.account + + + + + + + + + + + + + + + keychain.account + +
+ + + + + + + + + + +
+ + + + + ir.actions.act_window + + Accounts + keychain.account + form + tree,form + + + + +