From c7ba775048ec7d71f4795ae408492929bb5bfd14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 11:10:01 +0100 Subject: [PATCH 1/7] Removed some traces of Bugsnag --- pillar/__init__.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pillar/__init__.py b/pillar/__init__.py index 21d9813a..434dba43 100644 --- a/pillar/__init__.py +++ b/pillar/__init__.py @@ -210,14 +210,7 @@ class PillarServer(BlinkerCompatibleEve): 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) - self.log.info('Sentry setup complete') - - def __notify_bugsnag(self, sender, exception, **extra): - import bugsnag - bugsnag.auto_notify(exception) + self.log.debug('Sentry setup complete') def _config_google_cloud_storage(self): # Google Cloud project From ef2cc44cebc62e3be9b8027302bec7859b2461b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 11:12:17 +0100 Subject: [PATCH 2/7] Reduce log level when user lacks required roles/caps This prevents logging those at Sentry. --- pillar/api/utils/authorization.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pillar/api/utils/authorization.py b/pillar/api/utils/authorization.py index bdac5d5c..569020e1 100644 --- a/pillar/api/utils/authorization.py +++ b/pillar/api/utils/authorization.py @@ -345,13 +345,13 @@ def require_login(*, require_roles=set(), return render_error() if require_roles and not current_user.matches_roles(require_roles, require_all): - log.warning('User %s is authenticated, but does not have required roles %s to ' - 'access %s', current_user.user_id, require_roles, func) + log.info('User %s is authenticated, but does not have required roles %s to ' + 'access %s', current_user.user_id, require_roles, func) return render_error() if require_cap and not current_user.has_cap(require_cap): - log.warning('User %s is authenticated, but does not have required capability %s to ' - 'access %s', current_user.user_id, require_cap, func) + log.info('User %s is authenticated, but does not have required capability %s to ' + 'access %s', current_user.user_id, require_cap, func) return render_error() return func(*args, **kwargs) From 656a878c6a55495752c06145bd74f30b6321643b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 11:44:49 +0100 Subject: [PATCH 3/7] Include stack trace when looking an SDK exception. Possibly the exception shouldn't be logged at all (or just at debug level), since it's also re-raised and should be handled by the caller instead. --- pillar/sdk.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pillar/sdk.py b/pillar/sdk.py index 119e33e7..bfe23bda 100644 --- a/pillar/sdk.py +++ b/pillar/sdk.py @@ -42,7 +42,7 @@ class FlaskInternalApi(pillarsdk.Api): content = self.handle_response(response, response.data) except: log.warning("%s: Response[%s]: %s", url, response.status_code, - response.data) + response.data, exc_info=True) raise return content From a938342611b8eab650f18d50de5441070648dd4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 12:08:06 +0100 Subject: [PATCH 4/7] Reduced log level when checking user without email for org membership Service accounts may not have an email address, which is fine for now. --- pillar/api/users/hooks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pillar/api/users/hooks.py b/pillar/api/users/hooks.py index d4a5271c..7e8452a3 100644 --- a/pillar/api/users/hooks.py +++ b/pillar/api/users/hooks.py @@ -161,8 +161,8 @@ def grant_org_roles(user_doc): email = user_doc.get('email') if not email: - log.warning('Unable to check new user for organization membership, no email address! %r', - user_doc) + log.info('Unable to check new user for organization membership, no email address: %r', + user_doc) return org_roles = current_app.org_manager.unknown_member_roles(email) From 1c6599fc3090f8be3a1d7e0cf0042c725ec3e5d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 12:18:43 +0100 Subject: [PATCH 5/7] More detailed logging in fetch_blenderid_user --- pillar/api/blender_id.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pillar/api/blender_id.py b/pillar/api/blender_id.py index eb9e7537..3e8c91ed 100644 --- a/pillar/api/blender_id.py +++ b/pillar/api/blender_id.py @@ -209,11 +209,12 @@ def fetch_blenderid_user() -> dict: :raises LogoutUser: when Blender ID tells us the current token is invalid, and the user should be logged out. """ - import httplib2 # used by the oauth2 package + my_log = log.getChild('fetch_blenderid_user') + bid_url = '%s/api/user' % blender_id_endpoint() - log.debug('Fetching user info from %s', bid_url) + my_log.debug('Fetching user info from %s', bid_url) credentials = current_app.config['OAUTH_CREDENTIALS']['blender-id'] oauth_token = session['blender_id_oauth_token'] @@ -226,23 +227,23 @@ def fetch_blenderid_user() -> dict: try: bid_resp = oauth_session.get(bid_url) except httplib2.HttpLib2Error: - log.exception('Error getting %s from BlenderID', bid_url) + my_log.exception('Error getting %s from BlenderID', bid_url) return {} if bid_resp.status_code == 403: - log.warning('Error %i from BlenderID %s, logging out user', bid_resp.status_code, bid_url) + my_log.warning('Error %i from BlenderID %s, logging out user', bid_resp.status_code, bid_url) raise LogoutUser() if bid_resp.status_code != 200: - log.warning('Error %i from BlenderID %s: %s', bid_resp.status_code, bid_url, bid_resp.text) + my_log.warning('Error %i from BlenderID %s: %s', bid_resp.status_code, bid_url, bid_resp.text) return {} payload = bid_resp.json() if not payload: - log.warning('Empty data returned from BlenderID %s', bid_url) + my_log.warning('Empty data returned from BlenderID %s', bid_url) return {} - log.debug('BlenderID returned %s', payload) + my_log.debug('BlenderID returned %s', payload) return payload From fdb997079236f24fa2b0d959ee36a24e513094bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 12:19:03 +0100 Subject: [PATCH 6/7] Prevent crash when session['blender_id_oauth_token'] doesn't exist --- pillar/api/blender_id.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pillar/api/blender_id.py b/pillar/api/blender_id.py index 3e8c91ed..6291c5e2 100644 --- a/pillar/api/blender_id.py +++ b/pillar/api/blender_id.py @@ -217,7 +217,11 @@ def fetch_blenderid_user() -> dict: my_log.debug('Fetching user info from %s', bid_url) credentials = current_app.config['OAUTH_CREDENTIALS']['blender-id'] - oauth_token = session['blender_id_oauth_token'] + oauth_token = session.get('blender_id_oauth_token') + if not oauth_token: + my_log.warning('no Blender ID oauth token found in user session') + return {} + assert isinstance(oauth_token, str), f'oauth token must be str, not {type(oauth_token)}' oauth_session = OAuth2Session( From 91660fefe4b33cec36550404c5712f82150c94e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 3 Jan 2018 14:39:02 +0100 Subject: [PATCH 7/7] Lowering log level to DEBUG for internal SDK call exceptions. The exception is re-raised anyway, so it may be handled by the caller in a way that doesn't warrant a warning/error at all. --- pillar/sdk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pillar/sdk.py b/pillar/sdk.py index bfe23bda..647e528f 100644 --- a/pillar/sdk.py +++ b/pillar/sdk.py @@ -41,8 +41,8 @@ class FlaskInternalApi(pillarsdk.Api): try: content = self.handle_response(response, response.data) except: - log.warning("%s: Response[%s]: %s", url, response.status_code, - response.data, exc_info=True) + log.debug("%s: Response[%s]: %s", url, response.status_code, + response.data, exc_info=True) raise return content