Last Comment Bug 772072 - Add warning to Filelink about URL guessing
: Add warning to Filelink about URL guessing
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: FileLink (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 16.0
Assigned To: Mike Conley (:mconley)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-09 07:51 PDT by Mike Conley (:mconley)
Modified: 2012-07-13 08:41 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed warnings in HTML and plaintext mail (118.96 KB, image/png)
2012-07-09 08:13 PDT, Mike Conley (:mconley)
bwinton: ui‑review-
Details
Patch v1 (3.99 KB, patch)
2012-07-09 09:31 PDT, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review
Screenshot of applied patch (84.82 KB, image/png)
2012-07-09 09:35 PDT, Mike Conley (:mconley)
bwinton: ui‑review+
Details
Patch that gives off warning notification - includes test (7.34 KB, patch)
2012-07-09 09:59 PDT, Mike Conley (:mconley)
bwinton: review-
Details | Diff | Splinter Review
Patch v3 (11.79 KB, patch)
2012-07-10 07:06 PDT, Mike Conley (:mconley)
bwinton: review-
Details | Diff | Splinter Review
Patch v4 (12.54 KB, patch)
2012-07-12 07:58 PDT, Mike Conley (:mconley)
bwinton: review+
bwinton: ui‑review+
Details | Diff | Splinter Review

Description Mike Conley (:mconley) 2012-07-09 07:51:38 PDT
From the Filelink privacy review:

"We point the user at the privacy and terms of service of the providers. We also are requiring the providers not to expire files, so that the e-mail recipients don't end up with stale links.

Risk: A "surprise" may manifest itself is when a user mis-forwards the message to the wrong party and the file can be downloaded by an unintended recipient. This can happen with the current set-up if a user forwards an attachment to the wrong party, and is a risk inherent in email.

An added risk of this system is that the file is publicly available and not limited in accessibility to those who can access the message itself. This is like 'security by obscurity', which is not ideal, but acceptable in this case since users know the file will be uploaded to a sharing service.

Recommendation: Make it clear to users that uploaded files will be world-readable (to members of the world who know where to find it). "
Comment 1 Mike Conley (:mconley) 2012-07-09 08:13:13 PDT
Created attachment 640226 [details]
Proposed warnings in HTML and plaintext mail

Blake:

What do you think about removing the little ad at the bottom of the set of Filelinks, and including something like this warning instead?

-Mike
Comment 2 Blake Winton (:bwinton) (:☕️) 2012-07-09 08:17:05 PDT
Comment on attachment 640226 [details]
Proposed warnings in HTML and plaintext mail

I kind of like the ad, and I think that this warning is being displayed to the wrong people.  My understanding is that we're trying to warn the authors of the email, not the recipients…  So, I'm going to say ui-r-.

Thanks,
Blake.
Comment 3 Mike Conley (:mconley) 2012-07-09 09:31:00 PDT
Created attachment 640247 [details] [diff] [review]
Patch v1

Adds a one-time-per-composer notification one the first batch of uploads are finished, warning the user about the visibility of their files.
Comment 4 Mike Conley (:mconley) 2012-07-09 09:35:08 PDT
Created attachment 640249 [details]
Screenshot of applied patch

Closer? How's the language?
Comment 5 Blake Winton (:bwinton) (:☕️) 2012-07-09 09:38:18 PDT
Comment on attachment 640249 [details]
Screenshot of applied patch

Seems good.  I would say "may be accessible _to_ people"…  Or maybe "will be accessible to people"…

But other than that, ui-r-me.
Comment 6 Mike Conley (:mconley) 2012-07-09 09:59:50 PDT
Created attachment 640256 [details] [diff] [review]
Patch that gives off warning notification - includes test

Tada!
Comment 7 Blake Winton (:bwinton) (:☕️) 2012-07-09 11:04:01 PDT
Comment on attachment 640256 [details] [diff] [review]
Patch that gives off warning notification - includes test

r- due to the following:

Open a compose window.
Attach a file.
Link it.
Wait for the new notification to show up.
Close the compose window.
Open a new compose window.
Marvel at the notification that appears when there are no attachments!  ;)

(In a similar vein, I don't get notified for the second compose window if I collapse the notification on the first one.)

Tests for those would also be nice…

Thanks,
Blake.
Comment 8 Mike Conley (:mconley) 2012-07-10 07:06:06 PDT
Created attachment 640587 [details] [diff] [review]
Patch v3

Great catches! Fixed, and added tests for those cases.
Comment 9 Blake Winton (:bwinton) (:☕️) 2012-07-11 11:34:19 PDT
Comment on attachment 640587 [details] [diff] [review]
Patch v3

Open a compose window.
Attach a file.
Convert it to an UbuntuOne link.
Look at the pretty error message.
Close the window, without saving the draft.
Open a compose window.
Attach a file.
Convert it to an UbuntuOne link.
Wonder where the pretty error message is this time…  :(
Open a compose window.
Attach a file.
Convert it to an UbuntuOne link.
Hey, there it is!  :(

Apparently I missed my calling as a tester…  ;)

Later,
Blake.
Comment 10 Mike Conley (:mconley) 2012-07-11 13:37:04 PDT
(In reply to Blake Winton (:bwinton - Thunderbird UX) [On vacation until July 6th!] from comment #9)
> Comment on attachment 640587 [details] [diff] [review]
> Patch v3
> 
> Open a compose window.
> Attach a file.
> Convert it to an UbuntuOne link.
> Look at the pretty error message.
> Close the window, without saving the draft.
> Open a compose window.
> Attach a file.
> Convert it to an UbuntuOne link.
> Wonder where the pretty error message is this time…  :(
> Open a compose window.
> Attach a file.
> Convert it to an UbuntuOne link.
> Hey, there it is!  :(
> 
> Apparently I missed my calling as a tester…  ;)
> 
> Later,
> Blake.

Ah, figured it out. Dumb error. I'll have the fix and more test cases up soon.
Comment 11 Mike Conley (:mconley) 2012-07-12 07:58:04 PDT
Created attachment 641467 [details] [diff] [review]
Patch v4

Hey Blake - mind taking this one for a spin and seeing if you can shake anything else loose?

-Mike
Comment 12 Blake Winton (:bwinton) (:☕️) 2012-07-13 08:24:37 PDT
Comment on attachment 641467 [details] [diff] [review]
Patch v4

>+++ b/mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
>@@ -361,16 +361,17 @@ bigFileHideNotification.check=Never noti
>+cloudFilePrivacyNotification=Linking is complete. Please note that linked attachments may be accessible to people who can see or guess the links.

"may be" or "will be"?

Aside from that, I like it.  r=me!  (And ui-r=me, for completeness. ;)

Thanks,
Blake.
Comment 13 Mike Conley (:mconley) 2012-07-13 08:41:33 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/736f08622ee9

Note You need to log in before you can comment on or make changes to this bug.