launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/email_notice_extras_959482 into lp:launchpad with lp:~rharding/launchpad/email_notice_959482 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/email_notice_extras_959482/+merge/102341

= Summary =

This is a follow up branch [1] to add security emails to users when important
activities occur under their account. This branch deals with GPG keys and
OAuth tokens.

== Propsed Fix ==

Reuse the Person `security_field_changed` to send notifications when new GPG keys
and OAuth tokens are generated.

== Implementation Notes ==

Investigation found out that new GPG keys automatically get an email sent to
the preferred email address. It doesn't send the email to the address in the
key itself. This means we don't need to add any additional notifications in
this case.

It was decided that emails weren't necessary on deactivation of these. We can
look at adding support going forward if it becomes an issue, but deactivating
GPG keys are destructive since it's easy to reenable.

== Tests ==

lib/lp/services/oauth/tests/test_tokens.py

== Lint ==

Corrected existing lint errors in model/oauth.py in some drive by linting.

[1] https://code.launchpad.net/~rharding/launchpad/email_notice_959482/+merge/99995
-- 
https://code.launchpad.net/~rharding/launchpad/email_notice_extras_959482/+merge/102341
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/email_notice_extras_959482 into lp:launchpad.

=== modified file 'lib/lp/services/oauth/model.py'
--- lib/lp/services/oauth/model.py	2011-12-30 06:14:56 +0000
+++ lib/lp/services/oauth/model.py	2012-04-17 16:10:29 +0000
@@ -74,14 +74,14 @@
 # suggested by Robert, we use a window which is at least twice the size of our
 # hard time out. This is a safe bet since no requests should take more than
 # one hard time out.
-TIMESTAMP_ACCEPTANCE_WINDOW = 60 # seconds
+TIMESTAMP_ACCEPTANCE_WINDOW = 60  # seconds
 # If the timestamp is far in the future because of a client's clock skew,
 # it will effectively invalidate the authentication tokens when the clock is
 # corrected.  To prevent that from becoming too serious a problem, we raise an
 # exception if the timestamp is off by more than this amount from the server's
 # concept of "now".  We also reject timestamps that are too old by the same
 # amount.
-TIMESTAMP_SKEW_WINDOW = 60*60 # seconds, +/-
+TIMESTAMP_SKEW_WINDOW = 60 * 60  # seconds, +/-
 
 
 class OAuthBase:
@@ -252,14 +252,14 @@
         # Determine if the timestamp is too far off from now.
         skew = timedelta(seconds=TIMESTAMP_SKEW_WINDOW)
         now = datetime.now(pytz.UTC)
-        if date < (now-skew) or date > (now+skew):
+        if date < (now - skew) or date > (now + skew):
             raise ClockSkew('Timestamp appears to come from bad system clock')
         # Determine if the nonce was already used for this timestamp.
         store = OAuthNonce.getStore()
         oauth_nonce = store.find(OAuthNonce,
-                                 And(OAuthNonce.access_token==self,
-                                     OAuthNonce.nonce==nonce,
-                                     OAuthNonce.request_timestamp==date)
+                                 And(OAuthNonce.access_token == self,
+                                     OAuthNonce.nonce == nonce,
+                                     OAuthNonce.request_timestamp == date)
                                  ).one()
         if oauth_nonce is not None:
             raise NonceAlreadyUsed('This nonce has been used already.')
@@ -267,8 +267,8 @@
         # request.
         limit = date + timedelta(seconds=TIMESTAMP_ACCEPTANCE_WINDOW)
         match = store.find(OAuthNonce,
-                           And(OAuthNonce.access_token==self,
-                               OAuthNonce.request_timestamp>limit)
+                           And(OAuthNonce.access_token == self,
+                               OAuthNonce.request_timestamp > limit)
                            ).any()
         if match is not None:
             raise TimestampOrderingError(
@@ -377,6 +377,13 @@
             date_expires=self.date_expires, product=self.product,
             project=self.project, distribution=self.distribution,
             sourcepackagename=self.sourcepackagename)
+
+        # We want to notify the user that this oauth token has been generated
+        # for them for security reasons.
+        self.person.security_field_changed(
+            "OAuth token generated in Launchpad",
+            "A new OAuth token consumer was enabled in Launchpad.")
+
         self.destroySelf()
         return access_token
 

=== modified file 'lib/lp/services/oauth/tests/test_tokens.py'
--- lib/lp/services/oauth/tests/test_tokens.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/oauth/tests/test_tokens.py	2012-04-17 16:10:29 +0000
@@ -13,10 +13,12 @@
     )
 
 import pytz
+import transaction
 from zope.component import getUtility
 from zope.proxy import sameProxiedObjects
 from zope.security.interfaces import Unauthorized
 
+from lp.services.mail import stub
 from lp.services.oauth.interfaces import (
     IOAuthAccessToken,
     IOAuthConsumer,
@@ -273,6 +275,13 @@
             self._exchange_request_token_for_access_token())
         verifyObject(IOAuthAccessToken, access_token)
 
+        # Make sure the security notification email went out that the new
+        # token was created.
+        transaction.commit()
+        from_addr, to_addr, msg = stub.test_emails.pop()
+        self.assertIn('OAuth token generated', msg)
+        self.assertIn('@example.com', to_addr[0])
+
     def test_access_token_inherits_data_fields_from_request_token(self):
         request_token, access_token = (
             self._exchange_request_token_for_access_token())
@@ -298,7 +307,6 @@
         request_token.review(
             self.person, OAuthPermission.WRITE_PRIVATE,
             context=context, date_expires=self.in_a_while)
-
         access_token = request_token.createAccessToken()
         self.assertEquals(request_token.context, access_token.context)
         self.assertEquals(