652345 - SecureMail extension should encrypt password reset mail regardless of group membership if the user has provided a key

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Dave Miller [:justdave]]

What the summary says...

The SecureMail extension should encrypt password reset mail regardless of group membership if the user has provided a key to encrypt it with in their preferences.

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This should do the trick.

Gerv

Comment 2

[David Lawrence [:dkl]]

Comment on attachment 528118 [details] [diff] [review]
Patch v.1

Review of attachment 528118 [details] [diff] [review]:

::: extensions/SecureMail/Extension.pm
@@ +220,2 @@
                 $make_secure = 0;
+            

t/001compile.t ....... 30/176 Missing right curly or square bracket at ./extensions/SecureMail/Extension.pm line 326, at end of line
syntax error at ./extensions/SecureMail/Extension.pm line 326, at EOF
Compilation failed in require at Bugzilla/Extension.pm line 82.

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.2

No idea what happened there; a typo just before I uploaded. Try this.

Gerv

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 528146 [details] [diff] [review]
Patch v.2

Review of attachment 528146 [details] [diff] [review]:

Looks good. Today I added an extensions/SecureMail/template/en/default/pages/securemail/help.html.tmpl that is a copy of the BMO/Keys wiki text.
Please update the text to show that having a key uploaded will always encrypt password reset emails regardless of group membership. r=dkl

Comment 6

[Gervase Markham [:gerv]]

Committing to: bzr+ssh://bzr.mozilla.org/bmo/4.0/
modified extensions/SecureMail/Extension.pm
Committed revision 7630.   

dkl: why move that help text into a page.cgi page rather than the wiki page? Surely that just makes it harder to update?

Gerv

Comment 7

[Dave Miller [:justdave]]

If it's on the wiki, any Joe off the internet can come and edit it, unless we lock the page.  Locking the page makes it equivalently hard to edit as having it as a page.cgi template (find someone with the right permissions to edit it).

Comment 8

[Gervase Markham [:gerv]]

(In reply to comment #7)
> If it's on the wiki, any Joe off the internet can come and edit it, unless we
> lock the page.  Locking the page makes it equivalently hard to edit as having
> it as a page.cgi template (find someone with the right permissions to edit it).

Or we watch it and revert bogus edits. What could they change it to do? Do you think if it said "mail your private key to [e-mail]" anyone would be that dumb?

Gerv

Comment 9

[David Lawrence [:dkl]]

Reason being is that we want anyone that wants to be able to use the extension on their own Bugzilla instance. So the help for the extension needs to be self-contained and not always pointing to Mozilla's wiki. The wiki page could change or go away and then everyone's help links become broken.

dkl

Comment 10

[Dave Miller [:justdave]]

(In reply to comment #8)
> Do you think if it said "mail your private key to [e-mail]"
> anyone would be that dumb?

Yes.  Because I've seen it happen.  Phishing on the Internet wouldn't exist as a problem if everyone was smart enough to avoid that kind of thing.

Comment 11

[Luis]

Comment on attachment 528146 [details] [diff] [review]
Patch v.2

Review of attachment 528146 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/SecureMail/Extension.pm
@@ +208,5 @@
>              }
>          }
>          elsif ($is_passwordmail) {
> +            # Mail is made unsecure only if the user does not have a public
> +            # key and is not in any security groups. So specifying a public

cool test review

@@ +211,5 @@
> +            # Mail is made unsecure only if the user does not have a public
> +            # key and is not in any security groups. So specifying a public
> +            # key OR being in a security group means the mail is kept secure
> +            # (but, as noted above, the check is the other way around because
> +            # we default to secure).

test patch review