From 72404d0fd97213e0dcd56bb9255451c57c4a4796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 23 Aug 2017 13:41:17 +0200 Subject: [PATCH] Handle registration of previously unknown organization members. When a new user is created, two things happen: - before inserting into MongoDB, the organizational roles are given - after inserting, the organizations are updated to move the user from `unknown_members` to `members`. --- pillar/api/organizations/__init__.py | 41 ++++++++++++++++++++++ pillar/api/users/__init__.py | 3 ++ pillar/api/users/hooks.py | 49 ++++++++++++++++++++++++-- tests/test_api/test_organizations.py | 52 ++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 2 deletions(-) diff --git a/pillar/api/organizations/__init__.py b/pillar/api/organizations/__init__.py index 18b816ef..6c4f0d3c 100644 --- a/pillar/api/organizations/__init__.py +++ b/pillar/api/organizations/__init__.py @@ -252,6 +252,47 @@ class OrgManager: org = self._get_org(org_id, projection={'admin_uid': 1}) return org['admin_uid'] == uid + def unknown_member_roles(self, member_email: str) -> typing.Set[str]: + """Returns the set of organization roles for this user. + + Assumes the user is not yet known, i.e. part of the unknown_members lists. + """ + + org_coll = current_app.db('organizations') + + # Aggregate all org-given roles for this user. + query = org_coll.aggregate([ + {'$match': {'unknown_members': member_email}}, + {'$project': {'org_roles': 1}}, + {'$unwind': {'path': '$org_roles'}}, + {'$group': { + '_id': None, + 'org_roles': {'$addToSet': '$org_roles'}, + }}]) + + # If the user has no organizations at all, the query will have no results. + try: + org_roles_doc = query.next() + except StopIteration: + return set() + + return set(org_roles_doc['org_roles']) + + def make_member_known(self, member_uid: bson.ObjectId, member_email: str): + """Moves the given member from the unknown_members to the members lists.""" + + # This uses a direct PyMongo query rather than using Eve's put_internal, + # to prevent simultaneous updates from dropping users. + + org_coll = current_app.db('organizations') + for org in org_coll.find({'unknown_members': member_email}): + self._log.info('Updating organization %s, marking member %s/%s as known', + org['_id'], member_uid, member_email) + org_coll.update_one({'_id': org['_id']}, + {'$addToSet': {'members': member_uid}, + '$pull': {'unknown_members': member_email} + }) + def setup_app(app): from . import patch, hooks diff --git a/pillar/api/users/__init__.py b/pillar/api/users/__init__.py index bae7a2aa..664f78c0 100644 --- a/pillar/api/users/__init__.py +++ b/pillar/api/users/__init__.py @@ -71,6 +71,9 @@ def setup_app(app, api_prefix): app.on_fetched_item_users += hooks.after_fetching_user app.on_fetched_resource_users += hooks.after_fetching_user_resource + app.on_insert_users += hooks.before_inserting_users + app.on_inserted_users += hooks.after_inserting_users + app.register_api_blueprint(blueprint_api, url_prefix=api_prefix) service.signal_user_changed_role.connect(_update_algolia_user_changed_role) diff --git a/pillar/api/users/hooks.py b/pillar/api/users/hooks.py index 06699439..3ac21d61 100644 --- a/pillar/api/users/hooks.py +++ b/pillar/api/users/hooks.py @@ -1,11 +1,14 @@ import copy import json +import bson from eve.utils import parse_request -from flask import current_app, g +from flask import g +from werkzeug import exceptions as wz_exceptions + +from pillar import current_app from pillar.api.users.routes import log from pillar.api.utils.authorization import user_has_role -from werkzeug import exceptions as wz_exceptions USER_EDITABLE_FIELDS = {'full_name', 'username', 'email', 'settings'} @@ -152,3 +155,45 @@ def post_GET_user(request, payload): # json_data['computed_permissions'] = \ # compute_permissions(json_data['_id'], app.data.driver) payload.data = json.dumps(json_data) + + +def grant_org_roles(user_doc): + """Handle any organization this user may be part of.""" + + email = user_doc.get('email') + if not email: + log.warning('Unable to check new user for organization membership, no email address! %r', + user_doc) + return + + org_roles = current_app.org_manager.unknown_member_roles(email) + if not org_roles: + log.debug('No organization roles for user %r', email) + return + + log.info('Granting organization roles %r to user %r', org_roles, email) + new_roles = set(user_doc.get('roles') or []) | org_roles + user_doc['roles'] = list(new_roles) + + +def before_inserting_users(user_docs): + """Grants organization roles to the created users.""" + + for user_doc in user_docs: + grant_org_roles(user_doc) + + +def after_inserting_users(user_docs): + """Moves the users from the unknown_members to the members list of their organizations.""" + + om = current_app.org_manager + for user_doc in user_docs: + user_id = user_doc.get('_id') + user_email = user_doc.get('email') + + if not user_id or not user_email: + log.warning('User created with _id=%r and email=%r, unable to check organizations', + user_id, user_email) + continue + + om.make_member_known(user_id, user_email) diff --git a/tests/test_api/test_organizations.py b/tests/test_api/test_organizations.py index 75ed4689..9b7e9498 100644 --- a/tests/test_api/test_organizations.py +++ b/tests/test_api/test_organizations.py @@ -1,6 +1,7 @@ import typing import bson +import responses from pillar.tests import AbstractPillarTest from pillar.api.utils import remove_private_keys @@ -575,3 +576,54 @@ class OrganizationItemEveTest(AbstractPillarTest): def test_delete_anonymous(self): self._delete_test(None) + + +class UserCreationTest(AbstractPillarTest): + def setUp(self, **kwargs): + super().setUp(**kwargs) + + self.enter_app_context() + self.om = self.app.org_manager + + # Create an organization with admin who is not organization member. + self.admin_uid = self.create_user(24 * 'a', token='admin-token') + org_doc = self.om.create_new_org('Хакеры', self.admin_uid, 25, + org_roles={'org-크툴루'}) + self.org_id = org_doc['_id'] + + self.om.assign_users(self.org_id, ['newmember@example.com']) + + # Re-fetch the organization so that self.org_doc has the correct etag. + self.org_doc = self._from_db() + + def _from_db(self) -> dict: + return self.om._get_org(self.org_id) + + @responses.activate + def test_member_joins_later(self, **kwargs): + # Mock a Blender ID response for this user. + blender_id_response = {'status': 'success', + 'user': {'email': 'newmember@example.com', + 'full_name': 'Our lord and saviour Cthulhu', + 'id': 9999}, + 'token_expires': 'Mon, 1 Jan 2218 01:02:03 GMT'} + + responses.add(responses.POST, + '%s/u/validate_token' % self.app.config['BLENDER_ID_ENDPOINT'], + json=blender_id_response, + status=200) + + # Create a user by authenticating + resp = self.get('/api/users/me', auth_token='user-token') + user_info = resp.get_json() + my_id = bson.ObjectId(user_info['_id']) + + # Check that the user has the organization roles. + users_coll = self.app.db('users') + db_user = users_coll.find_one(my_id) + self.assertEqual({'org-크툴루'}, set(db_user['roles'])) + + # Check that the user is moved from 'unknown_members' to 'members' in the org. + db_org = self._from_db() + self.assertEqual([], db_org['unknown_members']) + self.assertEqual([my_id], db_org['members'])