add no_cookie blueprint

we have a hunch that some session related issues that we've seen over
the last few weeks might be related to weird race conditions where
cookies set by subresources (image previews of letters on the send flow)
arrive just as the img request is cancelled because the user has clicked
on a button to navigate to a new page, but still manage to set the
cookie? We're not entirely sure what's going on, but we've got a hunch
that not setting cookies on image fetches sounds sensible. Images are
always loaded as a subresource (ie: through a `src` tag in an html
element), so they should never need to change the cookies, so this seems
sensible. We've done this by creating a new blueprint that doesn't set
session.permanent, and doesn't call `save_serivce_or_org_after_request`
either.

cookies are sent back to the browser if:
`sesion.modified or (session.permanent and 'REFRESH_EVERY_REQUEST')`
(where the latter is a config setting).

Turning off REFRESH_EVERY_REQUEST (which is True by default) means that
we will only update the sesion if it's been modified. In practice,
literally every request is modified in the after_request handler
`save_service_or_org_after_request`. This is accidentally convenient,
as it guarantees that we'll still send back the cookie normally even
though refresh_every_request is disabled. Sending back the cookie
updates the expiry time (20 hours), so we need to keep doing this to
preserve existing session timeout behaviour.
This commit is contained in:
Leo Hemsted
2019-11-28 14:39:30 +00:00
parent 27119bc15b
commit 72acc4ebdc
3 changed files with 52 additions and 17 deletions

View File

@@ -173,11 +173,7 @@ def create_app(application):
# make sure we handle unicode correctly
redis_client.redis_store.decode_responses = True
from app.main import main as main_blueprint
application.register_blueprint(main_blueprint)
from .status import status as status_blueprint
application.register_blueprint(status_blueprint)
setup_blueprints(application)
add_template_filters(application)
@@ -188,21 +184,11 @@ def create_app(application):
def init_app(application):
application.after_request(useful_headers_after_request)
application.after_request(save_service_or_org_after_request)
application.before_request(load_service_before_request)
application.before_request(load_organisation_before_request)
application.before_request(request_helper.check_proxy_header_before_request)
@application.before_request
def make_session_permanent():
# this is dumb. You'd think, given that there's `config['PERMANENT_SESSION_LIFETIME']`, that you'd enable
# permanent sessions in the config too - but no, you have to declare it for each request.
# https://stackoverflow.com/questions/34118093/flask-permanent-session-where-to-define-them
# session.permanent is also, helpfully, a way of saying that the session isn't permanent - in that, it will
# expire on its own, as opposed to being controlled by the browser's session. Because session is a proxy, it's
# only accessible from within a request context, so we need to set this before every request :rolls_eyes:
session.permanent = True
@application.context_processor
def _attach_current_service():
return {'current_service': current_service}
@@ -487,6 +473,19 @@ def load_user(user_id):
return User.from_id(user_id)
def make_session_permanent():
"""
Make sessions permanent. By permanent, we mean "admin app sets when it expires". Normally the cookie would expire
whenever you close the browser. With this, the session expiry is set in `config['PERMANENT_SESSION_LIFETIME']`
(20 hours) and is refreshed after every request. IE: you will be logged out after twenty hours of inactivity.
We don't _need_ to set this every request (it's saved within the cookie itself under the `_permanent` flag), only
when you first log in/sign up/get invited/etc, but we do it just to be safe. For more reading, check here:
https://stackoverflow.com/questions/34118093/flask-permanent-session-where-to-define-them
"""
session.permanent = True
def load_service_before_request():
if '/static/' in request.url:
_request_ctx_stack.top.service = None
@@ -680,6 +679,38 @@ def register_errorhandlers(application): # noqa (C901 too complex)
return _error_response(500)
def setup_blueprints(application):
"""
There are three blueprints: status_blueprint, no_cookie_blueprint, and main_blueprint.
main_blueprint is the default for everything.
status_blueprint is only for the status page - unauthenticated, unstyled, no cookies, etc.
no_cookie_blueprint is for subresources (things loaded asynchronously) that we might be concerned are setting
cookies unnecessarily and potentially getting in to strange race conditions and overwriting other cookies, as we've
seen in the send message flow. Currently, this includes letter template previews, and the iframe from the platform
admin email branding preview pages.
This notably doesn't include the *.json ajax endpoints. If we included them in this, the cookies wouldn't be
updated, including the expiration date. If you have a dashboard open and in focus it'll refresh the expiration timer
every two seconds, and you will never log out, which is behaviour we want to preserve.
"""
from app.status import status as status_blueprint
from app.main import (
main as main_blueprint,
no_cookie as no_cookie_blueprint
)
main_blueprint.before_request(make_session_permanent)
main_blueprint.after_request(save_service_or_org_after_request)
application.register_blueprint(main_blueprint)
# no_cookie_blueprint specifically doesn't have `make_session_permanent` or `save_service_or_org_after_request`
application.register_blueprint(no_cookie_blueprint)
application.register_blueprint(status_blueprint)
def setup_event_handlers():
from flask_login import user_logged_in
from app.event_handlers import on_user_logged_in

View File

@@ -61,7 +61,10 @@ class Config(object):
SESSION_COOKIE_HTTPONLY = True
SESSION_COOKIE_NAME = 'notify_admin_session'
SESSION_COOKIE_SECURE = True
SESSION_REFRESH_EACH_REQUEST = True
# don't send back the cookie if it hasn't been modified by the request. this means that the expiry time won't be
# updated unless the session is changed - but it's generally refreshed by `save_service_or_org_after_request`
# every time anyway, except for specific endpoints (png/pdfs generally) where we've disabled that handler.
SESSION_REFRESH_EACH_REQUEST = False
SHOW_STYLEGUIDE = True
WTF_CSRF_ENABLED = True
WTF_CSRF_TIME_LIMIT = None

View File

@@ -1,6 +1,7 @@
from flask import Blueprint
main = Blueprint('main', __name__)
no_cookie = Blueprint('no_cookie', __name__)
from app.main.views import ( # noqa isort:skip
add_service,