From 4a3cde2faa5528b6b87a037c4d96ad25ebdee9d9 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 9 Jun 2022 15:16:47 +0200 Subject: [PATCH] [IMP] runbot_merge: provisioning features A few fixes and improvements after testing the feature: - ensure the provisioned users are created as internal (not portal) - assume oauth is installed and just crash if it's not - handle a user not having an email (ignore) - return value from json handler, otherwise JsonRequest sends no payload which is *weird* --- conftest.py | 13 ++-------- .../changelog/2022-06/provisioning.md | 1 + .../controllers/reviewer_provisioning.py | 26 ++++++++++++------- runbot_merge/tests/test_provisioning.py | 11 +++++++- 4 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 runbot_merge/changelog/2022-06/provisioning.md diff --git a/conftest.py b/conftest.py index 33c72ffc..d1b9b427 100644 --- a/conftest.py +++ b/conftest.py @@ -1129,17 +1129,8 @@ class Model: def create(self, values): return Model(self._env, self._model, [self._env(self._model, 'create', values)]) - def write(self, values): - return self._env(self._model, 'write', self._ids, values) - - def read(self, fields): - return self._env(self._model, 'read', self._ids, fields) - - def name_get(self): - return self._env(self._model, 'name_get', self._ids) - - def unlink(self): - return self._env(self._model, 'unlink', self._ids) + def check_object_reference(self, *args, **kwargs): + return self.env(self._model, 'check_object_reference', *args, **kwargs) def sorted(self, field): rs = sorted(self.read([field]), key=lambda r: r[field]) diff --git a/runbot_merge/changelog/2022-06/provisioning.md b/runbot_merge/changelog/2022-06/provisioning.md new file mode 100644 index 00000000..a0a47f27 --- /dev/null +++ b/runbot_merge/changelog/2022-06/provisioning.md @@ -0,0 +1 @@ +ADD: automated provisioning of accounts from odoo.com diff --git a/runbot_merge/controllers/reviewer_provisioning.py b/runbot_merge/controllers/reviewer_provisioning.py index 9d760dc4..1b7a9f2f 100644 --- a/runbot_merge/controllers/reviewer_provisioning.py +++ b/runbot_merge/controllers/reviewer_provisioning.py @@ -20,6 +20,7 @@ class MergebotReviewerProvisioning(Controller): 'email': u.email, } for u in env['res.users'].search([]) + if u.github_login ] @from_role('accounts') @@ -56,22 +57,27 @@ class MergebotReviewerProvisioning(Controller): # unique, and should not be able to collide with emails partners[p.github_login] = p - if 'oauth_provider_id' in Users: - odoo_provider = env.ref('auth_oauth.provider_openerp', False) - if odoo_provider: - for new in users: - if 'sub' in new: - new['oauth_provider_id'] = odoo_provider.id - new['oauth_uid'] = new.pop('sub') + internal = env.ref('base.group_user') + odoo_provider = env.ref('auth_oauth.provider_openerp') to_create = [] created = updated = 0 for new in users: + if 'sub' in new: + new['oauth_provider_id'] = odoo_provider.id + new['oauth_uid'] = new.pop('sub') + # prioritise by github_login as that's the unique-est point of information current = partners.get(new['github_login']) or partners.get(new['email']) or Partners # entry doesn't have user -> create user if not current.user_ids: + # skip users without an email (= login) as that + # fails + if not new['email']: + continue + new['login'] = new['email'] + new['groups_id'] = [(4, internal.id)] # entry has partner -> create user linked to existing partner # (and update partner implicitly) if current: @@ -94,8 +100,9 @@ class MergebotReviewerProvisioning(Controller): user.write(update_vals) updated += 1 if to_create: - Users.create(to_create) - created = len(to_create) + # only create 100 users at a time to avoid request timeout + Users.create(to_create[:100]) + created = len(to_create[:100]) _logger.info("Provisioning: created %d updated %d.", created, updated) return [created, updated] @@ -121,3 +128,4 @@ class MergebotReviewerProvisioning(Controller): partners.mapped('user_ids').write({ 'groups_id': [(6, 0, [request.env.ref('base.group_portal').id])] }) + return True diff --git a/runbot_merge/tests/test_provisioning.py b/runbot_merge/tests/test_provisioning.py index debfa3b5..6119bf17 100644 --- a/runbot_merge/tests/test_provisioning.py +++ b/runbot_merge/tests/test_provisioning.py @@ -8,7 +8,6 @@ GEORGE = { 'sub': '19321102' } def test_basic_provisioning(env, port): - r = provision_user(port, [GEORGE]) assert r == [1, 0] @@ -16,6 +15,10 @@ def test_basic_provisioning(env, port): assert g.partner_id.name == GEORGE['name'] assert g.partner_id.github_login == GEORGE['github_login'] assert g.oauth_uid == GEORGE['sub'] + (model, g_id) = env['ir.model.data']\ + .check_object_reference('base', 'group_user') + assert model == 'res.groups' + assert g.groups_id.id == g_id, "check that users were provisioned as internal (not portal)" # repeated provisioning should be a no-op r = provision_user(port, [GEORGE]) @@ -79,6 +82,12 @@ def test_upgrade_partner(env, port): p.user_ids.unlink() p.unlink() +def test_no_email(env, port): + """ Provisioning system should ignore email-less entries + """ + r = provision_user(port, [{**GEORGE, 'email': None}]) + assert r == [0, 0] + def provision_user(port, users): r = requests.post(f'http://localhost:{port}/runbot_merge/provision', json={ 'jsonrpc': '2.0',