Skip to content

Commit

Permalink
Fix some errors caused by raising 403 in get_current_user
Browse files Browse the repository at this point in the history
get_current_user is called in a few places that really shouldn’t raise

move the raising to `get_login_url`, which is called in `@web.authenticated`,
where we want to replace redirect logic with 403.
  • Loading branch information
minrk committed Oct 10, 2017
1 parent 2ee51ab commit 0141d82
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions notebook/base/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ def skip_check_origin(self):
For example: in the default LoginHandler, if a request is token-authenticated,
origin checking should be skipped.
"""
if self.request.method == 'OPTIONS':
# no origin-check on options requests, which are used to check origins!
return True
if self.login_handler is None or not hasattr(self.login_handler, 'should_check_origin'):
return False
return not self.login_handler.should_check_origin(self)
Expand Down Expand Up @@ -476,10 +479,16 @@ def get_current_user(self):
if hasattr(self, '_user_cache'):
return self._user_cache
self._user_cache = user = super(APIHandler, self).get_current_user()
if user is None:
raise web.HTTPError(403)
return user

def get_login_url(self):
# if get_login_url is invoked in an API handler,
# that means @web.authenticated is trying to trigger a redirect.
# instead of redirecting, raise 403 instead.
if not self.current_user:
raise web.HTTPError(403)
return super(APIHandler, self).get_login_url()

@property
def content_security_policy(self):
csp = '; '.join([
Expand All @@ -494,7 +503,7 @@ def content_security_policy(self):
def update_api_activity(self):
"""Update last_activity of API requests"""
# record activity of authenticated requests
if self._track_activity and self.get_current_user():
if self._track_activity and getattr(self, '_user_cache', None):
self.settings['api_last_activity'] = utcnow()

def finish(self, *args, **kwargs):
Expand All @@ -507,7 +516,6 @@ def options(self, *args, **kwargs):
'accept, content-type, authorization, x-xsrftoken')
self.set_header('Access-Control-Allow-Methods',
'GET, PUT, POST, PATCH, DELETE, OPTIONS')
self.finish()


class Template404(IPythonHandler):
Expand Down

0 comments on commit 0141d82

Please sign in to comment.