From 0b218eb656ca5d5f38269a1e21ce2b02bfcc5951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 30 Nov 2017 15:28:35 +0100 Subject: [PATCH 01/16] Use Blender ID to obtain subscription status. Instead of performing a call to the Blender Store, call to Blender ID to get the user's subscription status. Currently this is performed as a second HTTP call after logging in; in the future we may want to include the roles in the login response from Blender ID, so that we can do this in one call instead of two. --- pillar/api/blender_cloud/subscription.py | 155 ++++++++--------------- pillar/api/blender_id.py | 10 +- pillar/config.py | 8 +- pillar/web/users/routes.py | 3 +- tests/test_api/test_subscriptions.py | 37 ++---- 5 files changed, 82 insertions(+), 131 deletions(-) diff --git a/pillar/api/blender_cloud/subscription.py b/pillar/api/blender_cloud/subscription.py index 9e5aacb9..ea93b393 100644 --- a/pillar/api/blender_cloud/subscription.py +++ b/pillar/api/blender_cloud/subscription.py @@ -1,94 +1,37 @@ +import collections import logging import typing -from flask import current_app, Blueprint +from flask import Blueprint +from pillar.auth import UserClass from pillar.api.utils import authorization log = logging.getLogger(__name__) blueprint = Blueprint('blender_cloud.subscription', __name__) - -def fetch_subscription_info(email: str) -> typing.Optional[dict]: - """Returns the user info dict from the external subscriptions management server. - - :returns: the store user info, or None if the user can't be found or there - was an error communicating. A dict like this is returned: - { - "shop_id": 700, - "cloud_access": 1, - "paid_balance": 314.75, - "balance_currency": "EUR", - "start_date": "2014-08-25 17:05:46", - "expiration_date": "2016-08-24 13:38:45", - "subscription_status": "wc-active", - "expiration_date_approximate": true - } - """ - - import requests - from requests.adapters import HTTPAdapter - import requests.exceptions - - external_subscriptions_server = current_app.config['EXTERNAL_SUBSCRIPTIONS_MANAGEMENT_SERVER'] - - if log.isEnabledFor(logging.DEBUG): - import urllib.parse - - log_email = urllib.parse.quote(email) - log.debug('Connecting to store at %s?blenderid=%s', - external_subscriptions_server, log_email) - - # Retry a few times when contacting the store. - s = requests.Session() - s.mount(external_subscriptions_server, HTTPAdapter(max_retries=5)) - - try: - r = s.get(external_subscriptions_server, - params={'blenderid': email}, - verify=current_app.config['TLS_CERT_FILE'], - timeout=current_app.config.get('EXTERNAL_SUBSCRIPTIONS_TIMEOUT_SECS', 10)) - except requests.exceptions.ConnectionError as ex: - log.error('Error connecting to %s: %s', external_subscriptions_server, ex) - return None - except requests.exceptions.Timeout as ex: - log.error('Timeout communicating with %s: %s', external_subscriptions_server, ex) - return None - except requests.exceptions.RequestException as ex: - log.error('Some error communicating with %s: %s', external_subscriptions_server, ex) - return None - - if r.status_code != 200: - log.warning("Error communicating with %s, code=%i, unable to check " - "subscription status of user %s", - external_subscriptions_server, r.status_code, email) - return None - - store_user = r.json() - - if log.isEnabledFor(logging.DEBUG): - import json - log.debug('Received JSON from store API: %s', - json.dumps(store_user, sort_keys=False, indent=4)) - - return store_user +# Mapping from roles on Blender ID to roles here in Pillar. +# Roles not mentioned here will not be synced from Blender ID. +ROLES_BID_TO_PILLAR = { + 'cloud_subscriber': 'subscriber', + 'cloud_demo': 'demo', + 'cloud_has_subscription': 'has_subscription', +} @blueprint.route('/update-subscription') @authorization.require_login() -def update_subscription(): +def update_subscription() -> typing.Tuple[str, int]: """Updates the subscription status of the current user. Returns an empty HTTP response. """ - import pprint from pillar import auth - from pillar.api import blender_id, service - from pillar.api.utils import authentication + from pillar.api import blender_id my_log: logging.Logger = log.getChild('update_subscription') - user_id = authentication.current_user_id() + current_user = auth.get_current_user() try: bid_user = blender_id.fetch_blenderid_user() @@ -98,46 +41,60 @@ def update_subscription(): if not bid_user: my_log.warning('Logged in user %s has no BlenderID account! ' - 'Unable to update subscription status.', user_id) + 'Unable to update subscription status.', current_user.user_id) return '', 204 - # Use the Blender ID email address to check with the store. At least that reduces the - # number of email addresses that could be out of sync to two (rather than three when we - # use the email address from our local database). + do_update_subscription(current_user, bid_user) + return '', 204 + + +def do_update_subscription(local_user: UserClass, bid_user: dict): + """Updates the subscription status of the user given the Blender ID user info. + + Uses the badger service to update the user's roles from Blender ID. + + bid_user should be a dict like: + {'id': 1234, + 'full_name': 'मूंगफली मक्खन प्रेमी', + 'email': 'here@example.com', + 'roles': {'cloud_demo': True}} + """ + + from pillar.api import service + + my_log: logging.Logger = log.getChild('do_update_subscription') + try: email = bid_user['email'] except KeyError: - my_log.error('Blender ID response did not include an email address, ' - 'unable to update subscription status: %s', - pprint.pformat(bid_user, compact=True)) - return 'Internal error', 500 - store_user = fetch_subscription_info(email) or {} + email = '-missing email-' # Handle the role changes via the badger service functionality. - grant_subscriber = store_user.get('cloud_access', 0) == 1 - grant_demo = bid_user.get('roles', {}).get('cloud_demo', False) + bid_roles = collections.defaultdict(bool, **bid_user.get('roles', {})) + plr_roles = set(local_user.roles) - is_subscriber = authorization.user_has_role('subscriber') - is_demo = authorization.user_has_role('demo') + grant_roles = set() + revoke_roles = set() + for bid_role, plr_role in ROLES_BID_TO_PILLAR.items(): + if bid_roles[bid_role] and plr_role not in plr_roles: + grant_roles.add(plr_role) + continue + if not bid_roles[bid_role] and plr_role in plr_roles: + revoke_roles.add(plr_role) - if grant_subscriber != is_subscriber: - action = 'grant' if grant_subscriber else 'revoke' - my_log.info('%sing subscriber role to user %s (Blender ID email %s)', - action, user_id, email) - service.do_badger(action, role='subscriber', user_id=user_id) - else: - my_log.debug('Not changing subscriber role, grant=%r and is=%s', - grant_subscriber, is_subscriber) + user_id = local_user.user_id - if grant_demo != is_demo: - action = 'grant' if grant_demo else 'revoke' - my_log.info('%sing demo role to user %s (Blender ID email %s)', action, user_id, email) - service.do_badger(action, role='demo', user_id=user_id) - else: - my_log.debug('Not changing demo role, grant=%r and is=%s', - grant_demo, is_demo) + if grant_roles: + if my_log.isEnabledFor(logging.INFO): + my_log.info('granting roles to user %s (Blender ID %s): %s', + user_id, email, ', '.join(sorted(grant_roles))) + service.do_badger('grant', roles=grant_roles, user_id=user_id) - return '', 204 + if revoke_roles: + if my_log.isEnabledFor(logging.INFO): + my_log.info('revoking roles to user %s (Blender ID %s): %s', + user_id, email, ', '.join(sorted(revoke_roles))) + service.do_badger('revoke', roles=revoke_roles, user_id=user_id) def setup_app(app, url_prefix): diff --git a/pillar/api/blender_id.py b/pillar/api/blender_id.py index 06c8cae2..8d025243 100644 --- a/pillar/api/blender_id.py +++ b/pillar/api/blender_id.py @@ -181,7 +181,8 @@ def fetch_blenderid_user() -> dict: "roles": { "admin": true, "bfct_trainer": false, - "cloud_single_member": true, + "cloud_has_subscription": true, + "cloud_subscriber": true, "conference_speaker": true, "network_member": true } @@ -218,12 +219,13 @@ def fetch_blenderid_user() -> dict: log.warning('Error %i from BlenderID %s: %s', bid_resp.status_code, bid_url, bid_resp.text) return {} - if not bid_resp.json(): + payload = bid_resp.json() + if not payload: log.warning('Empty data returned from BlenderID %s', bid_url) return {} - log.debug('BlenderID returned %s', bid_resp.json()) - return bid_resp.json() + log.debug('BlenderID returned %s', payload) + return payload def setup_app(app, url_prefix): diff --git a/pillar/config.py b/pillar/config.py index 526ae4a6..e5938499 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -99,6 +99,11 @@ FULL_FILE_ACCESS_ROLES = {'admin', 'subscriber', 'demo'} BLENDER_ID_CLIENT_ID = 'SPECIAL-SNOWFLAKE-57' BLENDER_ID_SUBCLIENT_ID = 'PILLAR' +# Blender ID user info API endpoint URL and auth token, only used for +# reconciling subscribers. The token requires the 'userinfo' scope. +BLENDER_ID_USER_INFO_API = 'http://blender-id:8000/api/user/' +BLENDER_ID_USER_INFO_TOKEN = '-set-in-config-local-' + # Collection of supported OAuth providers (Blender ID, Facebook and Google). # Example entry: # OAUTH_CREDENTIALS = { @@ -173,9 +178,6 @@ URLER_SERVICE_AUTH_TOKEN = None # front-end. BLENDER_CLOUD_ADDON_VERSION = '1.4' -EXTERNAL_SUBSCRIPTIONS_MANAGEMENT_SERVER = 'https://store.blender.org/api/' -EXTERNAL_SUBSCRIPTIONS_TIMEOUT_SECS = 10 - # Certificate file for communication with other systems. TLS_CERT_FILE = requests.certs.where() diff --git a/pillar/web/users/routes.py b/pillar/web/users/routes.py index 85aa0462..713e5ffc 100644 --- a/pillar/web/users/routes.py +++ b/pillar/web/users/routes.py @@ -69,8 +69,7 @@ def oauth_callback(provider): pillar.auth.login_user(token['token'], load_from_db=True) if provider == 'blender-id' and current_user.is_authenticated: - # Check with the store for user roles. If the user has an active subscription, we apply - # the 'subscriber' role + # Check with Blender ID to update certain user roles. update_subscription() next_after_login = session.pop('next_after_login', None) diff --git a/tests/test_api/test_subscriptions.py b/tests/test_api/test_subscriptions.py index 6e8e69f2..08dec044 100644 --- a/tests/test_api/test_subscriptions.py +++ b/tests/test_api/test_subscriptions.py @@ -1,3 +1,4 @@ +import typing from unittest import mock import responses @@ -17,22 +18,16 @@ class RoleUpdatingTest(AbstractPillarTest): self.create_standard_groups() def _setup_testcase(self, mocked_fetch_blenderid_user, *, - store_says_cloud_access: bool, - bid_says_cloud_demo: bool): + bid_roles: typing.Set[str]): import urllib.parse + + # The Store API endpoint should not be called upon any more. url = '%s?blenderid=%s' % (self.app.config['EXTERNAL_SUBSCRIPTIONS_MANAGEMENT_SERVER'], urllib.parse.quote(TEST_EMAIL_ADDRESS)) responses.add('GET', url, - json={'shop_id': 58432, - 'cloud_access': 1 if store_says_cloud_access else 0, - 'paid_balance': 0, - 'balance_currency': 'EUR', - 'start_date': '2017-05-04 12:07:49', - 'expiration_date': '2017-08-04 10:07:49', - 'subscription_status': 'wc-active' - }, - status=200, + status=500, match_querystring=True) + self.mock_blenderid_validate_happy() mocked_fetch_blenderid_user.return_value = { 'email': TEST_EMAIL_ADDRESS, @@ -45,27 +40,25 @@ class RoleUpdatingTest(AbstractPillarTest): 'network_member': True } } - if bid_says_cloud_demo: - mocked_fetch_blenderid_user.return_value['roles']['cloud_demo'] = True + for role in bid_roles: + mocked_fetch_blenderid_user.return_value['roles'][role] = True @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_store_api_role_grant_subscriber(self, mocked_fetch_blenderid_user): self._setup_testcase(mocked_fetch_blenderid_user, - store_says_cloud_access=True, - bid_says_cloud_demo=False) + bid_roles={'cloud_subscriber', 'cloud_has_subscription'}) self.get('/api/bcloud/update-subscription', auth_token='my-happy-token', expected_status=204) user_info = self.get('/api/users/me', auth_token='my-happy-token').json() - self.assertEqual(['subscriber'], user_info['roles']) + self.assertEqual({'subscriber', 'has_subscription'}, set(user_info['roles'])) @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_store_api_role_revoke_subscriber(self, mocked_fetch_blenderid_user): self._setup_testcase(mocked_fetch_blenderid_user, - store_says_cloud_access=False, - bid_says_cloud_demo=False) + bid_roles={'conference_speaker'}) # Make sure this user is currently known as a subcriber. self.create_user(roles={'subscriber'}, token='my-happy-token') @@ -82,8 +75,7 @@ class RoleUpdatingTest(AbstractPillarTest): @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_bid_api_grant_demo(self, mocked_fetch_blenderid_user): self._setup_testcase(mocked_fetch_blenderid_user, - store_says_cloud_access=False, - bid_says_cloud_demo=True) + bid_roles={'cloud_demo'}) self.get('/api/bcloud/update-subscription', auth_token='my-happy-token', expected_status=204) @@ -93,10 +85,9 @@ class RoleUpdatingTest(AbstractPillarTest): @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') - def test_bid_api_role_revoke_subscriber(self, mocked_fetch_blenderid_user): + def test_bid_api_role_revoke_demo(self, mocked_fetch_blenderid_user): self._setup_testcase(mocked_fetch_blenderid_user, - store_says_cloud_access=False, - bid_says_cloud_demo=False) + bid_roles={'conference_speaker'}) # Make sure this user is currently known as demo user. self.create_user(roles={'demo'}, token='my-happy-token') From 517b28389331b0c65110508b007925a17f69fb23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 1 Dec 2017 18:10:33 +0100 Subject: [PATCH 02/16] Accept roles from Blender ID in two formats This supports {'role_name': bool} dicts (the old format) and any iterable of strings {'role_name', ...} --- pillar/api/blender_cloud/subscription.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pillar/api/blender_cloud/subscription.py b/pillar/api/blender_cloud/subscription.py index ea93b393..d523aa75 100644 --- a/pillar/api/blender_cloud/subscription.py +++ b/pillar/api/blender_cloud/subscription.py @@ -58,6 +58,8 @@ def do_update_subscription(local_user: UserClass, bid_user: dict): 'full_name': 'मूंगफली मक्खन प्रेमी', 'email': 'here@example.com', 'roles': {'cloud_demo': True}} + + The 'roles' key can also be an interable of role names instead of a dict. """ from pillar.api import service @@ -69,17 +71,25 @@ def do_update_subscription(local_user: UserClass, bid_user: dict): except KeyError: email = '-missing email-' + # Transform the BID roles from a dict to a set. + bidr = bid_user.get('roles', set()) + if isinstance(bidr, dict): + bid_roles = {role + for role, has_role in bid_user.get('roles', {}).items() + if has_role} + else: + bid_roles = set(bidr) + # Handle the role changes via the badger service functionality. - bid_roles = collections.defaultdict(bool, **bid_user.get('roles', {})) plr_roles = set(local_user.roles) grant_roles = set() revoke_roles = set() for bid_role, plr_role in ROLES_BID_TO_PILLAR.items(): - if bid_roles[bid_role] and plr_role not in plr_roles: + if bid_role in bid_roles and plr_role not in plr_roles: grant_roles.add(plr_role) continue - if not bid_roles[bid_role] and plr_role in plr_roles: + if bid_role not in bid_roles and plr_role in plr_roles: revoke_roles.add(plr_role) user_id = local_user.user_id From c8221ea0e44ad0a86f1be2fd6131f3d1bff6a0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 5 Dec 2017 11:44:05 +0100 Subject: [PATCH 03/16] Simplified javascript in users/edit_embed_base.pug There is no need to use JQuery, a unique ID for the a-element, and an invalid href value, just to bind on-click functionality to a link. --- src/templates/users/edit_embed_base.pug | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/templates/users/edit_embed_base.pug b/src/templates/users/edit_embed_base.pug index db6344c0..2f616666 100644 --- a/src/templates/users/edit_embed_base.pug +++ b/src/templates/users/edit_embed_base.pug @@ -72,7 +72,7 @@ | none | {% endif %} - a#button-cancel.btn.btn-default(href="#", data-user-id='{{user.user_id}}') Cancel + a.btn.btn-default(href="javascript:$('#user-edit-container').html('')") Cancel input#submit_edit_user.btn.btn-default( data-user-id="{{user.user_id}}", @@ -101,10 +101,6 @@ script(type="text/javascript"). //- $("#user-edit-form").submit(); }); - $('#button-cancel').click(function(e){ - $('#user-container').html('') - }); - new Clipboard('.copy-to-clipboard'); | {% endblock %} From 87fe1887e890198bb3a0ac672ce59eaf48fed5d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 5 Dec 2017 11:45:42 +0100 Subject: [PATCH 04/16] Added "Update from Blender ID" button Added this button in the /u/ user/embed view, so that admins can easily force a re-check from Blender ID without requiring the user themselves to perform any actions. --- pillar/api/blender_cloud/subscription.py | 71 +++++++++++++++++++++--- pillar/api/blender_id.py | 18 ++++++ pillar/config.py | 5 +- src/templates/users/edit_embed_base.pug | 13 +++++ 4 files changed, 97 insertions(+), 10 deletions(-) diff --git a/pillar/api/blender_cloud/subscription.py b/pillar/api/blender_cloud/subscription.py index d523aa75..e6fe6fe9 100644 --- a/pillar/api/blender_cloud/subscription.py +++ b/pillar/api/blender_cloud/subscription.py @@ -1,11 +1,14 @@ -import collections import logging import typing -from flask import Blueprint +from flask import Blueprint, Response +import requests +from requests.adapters import HTTPAdapter -from pillar.auth import UserClass -from pillar.api.utils import authorization +from pillar import auth, current_app +from pillar.api import blender_id +from pillar.api.utils import authorization, jsonify +from pillar.auth import current_user log = logging.getLogger(__name__) blueprint = Blueprint('blender_cloud.subscription', __name__) @@ -27,9 +30,6 @@ def update_subscription() -> typing.Tuple[str, int]: Returns an empty HTTP response. """ - from pillar import auth - from pillar.api import blender_id - my_log: logging.Logger = log.getChild('update_subscription') current_user = auth.get_current_user() @@ -48,7 +48,58 @@ def update_subscription() -> typing.Tuple[str, int]: return '', 204 -def do_update_subscription(local_user: UserClass, bid_user: dict): +@blueprint.route('/update-subscription-for/', methods=['POST']) +@authorization.require_login(require_cap='admin') +def update_subscription_for(user_id: str): + """Updates the user based on their info at Blender ID.""" + + from urllib.parse import urljoin + + from pillar.api.utils import str2id + + my_log = log.getChild('update_subscription_for') + + bid_session = requests.Session() + bid_session.mount('https://', HTTPAdapter(max_retries=5)) + bid_session.mount('http://', HTTPAdapter(max_retries=5)) + + users_coll = current_app.db('users') + db_user = users_coll.find_one({'_id': str2id(user_id)}) + if not db_user: + my_log.warning('User %s not found in database', user_id) + return Response(f'User {user_id} not found in our database', status=404) + + log.info('Updating user %s from Blender ID on behalf of %s', + db_user['email'], current_user.email) + + bid_user_id = blender_id.get_user_blenderid(db_user) + if not bid_user_id: + my_log.info('User %s has no Blender ID', user_id) + return Response('User has no Blender ID', status=404) + + # Get the user info from Blender ID, and handle errors. + api_url = current_app.config['BLENDER_ID_USER_INFO_API'] + api_token = current_app.config['BLENDER_ID_USER_INFO_TOKEN'] + url = urljoin(api_url, bid_user_id) + resp = bid_session.get(url, headers={'Authorization': f'Bearer {api_token}'}) + if resp.status_code == 404: + my_log.info('User %s has a Blender ID %s but Blender ID itself does not find it', + user_id, bid_user_id) + return Response(f'User {bid_user_id} does not exist at Blender ID', status=404) + if resp.status_code != 200: + my_log.info('Error code %s getting user %s from Blender ID (resp = %s)', + resp.status_code, user_id, resp.text) + return Response(f'Error code {resp.status_code} from Blender ID', status=resp.status_code) + + # Update the user in our database. + local_user = auth.UserClass.construct('', db_user) + bid_user = resp.json() + do_update_subscription(local_user, bid_user) + + return '', 204 + + +def do_update_subscription(local_user: auth.UserClass, bid_user: dict): """Updates the subscription status of the user given the Blender ID user info. Uses the badger service to update the user's roles from Blender ID. @@ -106,6 +157,10 @@ def do_update_subscription(local_user: UserClass, bid_user: dict): user_id, email, ', '.join(sorted(revoke_roles))) service.do_badger('revoke', roles=revoke_roles, user_id=user_id) + # Re-index the user in the search database. + from pillar.api.users import hooks + hooks.push_updated_user_to_algolia({'_id': user_id}, {}) + def setup_app(app, url_prefix): log.info('Registering blueprint at %s', url_prefix) diff --git a/pillar/api/blender_id.py b/pillar/api/blender_id.py index 8d025243..eb9e7537 100644 --- a/pillar/api/blender_id.py +++ b/pillar/api/blender_id.py @@ -168,6 +168,24 @@ def _compute_token_expiry(token_expires_string): return min(blid_expiry, our_expiry) +def get_user_blenderid(db_user: dict) -> str: + """Returns the Blender ID user ID for this Pillar user. + + Takes the string from 'auth.*.user_id' for the '*' where 'provider' + is 'blender-id'. + + :returns the user ID, or the empty string when the user has none. + """ + + bid_user_ids = [auth['user_id'] + for auth in db_user['auth'] + if auth['provider'] == 'blender-id'] + try: + return bid_user_ids[0] + except IndexError: + return '' + + def fetch_blenderid_user() -> dict: """Returns the user info of the currently logged in user from BlenderID. diff --git a/pillar/config.py b/pillar/config.py index e5938499..eb168fa7 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -99,8 +99,9 @@ FULL_FILE_ACCESS_ROLES = {'admin', 'subscriber', 'demo'} BLENDER_ID_CLIENT_ID = 'SPECIAL-SNOWFLAKE-57' BLENDER_ID_SUBCLIENT_ID = 'PILLAR' -# Blender ID user info API endpoint URL and auth token, only used for -# reconciling subscribers. The token requires the 'userinfo' scope. +# Blender ID user info API endpoint URL and auth token, used for +# reconciling subscribers and updating their info from /u/. +# The token requires the 'userinfo' scope. BLENDER_ID_USER_INFO_API = 'http://blender-id:8000/api/user/' BLENDER_ID_USER_INFO_TOKEN = '-set-in-config-local-' diff --git a/src/templates/users/edit_embed_base.pug b/src/templates/users/edit_embed_base.pug index 2f616666..7ce36b08 100644 --- a/src/templates/users/edit_embed_base.pug +++ b/src/templates/users/edit_embed_base.pug @@ -72,6 +72,7 @@ | none | {% endif %} + a.btn.btn-default(href="javascript:update_from_bid()") Update from Blender ID a.btn.btn-default(href="javascript:$('#user-edit-container').html('')") Cancel input#submit_edit_user.btn.btn-default( @@ -103,4 +104,16 @@ script(type="text/javascript"). new Clipboard('.copy-to-clipboard'); + function update_from_bid() { + var url = '{{ url_for("blender_cloud.subscription.update_subscription_for", user_id=user.user_id) }}'; + $.post(url) + .done(function(data) { + toastr.info('User updated from Blender ID'); + displayUser('{{ user.user_id }}'); + }) + .fail(function(data) { + toastr.error(data.responseText); + }); + } + | {% endblock %} From 1e012f860b09f3e958d51d3dd942c3bdcc20f453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 6 Dec 2017 11:58:21 +0100 Subject: [PATCH 05/16] Registered org-subscriber role so that it shows in the admin --- pillar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 49a3d4e3..1f15d21b 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -75,7 +75,7 @@ class PillarServer(Eve): # The default roles Pillar uses. Will probably all move to extensions at some point. self._user_roles: typing.Set[str] = { 'demo', 'admin', 'subscriber', 'homeproject', - 'protected', + 'protected', 'org-subscriber', 'service', 'badger', 'svner', 'urler', } self._user_roles_indexable: typing.Set[str] = {'demo', 'admin', 'subscriber'} From 2bcc26860f9ec3c9528ac4d812ee18c2ea54a4f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 6 Dec 2017 11:59:01 +0100 Subject: [PATCH 06/16] Removed 'subscriber' cap from 'admin' role This allows admins to test what happens when users do not have a subscription. To give the user subscriber capability, just grant demo role as well. --- pillar/auth/__init__.py | 2 +- pillar/config.py | 2 +- tests/test_api/test_auth.py | 2 +- tests/test_api/test_project_management.py | 7 ++++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pillar/auth/__init__.py b/pillar/auth/__init__.py index 9f9e3168..28b2c475 100644 --- a/pillar/auth/__init__.py +++ b/pillar/auth/__init__.py @@ -18,7 +18,7 @@ log = logging.getLogger(__name__) CAPABILITIES = collections.defaultdict(**{ 'subscriber': {'subscriber', 'home-project'}, 'demo': {'subscriber', 'home-project'}, - 'admin': {'subscriber', 'home-project', 'video-encoding', 'admin', + 'admin': {'video-encoding', 'admin', 'view-pending-nodes', 'edit-project-node-types'}, }, default_factory=frozenset) diff --git a/pillar/config.py b/pillar/config.py index eb168fa7..10602ac4 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -200,7 +200,7 @@ CELERY_BEAT_SCHEDULE = { USER_CAPABILITIES = defaultdict(**{ 'subscriber': {'subscriber', 'home-project'}, 'demo': {'subscriber', 'home-project'}, - 'admin': {'subscriber', 'home-project', 'video-encoding', 'admin', + 'admin': {'video-encoding', 'admin', 'view-pending-nodes', 'edit-project-node-types', 'create-organization'}, 'org-subscriber': {'subscriber', 'home-project'}, }, default_factory=frozenset) diff --git a/tests/test_api/test_auth.py b/tests/test_api/test_auth.py index 5c600cbb..13191454 100644 --- a/tests/test_api/test_auth.py +++ b/tests/test_api/test_auth.py @@ -677,7 +677,7 @@ class RequireRolesTest(AbstractPillarTest): self.assertFalse(called[0]) with self.app.test_request_context(): - self.login_api_as(ObjectId(24 * 'a'), ['admin']) + self.login_api_as(ObjectId(24 * 'a'), ['demo']) call_me() self.assertTrue(called[0]) diff --git a/tests/test_api/test_project_management.py b/tests/test_api/test_project_management.py index 04ada87a..d35126a1 100644 --- a/tests/test_api/test_project_management.py +++ b/tests/test_api/test_project_management.py @@ -95,7 +95,7 @@ class ProjectCreationTest(AbstractProjectTest): def test_project_creation_access_admin(self): """Admin-created projects should be public""" - proj = self._create_user_and_project(roles={'admin'}) + proj = self._create_user_and_project(roles={'admin', 'demo'}) self.assertEqual(['GET'], proj['permissions']['world']) def test_project_creation_access_subscriber(self): @@ -311,13 +311,14 @@ class ProjectEditTest(AbstractProjectTest): def test_delete_by_admin(self): # Create public test project. - project_info = self._create_user_and_project(['admin']) + project_info = self._create_user_and_project(['admin', 'demo']) project_id = project_info['_id'] project_url = '/api/projects/%s' % project_id # Create admin user that doesn't own the project, to check that # non-owner admins can delete projects too. - self._create_user_with_token(['admin'], 'admin-token', user_id='cafef00dbeefcafef00dbeef') + self._create_user_with_token(['admin'], 'admin-token', + user_id='cafef00dbeefcafef00dbeef') # Admin user should be able to DELETE. resp = self.client.delete(project_url, From 9fdcfff4fcd2efb59217ecead2cad35b8295b4b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 6 Dec 2017 14:39:30 +0100 Subject: [PATCH 07/16] Direct users to renewal page on Store instead of /join /join should only be used when someone can actually buy a new subscription. /renew should be used when someone already has a subscription that needs to be renewed. Since url_for('cloud.xxxx') makes no sense in Pillar, I just hard-coded /renew instead. --- pillar/web/nodes/custom/comments.py | 2 +- src/templates/_macros/_menu.pug | 5 +++++ src/templates/errors/layout.pug | 4 ++-- src/templates/nodes/custom/_node_details.pug | 9 +++++++++ .../nodes/custom/asset/video/view_embed.pug | 8 +++++++- .../nodes/custom/comment/list_embed.pug | 3 +++ src/templates/projects/index_dashboard.pug | 18 +++++++++++++++--- 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/pillar/web/nodes/custom/comments.py b/pillar/web/nodes/custom/comments.py index f61fae8a..14cc036f 100644 --- a/pillar/web/nodes/custom/comments.py +++ b/pillar/web/nodes/custom/comments.py @@ -134,7 +134,7 @@ def comments_for_node(node_id): project = Project({'_id': node.project}) can_post_comments = project.node_type_has_method('comment', 'POST', api=api) can_comment_override = request.args.get('can_comment', 'True') == 'True' - can_post_comments = can_post_comments and can_comment_override + can_post_comments = can_post_comments and can_comment_override and current_user.has_cap('subscriber') # Query for all children, i.e. comments on the node. comments = Node.all({ diff --git a/src/templates/_macros/_menu.pug b/src/templates/_macros/_menu.pug index 6fa73729..88ebba4d 100644 --- a/src/templates/_macros/_menu.pug +++ b/src/templates/_macros/_menu.pug @@ -40,6 +40,11 @@ li(class="dropdown") title="View subscription info") i.pi-heart-filled span You have a free account. + | {% elif current_user.has_cap('can-renew-subscription') %} + a.navbar-item(target='_blank', href="/renew", title="Renew subscription") + i.pi-heart + span.info Your subscription is not active. + span.renew Click here to renew. | {% else %} a.navbar-item( href="https://store.blender.org/product/membership/" diff --git a/src/templates/errors/layout.pug b/src/templates/errors/layout.pug index 0335f77b..50ddefb8 100644 --- a/src/templates/errors/layout.pug +++ b/src/templates/errors/layout.pug @@ -2,7 +2,7 @@ doctype html(lang="en") head meta(charset="utf-8") - title Error + title {% block title %}Error{% endblock %} meta(name="viewport", content="width=device-width, initial-scale=1.0") link(href="{{ url_for('static_pillar', filename='assets/ico/favicon.png') }}", rel="shortcut icon") @@ -10,7 +10,7 @@ html(lang="en") link(href="{{ url_for('static_pillar', filename='assets/css/font-pillar.css') }}", rel="stylesheet") link(href="{{ url_for('static_pillar', filename='assets/css/base.css') }}", rel="stylesheet") link(href='//fonts.googleapis.com/css?family=Roboto:300,400', rel='stylesheet', type='text/css') + | {% block head %}{% endblock %} body.error | {% block body %}{% endblock %} - diff --git a/src/templates/nodes/custom/_node_details.pug b/src/templates/nodes/custom/_node_details.pug index 511815d1..a4384169 100644 --- a/src/templates/nodes/custom/_node_details.pug +++ b/src/templates/nodes/custom/_node_details.pug @@ -74,6 +74,15 @@ | Download | {% endif %} + | {% elif current_user.has_cap('can-renew-subscription') %} + li.download + a.btn.btn-success( + title="Renew your subscription to download", + target="_blank", + href="/renew") + i.pi-heart + | Renew subscription to download + | {% elif current_user.is_authenticated %} li.download a.btn( diff --git a/src/templates/nodes/custom/asset/video/view_embed.pug b/src/templates/nodes/custom/asset/video/view_embed.pug index 528ef3b5..ae1faf90 100644 --- a/src/templates/nodes/custom/asset/video/view_embed.pug +++ b/src/templates/nodes/custom/asset/video/view_embed.pug @@ -27,8 +27,14 @@ span small Support Blender and get awesome stuff! hr - a.subscribe(href="{{ url_for('cloud.join') }}") Subscribe + | {% if current_user.has_cap('can-renew-subscription') %} + a.subscribe(href="/renew") You have a subscription, it just needs to be renewed. Renew your subscription now! + | {% else %} + a.subscribe(href="{{ url_for('cloud.join') }}") Subscribe to Blender Cloud. + | {% endif %} + | {% if current_user.is_anonymous %} a(href="{{ url_for('users.login') }}") Already a subscriber? Log in + | {% endif %} | {% endif %} diff --git a/src/templates/nodes/custom/comment/list_embed.pug b/src/templates/nodes/custom/comment/list_embed.pug index 79b167c2..bdc1c910 100644 --- a/src/templates/nodes/custom/comment/list_embed.pug +++ b/src/templates/nodes/custom/comment/list_embed.pug @@ -49,6 +49,9 @@ | {% if current_user.has_cap('subscriber') %} i.pi-lock | Only project members can comment. + | {% elif current_user.has_cap('can-renew-subscription') %} + i.pi-heart + a(href='/renew', target='_blank') Renew your subscription to join the conversation! | {% else %} | Join the conversation! Subscribe to Blender Cloud now. | {% endif %} diff --git a/src/templates/projects/index_dashboard.pug b/src/templates/projects/index_dashboard.pug index 67b313c8..41b3136a 100644 --- a/src/templates/projects/index_dashboard.pug +++ b/src/templates/projects/index_dashboard.pug @@ -40,12 +40,17 @@ meta(name="twitter:image", content="{{ url_for('static', filename='assets/img/ba | {% endif %} | {% if current_user.has_cap('subscriber') %} - li.create( + li.create#project-create( data-url="{{ url_for('projects.create') }}") - a.btn.btn-success#project-create( + a.btn.btn-success( href="{{ url_for('projects.create') }}") i.pi-plus | Create Project + | {% elif current_user.has_cap('can-renew-subscription') %} + li.create + a.btn(href="/renew", target="_blank") + i.pi-heart + | Renew subscription to create a project | {% endif %} nav.nav-tabs__tab.active#own_projects @@ -83,6 +88,13 @@ meta(name="twitter:image", content="{{ url_for('static', filename='assets/img/ba .projects__list-details a.title(href="{{ url_for('projects.create') }}") | Create a project to get started! + | {% elif current_user.has_cap('can-renew-subscription') %} + li.projects__list-item(data-url="https://store.blender.org/renew-my-subscription.php") + a.projects__list-thumbnail + i.pi-plus + .projects__list-details + a.title(href="https://store.blender.org/renew-my-subscription.php") + | Renew your Blender Cloud subscription to create your own projects! | {% else %} li.projects__list-item(data-url="/join") a.projects__list-thumbnail @@ -222,7 +234,7 @@ script. }); // Create project - $nav_tabs_list.find('li.create').on('click', function(e){ + $('#project-create').on('click', function(e){ e.preventDefault(); $(this).addClass('disabled'); From 6c4dd8ae0281ab17ffe26b5bfb7793189449ede8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 12:44:05 +0100 Subject: [PATCH 08/16] =?UTF-8?q?Fix=20T53339:=20Downgraded=20Werkzeug=200?= =?UTF-8?q?.12.2=20=E2=86=92=200.11.15?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index e5d7beb0..33b12b8f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -55,4 +55,4 @@ simplejson==3.10.0 six==1.10.0 vine==1.1.3 WTForms==2.1 -Werkzeug==0.12.2 +Werkzeug==0.11.15 From fc25ca9c03809affe2327ad6c70e9f0d7eb85559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 12:58:21 +0100 Subject: [PATCH 09/16] Replaced Bugsnag with Sentry - requires config changes! Note that pillar/bugsnag_extra.py still exists; I'm keeping it around for a while until we know what info we miss in Sentry, can port it, and then remove/refactor it. --- pillar/__init__.py | 57 ++++++++++++++++++++-------------------------- pillar/config.py | 9 ++++++-- requirements.txt | 2 +- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 1f15d21b..c1d0d883 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -21,6 +21,7 @@ from flask_babel import Babel, gettext as _ from flask.templating import TemplateNotFound import pymongo.collection import pymongo.database +from raven.contrib.flask import Sentry from werkzeug.local import LocalProxy @@ -59,7 +60,17 @@ class ConfigurationMissingError(SystemExit): """ -class PillarServer(Eve): +class BlinkerCompatibleEve(Eve): + """Workaround for https://github.com/pyeve/eve/issues/1087""" + + def __getattr__(self, name): + if name in {"im_self", "im_func"}: + raise AttributeError("type object '%s' has no attribute '%s'" % + (self.__class__.__name__, name)) + return super().__getattr__(name) + + +class PillarServer(BlinkerCompatibleEve): def __init__(self, app_root, **kwargs): from .extension import PillarExtension from celery import Celery @@ -94,7 +105,9 @@ class PillarServer(Eve): self._config_auth_token_hmac_key() self._config_tempdirs() self._config_git() - self._config_bugsnag() + + self.sentry: typing.Optional[Sentry] = None + self._config_sentry() self._config_google_cloud_storage() self.algolia_index_users = None @@ -187,39 +200,19 @@ class PillarServer(Eve): self.config['GIT_REVISION'] = 'unknown' self.log.info('Git revision %r', self.config['GIT_REVISION']) - def _config_bugsnag(self): - bugsnag_api_key = self.config.get('BUGSNAG_API_KEY') - if self.config.get('TESTING') or not bugsnag_api_key: - self.log.info('Bugsnag NOT configured.') + def _config_sentry(self): + sentry_dsn = self.config.get('SENTRY_CONFIG', {}).get('dsn') + if self.config.get('TESTING') or sentry_dsn in {'', '-set-in-config-local-'}: + self.log.warning('Sentry NOT configured.') + self.sentry = None return - import bugsnag - from bugsnag.handlers import BugsnagHandler + self.sentry = Sentry(self, logging=True, level=logging.WARNING, + logging_exclusions=('werkzeug',)) - release_stage = self.config.get('BUGSNAG_RELEASE_STAGE', 'unconfigured') - if self.config.get('DEBUG'): - release_stage += '-debug' - - bugsnag.configure( - api_key=bugsnag_api_key, - project_root="/data/git/pillar/pillar", - release_stage=release_stage - ) - - bs_handler = BugsnagHandler() - bs_handler.setLevel(logging.ERROR) - self.log.addHandler(bs_handler) - - # This is what bugsnag.flask.handle_exceptions also tries to do, - # but it passes the app to the connect() call, which causes an - # error. Since we only have one app, we can do without. - from flask import got_request_exception - from . import bugsnag_extra - - bugsnag.before_notify(bugsnag_extra.add_pillar_request_to_notification) - got_request_exception.connect(self.__notify_bugsnag) - - self.log.info('Bugsnag setup complete') + # bugsnag.before_notify(bugsnag_extra.add_pillar_request_to_notification) + # got_request_exception.connect(self.__notify_bugsnag) + self.log.info('Sentry setup complete') def __notify_bugsnag(self, sender, exception, **extra): import bugsnag diff --git a/pillar/config.py b/pillar/config.py index 10602ac4..2a263f25 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -65,8 +65,13 @@ GOOGLE_SITE_VERIFICATION = '' ADMIN_USER_GROUP = '5596e975ea893b269af85c0e' SUBSCRIBER_USER_GROUP = '5596e975ea893b269af85c0f' -BUGSNAG_API_KEY = '' -BUGSNAG_RELEASE_STAGE = 'development' + +SENTRY_CONFIG = { + 'dsn': '-set-in-config-local-', + # 'release': raven.fetch_git_sha(os.path.dirname(__file__)), +} +# See https://docs.sentry.io/clients/python/integrations/flask/#settings +SENTRY_USER_ATTRS = ['username', 'full_name', 'email', 'objectid'] ALGOLIA_USER = '-SECRET-' ALGOLIA_API_KEY = '-SECRET-' diff --git a/requirements.txt b/requirements.txt index 33b12b8f..4ec1b127 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,6 @@ attrs==16.2.0 algoliasearch==1.12.0 bcrypt==3.1.3 blinker==1.4 -bugsnag[flask]==3.1.1 bleach==1.4.3 celery[redis]==4.0.2 CommonMark==0.7.2 @@ -24,6 +23,7 @@ ndg-httpsclient==0.4.0 Pillow==4.1.1 python-dateutil==2.5.3 rauth==0.7.3 +raven[flask]==6.3.0 redis==2.10.5 WebOb==1.5.0 wheel==0.29.0 From 5c7f37a10014bbbadf47613d3ef4ff68bbbe19ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 13:02:23 +0100 Subject: [PATCH 10/16] Lowered dependency versions to satisfy Eve --- requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 4ec1b127..43a698ac 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,7 @@ bleach==1.4.3 celery[redis]==4.0.2 CommonMark==0.7.2 Eve==0.7.3 -Flask==0.12.2 +Flask==0.12 Flask-Babel==0.11.2 Flask-Cache==0.13.1 Flask-Script==2.0.6 @@ -18,7 +18,7 @@ Flask-WTF==0.12 gcloud==0.12.0 google-apitools==0.4.11 httplib2==0.9.2 -MarkupSafe==1.0 +MarkupSafe==0.23 ndg-httpsclient==0.4.0 Pillow==4.1.1 python-dateutil==2.5.3 From dfc224d8a9fbc3466bafe58d55940093b40c6eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 16:51:16 +0100 Subject: [PATCH 11/16] Added capability 'encode-video' and role 'video-encoder'. Both 'video-encoder' and 'admin' roles get 'encode-video' capability, which allows users to upload video that gets encoded & displayed as a video. For users without this capability videos are handled as regular downloads. --- pillar/__init__.py | 2 +- pillar/api/file_storage/__init__.py | 8 ++++---- pillar/config.py | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index c1d0d883..d98cc71e 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -86,7 +86,7 @@ class PillarServer(BlinkerCompatibleEve): # The default roles Pillar uses. Will probably all move to extensions at some point. self._user_roles: typing.Set[str] = { 'demo', 'admin', 'subscriber', 'homeproject', - 'protected', 'org-subscriber', + 'protected', 'org-subscriber', 'video-encoder', 'service', 'badger', 'svner', 'urler', } self._user_roles_indexable: typing.Set[str] = {'demo', 'admin', 'subscriber'} diff --git a/pillar/api/file_storage/__init__.py b/pillar/api/file_storage/__init__.py index 4e98cdc9..30c8f40c 100644 --- a/pillar/api/file_storage/__init__.py +++ b/pillar/api/file_storage/__init__.py @@ -26,7 +26,7 @@ from flask import url_for, helpers from pillar.api import utils from pillar.api.file_storage_backends.gcs import GoogleCloudStorageBucket, \ GoogleCloudStorageBlob -from pillar.api.utils import remove_private_keys, authentication +from pillar.api.utils import remove_private_keys from pillar.api.utils.authorization import require_login, user_has_role, \ user_matches_roles from pillar.api.utils.cdn import hash_file_path @@ -291,8 +291,8 @@ def process_file(bucket: Bucket, # TODO: overrule the content type based on file extention & magic numbers. mime_category, src_file['format'] = src_file['content_type'].split('/', 1) - # Prevent video handling for non-admins. - if not user_has_role('admin') and mime_category == 'video': + # Only allow video encoding when the user has the correct capability. + if not current_user.has_cap('encode-video') and mime_category == 'video': if src_file['format'].startswith('x-'): xified = src_file['format'] else: @@ -300,7 +300,7 @@ def process_file(bucket: Bucket, src_file['content_type'] = 'application/%s' % xified mime_category = 'application' - log.info('Not processing video file %s for non-admin user', file_id) + log.info('Not processing video file %s for non-video-encoding user', file_id) # Run the required processor, based on the MIME category. processors: typing.Mapping[str, typing.Callable] = { diff --git a/pillar/config.py b/pillar/config.py index 2a263f25..291787a3 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -205,8 +205,9 @@ CELERY_BEAT_SCHEDULE = { USER_CAPABILITIES = defaultdict(**{ 'subscriber': {'subscriber', 'home-project'}, 'demo': {'subscriber', 'home-project'}, - 'admin': {'video-encoding', 'admin', + 'admin': {'encode-video', 'admin', 'view-pending-nodes', 'edit-project-node-types', 'create-organization'}, + 'video-encoder': {'encode-video'}, 'org-subscriber': {'subscriber', 'home-project'}, }, default_factory=frozenset) From d20f3d56682596097dcd20d16c69030f5eac4211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 17:29:49 +0100 Subject: [PATCH 12/16] Removed manual bad JSON-encoding --- pillar/web/users/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/web/users/forms.py b/pillar/web/users/forms.py index 28910429..02a428cf 100644 --- a/pillar/web/users/forms.py +++ b/pillar/web/users/forms.py @@ -42,7 +42,7 @@ class UserProfileForm(Form): user = User.find(current_user.objectid, api=api) if user.username != self.username.data: username = User.find_first( - {'where': '{"username": "%s"}' % self.username.data}, + {'where': {"username": self.username.data}}, api=api) if username: From e1646adff65166b27ef641d3a7bc47296bc3ae14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 17:30:10 +0100 Subject: [PATCH 13/16] More modern use of super() --- pillar/web/users/forms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pillar/web/users/forms.py b/pillar/web/users/forms.py index 02a428cf..fa0fd108 100644 --- a/pillar/web/users/forms.py +++ b/pillar/web/users/forms.py @@ -31,10 +31,10 @@ class UserProfileForm(Form): r'^[\w.@+-]+$', message="Please do not use spaces")]) def __init__(self, csrf_enabled=False, *args, **kwargs): - super(UserProfileForm, self).__init__(csrf_enabled=False, *args, **kwargs) + super().__init__(csrf_enabled=csrf_enabled, *args, **kwargs) def validate(self): - rv = Form.validate(self) + rv = super().validate() if not rv: return False From 785145e1c1fcd4b0e5c33dd629bfe4d85f69b5fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 17:31:07 +0100 Subject: [PATCH 14/16] Nicer message when username already exists --- pillar/web/users/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/web/users/forms.py b/pillar/web/users/forms.py index fa0fd108..316813c9 100644 --- a/pillar/web/users/forms.py +++ b/pillar/web/users/forms.py @@ -46,7 +46,7 @@ class UserProfileForm(Form): api=api) if username: - self.username.errors.append('Sorry, username already exists!') + self.username.errors.append('Sorry, this username is already taken.') return False self.user = user From ca25078b3077232072d8d1cb369d5138b03bc776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 7 Dec 2017 17:31:26 +0100 Subject: [PATCH 15/16] Removed editing of full name from Cloud profile We take the full name from Blender ID instead. --- pillar/web/settings/routes.py | 5 +---- pillar/web/users/forms.py | 3 +-- src/templates/users/settings/profile.pug | 18 +++++++++--------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/pillar/web/settings/routes.py b/pillar/web/settings/routes.py index 5ed1d9fd..9623c79f 100644 --- a/pillar/web/settings/routes.py +++ b/pillar/web/settings/routes.py @@ -23,13 +23,10 @@ def profile(): api = system_util.pillar_api() user = User.find(current_user.objectid, api=api) - form = forms.UserProfileForm( - full_name=user.full_name, - username=user.username) + form = forms.UserProfileForm(username=user.username) if form.validate_on_submit(): try: - user.full_name = form.full_name.data user.username = form.username.data user.update(api=api) flash("Profile updated", 'success') diff --git a/pillar/web/users/forms.py b/pillar/web/users/forms.py index 316813c9..3ba76c65 100644 --- a/pillar/web/users/forms.py +++ b/pillar/web/users/forms.py @@ -24,14 +24,13 @@ class UserLoginForm(Form): class UserProfileForm(Form): - full_name = StringField('Full Name', validators=[DataRequired(), Length( - min=3, max=128, message="Min. 3 and max. 128 chars please")]) username = StringField('Username', validators=[DataRequired(), Length( min=3, max=128, message="Min. 3, max. 128 chars please"), Regexp( r'^[\w.@+-]+$', message="Please do not use spaces")]) def __init__(self, csrf_enabled=False, *args, **kwargs): super().__init__(csrf_enabled=csrf_enabled, *args, **kwargs) + self.user = None def validate(self): rv = super().validate() diff --git a/src/templates/users/settings/profile.pug b/src/templates/users/settings/profile.pug index 5d872da0..339b8e66 100644 --- a/src/templates/users/settings/profile.pug +++ b/src/templates/users/settings/profile.pug @@ -11,13 +11,6 @@ .settings-form form#settings-form(method='POST', action="{{url_for('settings.profile')}}") .left - .form-group - | {{ form.full_name.label }} - | {{ form.full_name(size=20, class='form-control') }} - | {% if form.full_name.errors %} - | {% for error in form.full_name.errors %}{{ error|e }}{% endfor %} - | {% endif %} - .form-group | {{ form.username.label }} | {{ form.username(size=20, class='form-control') }} @@ -25,8 +18,15 @@ | {% for error in form.username.errors %}{{ error|e }}{% endfor %} | {% endif %} - .form-group.settings-password - | {{ _("Change your password at") }} #[a(href="https://blender.org/id/change") Blender ID] + .form-group + label {{ _("Full name") }} + p {{ current_user.full_name }} + .form-group + label {{ _("E-mail") }} + p {{ current_user.email }} + + .form-group + | {{ _("Change your full name, email, and password at") }} #[a(href="https://blender.org/id/",target='_blank') Blender ID]. .right .settings-avatar From 821f11393c5c2cef843520092682dd4fbf4f8f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 10:42:43 +0100 Subject: [PATCH 16/16] Link to 'edit profile' page on Blender ID directly --- src/templates/users/settings/profile.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/templates/users/settings/profile.pug b/src/templates/users/settings/profile.pug index 339b8e66..f2d93c29 100644 --- a/src/templates/users/settings/profile.pug +++ b/src/templates/users/settings/profile.pug @@ -26,7 +26,7 @@ p {{ current_user.email }} .form-group - | {{ _("Change your full name, email, and password at") }} #[a(href="https://blender.org/id/",target='_blank') Blender ID]. + | {{ _("Change your full name, email, and password at") }} #[a(href="https://www.blender.org/id/settings/profile",target='_blank') Blender ID]. .right .settings-avatar