Handle multiple instances of /generate

[DrGFreeman]

Status

Ready for review.

Description of Changes

Fixes #4458.

Changes proposed in this pull request:

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 /lookup 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.

Testing

In the dev VM:

1. Open one browser tab (A) to /generate and note the codename (A).
2. Open a second browser tab (B) to /generate.
3. Return to tab A and click "SUBMIT DOCUMENTS".
4. Verify that tab A redirects to /lookup.
5. Verify that the codename at bottom of /lookup in tab A corresponds to codename A.
6. Verify that a message can be submitted successfully from tab A.
7. Return to tab B and click "SUBMIT DOCUMENTS".
8. Verify that tab B redirects to /lookup.
9. Verify that a flash message indicates the source is already logged-in and recommends verifying the codename.
10. Verify that the codename at the bottom of /lookup in tab B is visible and matches codename A.
11. Verify that a message can be submitted successfully from tab B.

Additional tests:

1. Logout of both tabs A & B
2. Proceed to /generate.
3. Verify that refreshing the TAB (via the browser button or the button to the right of the codename) refreshes the page as expected.

Deployment

Any special considerations for deployment?

N/A

Checklist

If you made changes to the server application code:

• Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

• Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

• Configuration tests pass

If you made non-trivial code changes:

• I have written a test plan and validated it for this PR

• Updated existing tests in test_source.py and test_integration.py.
• Added functional tests in functional/test_source.py.

If you made changes to documentation:

• Doc linting (make docs-lint) passed locally

If you added or updated a code dependency:

Choose one of the following:

• I have performed a diff review and pasted the contents to the packaging wiki
• I would like someone else to do the diff review

[zenmonkeykstop]

Test plan as described passes against dev container. Also tried logging out on Tab A and then attempting to submit on Tab B, which throws to the index page with the "session timed out due to inactivity" flash message.

Given the nature of the change I'm running through it on staging and Tor Browser now as well.

[zenmonkeykstop]

Test plan also passes in staging.

[DrGFreeman]

Great! Thanks @zenmonkeykstop. I'll start working on the additional functional tests.

[DrGFreeman]

@zenmonkeykstop. I have added a functional test for the main scenario described in the PR template and switched the status to Ready for review.

I wonder if I should also add tests for the "Refresh" scenario and the "Logout" scenario you tested as well?

[zenmonkeykstop]

@DrGFreeman more tests the better, if you have time :)

[eloquence]

I would recommend removing the #codename-hint-visible anchor from the redirect for now. Redirecting to the bottom of the page while displaying a message at the top of the page is confusing, and potentially clips off the message, like so:

We've kicked this around from the UX perspective a bit more, and I will file an issue for a couple of small follow-up tweaks. Once this anchor redirect is gone, this PR is IMO good to merge as a first pass, from a UX perspective.

[DrGFreeman]

Good point @eloquence. I had tested on a high res. monitor where the whole page fits in the default Tor Browser window size. I appreciate this kind of feedback from the UX perspective. It helps me consider additional aspects when proposing a change.

I will remove the anchor, update the tests accordingly and as well as add functional tests for the "refresh" and "logout" cases mentioned above.

Also, I read the suggestions in #5080. Understanding there are potential further improvements to be made, would you suggest a different (better) wording for the message introduced by the current PR?

[eloquence]

Also, I read the suggestions in #5080. Understanding there are potential further improvements to be made, would you suggest a different (better) wording for the message introduced by the current PR?

I'd only suggest a slight edit for now:

From:

You have already logged-in from a different browser tab.

To:

You are already logged in.

(We don't know for sure if it's a different tab, or a window, and it may be closed by the time they see this.)

The rest we can handle in follow-up as we get consensus on any further tweaks per #5080.

[DrGFreeman]

@DrGFreeman more tests the better, if you have time :)

Two other functional test on their way. Will push them shortly.

[DrGFreeman]

Added a test for the "refresh" scenario via 32ab477.

I've spent the two last nights trying to implement a test of the "logout" scenario described by @zenmonkeykstop:

Also tried logging out on Tab A and then attempting to submit on Tab B, which throws to the index page with the "session timed out due to inactivity" flash message.

When the source logs-out, the session is cleared. In the browser, when trying to submit on Tab B after the logout, the redirect to /index with the "session timed out" message results from the handle_csrf_error function (at least, that's my understanding).

However, when I try to replicate this with Selemium, trying to submit from Tab B after the logout results in the driver being stuck on the /create route. There is no handling of an empty session in /create which may explain this behavior but I can't figure out why the behavior differs between the actual browser and Selenium.

I will keep investigating this but I currently do not have a solution in view.

[zenmonkeykstop]

Thanks, @DrGFreeman, test coverage you have for this is already great. Gonna review once more today!

[zenmonkeykstop]

Tests and test plan passing. LGTM!