Skip to content

Commit

Permalink
Handle multiple instances of /generate
Browse files Browse the repository at this point in the history
Associate codenames from different browser tabs open to /generate with
a unique tab id hidden in the form. This allows to register the source
using the codename displayed in the browser tab from which the source
proceeds to submit documents instead of the last codename generated
(current behavior).

Proceeding to submit documents from another tab open to /generate
redirects to /lookout with a flash message informing the source they are
already logged-in and to verify their codename. The codename is made
visible at the bottom of the /lookup page.
  • Loading branch information
DrGFreeman committed Dec 14, 2019
1 parent 2300bdf commit 3635a4a
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 53 deletions.
76 changes: 48 additions & 28 deletions securedrop/source_app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
import io

from base64 import urlsafe_b64encode
from datetime import datetime
from flask import (Blueprint, render_template, flash, redirect, url_for, g,
session, current_app, request, Markup, abort)
Expand Down Expand Up @@ -37,9 +38,17 @@ def generate():
return redirect(url_for('.lookup'))

codename = generate_unique_codename(config)
session['codename'] = codename

# Generate a unique id for each browser tab and associate the codename with this id.
# This will allow retrieval of the codename displayed in the tab from which the source has
# clicked to proceed to /generate (ref. issue #4458)
tab_id = urlsafe_b64encode(os.urandom(64)).decode()
codenames = session.get('codenames', {})
codenames[tab_id] = codename
session['codenames'] = codenames

session['new_user'] = True
return render_template('generate.html', codename=codename)
return render_template('generate.html', codename=codename, tab_id=tab_id)

@view.route('/org-logo')
def select_logo():
Expand All @@ -51,34 +60,45 @@ def select_logo():

@view.route('/create', methods=['POST'])
def create():
filesystem_id = current_app.crypto_util.hash_codename(
session['codename'])

source = Source(filesystem_id, current_app.crypto_util.display_id())
db.session.add(source)
try:
db.session.commit()
except IntegrityError as e:
db.session.rollback()
current_app.logger.error(
"Attempt to create a source with duplicate codename: %s" %
(e,))

# Issue 2386: don't log in on duplicates
del session['codename']

# Issue 4361: Delete 'logged_in' if it's in the session
try:
del session['logged_in']
except KeyError:
pass

abort(500)
if session.get('logged_in', False):
flash(gettext("You have already logged-in from a different browser tab. " +
"Please verify your codename below as it may differ from " +
"the one displayed on the previous page."), 'notification')
return redirect(url_for('.lookup', _anchor='codename-hint-visible'))
else:
os.mkdir(current_app.storage.path(filesystem_id))
tab_id = request.form['tab_id']
codename = session['codenames'][tab_id]
session['codename'] = codename

session['logged_in'] = True
return redirect(url_for('.lookup'))
del session['codenames']

filesystem_id = current_app.crypto_util.hash_codename(codename)

source = Source(filesystem_id, current_app.crypto_util.display_id())
db.session.add(source)
try:
db.session.commit()
except IntegrityError as e:
db.session.rollback()
current_app.logger.error(
"Attempt to create a source with duplicate codename: %s" %
(e,))

# Issue 2386: don't log in on duplicates
del session['codename']

# Issue 4361: Delete 'logged_in' if it's in the session
try:
del session['logged_in']
except KeyError:
pass

abort(500)
else:
os.mkdir(current_app.storage.path(filesystem_id))

session['logged_in'] = True
return redirect(url_for('.lookup'))

@view.route('/lookup', methods=('GET',))
@login_required
Expand Down
1 change: 1 addition & 0 deletions securedrop/source_templates/generate.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ <h1>{{ gettext('Welcome') }}</h1>

<form id="create-form" method="post" action="/create" autocomplete="off">
<input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
<input name="tab_id" type="hidden" value="{{ tab_id }}">
<button type="submit" class="btn--space pull-right" id="continue-button">
{{ gettext('SUBMIT DOCUMENTS') }}
</button>
Expand Down
54 changes: 31 additions & 23 deletions securedrop/tests/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_generate(source_app):
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200
session_codename = session['codename']
session_codename = next(iter(session['codenames'].values()))

text = resp.data.decode('utf-8')
assert "This codename is what you will use in future visits" in text
Expand Down Expand Up @@ -113,11 +113,13 @@ def test_create_new_source(source_app):
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200
resp = app.post(url_for('main.create'), follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
resp = app.post(url_for('main.create'), data={'tab_id': tab_id}, follow_redirects=True)
assert session['logged_in'] is True
# should be redirected to /lookup
text = resp.data.decode('utf-8')
assert "Submit Files" in text
assert 'codenames' not in session


def test_generate_too_long_codename(source_app):
Expand All @@ -143,17 +145,18 @@ def test_create_duplicate_codename_logged_in_not_in_session(source_app):
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200
tab_id = next(iter(session['codenames'].keys()))

# Create a source the first time
resp = app.post(url_for('main.create'), follow_redirects=True)
resp = app.post(url_for('main.create'), data={'tab_id': tab_id}, follow_redirects=True)
assert resp.status_code == 200
codename = session['codename']

with source_app.test_client() as app:
# Attempt to add the same source
with app.session_transaction() as sess:
sess['codename'] = codename
resp = app.post(url_for('main.create'), follow_redirects=True)
sess['codenames'] = {tab_id: codename}
resp = app.post(url_for('main.create'), data={'tab_id': tab_id}, follow_redirects=True)
logger.assert_called_once()
assert ("Attempt to create a source with duplicate codename"
in logger.call_args[0][0])
Expand All @@ -163,26 +166,31 @@ def test_create_duplicate_codename_logged_in_not_in_session(source_app):


def test_create_duplicate_codename_logged_in_in_session(source_app):
with patch.object(source.app.logger, 'error') as logger:
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200

# Create a source the first time
resp = app.post(url_for('main.create'), follow_redirects=True)
assert resp.status_code == 200
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200
tab_id = next(iter(session['codenames'].keys()))

# Attempt to add the same source
resp = app.post(url_for('main.create'), follow_redirects=True)
logger.assert_called_once()
assert ("Attempt to create a source with duplicate codename"
in logger.call_args[0][0])
assert resp.status_code == 500
assert 'codename' not in session
# Create a source the first time
resp = app.post(url_for('main.create'), data={'tab_id': tab_id}, follow_redirects=True)
assert resp.status_code == 200
codename = session['codename']
logged_in = session['logged_in']

# Reproducer for bug #4361
resp = app.post(url_for('main.index'), follow_redirects=True)
assert 'logged_in' not in session
# Attempt to add another source in the same session
with source_app.test_client() as app:
resp = app.get(url_for('main.generate'))
assert resp.status_code == 200
tab_id = next(iter(session['codenames'].keys()))
with app.session_transaction() as sess:
sess['codename'] = codename
sess['logged_in'] = logged_in
resp = app.post(url_for('main.create'), data={'tab_id': tab_id}, follow_redirects=True)
assert resp.status_code == 200
assert session['codename'] == codename
text = resp.data.decode('utf-8')
assert "You have already logged-in" in text
assert "Submit Files" in text


def test_lookup(source_app):
Expand Down
4 changes: 2 additions & 2 deletions securedrop/tests/utils/db_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,6 @@ def new_codename(client, session):
"""Helper function to go through the "generate codename" flow.
"""
client.get('/generate')
codename = session['codename']
client.post('/create')
tab_id, codename = next(iter(session['codenames'].items()))
client.post('/create', data={'tab_id': tab_id})
return codename

0 comments on commit 3635a4a

Please sign in to comment.