From fd68f3fc8b4ad00fe37a593756d3a07c0210f3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 20 Dec 2017 12:56:48 +0100 Subject: [PATCH] Allow user creation from Blender ID webhook "user modified" When the webhook indicates that the user has a Cloud subscription (demo, active, or renewable), the user is immediately created. --- cloud/webhooks.py | 58 +++++++++--- tests/test_webhooks.py | 200 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 246 insertions(+), 12 deletions(-) diff --git a/cloud/webhooks.py b/cloud/webhooks.py index f49b6bf..33e3c61 100644 --- a/cloud/webhooks.py +++ b/cloud/webhooks.py @@ -13,6 +13,7 @@ import werkzeug.exceptions as wz_exceptions from pillar import current_app from pillar.api.blender_cloud import subscription +from pillar.api.utils.authentication import create_new_user_document, make_unique_username from pillar.auth import UserClass blueprint = Blueprint('cloud-webhooks', __name__) @@ -76,24 +77,30 @@ def score(wh_payload: dict, user: dict) -> int: return match_on_bid * 10 + match_on_old_email + match_on_new_email * 2 -def fetch_user(wh_payload: dict) -> typing.Optional[dict]: - """Fetch the user from the DB +def insert_or_fetch_user(wh_payload: dict) -> typing.Optional[dict]: + """Fetch the user from the DB or create it. - :returns the user document, or None when not found. + Only creates it if the webhook payload indicates they could actually use + Blender Cloud (i.e. demo or subscriber). This prevents us from creating + Cloud accounts for Blender Network users. + + :returns the user document, or None when not created. """ users_coll = current_app.db('users') my_log = log.getChild('insert_or_fetch_user') + bid_str = str(wh_payload['id']) + email = wh_payload['email'] + # Find the user by their Blender ID, or any of their email addresses. # We use one query to find all matching users. This is done as a # consistency check; if more than one user is returned, we know the # database is inconsistent with Blender ID and can emit a warning # about this. - bid_str = str(wh_payload['id']) query = {'$or': [ {'auth.provider': 'blender-id', 'auth.user_id': bid_str}, - {'email': {'$in': [wh_payload['old_email'], wh_payload['email']]}}, + {'email': {'$in': [wh_payload['old_email'], email]}}, ]} db_users = users_coll.find(query) user_count = db_users.count() @@ -103,21 +110,50 @@ def fetch_user(wh_payload: dict) -> typing.Optional[dict]: calc_score = functools.partial(score, wh_payload) best_score = max(db_users, key=calc_score) - my_log.warning('%d users found for query %s, picking %s', - user_count, query, best_score['email']) + my_log.error('%d users found for query %s, picking user %s (%s)', + user_count, query, best_score['_id'], best_score['email']) return best_score if user_count: db_user = db_users[0] my_log.debug('found user %s', db_user['email']) return db_user - my_log.info('Received update for unknown user %r', wh_payload['old_email']) - return None + # Pretend to create the user, so that we can inspect the resulting + # capabilities. This is more future-proof than looking at the list + # of roles in the webhook payload. + username = make_unique_username(email) + user_doc = create_new_user_document(email, bid_str, username, + provider='blender-id', + full_name=wh_payload['full_name']) + + user_doc['roles'] = [subscription.ROLES_BID_TO_PILLAR[r] + for r in wh_payload.get('roles', []) + if r in subscription.ROLES_BID_TO_PILLAR] + + user_ob = UserClass.construct('', user_doc) + create = user_ob.has_cap('subscriber') or user_ob.has_cap('can-renew-subscription') + if not create: + my_log.info('Received update for unknown user %r without Cloud access (caps=%s)', + wh_payload['old_email'], user_ob.capabilities) + return None + + # Actually create the user in the database. + r, _, _, status = current_app.post_internal('users', user_doc) + if status != 201: + my_log.error('unable to create user %s: : %r %r', email, status, r) + raise wz_exceptions.InternalServerError('unable to create user') + + user_doc.update(r) + my_log.info('created user %r = %s to allow immediate Cloud access', email, user_doc['_id']) + return user_doc @blueprint.route('/user-modified', methods=['POST']) def user_modified(): - """Updates the local user based on the info from Blender ID. + """Update the local user based on the info from Blender ID. + + If the payload indicates the user has access to Blender Cloud (or at least + a renewable subscription), create the user if not already in our DB. The payload we expect is a dictionary like: {'id': 12345, # the user's ID in Blender ID @@ -135,7 +171,7 @@ def user_modified(): my_log.info('payload: %s', payload) # Update the user - db_user = fetch_user(payload) + db_user = insert_or_fetch_user(payload) if not db_user: my_log.info('Received update for unknown user %r', payload['old_email']) return '', 204 diff --git a/tests/test_webhooks.py b/tests/test_webhooks.py index 2d857cd..17992c4 100644 --- a/tests/test_webhooks.py +++ b/tests/test_webhooks.py @@ -4,16 +4,19 @@ import json from abstract_cloud_test import AbstractCloudTest -class UserModifiedTest(AbstractCloudTest): +class AbstractWebhookTest(AbstractCloudTest): def setUp(self, **kwargs): super().setUp(**kwargs) self.enter_app_context() + self.create_standard_groups() self.hmac_secret = b'1234 je moeder' self.app.config['BLENDER_ID_WEBHOOK_USER_CHANGED_SECRET'] = self.hmac_secret.decode() self.uid = self.create_user(24 * 'a', roles={'subscriber'}, email='old@email.address') + +class UserModifiedTest(AbstractWebhookTest): def test_change_full_name(self): payload = {'id': 1112333, 'old_email': 'old@email.address', @@ -128,6 +131,45 @@ class UserModifiedTest(AbstractCloudTest): self.assertEqual('ကြယ်ဆွတ်', db_user['full_name']) self.assertEqual(['demo'], db_user['roles']) + def test_multiple_users_matching(self): + users_coll = self.app.db('users') + users_coll.update_one({'_id': self.uid}, + {'$set': {'auth': [ + {'provider': 'mastodon', 'user_id': 'hey@there'}, + {'provider': 'blender-id', 'user_id': '1112333'} + ]}}) + + # Create another user with email=new@elsewhere.address + other_uid = self.create_user(24 * 'b', email='new@elsewhere.address') + + payload = {'id': 1112333, + 'old_email': 'new@elsewhere.address', + 'full_name': 'ကြယ်ဆွတ်', + 'email': 'new@elsewhere.address', + 'roles': ['cloud_demo']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check the effect on the user + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('new@elsewhere.address', db_user['email']) + self.assertEqual('ကြယ်ဆွတ်', db_user['full_name']) + self.assertEqual(['demo'], db_user['roles']) + + # The other user with the email address should still be there. + # This *will* cause problems later, so the code should log this + # as error condition! + other_user = self.fetch_user_from_db(other_uid) + self.assertEqual('new@elsewhere.address', other_user['email']) + + + def test_change_roles(self): payload = {'id': 1112333, 'old_email': 'old@email.address', @@ -218,6 +260,21 @@ class UserModifiedTest(AbstractCloudTest): self.assertEqual('คนรักของผัดไทย', db_user['full_name']) self.assertEqual({'subscriber'}, set(db_user['roles'])) + def test_text_plain(self): + payload = b'{"valid": false}' + mac = hmac.new(self.hmac_secret, payload, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=payload, + content_type='text/plain', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=400) + + # Check the effect on the user + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('old@email.address', db_user['email']) + self.assertEqual('คนรักของผัดไทย', db_user['full_name']) + self.assertEqual({'subscriber'}, set(db_user['roles'])) + class UserScoreTest(AbstractCloudTest): def setUp(self, **kwargs): @@ -254,3 +311,144 @@ class UserScoreTest(AbstractCloudTest): ], 'email': 'new@email.address' })) + + +class UserModifiedUserCreationTest(AbstractWebhookTest): + def test_unknown_email(self): + payload = {'id': 1112333, + 'old_email': 'unknown@email.address', + 'full_name': 'ကြယ်ဆွတ်', + 'email': 'new@email.address', + 'roles': ['cloud_demo']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check that the user has been created, and the existing user has not been touched. + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('old@email.address', db_user['email']) + self.assertEqual('คนรักของผัดไทย', db_user['full_name']) + self.assertEqual({'subscriber'}, set(db_user['roles'])) + + users_coll = self.app.db('users') + new_user = users_coll.find_one({'email': 'new@email.address'}) + self.assertIsNotNone(new_user) + self.assertEqual('ကြယ်ဆွတ်', new_user['full_name']) + self.assertEqual('new', new_user['username']) # based on email address + self.assertEqual(['demo'], new_user['roles']) + self.assertEqual({ + 'provider': 'blender-id', + 'user_id': '1112333', + 'token': '', + }, new_user['auth'][0]) + + def test_create_subscriber(self): + payload = {'id': 1112333, + 'old_email': 'unknown@email.address', + 'full_name': 'ကြယ်ဆွတ်', + 'email': 'new@email.address', + 'roles': ['cloud_subscriber', 'cloud_has_subscription']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check that the user has been created, and the existing user has not been touched. + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('old@email.address', db_user['email']) + self.assertEqual('คนรักของผัดไทย', db_user['full_name']) + self.assertEqual({'subscriber'}, set(db_user['roles'])) + + users_coll = self.app.db('users') + new_user = users_coll.find_one({'email': 'new@email.address'}) + self.assertIsNotNone(new_user) + self.assertEqual('new', new_user['username']) + self.assertEqual('ကြယ်ဆွတ်', new_user['full_name']) + self.assertEqual(['subscriber', 'has_subscription'], new_user['roles']) + + def test_create_renewable(self): + payload = {'id': 1112333, + 'old_email': 'unknown@email.address', + 'full_name': 'ကြယ်ဆွတ်', + 'email': 'new@email.address', + 'roles': ['cloud_has_subscription']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check that the user has been created, and the existing user has not been touched. + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('old@email.address', db_user['email']) + self.assertEqual('คนรักของผัดไทย', db_user['full_name']) + self.assertEqual({'subscriber'}, set(db_user['roles'])) + + users_coll = self.app.db('users') + new_user = users_coll.find_one({'email': 'new@email.address'}) + self.assertIsNotNone(new_user) + self.assertEqual('new', new_user['username']) + self.assertEqual('ကြယ်ဆွတ်', new_user['full_name']) + self.assertEqual(['has_subscription'], new_user['roles']) + + def test_no_full_name(self): + """Blender ID doesn't enforce full names on creation.""" + payload = {'id': 1112333, + 'old_email': 'unknown@email.address', + 'full_name': '', + 'email': 'new@email.address', + 'roles': ['cloud_subscriber', 'cloud_has_subscription']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check that the user has been created correctly. + users_coll = self.app.db('users') + new_user = users_coll.find_one({'email': 'new@email.address'}) + self.assertIsNotNone(new_user) + self.assertEqual('new', new_user['username']) + self.assertEqual('new', new_user['full_name']) # defaults to username + self.assertEqual(['subscriber', 'has_subscription'], new_user['roles']) + + def test_no_create_when_not_subscriber(self): + """Don't create local users when they are not subscriber.""" + payload = {'id': 1112333, + 'old_email': 'unknown@email.address', + 'full_name': 'ကြယ်ဆွတ်', + 'email': 'new@email.address', + 'roles': ['blender_network']} + as_json = json.dumps(payload).encode() + mac = hmac.new(self.hmac_secret, + as_json, hashlib.sha256) + self.post('/api/webhooks/user-modified', + data=as_json, + content_type='application/json', + headers={'X-Webhook-HMAC': mac.hexdigest()}, + expected_status=204) + + # Check that the user has been not been created, and the existing user has not been touched. + db_user = self.fetch_user_from_db(self.uid) + self.assertEqual('old@email.address', db_user['email']) + self.assertEqual('คนรักของผัดไทย', db_user['full_name']) + self.assertEqual({'subscriber'}, set(db_user['roles'])) + + users_coll = self.app.db('users') + new_user = users_coll.find_one({'email': 'new@email.address'}) + self.assertIsNone(new_user)