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 /generate #5075

Merged
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
73 changes: 46 additions & 27 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,33 +60,43 @@ 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 are already logged in. Please verify your codename below as it " +
"may differ from the one displayed on the previous page."),
'notification')
else:
os.mkdir(current_app.storage.path(filesystem_id))
tab_id = request.form['tab_id']
codename = session['codenames'][tab_id]
session['codename'] = codename

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
session['logged_in'] = True
return redirect(url_for('.lookup'))

@view.route('/lookup', methods=('GET',))
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
22 changes: 20 additions & 2 deletions securedrop/tests/functional/source_navigation_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ def _source_clicks_submit_documents_on_homepage(self):
# a diceware codename they can use for subsequent logins
assert self._is_on_generate_page()

def _source_regenerates_codename(self):
self.safe_click_by_id("regenerate-submit")

def _source_chooses_to_submit_documents(self):
self._source_clicks_submit_documents_on_homepage()

Expand All @@ -48,7 +51,7 @@ def _source_chooses_to_submit_documents(self):
assert len(codename.text) > 0
self.source_name = codename.text

def _source_shows_codename(self):
def _source_shows_codename(self, verify_source_name=True):
content = self.driver.find_element_by_id("codename-hint-content")
assert not content.is_displayed()

Expand All @@ -57,7 +60,8 @@ def _source_shows_codename(self):
self.wait_for(lambda: content.is_displayed())
assert content.is_displayed()
content_content = self.driver.find_element_by_css_selector("#codename-hint-content p")
assert content_content.text == self.source_name
if verify_source_name:
assert content_content.text == self.source_name

def _source_hides_codename(self):
content = self.driver.find_element_by_id("codename-hint-content")
Expand Down Expand Up @@ -223,3 +227,17 @@ def _source_sees_document_attachment_item(self):
def _source_does_not_sees_document_attachment_item(self):
with pytest.raises(NoSuchElementException):
self.driver.find_element_by_class_name("attachment")

def _source_sees_already_logged_in_in_other_tab_message(self):
notification = self.driver.find_element_by_css_selector(".notification")

if not hasattr(self, "accepted_languages"):
expected_text = "You are already logged in."
assert expected_text in notification.text

def _source_sees_redirect_already_logged_in_message(self):
notification = self.driver.find_element_by_css_selector(".notification")

if not hasattr(self, "accepted_languages"):
expected_text = "You were redirected because you are already logged in."
assert expected_text in notification.text
103 changes: 103 additions & 0 deletions securedrop/tests/functional/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,106 @@ def test_journalist_key_from_source_interface(self):

data = data.decode('utf-8')
assert "BEGIN PGP PUBLIC KEY BLOCK" in data


class TestDuplicateSourceInterface(
functional_test.FunctionalTest,
source_navigation_steps.SourceNavigationStepsMixin):

def get_codename_generate(self):
return self.driver.find_element_by_css_selector("#codename").text

def get_codename_lookup(self):
return self.driver.find_element_by_css_selector("#codename-hint-content p").text

def test_duplicate_generate_pages(self):
# Test generation of multiple codenames in different browser tabs, ref. issue 4458.

# Generate a codename in Tab A
assert len(self.driver.window_handles) == 1
tab_a = self.driver.current_window_handle
self._source_visits_source_homepage()
self._source_chooses_to_submit_documents()
codename_a = self.get_codename_generate()

# Generate a different codename in Tab B
self.driver.execute_script("window.open()")
tab_b = self.driver.window_handles[1]
self.driver.switch_to.window(tab_b)
assert self.driver.current_window_handle == tab_b
self._source_visits_source_homepage()
self._source_chooses_to_submit_documents()
codename_b = self.get_codename_generate()

assert tab_a != tab_b
assert codename_a != codename_b

# Proceed to submit documents in Tab A
self.driver.switch_to.window(tab_a)
assert self.driver.current_window_handle == tab_a
self._source_continues_to_submit_page()
assert self._is_on_lookup_page()
self._source_shows_codename(verify_source_name=False)
codename_lookup_a = self.get_codename_lookup()
assert codename_lookup_a == codename_a
self._source_submits_a_message()

# Proceed to submit documents in Tab B
self.driver.switch_to.window(tab_b)
assert self.driver.current_window_handle == tab_b
self._source_continues_to_submit_page()
assert self._is_on_lookup_page()
self._source_sees_already_logged_in_in_other_tab_message()
self._source_shows_codename(verify_source_name=False)
codename_lookup_b = self.get_codename_lookup()
# We expect the codename to be the one from Tab A
assert codename_lookup_b == codename_a
self._source_submits_a_message()

def test_duplicate_generate_pages_with_refresh(self):
# Test generation of multiple codenames in different browser tabs, including behavior
# of refreshing the codemae in each tab. Ref. issue 4458.

# Generate a codename in Tab A
assert len(self.driver.window_handles) == 1
tab_a = self.driver.current_window_handle
self._source_visits_source_homepage()
self._source_chooses_to_submit_documents()
codename_a1 = self.get_codename_generate()
# Regenerate codename in Tab A
self._source_regenerates_codename()
codename_a2 = self.get_codename_generate()
assert codename_a1 != codename_a2

# Generate a different codename in Tab B
self.driver.execute_script("window.open()")
tab_b = self.driver.window_handles[1]
self.driver.switch_to.window(tab_b)
assert self.driver.current_window_handle == tab_b
self._source_visits_source_homepage()
self._source_chooses_to_submit_documents()
codename_b = self.get_codename_generate()
assert codename_b != codename_a1 != codename_a2

# Proceed to submit documents in Tab A
self.driver.switch_to.window(tab_a)
assert self.driver.current_window_handle == tab_a
self._source_continues_to_submit_page()
assert self._is_on_lookup_page()
self._source_shows_codename(verify_source_name=False)
codename_lookup_a = self.get_codename_lookup()
assert codename_lookup_a == codename_a2
self._source_submits_a_message()

# Regenerate codename in Tab B
self.driver.switch_to.window(tab_b)
assert self.driver.current_window_handle == tab_b
self._source_regenerates_codename()
# We expect the source to be directed to /lookup with a flash message
assert self._is_on_lookup_page()
self._source_sees_redirect_already_logged_in_message()
# Check codename
self._source_shows_codename(verify_source_name=False)
codename_lookup_b = self.get_codename_lookup()
assert codename_lookup_b == codename_a2
self._source_submits_a_message()
27 changes: 18 additions & 9 deletions securedrop/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def test_submit_message(source_app, journalist_app, test_journo):

with source_app.test_client() as app:
app.get('/generate')
app.post('/create', follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id}, follow_redirects=True)
filesystem_id = g.filesystem_id
# redirected to submission form
resp = app.post('/submit', data=dict(
Expand Down Expand Up @@ -153,7 +154,8 @@ def test_submit_file(source_app, journalist_app, test_journo):

with source_app.test_client() as app:
app.get('/generate')
app.post('/create', follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id}, follow_redirects=True)
filesystem_id = g.filesystem_id
# redirected to submission form
resp = app.post('/submit', data=dict(
Expand Down Expand Up @@ -254,7 +256,8 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo,

with source_app.test_client() as app:
app.get('/generate')
app.post('/create', follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id}, follow_redirects=True)
codename = session['codename']
filesystem_id = g.filesystem_id
# redirected to submission form
Expand Down Expand Up @@ -474,7 +477,8 @@ def test_delete_collection(mocker, source_app, journalist_app, test_journo):
# first, add a source
with source_app.test_client() as app:
app.get('/generate')
app.post('/create')
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id})
resp = app.post('/submit', data=dict(
msg="This is a test.",
fh=(BytesIO(b''), ''),
Expand Down Expand Up @@ -523,7 +527,8 @@ def test_delete_collections(mocker, journalist_app, source_app, test_journo):
num_sources = 2
for i in range(num_sources):
app.get('/generate')
app.post('/create')
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id})
app.post('/submit', data=dict(
msg="This is a test " + str(i) + ".",
fh=(BytesIO(b''), ''),
Expand Down Expand Up @@ -577,7 +582,8 @@ def test_filenames(source_app, journalist_app, test_journo):
# add a source and submit stuff
with source_app.test_client() as app:
app.get('/generate')
app.post('/create')
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id})
_helper_filenames_submit(app)

# navigate to the collection page
Expand All @@ -603,7 +609,8 @@ def test_filenames_delete(journalist_app, source_app, test_journo):
# add a source and submit stuff
with source_app.test_client() as app:
app.get('/generate')
app.post('/create')
tab_id = next(iter(session['codenames'].keys()))
app.post('/create', data={'tab_id': tab_id})
_helper_filenames_submit(app)

# navigate to the collection page
Expand Down Expand Up @@ -714,7 +721,8 @@ def test_prevent_document_uploads(source_app, journalist_app, test_admin):
# Check that the source interface accepts only messages:
with source_app.test_client() as app:
app.get('/generate')
resp = app.post('/create', follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
resp = app.post('/create', data={'tab_id': tab_id}, follow_redirects=True)
assert resp.status_code == 200

text = resp.data.decode('utf-8')
Expand All @@ -739,7 +747,8 @@ def test_no_prevent_document_uploads(source_app, journalist_app, test_admin):
# Check that the source interface accepts both files and messages:
with source_app.test_client() as app:
app.get('/generate')
resp = app.post('/create', follow_redirects=True)
tab_id = next(iter(session['codenames'].keys()))
resp = app.post('/create', data={'tab_id': tab_id}, follow_redirects=True)
assert resp.status_code == 200

text = resp.data.decode('utf-8')
Expand Down
Loading