launchpad-reviewers team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

Celso Providelo has proposed merging lp:~cprov/launchpad/p3a-api-token into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1290543 in Launchpad itself: "getArchiveSubscriptionURL(archive) should return the generated URL, not the user information"
  https://bugs.launchpad.net/launchpad/+bug/1290543

For more details, see:
https://code.launchpad.net/~cprov/launchpad/p3a-api-token/+merge/210344

Requiring a valid subscription in IPerson.getArchiveSubscriptionURL() before querying/creating authorization tokens.
-- 
https://code.launchpad.net/~cprov/launchpad/p3a-api-token/+merge/210344
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cprov/launchpad/p3a-api-token into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2014-03-07 01:35:46 +0000
+++ lib/lp/registry/interfaces/person.py	2014-03-11 05:42:28 +0000
@@ -1054,7 +1054,9 @@
         """Get a text line that is suitable to be used for a sources.list
         entry.
 
-        It will create a new IArchiveAuthToken if one doesn't already exist.
+        It retuns None if the user does not have a valid subscription
+        for the given archive. Otherwise, it will create a new
+        IArchiveAuthToken if one doesn't already exist.
         """
 
     def getVisiblePPAs(user):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2014-03-07 00:57:38 +0000
+++ lib/lp/registry/model/person.py	2014-03-11 05:42:28 +0000
@@ -3004,6 +3004,14 @@
         if requester.id != agent.id:
             if self.id != requester.id:
                 raise Unauthorized
+        # Verify if the user has a valid subscription on the given
+        # archive and return None if it doesn't.
+        subscription = getUtility(
+            IArchiveSubscriberSet).getBySubscriber(
+                subscriber=self, archive=archive)
+        if len(list(subscription)) == 0:
+            return None
+        # Find the corresponding authorization token or create a new one.
         token = archive.getAuthToken(self)
         if token is None:
             token = archive.newAuthToken(self)

=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt	2014-03-07 01:35:46 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt	2014-03-11 05:42:28 +0000
@@ -29,13 +29,30 @@
     >>> joe = webservice.get('/~joe').jsonBody()
     >>> cprov = webservice.get('/~cprov').jsonBody()
     >>> cp3a = webservice.get(url).jsonBody()
+
+Setup webservice handler for the agent and the test user:
+
     >>> agent_webservice = webservice_for_person(
     ...   celebrity, permission=OAuthPermission.WRITE_PRIVATE)
 
-Subscribe our test user to the commercial archive.
-
     >>> joe_webservice = webservice_for_person(
     ...   person, permission=OAuthPermission.WRITE_PRIVATE)
+
+When the agent tries to get a URL for accessing the commercial
+archive as the test user, nothing is returned since there is no
+valid subscription for it.
+
+    >>> response = agent_webservice.named_post(
+    ...   joe['self_link'], 'getArchiveSubscriptionURL', {},
+    ...   archive=cp3a['self_link'])
+    >>> print response
+    HTTP/1.1 200 Ok
+    ...
+    null
+
+In order to allow access for the test user, the agent has to subscribe
+him first.
+
     >>> response = agent_webservice.named_post(cp3a['self_link'],
     ...   'newSubscription', subscriber=joe['self_link'])
     >>> print response
@@ -44,8 +61,9 @@
     Location: http://api.launchpad.dev/beta/.../+subscriptions/joe
     ...
 
-The agent can query the sources.list entry for an archive for any user, which
-will include an AuthToken, and create it if needed:
+Now the agent can query the sources.list entry for an archive for the
+the test user (or any other user), which will include an AuthToken,
+which is create on demand if necessary:
 
     >>> response = agent_webservice.named_post(
     ...   joe['self_link'], 'getArchiveSubscriptionURL', {},
@@ -55,7 +73,8 @@
     ...
     "http://joe:...@private-ppa.launchpad.dev/.../commercial/ubuntu";
 
-The agent can also query all sources.list entries for any user:
+The agent can also query all sources.list entries for the test user
+(and any other user too):
 
     >>> response = agent_webservice.named_get(
     ...   joe['self_link'], 'getArchiveSubscriptionURLs')