Skip to content

Commit

Permalink
Merge pull request #1494 from heartsucker/session-timeouts
Browse files Browse the repository at this point in the history
Added expiration to sessions
  • Loading branch information
redshiftzero authored Oct 5, 2017
2 parents 089eba7 + 5090c3d commit d57f141
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 130 deletions.
3 changes: 3 additions & 0 deletions securedrop/config.py.example
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,6 @@ DATABASE_FILE = os.path.join(SECUREDROP_DATA_ROOT, 'db.sqlite')
#SUPPORTED_LOCALES = ['en_US', 'fr_FR', 'nb_NO']
# Which of the available locales should be displayed by default ?
DEFAULT_LOCALE = 'en_US'

# How long a session is valid before it expires and logs a user out
SESSION_EXPIRATION_MINUTES = 30
10 changes: 9 additions & 1 deletion securedrop/journalist.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-

from datetime import datetime
from datetime import datetime, timedelta
import functools

from flask import (Flask, request, render_template, send_file, redirect, flash,
Expand Down Expand Up @@ -65,6 +65,13 @@ def get_source(filesystem_id):
@app.before_request
def setup_g():
"""Store commonly used values in Flask's special g object"""
if 'expires' in session and datetime.utcnow() >= session['expires']:
session.clear()
flash(gettext('You have been logged out due to inactivity'), 'error')

session['expires'] = datetime.utcnow() + \
timedelta(minutes=getattr(config, 'SESSION_EXPIRATION_MINUTES', 30))

uid = session.get('uid', None)
if uid:
g.user = Journalist.query.get(uid)
Expand Down Expand Up @@ -162,6 +169,7 @@ def login():
@app.route('/logout')
def logout():
session.pop('uid', None)
session.pop('expires', None)
return redirect(url_for('index'))


Expand Down
1 change: 1 addition & 0 deletions securedrop/source.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-

import config

from source_app import create_app
Expand Down
11 changes: 11 additions & 0 deletions securedrop/source_app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import datetime, timedelta
from flask import (Flask, render_template, flash, Markup, request, g, session,
url_for, redirect)
from flask_babel import gettext
Expand Down Expand Up @@ -76,6 +77,16 @@ def setup_g():
g.html_lang = i18n.locale_to_rfc_5646(g.locale)
g.locales = i18n.get_locale2name()

if 'expires' in session and datetime.utcnow() >= session['expires']:
session.clear()
flash(gettext('You have been logged out due to inactivity'),
'error')

session['expires'] = datetime.utcnow() + \
timedelta(minutes=getattr(config,
'SESSION_EXPIRATION_MINUTES',
30))

# ignore_static here because `crypto_util.hash_codename` is scrypt
# (very time consuming), and we don't need to waste time running if
# we're just serving a static resource that won't need to access
Expand Down
38 changes: 37 additions & 1 deletion securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import unittest
import zipfile

from flask import url_for, escape
from flask import url_for, escape, session
from flask_testing import TestCase
from mock import patch, ANY, MagicMock
from sqlalchemy.orm.exc import StaleDataError
Expand Down Expand Up @@ -965,6 +965,42 @@ def test_add_star_redirects_to_index(self):
filesystem_id=source.filesystem_id))
self.assertRedirects(resp, url_for('index'))

def test_journalist_session_expiration(self):
try:
old_expiration = config.SESSION_EXPIRATION_MINUTES
has_session_expiration = True
except AttributeError:
has_session_expiration = False

try:
with self.client as client:
# do a real login to get a real session
# (none of the mocking `g` hacks)
resp = self.client.post(url_for('login'),
data=dict(username=self.user.username,
password=VALID_PASSWORD,
token='mocked'))
assert resp.status_code == 200

# set the expiration to ensure we trigger an expiration
config.SESSION_EXPIRATION_MINUTES = -1

resp = client.get(url_for('edit_account'),
follow_redirects=True)

# check that the session was cleared (apart from 'expires'
# which is always present and 'csrf_token' which leaks no info)
session.pop('expires', None)
session.pop('csrf_token', None)
assert not session, session
assert ('You have been logged out due to inactivity' in
resp.data.decode('utf-8'))
finally:
if has_session_expiration:
config.SESSION_EXPIRATION_MINUTES = old_expiration
else:
del config.SESSION_EXPIRATION_MINUTES


class TestJournalistAppTwo(unittest.TestCase):

Expand Down
Loading

0 comments on commit d57f141

Please sign in to comment.