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

"Awaiting action from the source to enable replies:" message appears for new sources #803

Closed
sssoleileraaa opened this issue Feb 13, 2020 · 8 comments · Fixed by #830
Closed
Labels

Comments

@sssoleileraaa
Copy link
Contributor

STR

  1. From Qubes, create a new source and send a file.
  2. Select the new source in the client right after you see the source appear after a sync

Expected

The reg "Compose a reply to " message in the reply box.

Actual

An "Awaiting action from the source to enable replies" in a gray box that doesn't span for the entire size of the reply box. Also, no action from the source was actually necessary for this message to go away eventually.

signal-attachment-2020-02-12-192645

@sssoleileraaa
Copy link
Contributor Author

here is a clearer screenshot (just saw this today from outside of Qubes on master by selecting a new source immediately after a submission):
Screenshot from 2020-02-18 14-01-49

@rmol rmol self-assigned this Feb 20, 2020
@rmol
Copy link
Contributor

rmol commented Feb 24, 2020

I think this is a state we can't avoid without changing the SecureDrop server code. When the new source makes a submission, the server launches the generation of its key pair, but that might not complete by the time the client syncs. In that case, the client has a new source without a key, so the reply box is necessarily disabled with the Awaiting action from the source to enable replies message1. Until the key is delivered at the next sync, there's nothing to be done.

We could try to force the retrieval of the source when the selection changes, in the hope that the key has been generated and we can enable replies, but it still wouldn't be guaranteed.

I've added the update of the reply box state in 803-refresh-replybox but I hesitate to file a PR for this based on it because I don't think it will change the user's experience.

1 It does seem like we should replace the misleading placeholder text. We're really waiting for the server to enable encrypted replies to the source.

@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Feb 24, 2020

@rmol nice investigation and explanation for what's going on. i think waiting until the next sync is fine, espeically if we decrease wait period to less than 15 seconds, but even 15 seconds is not long.

1 It does seem like we should replace the misleading placeholder text. We're really waiting for the server to enable encrypted replies to the source.

agreed. the messaging needs to change from "Awaiting action from the source to enable replies" to something more like "Awaiting source key before we can encrypt replies".

we also need to update the gray background to white.

@eloquence
Copy link
Member

eloquence commented Feb 24, 2020

The UI text was composed with cases in mind where a source key was not generated for other reasons upon the source's initial login (e.g. due to a reboot, or the elusive "flag for reply" entropy exhaustion case). In those cases, another login by the source will trigger another keypair generation attempt, hence the "Awaiting action".

I agree we can generalize it for now, and we should fix the styling as you say.

I'd suggest something like this:

Awaiting encryption key to enable replies.

@ninavizz: Your input on the final wording is appreciated, happy to talk this through to give context.

@ninavizz
Copy link
Member

As I understand it, there are 2 distinct use-cases...

  1. Upton (source) submits to NYWorld, < 60sec after loading the GetCodename page. Ely Bly at NYWorld is also working on her SD Workstation, and immediately receives Erik's tip. She will not be able to reply to him, however, until the keypair has been generated... which can take up to 60 seconds.

  2. Upton (source) submits to NYWorld, right at the same time the server is rebooting for its nightly purge. Because of the timing, a keypair is not generated. A keypair will unfortunately not be generated, until Upton logs in again. The next day, Ely checks NYWorld's SD. She will not be able to send Upton a reply, until Upton re-checks SecureDrop.

After thinking it through, I feel the best solution for #1 is to not block Ely from replying, for the first 2 syncs after the Workstation receives Upton's submission. Instead, add a reply to the Queue and let it get sent when it gets sent. This assumes that #2 is much less likely to happen.

After 2 syncs, if a keypair is still absent, then show the below screen—as #2 is then to be the most likely scenario. On the chance that Ely sends a reply within one minute of a server reboot failed keypair, then simply bounce it with an error—and then show the below screen, with the failed reply above it.

I feel that anything more complex, would be over-engineering and introducing confusion to users with little benefit.

For Beta, the "Learn More" would open a page in the docs. Beyond Beta, it'd open a contextual help window.

image

@eloquence
Copy link
Member

Thanks @ninavizz! There is also the entropy pool exhaustion case, since that logic has not been removed yet (that's tracked in freedomofpress/securedrop#1584). There's an open question on that ticket about how likely journalists are to hit that case in prod. The answer to that question is IMO important to decide whether the approach you propose is viable.

That's because we of course don't want to fail replies too often -- it would be very frustrating for a journo to start typing a reply to a new source, only to be told after a couple of syncs that we're still waiting for a keypair.

Separately, there's the question of implementation complexity:

  • stashing unencrypted draft replies (assuming that's not already the behavior for all drafts)
  • retrying up to a limited number of successful syncs
  • ensuring no regression in normal reply retry/sending behavior

Would appreciate Jen's/Allie's comments on the implementation complexity, but my intuition is that we have to do a more minimal UI iteration to resolve this release blocker, and then focus on other remaining blockers, leaving us no time to do what you describe for the pilot.

@eloquence
Copy link
Member

Unfortunately we won't have time to change the business logic for this case.

I'll put in a small PR with the wording suggested in #803 (comment) for now, we can always tweak more later.

@eloquence
Copy link
Member

eloquence commented Feb 27, 2020

I've marked #830 to resolve this issue; if it does what it says on the tin, it will fix both the styling bug and display a clearer message.

Per previous discussion, we'll likely want to follow up on the user experience for this case. We can prioritize specific actions and mitigations after the pilot kicks off, which should also help us to understand better how likely the "Flag for reply" case (which will also trigger this message to appear) is to occur in practice.

I've kept the "release blocker" label in place for issue and PR because the previous message is very confusing and the styling bug is very visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants