From 8a400c5c0f3a658096f52fb04d6feb1d0c922183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 14:03:30 +0100 Subject: [PATCH 01/27] Gracefully handle users with empty full_name --- pillar/api/utils/authentication.py | 4 ++ tests/test_api/test_auth.py | 68 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/pillar/api/utils/authentication.py b/pillar/api/utils/authentication.py index 65a50b79..4ac9c84b 100644 --- a/pillar/api/utils/authentication.py +++ b/pillar/api/utils/authentication.py @@ -371,6 +371,10 @@ def upsert_user(db_user): raise wz_exceptions.InternalServerError( 'Non-ObjectID string found in user.groups: %s' % db_user) + if not db_user['full_name']: + # Blender ID doesn't need a full name, but we do. + db_user['full_name'] = db_user['username'] + r = {} for retry in range(5): if '_id' in db_user: diff --git a/tests/test_api/test_auth.py b/tests/test_api/test_auth.py index 13191454..8c930db8 100644 --- a/tests/test_api/test_auth.py +++ b/tests/test_api/test_auth.py @@ -717,6 +717,74 @@ class UserCreationTest(AbstractPillarTest): db_user = users_coll.find()[0] self.assertEqual(db_user['email'], TEST_EMAIL_ADDRESS) + @responses.activate + def test_create_by_auth_no_full_name(self): + """Blender ID does not require full name, we do.""" + + with self.app.test_request_context(): + users_coll = self.app.db().users + self.assertEqual(0, users_coll.count()) + + bid_resp = {'status': 'success', + 'user': {'email': TEST_EMAIL_ADDRESS, + 'full_name': '', + 'id': ctd.BLENDER_ID_TEST_USERID}, + '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=bid_resp, + status=200) + + token = 'this is my life now' + self.get('/api/users/me', auth_token=token) + + with self.app.test_request_context(): + users_coll = self.app.db().users + self.assertEqual(1, users_coll.count()) + + db_user = users_coll.find()[0] + self.assertEqual(db_user['email'], TEST_EMAIL_ADDRESS) + self.assertNotEqual('', db_user['full_name']) + + @responses.activate + def test_update_by_auth_no_full_name(self): + """Blender ID does not require full name, we do.""" + self.enter_app_context() + users_coll = self.app.db().users + self.assertEqual(0, users_coll.count()) + + # First request will create the user, the 2nd request will update. + self.mock_blenderid_validate_happy() + bid_resp = {'status': 'success', + 'user': {'email': TEST_EMAIL_ADDRESS, + 'full_name': '', + 'id': ctd.BLENDER_ID_TEST_USERID}, + '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=bid_resp, + status=200) + + token = 'this is my life now' + self.get('/api/users/me', auth_token=token) + + # Clear out the full name of the user. This could happen for some + # reason, and it shouldn't break the login flow. + users_coll.update_many({}, {'$set': {'full_name': ''}}) + + # Delete all tokens to force a re-check with Blender ID + tokens_coll = self.app.db('tokens') + tokens_coll.delete_many({}) + + self.get('/api/users/me', auth_token=token) + + self.assertEqual(1, users_coll.count()) + + db_user = users_coll.find()[0] + self.assertEqual(db_user['email'], TEST_EMAIL_ADDRESS) + self.assertNotEqual('', db_user['full_name']) + def test_user_without_email_address(self): """Regular users should always have an email address. From 8eee0d57b61ecf8388ca27bde94728ba496db253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 14:03:45 +0100 Subject: [PATCH 02/27] Update token expiry in tests to be a bit more into the future. --- pillar/tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/tests/__init__.py b/pillar/tests/__init__.py index d29ebec1..7336c0ba 100644 --- a/pillar/tests/__init__.py +++ b/pillar/tests/__init__.py @@ -42,7 +42,7 @@ BLENDER_ID_USER_RESPONSE = {'status': 'success', 'user': {'email': TEST_EMAIL_ADDRESS, 'full_name': TEST_FULL_NAME, 'id': ctd.BLENDER_ID_TEST_USERID}, - 'token_expires': 'Mon, 1 Jan 2018 01:02:03 GMT'} + 'token_expires': 'Mon, 1 Jan 2218 01:02:03 GMT'} class PillarTestServer(pillar.PillarServer): From 3ea2504e8cb90c4bc10f2b5d6559539d7f1d07e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 14:46:01 +0100 Subject: [PATCH 03/27] Log more information in Sentry --- pillar/__init__.py | 9 +++++---- pillar/sentry_extra.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 pillar/sentry_extra.py diff --git a/pillar/__init__.py b/pillar/__init__.py index d98cc71e..0ab8ab6c 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -21,7 +21,6 @@ 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 @@ -42,6 +41,7 @@ import pillar.web.jinja from . import api from . import web from . import auth +from . import sentry_extra import pillar.api.organizations empty_settings = { @@ -106,7 +106,7 @@ class PillarServer(BlinkerCompatibleEve): self._config_tempdirs() self._config_git() - self.sentry: typing.Optional[Sentry] = None + self.sentry: typing.Optional[sentry_extra.PillarSentry] = None self._config_sentry() self._config_google_cloud_storage() @@ -207,8 +207,9 @@ class PillarServer(BlinkerCompatibleEve): self.sentry = None return - self.sentry = Sentry(self, logging=True, level=logging.WARNING, - logging_exclusions=('werkzeug',)) + self.sentry = sentry_extra.PillarSentry( + self, logging=True, level=logging.WARNING, + logging_exclusions=('werkzeug',)) # bugsnag.before_notify(bugsnag_extra.add_pillar_request_to_notification) # got_request_exception.connect(self.__notify_bugsnag) diff --git a/pillar/sentry_extra.py b/pillar/sentry_extra.py new file mode 100644 index 00000000..01b033fe --- /dev/null +++ b/pillar/sentry_extra.py @@ -0,0 +1,42 @@ +from raven.contrib.flask import Sentry + +from .auth import current_user +from . import current_app + + +class PillarSentry(Sentry): + """Flask Sentry with Pillar support. + + This is mostly for obtaining user information on API calls, + and for preventing the auth tokens to be logged as user ID. + """ + + def get_user_info(self, request): + user_info = super().get_user_info(request) + + # The auth token is stored as the user ID in the flask_login + # current_user object, so don't send that to Sentry. + user_info.pop('id', None) + + if len(user_info) > 1: + # Sentry always includes the IP address, but when they find a + # logged-in user, they add more info. In that case we're done. + return user_info + + # This is pretty much a copy-paste from Sentry, except that it uses + # pillar.auth.current_user instead. + try: + if not current_user.is_authenticated: + return user_info + except AttributeError: + # HACK: catch the attribute error thrown by flask-login is not attached + # > current_user = LocalProxy(lambda: _request_ctx_stack.top.user) + # E AttributeError: 'RequestContext' object has no attribute 'user' + return user_info + + if 'SENTRY_USER_ATTRS' in current_app.config: + for attr in current_app.config['SENTRY_USER_ATTRS']: + if hasattr(current_user, attr): + user_info[attr] = getattr(current_user, attr) + + return user_info From 199c6b1f77ccb32335ec155a4a778b8bacc417a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 14:46:58 +0100 Subject: [PATCH 04/27] Auth: also support Bearer token authentication This is commonly used in OAuth-authenticated calls, and can help us break away from the username-is-auth-token stuff currently in use. --- pillar/api/utils/authentication.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pillar/api/utils/authentication.py b/pillar/api/utils/authentication.py index 4ac9c84b..e0d14cb7 100644 --- a/pillar/api/utils/authentication.py +++ b/pillar/api/utils/authentication.py @@ -118,9 +118,13 @@ def validate_token(): from pillar.auth import AnonymousUser + auth_header = request.headers.get('Authorization') or '' if request.authorization: token = request.authorization.username oauth_subclient = request.authorization.password + elif auth_header.startswith('Bearer '): + token = auth_header[7:].strip() + oauth_subclient = '' else: # Check the session, the user might be logged in through Flask-Login. from pillar import auth From b77527e9a2e9aa86229b2d1d60cbfb7a27009c68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Dec 2017 14:52:38 +0100 Subject: [PATCH 05/27] =?UTF-8?q?No=20'=E2=80=A6'.format(=E2=80=A6)=20in?= =?UTF-8?q?=20logging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pillar/api/utils/authentication.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pillar/api/utils/authentication.py b/pillar/api/utils/authentication.py index e0d14cb7..261c9eb5 100644 --- a/pillar/api/utils/authentication.py +++ b/pillar/api/utils/authentication.py @@ -69,19 +69,19 @@ def find_user_in_db(user_info: dict, provider='blender-id'): users = current_app.data.driver.db['users'] + user_id = user_info['id'] query = {'$or': [ {'auth': {'$elemMatch': { - 'user_id': str(user_info['id']), + 'user_id': str(user_id), 'provider': provider}}}, {'email': user_info['email']}, - ]} + ]} log.debug('Querying: %s', query) db_user = users.find_one(query) if db_user: - log.debug('User with {provider} id {user_id} already in our database, ' - 'updating with info from {provider}.'.format( - provider=provider, user_id=user_info['id'])) + log.debug('User with %s id %s already in our database, updating with info from %s', + provider, user_id, provider) db_user['email'] = user_info['email'] # Find out if an auth entry for the current provider already exists @@ -89,13 +89,13 @@ def find_user_in_db(user_info: dict, provider='blender-id'): if not provider_entry: db_user['auth'].append({ 'provider': provider, - 'user_id': str(user_info['id']), + 'user_id': str(user_id), 'token': ''}) else: - log.debug('User %r not yet in our database, create a new one.', user_info['id']) + log.debug('User %r not yet in our database, create a new one.', user_id) db_user = create_new_user_document( email=user_info['email'], - user_id=user_info['id'], + user_id=user_id, username=user_info['full_name'], provider=provider) db_user['username'] = make_unique_username(user_info['email']) @@ -184,7 +184,6 @@ def validate_this_token(token, oauth_subclient=None): def remove_token(token: str): """Removes the token from the database.""" - tokens_coll = current_app.db('tokens') token_hashed = hash_auth_token(token) From 1d1e588d57017df03b70ccb7440c45f522f2e379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 12 Dec 2017 10:56:34 +0100 Subject: [PATCH 06/27] Switch: Always follow PREFERRED_URL_SCHEME instead of the request scheme When getting an _external=True URL, we shouldn't use the scheme of the current request at all (this depends on HaProxy forwarding the correct headers, which might fail when misconfigured) and just always use the preferred URL scheme. This fixes it at least for the user switching, because Blender ID will refuse to redirect back to a http:// URL. --- pillar/web/users/routes.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pillar/web/users/routes.py b/pillar/web/users/routes.py index 713e5ffc..dd04bb34 100644 --- a/pillar/web/users/routes.py +++ b/pillar/web/users/routes.py @@ -5,6 +5,7 @@ from flask import abort, Blueprint, redirect, render_template, request, session, from flask_login import login_required from werkzeug import exceptions as wz_exceptions +from pillar import current_app import pillar.api.blender_cloud.subscription import pillar.auth from pillar.api.blender_cloud.subscription import update_subscription @@ -16,6 +17,7 @@ from pillar.auth.oauth import OAuthSignIn, ProviderConfigurationMissing, Provide from pillar.web import system_util from pillarsdk import exceptions as sdk_exceptions from pillarsdk.users import User + from . import forms log = logging.getLogger(__name__) @@ -121,9 +123,11 @@ def switch(): # Without this URL, the user will remain on the Blender ID site. We want them to come # back to the Cloud after switching users. + scheme = current_app.config.get('PREFERRED_URL_SCHEME', 'https') next_url_after_bid_login = url_for('users.login', next=next_url_after_cloud_login, force='yes', + _scheme=scheme, _external=True) return redirect(blender_id.switch_user_url(next_url=next_url_after_bid_login)) From ae8c6e92fc6b956e5b6297198b1b728440bba803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 12 Dec 2017 11:25:32 +0100 Subject: [PATCH 07/27] Fix forced login for user switching --- pillar/web/users/routes.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pillar/web/users/routes.py b/pillar/web/users/routes.py index dd04bb34..44f9ccd2 100644 --- a/pillar/web/users/routes.py +++ b/pillar/web/users/routes.py @@ -85,8 +85,7 @@ def oauth_callback(provider): def login(): if request.args.get('force'): log.debug('Forcing logout of user before rendering login page.') - logout_user() - session.clear() + pillar.auth.logout_user() session['next_after_login'] = request.args.get('next') or request.referrer return render_template('login.html') From 6d37046933bb9f8afe8f06d6ee85928a8d2f67bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 12 Dec 2017 11:48:48 +0100 Subject: [PATCH 08/27] Fixed "leave shared project" javascript Now the project is actually removed from the page. This isn't optimal; see T53546 for a followup. --- src/templates/projects/index_dashboard.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/templates/projects/index_dashboard.pug b/src/templates/projects/index_dashboard.pug index 41b3136a..d51d7f4b 100644 --- a/src/templates/projects/index_dashboard.pug +++ b/src/templates/projects/index_dashboard.pug @@ -264,7 +264,7 @@ script. $projects_list.find('span.user-remove-confirm').on('click', function(e){ e.stopPropagation(); e.preventDefault(); - var parent = $(this).closest('projects__list-item'); + var parent = $(this).closest('.projects__list-item'); function removeUser(userId, projectUrl){ $.post(projectUrl, {user_id: userId, action: 'remove'}) From 20ca3f8ee40ce26865ae68dc418c380a2f05f639 Mon Sep 17 00:00:00 2001 From: Francesco Siddi Date: Tue, 12 Dec 2017 18:49:52 +0100 Subject: [PATCH 09/27] Rename blender_id url to blender-id This fixes a non-compliant to RFC 1178 exception raised by the Django implementation of Blender ID. The issue is debated here https://code.djangoproject.com/ticket/20264. --- pillar/config.py | 4 ++-- pillar/tests/config_testing.py | 2 +- tests/test_api/test_oauth.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pillar/config.py b/pillar/config.py index 291787a3..11a5a0ab 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -28,7 +28,7 @@ SECRET_KEY = '' AUTH_TOKEN_HMAC_KEY = b'' # Authentication settings -BLENDER_ID_ENDPOINT = 'http://blender_id:8000/' +BLENDER_ID_ENDPOINT = 'http://blender-id:8000/' PILLAR_SERVER_ENDPOINT = 'http://pillar:5001/api/' @@ -116,7 +116,7 @@ BLENDER_ID_USER_INFO_TOKEN = '-set-in-config-local-' # 'blender-id': { # 'id': 'CLOUD-OF-SNOWFLAKES-43', # 'secret': 'thesecret', -# 'base_url': 'http://blender_id:8000/' +# 'base_url': 'http://blender-id:8000/' # } # } # OAuth providers are defined in pillar.auth.oauth diff --git a/pillar/tests/config_testing.py b/pillar/tests/config_testing.py index c81994b6..f0f4ba21 100644 --- a/pillar/tests/config_testing.py +++ b/pillar/tests/config_testing.py @@ -24,7 +24,7 @@ OAUTH_CREDENTIALS = { 'blender-id': { 'id': 'blender-id-app-id', 'secret': 'blender-id–secret', - 'base_url': 'http://blender_id:8000/' + 'base_url': 'http://blender-id:8000/' }, 'facebook': { 'id': 'fb-app-id', diff --git a/tests/test_api/test_oauth.py b/tests/test_api/test_oauth.py index 152a8d46..6c5c358b 100644 --- a/tests/test_api/test_oauth.py +++ b/tests/test_api/test_oauth.py @@ -12,7 +12,7 @@ class OAuthTests(AbstractPillarTest): oauth_provider = OAuthSignIn.get_provider('blender-id') self.assertIsInstance(oauth_provider, BlenderIdSignIn) - self.assertEqual(oauth_provider.service.base_url, 'http://blender_id:8000/api/') + self.assertEqual(oauth_provider.service.base_url, 'http://blender-id:8000/api/') def test_provider_not_implemented(self): from pillar.auth.oauth import OAuthSignIn, ProviderNotImplemented @@ -46,11 +46,11 @@ class OAuthTests(AbstractPillarTest): def test_provider_callback_happy(self): from pillar.auth.oauth import OAuthSignIn - responses.add(responses.POST, 'http://blender_id:8000/oauth/token', + responses.add(responses.POST, 'http://blender-id:8000/oauth/token', json={'access_token': 'successful-token'}, status=200) - responses.add(responses.GET, 'http://blender_id:8000/api/user', + responses.add(responses.GET, 'http://blender-id:8000/api/user', json={'id': '7', 'email': 'harry@blender.org'}, status=200) From a7693aa78dcf0a0a77e113f34afa63fb4f615441 Mon Sep 17 00:00:00 2001 From: Francesco Siddi Date: Wed, 13 Dec 2017 11:08:33 +0100 Subject: [PATCH 10/27] Switch from macros to blocks for navigation menus This affects the user and notifications menus. It happens for two reasons: - the only argument passed to the macros was current_user, which is always available - we want to enable overriding and adding items to the menus via extensions At the moment only the user menu takes advantage of the base template, since the blender-cloud extension makes use of it, while notifications.pug does not need it yet. --- src/templates/_macros/_menu.pug | 142 -------------------------- src/templates/menus/notifications.pug | 23 +++++ src/templates/menus/user.pug | 1 + src/templates/menus/user_base.pug | 66 ++++++++++++ 4 files changed, 90 insertions(+), 142 deletions(-) delete mode 100644 src/templates/_macros/_menu.pug create mode 100644 src/templates/menus/notifications.pug create mode 100644 src/templates/menus/user.pug create mode 100644 src/templates/menus/user_base.pug diff --git a/src/templates/_macros/_menu.pug b/src/templates/_macros/_menu.pug deleted file mode 100644 index 88ebba4d..00000000 --- a/src/templates/_macros/_menu.pug +++ /dev/null @@ -1,142 +0,0 @@ -| {% macro navigation_menu_user(current_user) %} - -| {% if current_user.is_authenticated %} - -| {% if current_user.has_role('demo') %} -| {% set subscription = 'demo' %} -| {% elif current_user.has_cap('subscriber') %} -| {% set subscription = 'subscriber' %} -| {% else %} -| {% set subscription = 'none' %} -| {% endif %} - -li(class="dropdown") - a.navbar-item.dropdown-toggle(href="#", data-toggle="dropdown", title="{{ current_user.email }}") - img.gravatar( - src="{{ current_user.gravatar }}", - class="{{ subscription }}", - alt="Avatar") - .special(class="{{ subscription }}") - | {% if subscription == 'subscriber' %} - i.pi-check - | {% elif subscription == 'demo' %} - i.pi-heart-filled - | {% else %} - i.pi-attention - | {% endif %} - - ul.dropdown-menu - | {% if not current_user.has_role('protected') %} - li.subscription-status(class="{{ subscription }}") - | {% if subscription == 'subscriber' %} - a.navbar-item( - href="{{url_for('settings.billing')}}" - title="View subscription info") - i.pi-grin - span Your subscription is active! - | {% elif subscription == 'demo' %} - a.navbar-item( - href="{{url_for('settings.billing')}}" - 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/" - title="Renew subscription") - i.pi-unhappy - span.info Your subscription is not active. - span.renew Click here to renew. - | {% endif %} - - li - a.navbar-item( - href="{{ url_for('projects.home_project') }}" - title="Home") - i.pi-home - | Home - - li - a.navbar-item( - href="{{ url_for('projects.index') }}" - title="My Projects") - i.pi-star - | My Projects - - | {% if current_user.has_organizations() %} - li - a.navbar-item( - href="{{ url_for('pillar.web.organizations.index') }}" - title="My Organizations") - i.pi-users - | My Organizations - | {% endif %} - - li - a.navbar-item( - href="{{ url_for('settings.profile') }}" - title="Settings") - i.pi-cog - | Settings - - li - a.navbar-item( - href="{{ url_for('settings.billing') }}" - title="Billing") - i.pi-credit-card - | Subscription - - li.divider(role="separator") - | {% endif %} - - li - a.navbar-item( - href="{{ url_for('users.logout') }}") - i.pi-log-out(title="Log Out") - | Log out - a.navbar-item.subitem( - href="{{ url_for('users.switch') }}") - i.pi-blank - | Not {{ current_user.full_name }}? - -| {% else %} - -li.nav-item-sign-in - a.navbar-item(href="{{ url_for('users.login') }}") - | Log in -| {% endif %} - -| {% endmacro %} - - -| {% macro navigation_menu_notifications(current_user) %} - -| {% if current_user.is_authenticated %} - -li.nav-notifications - a.navbar-item#notifications-toggle( - title="Notifications", - data-toggle="tooltip", - data-placement="bottom") - i.pi-notifications-none.nav-notifications-icon - span#notifications-count - span - .flyout-hat - - #notifications.flyout.notifications - .flyout-content - span.flyout-title Notifications - a#notifications-markallread( - title="Mark All as Read", - href="/notifications/read-all") - | Mark All as Read - - | {% include '_notifications.html' %} - -| {% endif %} -| {% endmacro %} diff --git a/src/templates/menus/notifications.pug b/src/templates/menus/notifications.pug new file mode 100644 index 00000000..ed74b481 --- /dev/null +++ b/src/templates/menus/notifications.pug @@ -0,0 +1,23 @@ +| {% if current_user.is_authenticated %} + +li.nav-notifications + a.navbar-item#notifications-toggle( + title="Notifications", + data-toggle="tooltip", + data-placement="bottom") + i.pi-notifications-none.nav-notifications-icon + span#notifications-count + span + .flyout-hat + + #notifications.flyout.notifications + .flyout-content + span.flyout-title Notifications + a#notifications-markallread( + title="Mark All as Read", + href="/notifications/read-all") + | Mark All as Read + + | {% include '_notifications.html' %} + +| {% endif %} diff --git a/src/templates/menus/user.pug b/src/templates/menus/user.pug new file mode 100644 index 00000000..24f56d53 --- /dev/null +++ b/src/templates/menus/user.pug @@ -0,0 +1 @@ +| {% extends 'menus/user_base.html' %} diff --git a/src/templates/menus/user_base.pug b/src/templates/menus/user_base.pug new file mode 100644 index 00000000..079fc589 --- /dev/null +++ b/src/templates/menus/user_base.pug @@ -0,0 +1,66 @@ +| {% block menu_body %} +| {% if current_user.is_authenticated %} + +li(class="dropdown") + | {% block menu_avatar %} + a.navbar-item.dropdown-toggle(href="#", data-toggle="dropdown", title="{{ current_user.email }}") + img.gravatar( + src="{{ current_user.gravatar }}", + alt="Avatar") + | {% endblock menu_avatar %} + + ul.dropdown-menu + | {% if not current_user.has_role('protected') %} + | {% block menu_list %} + li + a.navbar-item( + href="{{ url_for('projects.home_project') }}" + title="Home") + i.pi-home + | Home + + li + a.navbar-item( + href="{{ url_for('projects.index') }}" + title="My Projects") + i.pi-star + | My Projects + + | {% if current_user.has_organizations() %} + li + a.navbar-item( + href="{{ url_for('pillar.web.organizations.index') }}" + title="My Organizations") + i.pi-users + | My Organizations + | {% endif %} + + li + a.navbar-item( + href="{{ url_for('settings.profile') }}" + title="Settings") + i.pi-cog + | Settings + + | {% endblock menu_list %} + + li.divider(role="separator") + | {% endif %} + + li + a.navbar-item( + href="{{ url_for('users.logout') }}") + i.pi-log-out(title="Log Out") + | Log out + a.navbar-item.subitem( + href="{{ url_for('users.switch') }}") + i.pi-blank + | Not {{ current_user.full_name }}? + +| {% else %} + +li.nav-item-sign-in + a.navbar-item(href="{{ url_for('users.login') }}") + | Log in +| {% endif %} +| {% endblock menu_body %} From e0604fc217ebef8dafcd3c6a589caa9ad846bfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 15 Dec 2017 11:23:16 +0100 Subject: [PATCH 11/27] Reduce log level for something that's fine Missing emails can happen when creating a service account, we shouldn't log a warning for this. --- pillar/api/users/hooks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pillar/api/users/hooks.py b/pillar/api/users/hooks.py index 46e4ef9a..d4a5271c 100644 --- a/pillar/api/users/hooks.py +++ b/pillar/api/users/hooks.py @@ -191,8 +191,9 @@ def after_inserting_users(user_docs): 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) + # Missing emails can happen when creating a service account, it's fine. + log.info('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) From 92fe39ddac5703e8198a4f1e2bcf81901b14b8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 19 Dec 2017 10:45:34 +0100 Subject: [PATCH 12/27] Prevent shadowing of name from outer scope --- pillar/api/blender_cloud/subscription.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pillar/api/blender_cloud/subscription.py b/pillar/api/blender_cloud/subscription.py index e6fe6fe9..ef906579 100644 --- a/pillar/api/blender_cloud/subscription.py +++ b/pillar/api/blender_cloud/subscription.py @@ -31,7 +31,7 @@ def update_subscription() -> typing.Tuple[str, int]: """ my_log: logging.Logger = log.getChild('update_subscription') - current_user = auth.get_current_user() + real_current_user = auth.get_current_user() # multiple accesses, just get unproxied. try: bid_user = blender_id.fetch_blenderid_user() @@ -41,10 +41,10 @@ def update_subscription() -> typing.Tuple[str, int]: if not bid_user: my_log.warning('Logged in user %s has no BlenderID account! ' - 'Unable to update subscription status.', current_user.user_id) + 'Unable to update subscription status.', real_current_user.user_id) return '', 204 - do_update_subscription(current_user, bid_user) + do_update_subscription(real_current_user, bid_user) return '', 204 From 05ad824dcb8fe71916291a7a03231ef66ed15a31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 20 Dec 2017 13:34:11 +0100 Subject: [PATCH 13/27] Allow UserClass instantiation without database ID This allows us to inspect the capabilities that would be given to a user, without actually creating the user in the database first. --- pillar/auth/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pillar/auth/__init__.py b/pillar/auth/__init__.py index 28b2c475..a00e90c0 100644 --- a/pillar/auth/__init__.py +++ b/pillar/auth/__init__.py @@ -49,7 +49,7 @@ class UserClass(flask_login.UserMixin): user = cls(token) - user.user_id = db_user['_id'] + user.user_id = db_user.get('_id') user.roles = db_user.get('roles') or [] user.group_ids = db_user.get('groups') or [] user.email = db_user.get('email') or '' @@ -57,7 +57,7 @@ class UserClass(flask_login.UserMixin): user.full_name = db_user.get('full_name') or '' # Derived properties - user.objectid = str(db_user['_id']) + user.objectid = str(user.user_id or '') user.gravatar = utils.gravatar(user.email) user.groups = [str(g) for g in user.group_ids] user.collect_capabilities() From c545053b859d8d508b3b0589402e8da5ad84d7f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 20 Dec 2017 13:34:17 +0100 Subject: [PATCH 14/27] Declare return type --- pillar/tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/tests/__init__.py b/pillar/tests/__init__.py index 7336c0ba..453cb21d 100644 --- a/pillar/tests/__init__.py +++ b/pillar/tests/__init__.py @@ -251,7 +251,7 @@ class AbstractPillarTest(TestMinimal): return result.inserted_id def create_user(self, user_id='cafef00dc379cf10c4aaceaf', roles=('subscriber',), - groups=None, *, token: str = None, email: str = TEST_EMAIL_ADDRESS): + groups=None, *, token: str = None, email: str = TEST_EMAIL_ADDRESS) -> ObjectId: from pillar.api.utils.authentication import make_unique_username import uuid From dab8fbae6df73ccedd69b4fb32a3496d1120c082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 20 Dec 2017 13:34:27 +0100 Subject: [PATCH 15/27] create_new_user_document: allow passing the full name --- pillar/api/utils/authentication.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pillar/api/utils/authentication.py b/pillar/api/utils/authentication.py index 261c9eb5..30f4874d 100644 --- a/pillar/api/utils/authentication.py +++ b/pillar/api/utils/authentication.py @@ -264,13 +264,13 @@ def create_new_user(email, username, user_id): def create_new_user_document(email, user_id, username, provider='blender-id', - token=''): + token='', *, full_name=''): """Creates a new user document, without storing it in MongoDB. The token parameter is a password in case provider is "local". """ user_data = { - 'full_name': username, + 'full_name': full_name or username, 'username': username, 'email': email, 'auth': [{ From b7bf29c06e1efc7a484cbe2c0d2278450f94910f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 20 Dec 2017 14:57:55 +0100 Subject: [PATCH 16/27] Added user_is_unknown_member() to OrgManager --- pillar/api/organizations/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pillar/api/organizations/__init__.py b/pillar/api/organizations/__init__.py index 27463eb9..1c80d981 100644 --- a/pillar/api/organizations/__init__.py +++ b/pillar/api/organizations/__init__.py @@ -376,6 +376,13 @@ class OrgManager: return bool(org_count) + def user_is_unknown_member(self, member_email: str) -> bool: + """Return True iff the email is an unknown member of some org.""" + + org_coll = current_app.db('organizations') + org_count = org_coll.count({'unknown_members': member_email}) + return bool(org_count) + def setup_app(app): from . import patch, hooks From ef1609efc2c667cb9ebe86f6f0bf503696310e77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 21 Dec 2017 12:58:06 +0100 Subject: [PATCH 17/27] Added abs_url() Jinja function for proper absolute URLs abs_url(x) is a shortcut for url_for(x, _external=True, _schema=app.config['SCHEMA']), and should be used for all URLs that should include the hostname and schema. --- pillar/__init__.py | 2 +- pillar/web/jinja.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 0ab8ab6c..2627692e 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -425,7 +425,7 @@ class PillarServer(BlinkerCompatibleEve): custom_jinja_loader = jinja2.ChoiceLoader(paths_list) self.jinja_loader = custom_jinja_loader - pillar.web.jinja.setup_jinja_env(self.jinja_env) + pillar.web.jinja.setup_jinja_env(self.jinja_env, self.config) # Register context processors from extensions for ext in self.pillar_extensions.values(): diff --git a/pillar/web/jinja.py b/pillar/web/jinja.py index 04a7c280..76d6a219 100644 --- a/pillar/web/jinja.py +++ b/pillar/web/jinja.py @@ -1,5 +1,6 @@ """Our custom Jinja filters and other template stuff.""" +import functools import logging import typing @@ -146,7 +147,7 @@ def do_yesno(value, arg=None): return no -def setup_jinja_env(jinja_env): +def setup_jinja_env(jinja_env, app_config: dict): jinja_env.filters['pretty_date'] = format_pretty_date jinja_env.filters['pretty_date_time'] = format_pretty_date_time jinja_env.filters['undertitle'] = format_undertitle @@ -157,5 +158,8 @@ def setup_jinja_env(jinja_env): jinja_env.filters['yesno'] = do_yesno jinja_env.filters['repr'] = repr jinja_env.globals['url_for_node'] = do_url_for_node + jinja_env.globals['abs_url'] = functools.partial(flask.url_for, + _external=True, + _scheme=app_config['SCHEME']) jinja_env.globals['session'] = flask.session jinja_env.globals['current_user'] = flask_login.current_user From 01f81ce4d5f81ac5063dfe909e6ccbd39237801d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 21 Dec 2017 12:59:32 +0100 Subject: [PATCH 18/27] Send a Blinker signal when someone's subscription status changes This is very close to the 'roles changed' signal, with the difference that it is sent only once for multiple role changes. --- pillar/api/blender_cloud/subscription.py | 13 ++++++++++ tests/test_api/test_subscriptions.py | 30 ++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/pillar/api/blender_cloud/subscription.py b/pillar/api/blender_cloud/subscription.py index ef906579..569d6d13 100644 --- a/pillar/api/blender_cloud/subscription.py +++ b/pillar/api/blender_cloud/subscription.py @@ -1,6 +1,7 @@ import logging import typing +import blinker from flask import Blueprint, Response import requests from requests.adapters import HTTPAdapter @@ -21,6 +22,10 @@ ROLES_BID_TO_PILLAR = { 'cloud_has_subscription': 'has_subscription', } +user_subscription_updated = blinker.NamedSignal( + 'user_subscription_updated', + 'The sender is a UserClass instance, kwargs includes "revoke_roles" and "grant_roles".') + @blueprint.route('/update-subscription') @authorization.require_login() @@ -157,6 +162,14 @@ def do_update_subscription(local_user: auth.UserClass, bid_user: dict): user_id, email, ', '.join(sorted(revoke_roles))) service.do_badger('revoke', roles=revoke_roles, user_id=user_id) + # Let the world know this user's subscription was updated. + final_roles = (plr_roles - revoke_roles).union(grant_roles) + local_user.roles = list(final_roles) + local_user.collect_capabilities() + user_subscription_updated.send(local_user, + grant_roles=grant_roles, + revoke_roles=revoke_roles) + # Re-index the user in the search database. from pillar.api.users import hooks hooks.push_updated_user_to_algolia({'_id': user_id}, {}) diff --git a/tests/test_api/test_subscriptions.py b/tests/test_api/test_subscriptions.py index 08dec044..e5b4a3c0 100644 --- a/tests/test_api/test_subscriptions.py +++ b/tests/test_api/test_subscriptions.py @@ -17,6 +17,13 @@ class RoleUpdatingTest(AbstractPillarTest): with self.app.test_request_context(): self.create_standard_groups() + from pillar.api.blender_cloud import subscription as sub + self.user_subs_signal_calls = [] + sub.user_subscription_updated.connect(self._user_subs_signal) + + def _user_subs_signal(self, sender, **kwargs): + self.user_subs_signal_calls.append((sender, kwargs)) + def _setup_testcase(self, mocked_fetch_blenderid_user, *, bid_roles: typing.Set[str]): import urllib.parse @@ -54,6 +61,12 @@ class RoleUpdatingTest(AbstractPillarTest): user_info = self.get('/api/users/me', auth_token='my-happy-token').json() self.assertEqual({'subscriber', 'has_subscription'}, set(user_info['roles'])) + # Check the signals + self.assertEqual(1, len(self.user_subs_signal_calls)) + sender, kwargs = self.user_subs_signal_calls[0] + self.assertEqual({'revoke_roles': set(), 'grant_roles': {'subscriber', 'has_subscription'}}, + kwargs) + @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_store_api_role_revoke_subscriber(self, mocked_fetch_blenderid_user): @@ -61,9 +74,9 @@ class RoleUpdatingTest(AbstractPillarTest): bid_roles={'conference_speaker'}) # Make sure this user is currently known as a subcriber. - self.create_user(roles={'subscriber'}, token='my-happy-token') + self.create_user(roles={'subscriber', 'has_subscription'}, token='my-happy-token') 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'])) # And after updating, it shouldn't be. self.get('/api/bcloud/update-subscription', auth_token='my-happy-token', @@ -71,6 +84,11 @@ class RoleUpdatingTest(AbstractPillarTest): user_info = self.get('/api/users/me', auth_token='my-happy-token').json() self.assertEqual([], user_info['roles']) + self.assertEqual(1, len(self.user_subs_signal_calls)) + sender, kwargs = self.user_subs_signal_calls[0] + self.assertEqual({'revoke_roles': {'subscriber', 'has_subscription'}, 'grant_roles': set()}, + kwargs) + @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_bid_api_grant_demo(self, mocked_fetch_blenderid_user): @@ -83,6 +101,10 @@ class RoleUpdatingTest(AbstractPillarTest): user_info = self.get('/api/users/me', auth_token='my-happy-token').json() self.assertEqual(['demo'], user_info['roles']) + self.assertEqual(1, len(self.user_subs_signal_calls)) + sender, kwargs = self.user_subs_signal_calls[0] + self.assertEqual({'revoke_roles': set(), 'grant_roles': {'demo'}}, kwargs) + @responses.activate @mock.patch('pillar.api.blender_id.fetch_blenderid_user') def test_bid_api_role_revoke_demo(self, mocked_fetch_blenderid_user): @@ -99,3 +121,7 @@ class RoleUpdatingTest(AbstractPillarTest): expected_status=204) user_info = self.get('/api/users/me', auth_token='my-happy-token').json() self.assertEqual([], user_info['roles']) + + self.assertEqual(1, len(self.user_subs_signal_calls)) + sender, kwargs = self.user_subs_signal_calls[0] + self.assertEqual({'revoke_roles': {'demo'}, 'grant_roles': set()}, kwargs) From 8ca6b4cdb0217ec479eac09741e947df9b04211f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 21 Dec 2017 13:17:57 +0100 Subject: [PATCH 19/27] Added Celery task for queued email sending. Upon IOError or OSError (which includes SMTP protocol errors) the mail sending task is retried after MAIL_RETRY seconds. It is retried three times (default setting of Celery) only. --- pillar/__init__.py | 1 + pillar/celery/email_tasks.py | 48 ++++++++++++++++++++++++++++++++++++ pillar/config.py | 8 ++++++ pillar/tests/__init__.py | 12 ++++++--- 4 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 pillar/celery/email_tasks.py diff --git a/pillar/__init__.py b/pillar/__init__.py index 2627692e..3b0da365 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -462,6 +462,7 @@ class PillarServer(BlinkerCompatibleEve): 'pillar.celery.tasks', 'pillar.celery.algolia_tasks', 'pillar.celery.file_link_tasks', + 'pillar.celery.email_tasks', ] # Allow Pillar extensions from defining their own Celery tasks. diff --git a/pillar/celery/email_tasks.py b/pillar/celery/email_tasks.py new file mode 100644 index 00000000..e6612c2a --- /dev/null +++ b/pillar/celery/email_tasks.py @@ -0,0 +1,48 @@ +"""Deferred email support. + +Note that this module can only be imported when an application context is +active. Best to late-import this in the functions where it's needed. +""" +from email.message import EmailMessage +from email.headerregistry import Address +import logging +import smtplib + +import celery + +from pillar import current_app + +log = logging.getLogger(__name__) + + +@current_app.celery.task(bind=True, ignore_result=True, acks_late=True) +def send_email(self: celery.Task, to_name: str, to_addr: str, subject: str, text: str, html: str): + """Send an email to a single address.""" + # WARNING: when changing the signature of this function, also change the + # self.retry() call below. + cfg = current_app.config + + # Construct the message + msg = EmailMessage() + msg['Subject'] = subject + msg['From'] = Address(cfg['MAIL_DEFAULT_FROM_NAME'], addr_spec=cfg['MAIL_DEFAULT_FROM_ADDR']) + msg['To'] = (Address(to_name, addr_spec=to_addr),) + msg.set_content(text) + msg.add_alternative(html, subtype='html') + + # Refuse to send mail when we're testing. + if cfg['TESTING']: + log.warning('not sending mail to %s <%s> because we are TESTING', to_name, to_addr) + return + log.info('sending email to %s <%s>', to_name, to_addr) + + # Send the message via local SMTP server. + try: + with smtplib.SMTP(cfg['SMTP_HOST'], cfg['SMTP_PORT'], timeout=cfg['SMTP_TIMEOUT']) as smtp: + smtp.send_message(msg) + except (IOError, OSError) as ex: + log.exception('error sending email to %s <%s>, will retry later: %s', + to_name, to_addr, ex) + self.retry((to_name, to_addr, subject, text, html), countdown=cfg['MAIL_RETRY']) + else: + log.info('mail to %s <%s> successfully sent', to_name, to_addr) diff --git a/pillar/config.py b/pillar/config.py index 11a5a0ab..261b12e5 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -230,3 +230,11 @@ DEFAULT_LOCALE = 'en_US' # never show the site in English. SUPPORT_ENGLISH = True + +# Mail options, see pillar.celery.email_tasks. +SMTP_HOST = 'localhost' +SMTP_PORT = 2525 +SMTP_TIMEOUT = 30 # timeout in seconds, https://docs.python.org/3/library/smtplib.html#smtplib.SMTP +MAIL_RETRY = 180 # in seconds, delay until trying to send an email again. +MAIL_DEFAULT_FROM_NAME = 'Blender Cloud' +MAIL_DEFAULT_FROM_ADDR = 'cloudsupport@localhost' diff --git a/pillar/tests/__init__.py b/pillar/tests/__init__.py index 453cb21d..3e026a56 100644 --- a/pillar/tests/__init__.py +++ b/pillar/tests/__init__.py @@ -67,20 +67,26 @@ class PillarTestServer(pillar.PillarServer): Without this, actual Celery tasks will be created while the tests are running. """ - from celery import Celery + from celery import Celery, Task self.celery = unittest.mock.MagicMock(Celery) - def fake_task(*task_args, **task_kwargs): + def fake_task(*task_args, bind=False, **task_kwargs): def decorator(f): def delay(*args, **kwargs): - return f(*args, **kwargs) + if bind: + return f(decorator.sender, *args, **kwargs) + else: + return f(*args, **kwargs) f.delay = delay f.si = unittest.mock.MagicMock() f.s = unittest.mock.MagicMock() return f + if bind: + decorator.sender = unittest.mock.MagicMock(Task) + return decorator self.celery.task = fake_task From 054eced7de158d8df4b5f17376abe89c3d4fa354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 10:59:15 +0100 Subject: [PATCH 20/27] Added SMTP Auth support --- pillar/celery/email_tasks.py | 2 ++ pillar/config.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pillar/celery/email_tasks.py b/pillar/celery/email_tasks.py index e6612c2a..6e1b0278 100644 --- a/pillar/celery/email_tasks.py +++ b/pillar/celery/email_tasks.py @@ -39,6 +39,8 @@ def send_email(self: celery.Task, to_name: str, to_addr: str, subject: str, text # Send the message via local SMTP server. try: with smtplib.SMTP(cfg['SMTP_HOST'], cfg['SMTP_PORT'], timeout=cfg['SMTP_TIMEOUT']) as smtp: + if cfg.get('SMTP_USERNAME') and cfg.get('SMTP_PASSWORD'): + smtp.login(cfg['SMTP_USERNAME'], cfg['SMTP_PASSWORD']) smtp.send_message(msg) except (IOError, OSError) as ex: log.exception('error sending email to %s <%s>, will retry later: %s', diff --git a/pillar/config.py b/pillar/config.py index 261b12e5..9990c574 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -234,6 +234,8 @@ SUPPORT_ENGLISH = True # Mail options, see pillar.celery.email_tasks. SMTP_HOST = 'localhost' SMTP_PORT = 2525 +SMTP_USERNAME = '' +SMTP_PASSWORD = '' SMTP_TIMEOUT = 30 # timeout in seconds, https://docs.python.org/3/library/smtplib.html#smtplib.SMTP MAIL_RETRY = 180 # in seconds, delay until trying to send an email again. MAIL_DEFAULT_FROM_NAME = 'Blender Cloud' From 8f9d21cdd88d3aea4fc5fcd700ca4816ef238691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 12:29:51 +0100 Subject: [PATCH 21/27] Fixed bug in parsing jstree A projection was missing, causing warnings that the node doesn't have a content type. --- pillar/web/utils/jstree.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pillar/web/utils/jstree.py b/pillar/web/utils/jstree.py index c9cc3d4b..978ea4f4 100644 --- a/pillar/web/utils/jstree.py +++ b/pillar/web/utils/jstree.py @@ -115,15 +115,17 @@ def jstree_build_from_node(node): # Get the parent node parent = None + parent_projection = {'projection': { + 'name': 1, + 'parent': 1, + 'project': 1, + 'node_type': 1, + 'properties.content_type': 1, + }} + if node.parent: try: - parent = Node.find(node.parent, { - 'projection': { - 'name': 1, - 'node_type': 1, - 'parent': 1, - 'properties.content_type': 1, - }}, api=api) + parent = Node.find(node.parent, parent_projection, api=api) # Define the child node of the tree (usually an asset) except ResourceNotFound: # If not found, we might be on the top level, in which case we skip the @@ -147,10 +149,7 @@ def jstree_build_from_node(node): # If we have a parent if parent.parent: try: - parent = Node.find(parent.parent, { - 'projection': { - 'name': 1, 'parent': 1, 'project': 1, 'node_type': 1}, - }, api=api) + parent = Node.find(parent.parent, parent_projection, api=api) except ResourceNotFound: parent = None else: From 8fb22931f5edc6649a64dcaa05054c990898a4dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 14:42:18 +0100 Subject: [PATCH 22/27] Remove unused imports --- pillar/api/projects/hooks.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pillar/api/projects/hooks.py b/pillar/api/projects/hooks.py index b1410020..0365e1d5 100644 --- a/pillar/api/projects/hooks.py +++ b/pillar/api/projects/hooks.py @@ -2,7 +2,6 @@ import copy import logging from flask import request, abort, current_app -from gcloud import exceptions as gcs_exceptions from pillar.api.node_types.asset import node_type_asset from pillar.api.node_types.comment import node_type_comment @@ -10,7 +9,6 @@ from pillar.api.node_types.group import node_type_group from pillar.api.node_types.group_texture import node_type_group_texture from pillar.api.node_types.texture import node_type_texture from pillar.api.file_storage_backends import default_storage_backend -from pillar.api.file_storage_backends.gcs import GoogleCloudStorageBucket from pillar.api.utils import authorization, authentication from pillar.api.utils import remove_private_keys from pillar.api.utils.authorization import user_has_role, check_permissions From f47a45f9a3af7126f09dc0a0ffcb59f337c5e3f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 14:42:24 +0100 Subject: [PATCH 23/27] Removed unused code --- src/scripts/project-edit.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/scripts/project-edit.js b/src/scripts/project-edit.js index 96890c44..544affae 100644 --- a/src/scripts/project-edit.js +++ b/src/scripts/project-edit.js @@ -182,8 +182,6 @@ $( document ).ready(function() { $('#item_delete').click(function(e){ e.preventDefault(); if (ProjectUtils.isProject()) { - // url = window.location.href.split('#')[0] + 'delete'; - // window.location.replace(url); $.post(urlProjectDelete, {project_id: ProjectUtils.projectId()}, function (data) { // Feedback logic From 766e766f50608d2a2db4460d7eb1ff88c780aa69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 16:25:12 +0100 Subject: [PATCH 24/27] Declare some parameter types --- pillar/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 3b0da365..21d9813a 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -755,7 +755,7 @@ class PillarServer(BlinkerCompatibleEve): return 'basic ' + base64.b64encode('%s:%s' % (username, subclient_id)) - def post_internal(self, resource, payl=None, skip_validation=False): + def post_internal(self, resource: str, payl=None, skip_validation=False): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.post import post_internal @@ -764,7 +764,7 @@ class PillarServer(BlinkerCompatibleEve): with self.__fake_request_url_rule('POST', path): return post_internal(resource, payl=payl, skip_validation=skip_validation)[:4] - def put_internal(self, resource, payload=None, concurrency_check=False, + def put_internal(self, resource: str, payload=None, concurrency_check=False, skip_validation=False, **lookup): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.put import put_internal @@ -775,7 +775,7 @@ class PillarServer(BlinkerCompatibleEve): return put_internal(resource, payload=payload, concurrency_check=concurrency_check, skip_validation=skip_validation, **lookup)[:4] - def patch_internal(self, resource, payload=None, concurrency_check=False, + def patch_internal(self, resource: str, payload=None, concurrency_check=False, skip_validation=False, **lookup): """Workaround for Eve issue https://github.com/nicolaiarocci/eve/issues/810""" from eve.methods.patch import patch_internal From 46612a9f68775a50dbd797040796d7871ea80f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 16:25:39 +0100 Subject: [PATCH 25/27] JavaScript function for getting reasonable error message from an XHR response --- src/scripts/tutti/5_1_notifications.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/scripts/tutti/5_1_notifications.js b/src/scripts/tutti/5_1_notifications.js index 434aa7d8..2a263d47 100644 --- a/src/scripts/tutti/5_1_notifications.js +++ b/src/scripts/tutti/5_1_notifications.js @@ -361,6 +361,20 @@ function getNotificationsLoop() { }, 30000); } +/* Returns a more-or-less reasonable message given an error response object. */ +function xhrErrorResponseMessage(err) { + if (typeof err.responseJSON == 'undefined') + return err.statusText; + + if (typeof err.responseJSON._error != 'undefined' && typeof err.responseJSON._error.message != 'undefined') + return err.responseJSON._error.message; + + if (typeof err.responseJSON._message != 'undefined') + return err.responseJSON._message + + return err.statusText; +} + /* Notifications: Toastr Defaults */ toastr.options.showDuration = 50; toastr.options.progressBar = true; From 8f73dab36e096f217e4a1972fca2bb2a23e22154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 16:27:05 +0100 Subject: [PATCH 26/27] Allow project undeletion, fixes T51244 Projects can be undeleted within a month of deletion. --- pillar/web/projects/routes.py | 59 +++++++++++++++++++-- src/scripts/project-edit.js | 12 +++-- src/styles/_project-dashboard.sass | 14 +++-- src/templates/projects/index_dashboard.pug | 61 +++++++++++++++++++++- src/templates/projects/view.pug | 2 +- 5 files changed, 132 insertions(+), 16 deletions(-) diff --git a/pillar/web/projects/routes.py b/pillar/web/projects/routes.py index 1c5598de..aa6a71e6 100644 --- a/pillar/web/projects/routes.py +++ b/pillar/web/projects/routes.py @@ -1,3 +1,4 @@ +import datetime import json import logging import itertools @@ -7,6 +8,7 @@ from pillarsdk import Node from pillarsdk import Project from pillarsdk.exceptions import ResourceNotFound from pillarsdk.exceptions import ForbiddenAccess +import flask from flask import Blueprint from flask import render_template from flask import request @@ -78,6 +80,19 @@ def index(): 'sort': '-_created' }, api=api) + show_deleted_projects = request.args.get('deleted') is not None + if show_deleted_projects: + timeframe = utils.datetime_now() - datetime.timedelta(days=31) + projects_deleted = Project.all({ + 'where': {'user': current_user.objectid, + 'category': {'$ne': 'home'}, + '_deleted': True, + '_updated': {'$gt': timeframe}}, + 'sort': '-_created' + }, api=api) + else: + projects_deleted = {'_items': []} + projects_shared = Project.all({ 'where': {'user': {'$ne': current_user.objectid}, 'permissions.groups.group': {'$in': current_user.groups}, @@ -87,17 +102,17 @@ def index(): }, api=api) # Attach project images - for project in projects_user['_items']: - utils.attach_project_pictures(project, api) - - for project in projects_shared['_items']: - utils.attach_project_pictures(project, api) + for project_list in (projects_user, projects_deleted, projects_shared): + for project in project_list['_items']: + utils.attach_project_pictures(project, api) return render_template( 'projects/index_dashboard.html', gravatar=utils.gravatar(current_user.email, size=128), projects_user=projects_user['_items'], + projects_deleted=projects_deleted['_items'], projects_shared=projects_shared['_items'], + show_deleted_projects=show_deleted_projects, api=api) @@ -847,3 +862,37 @@ def edit_extension(project: Project, extension_name): return ext.project_settings(project, ext_pages=find_extension_pages()) + + +@blueprint.route('/undelete', methods=['POST']) +@login_required +def undelete(): + """Undelete a deleted project. + + Can only be done by the owner of the project or an admin. + """ + # This function takes an API-style approach, even though it's a web + # endpoint. Undeleting via a REST approach would mean GETting the + # deleted project, which now causes a 404 exception to bubble to the + # client. + from pillar.api.utils import mongo, remove_private_keys + from pillar.api.utils.authorization import check_permissions + + project_id = request.form.get('project_id') + if not project_id: + raise wz_exceptions.BadRequest('missing project ID') + + # Check that the user has PUT permissions on the project itself. + project = mongo.find_one_or_404('projects', project_id) + check_permissions('projects', project, 'PUT') + + pid = project['_id'] + log.info('Undeleting project %s on behalf of %s', pid, current_user.email) + r, _, _, status = current_app.put_internal('projects', remove_private_keys(project), _id=pid) + if status != 200: + log.warning('Error %d un-deleting project %s: %s', status, pid, r) + return 'Error un-deleting project', 500 + + resp = flask.Response('', status=204) + resp.location = flask.url_for('projects.view', project_url=project['url']) + return resp diff --git a/src/scripts/project-edit.js b/src/scripts/project-edit.js index 544affae..4d4fa498 100644 --- a/src/scripts/project-edit.js +++ b/src/scripts/project-edit.js @@ -182,11 +182,13 @@ $( document ).ready(function() { $('#item_delete').click(function(e){ e.preventDefault(); if (ProjectUtils.isProject()) { - $.post(urlProjectDelete, {project_id: ProjectUtils.projectId()}, - function (data) { - // Feedback logic - }).done(function () { - window.location.replace('/p/'); + $.post(urlProjectDelete, {project_id: ProjectUtils.projectId()}) + .done(function () { + // Redirect to the /p/ URL that shows deleted projects. + window.location.replace('/p/?deleted=1'); + }) + .fail(function(err) { + toastr.error(xhrErrorResponseMessage(err), 'Project deletion failed'); }); } else { $.post(urlNodeDelete, {node_id: ProjectUtils.nodeId()}, diff --git a/src/styles/_project-dashboard.sass b/src/styles/_project-dashboard.sass index 0df2bd98..c07c0dca 100644 --- a/src/styles/_project-dashboard.sass +++ b/src/styles/_project-dashboard.sass @@ -245,7 +245,13 @@ box-shadow: 1px 1px 0 rgba(black, .1) display: flex margin: 10px 15px - padding: 10px 0 + padding: 10px 10px + + &.deleted + background-color: $color-background-light + + .title + color: $color-text-dark-hint !important &:hover cursor: pointer @@ -259,9 +265,9 @@ .projects__list-details a.title color: $color-primary - a.projects__list-thumbnail + .projects__list-thumbnail position: relative - margin: 0 15px + margin-right: 15px width: 50px height: 50px border-radius: 3px @@ -280,7 +286,7 @@ display: flex flex-direction: column - a.title + .title font-size: 1.2em padding-bottom: 2px color: $color-text-dark-primary diff --git a/src/templates/projects/index_dashboard.pug b/src/templates/projects/index_dashboard.pug index d51d7f4b..c721a4f5 100644 --- a/src/templates/projects/index_dashboard.pug +++ b/src/templates/projects/index_dashboard.pug @@ -18,6 +18,25 @@ meta(name="twitter:image", content="{{ url_for('static', filename='assets/img/ba | {{current_user.full_name}} | {% endblock %} +| {% block css %} +| {{ super() }} +style. + .deleted-projects-toggle { + z-index: 10; + position: absolute; + right: 0; + font-size: 20px; + padding: 3px; + text-shadow: 0 0 2px white; + } + .deleted-projects-toggle .show-deleted { + color: #aaa; + } + .deleted-projects-toggle .hide-deleted { + color: #bbb; + } +| {% endblock %} + | {% block body %} .dashboard-container section.dashboard-main @@ -54,7 +73,36 @@ meta(name="twitter:image", content="{{ url_for('static', filename='assets/img/ba | {% endif %} nav.nav-tabs__tab.active#own_projects + .deleted-projects-toggle + | {% if show_deleted_projects %} + a.hide-deleted(href="{{ request.base_url }}", title='Hide deleted projects') + i.pi-trash + | {% else %} + a.show-deleted(href="{{ request.base_url }}?deleted=1", title='Show deleted projects') + i.pi-trash + | {% endif %} + ul.projects__list + | {% for project in projects_deleted %} + li.projects__list-item.deleted + span.projects__list-thumbnail + | {% if project.picture_square %} + img(src="{{ project.picture_square.thumbnail('s', api=api) }}") + | {% else %} + i.pi-blender-cloud + | {% endif %} + .projects__list-details + span.title {{ project.name }} + ul.meta + li.status.deleted Deleted + li.edit + a(href="javascript:undelete_project('{{ project._id }}')") Restore project + | {% else %} + | {% if show_deleted_projects %} + li.projects__list-item.deleted You have no recenly deleted projects. Deleted projects can be restored within a month after deletion. + | {% endif %} + | {% endfor %} + | {% for project in projects_user %} li.projects__list-item( data-url="{{ url_for('projects.view', project_url=project.url) }}") @@ -105,7 +153,7 @@ meta(name="twitter:image", content="{{ url_for('static', filename='assets/img/ba | {% endif %} | {% endfor %} - section.nav-tabs__tab#shared + section.nav-tabs__tab#shared(style='display: none') ul.projects__list | {% if projects_shared %} | {% for project in projects_shared %} @@ -278,4 +326,15 @@ script. hopToTop(); // Display jump to top button }); + + function undelete_project(project_id) { + console.log('undeleting project', project_id); + $.post('{{ url_for('projects.undelete') }}', {project_id: project_id}) + .done(function(data, textStatus, jqXHR) { + location.href = jqXHR.getResponseHeader('Location'); + }) + .fail(function(err) { + toastr.error(xhrErrorResponseMessage(err), 'Undeletion failed'); + }) + } | {% endblock %} diff --git a/src/templates/projects/view.pug b/src/templates/projects/view.pug index 540a8efb..66a12e41 100644 --- a/src/templates/projects/view.pug +++ b/src/templates/projects/view.pug @@ -222,7 +222,7 @@ link(href="{{ url_for('static_pillar', filename='assets/css/project-main.css', v li.button-delete a#item_delete( href="javascript:void(0);", - title="Delete (Warning: no undo)", + title="Can be undone within a month", data-toggle="tooltip", data-placement="left") i.pi-trash From d73146ff625d70f022e6abb3c7fec7aad669c42c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 22 Dec 2017 16:27:16 +0100 Subject: [PATCH 27/27] Formatting --- pillar/web/projects/routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/web/projects/routes.py b/pillar/web/projects/routes.py index aa6a71e6..9e7ba5ce 100644 --- a/pillar/web/projects/routes.py +++ b/pillar/web/projects/routes.py @@ -289,7 +289,7 @@ def view(project_url): header_video_file = None header_video_node = None if project.header_node and project.header_node.node_type == 'asset' and \ - project.header_node.properties.content_type == 'video': + project.header_node.properties.content_type == 'video': header_video_node = project.header_node header_video_file = utils.get_file(project.header_node.properties.file) header_video_node.picture = utils.get_file(header_video_node.picture)