Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle multiple instances of source /generate #5048

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions securedrop/source_app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ def generate():
"notification")
return redirect(url_for('.lookup'))

# Note if a codename was already generated
if 'codename' in session:
session['multiple_codenames'] = True
else:
session['multiple_codenames'] = False

codename = generate_unique_codename(config)
session['codename'] = codename
session['new_user'] = True
Expand All @@ -51,34 +57,40 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirming that #4391 was a fix required when we were deleting session['codename'] in this view function and so is good to remove now

if logged_in():
flash(gettext("You have already logged-in from a different browser tab.\n"
"Please verify your codename below as it may differ from "
"the one displayed on the previous page."),
'notification')
else:
if session['multiple_codenames']:
flash(gettext("Multiple codenames have been generated in different browser "
"tabs. Please verify your codename below as it may differ from "
"the one displayed on the previous page."),
'notification')

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:
del session['logged_in']
except KeyError:
pass
db.session.commit()
except IntegrityError as e:
db.session.rollback()
current_app.logger.error(
"Attempt to create a source with duplicate codename: %s" %
(e,))

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

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

if session['multiple_codenames']:
return redirect(url_for('.lookup', _anchor='codename-hint-visible'))
else:
return redirect(url_for('.lookup'))

@view.route('/lookup', methods=('GET',))
@login_required
Expand Down
37 changes: 25 additions & 12 deletions securedrop/tests/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,18 @@ def test_create_duplicate_codename_logged_in_not_in_session(source_app):
# Attempt to add the same source
with app.session_transaction() as sess:
sess['codename'] = codename
sess['multiple_codenames'] = True
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
assert 'logged_in' not in session
assert resp.status_code == 200
assert 'codename' in session
assert 'logged_in' in session
# Should be redirected to /lookup with a flashed message
text = resp.data.decode('utf-8')
assert "Multiple codenames have been generated in different browser" in text
assert "Submit Files" in text


def test_create_duplicate_codename_logged_in_in_session(source_app):
Expand All @@ -171,18 +176,26 @@ def test_create_duplicate_codename_logged_in_in_session(source_app):
# Create a source the first time
resp = app.post(url_for('main.create'), follow_redirects=True)
assert resp.status_code == 200
codename = session['codename']
multiple_codenames = session['multiple_codenames']
logged_in = session['logged_in']

with source_app.test_client() as app:
# Attempt to add the same source
with app.session_transaction() as sess:
sess['codename'] = codename
sess['multiple_codenames'] = multiple_codenames
sess['logged_in'] = logged_in
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

# Reproducer for bug #4361
resp = app.post(url_for('main.index'), follow_redirects=True)
assert 'logged_in' not in session
# Since the user is logged-in we expect that they will be redirected
# and no attempt to create a soure with duplicate codename will be made.
# No error should therefore be logged.
assert logger.call_args is None
assert resp.status_code == 200
# Should be redirected to /lookup with a flashed message
text = resp.data.decode('utf-8')
assert "You have already logged-in from a different browser tab" in text
assert "Submit Files" in text


def test_lookup(source_app):
Expand Down