Merge lp:~rharding/launchpad/email_notice_extras_959482 into lp:launchpad

Proposed by Richard Harding on 2012-04-17
Status: Merged
Approved by: Aaron Bentley on 2012-04-17
Approved revision: no longer in the source branch.
Merged at revision: 15111
Proposed branch: lp:~rharding/launchpad/email_notice_extras_959482
Merge into: lp:launchpad
Prerequisite: lp:~rharding/launchpad/email_notice_959482
Diff against target: 103 lines (+24/-9)
2 files modified
lib/lp/services/oauth/model.py (+15/-8)
lib/lp/services/oauth/tests/test_tokens.py (+9/-1)
To merge this branch: bzr merge lp:~rharding/launchpad/email_notice_extras_959482
Reviewer Review Type Date Requested Status
Aaron Bentley (community) 2012-04-17 Approve on 2012-04-17
Review via email: mp+102341@code.launchpad.net

Commit Message

Send a security notice email to user when an oauth token is created for them.

Description of the Change

= 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

To post a comment you must log in.
Aaron Bentley (abentley) wrote :

Looks good. Thanks for the cleanup.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/oauth/model.py'
2--- lib/lp/services/oauth/model.py 2011-12-30 06:14:56 +0000
3+++ lib/lp/services/oauth/model.py 2012-04-17 16:10:29 +0000
4@@ -74,14 +74,14 @@
5 # suggested by Robert, we use a window which is at least twice the size of our
6 # hard time out. This is a safe bet since no requests should take more than
7 # one hard time out.
8-TIMESTAMP_ACCEPTANCE_WINDOW = 60 # seconds
9+TIMESTAMP_ACCEPTANCE_WINDOW = 60 # seconds
10 # If the timestamp is far in the future because of a client's clock skew,
11 # it will effectively invalidate the authentication tokens when the clock is
12 # corrected. To prevent that from becoming too serious a problem, we raise an
13 # exception if the timestamp is off by more than this amount from the server's
14 # concept of "now". We also reject timestamps that are too old by the same
15 # amount.
16-TIMESTAMP_SKEW_WINDOW = 60*60 # seconds, +/-
17+TIMESTAMP_SKEW_WINDOW = 60 * 60 # seconds, +/-
18
19
20 class OAuthBase:
21@@ -252,14 +252,14 @@
22 # Determine if the timestamp is too far off from now.
23 skew = timedelta(seconds=TIMESTAMP_SKEW_WINDOW)
24 now = datetime.now(pytz.UTC)
25- if date < (now-skew) or date > (now+skew):
26+ if date < (now - skew) or date > (now + skew):
27 raise ClockSkew('Timestamp appears to come from bad system clock')
28 # Determine if the nonce was already used for this timestamp.
29 store = OAuthNonce.getStore()
30 oauth_nonce = store.find(OAuthNonce,
31- And(OAuthNonce.access_token==self,
32- OAuthNonce.nonce==nonce,
33- OAuthNonce.request_timestamp==date)
34+ And(OAuthNonce.access_token == self,
35+ OAuthNonce.nonce == nonce,
36+ OAuthNonce.request_timestamp == date)
37 ).one()
38 if oauth_nonce is not None:
39 raise NonceAlreadyUsed('This nonce has been used already.')
40@@ -267,8 +267,8 @@
41 # request.
42 limit = date + timedelta(seconds=TIMESTAMP_ACCEPTANCE_WINDOW)
43 match = store.find(OAuthNonce,
44- And(OAuthNonce.access_token==self,
45- OAuthNonce.request_timestamp>limit)
46+ And(OAuthNonce.access_token == self,
47+ OAuthNonce.request_timestamp > limit)
48 ).any()
49 if match is not None:
50 raise TimestampOrderingError(
51@@ -377,6 +377,13 @@
52 date_expires=self.date_expires, product=self.product,
53 project=self.project, distribution=self.distribution,
54 sourcepackagename=self.sourcepackagename)
55+
56+ # We want to notify the user that this oauth token has been generated
57+ # for them for security reasons.
58+ self.person.security_field_changed(
59+ "OAuth token generated in Launchpad",
60+ "A new OAuth token consumer was enabled in Launchpad.")
61+
62 self.destroySelf()
63 return access_token
64
65
66=== modified file 'lib/lp/services/oauth/tests/test_tokens.py'
67--- lib/lp/services/oauth/tests/test_tokens.py 2012-01-01 02:58:52 +0000
68+++ lib/lp/services/oauth/tests/test_tokens.py 2012-04-17 16:10:29 +0000
69@@ -13,10 +13,12 @@
70 )
71
72 import pytz
73+import transaction
74 from zope.component import getUtility
75 from zope.proxy import sameProxiedObjects
76 from zope.security.interfaces import Unauthorized
77
78+from lp.services.mail import stub
79 from lp.services.oauth.interfaces import (
80 IOAuthAccessToken,
81 IOAuthConsumer,
82@@ -273,6 +275,13 @@
83 self._exchange_request_token_for_access_token())
84 verifyObject(IOAuthAccessToken, access_token)
85
86+ # Make sure the security notification email went out that the new
87+ # token was created.
88+ transaction.commit()
89+ from_addr, to_addr, msg = stub.test_emails.pop()
90+ self.assertIn('OAuth token generated', msg)
91+ self.assertIn('@example.com', to_addr[0])
92+
93 def test_access_token_inherits_data_fields_from_request_token(self):
94 request_token, access_token = (
95 self._exchange_request_token_for_access_token())
96@@ -298,7 +307,6 @@
97 request_token.review(
98 self.person, OAuthPermission.WRITE_PRIVATE,
99 context=context, date_expires=self.in_a_while)
100-
101 access_token = request_token.createAccessToken()
102 self.assertEquals(request_token.context, access_token.context)
103 self.assertEquals(