launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:gdpr-add-initial-endpoint into launchpad:master.

Commit message:
Allow retrieval of account existence via webservice

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/405237

Add a webservice endpoint for retrieving user data for GDPR services.
Limited to admins and registry experts.

Currently only confirms the account existence, to be extended later.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:gdpr-add-initial-endpoint into launchpad:master.

diff --git a/lib/lp/registry/browser/tests/test_person_webservice.py b/lib/lp/registry/browser/tests/test_person_webservice.py
index e59c87d..3af358c 100644
--- a/lib/lp/registry/browser/tests/test_person_webservice.py
+++ b/lib/lp/registry/browser/tests/test_person_webservice.py
@@ -12,6 +12,7 @@ from zope.component import getUtility
 from zope.security.management import endInteraction
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.person import (
     IPersonSet,
     TeamMembershipStatus,
@@ -37,6 +38,7 @@ from lp.testing import (
     record_two_runs,
     TestCaseWithFactory,
     )
+from lp.services.webapp.publisher import canonical_url
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import (
@@ -759,3 +761,27 @@ class PersonSetWebServiceTests(TestCaseWithFactory):
 
     def test_deleteSSHKeyFromSSO_is_restricted_dry_run(self):
         self.test_deleteSSHKeyFromSSO_is_restricted(True)
+
+    def test_user_data_retrieval(self):
+        with admin_logged_in():
+            target = self.factory.makePerson(email="test@xxxxxxxxxxx")
+            webservice = webservice_for_person(
+                getUtility(ILaunchpadCelebrities).admin.teamowner,
+                permission=OAuthPermission.WRITE_PRIVATE)
+        response = webservice.named_get(
+            "/people", "getUserData", email="test@xxxxxxxxxxx",
+            api_version="devel").jsonBody()
+        with admin_logged_in():
+            self.assertDictEqual({
+                "status": "account only; no other data",
+                "account": canonical_url(target)},
+                response)
+
+    def test_user_data_retrieval_protected(self):
+        with admin_logged_in():
+            self.factory.makePerson(email="test@xxxxxxxxxxx")
+            webservice = webservice_for_person(self.factory.makePerson())
+        response = webservice.named_get(
+            "/people", "getUserData", email="test@xxxxxxxxxxx",
+            api_version="devel")
+        self.assertEqual(401, response.status)
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index 2e05088..9d07930 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -1144,14 +1144,20 @@
 
         <class
             class="lp.registry.model.person.PersonSet">
+            <require
+                permission="launchpad.Moderate"
+                interface="lp.registry.interfaces.person.IPersonSetModerate" />
             <allow
-                interface="lp.registry.interfaces.person.IPersonSet"/>
+                interface="lp.registry.interfaces.person.IPersonSetPublic"/>
         </class>
         <securedutility
             class="lp.registry.model.person.PersonSet"
             provides="lp.registry.interfaces.person.IPersonSet">
+            <require
+                permission="launchpad.Moderate"
+                interface="lp.registry.interfaces.person.IPersonSetModerate" />
             <allow
-                interface="lp.registry.interfaces.person.IPersonSet"/>
+                interface="lp.registry.interfaces.person.IPersonSetPublic"/>
         </securedutility>
         <securedutility
             class="lp.services.webservice.me.MeLink"
diff --git a/lib/lp/registry/interfaces/person.py b/lib/lp/registry/interfaces/person.py
index 971cadf..19d13de 100644
--- a/lib/lp/registry/interfaces/person.py
+++ b/lib/lp/registry/interfaces/person.py
@@ -2068,8 +2068,18 @@ class ITeam(IPerson, ITeamPublic):
             "Launchpad."))
 
 
-@exported_as_webservice_collection(IPerson)
-class IPersonSet(Interface):
+class IPersonSetModerate(Interface):
+    """Actions for the set of Persons that require launchpad.Moderate"""
+
+    @export_read_operation()
+    @operation_parameters(
+        email=TextLine(required=True, constraint=email_validator))
+    @operation_for_version("devel")
+    def getUserData(email):
+        """Get GDRP related data for a user from their email address."""
+
+
+class IPersonSetPublic(Interface):
     """The set of Persons."""
 
     title = Attribute('Title')
@@ -2571,6 +2581,11 @@ class IPersonSet(Interface):
         """
 
 
+@exported_as_webservice_collection(IPerson)
+class IPersonSet(IPersonSetPublic, IPersonSetModerate):
+    """Combined schema for operations on a group of Persons."""
+
+
 class IRequestPeopleMerge(Interface):
     """This schema is used only because we want a very specific vocabulary."""
 
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 4127c91..6b08e56 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -310,6 +310,7 @@ from lp.services.verification.interfaces.authtoken import LoginTokenType
 from lp.services.verification.interfaces.logintoken import ILoginTokenSet
 from lp.services.verification.model.logintoken import LoginToken
 from lp.services.webapp.interfaces import ILaunchBag
+from lp.services.webapp.publisher import canonical_url
 from lp.services.worlddata.model.language import Language
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -4069,6 +4070,28 @@ class PersonSet:
             pre_iter_hook=preload_for_people,
             result_decorator=prepopulate_person)
 
+    def getUserData(self, email):
+        """See `IPersonSet`."""
+        find_results = list(self.find(email))
+        email_results = list(x[1] for x in self.getByEmails(
+            [email], include_hidden=True, filter_status=False))
+
+        # ideally, this should be a .union, but the order_by and filters
+        # make the result sets incompatible
+        overall_results = list(set(find_results + email_results))
+        # We should only have one result
+        if len(overall_results) > 1:
+            raise ValueError("Multiple records for {}".format(email))
+
+        # If we don't have any results at all, we have no data!
+        if len(overall_results) == 0:
+            return {"status": "no data held"}
+
+        account = overall_results[0]
+        return_data = {"status": "account only; no other data"}
+        return_data["account"] = canonical_url(account)
+        return return_data
+
 
 # Provide a storm alias from Person to Owner. This is useful in queries on
 # objects that have more than just an owner associated with them.
diff --git a/lib/lp/registry/tests/test_personset.py b/lib/lp/registry/tests/test_personset.py
index a0fee14..41a7443 100644
--- a/lib/lp/registry/tests/test_personset.py
+++ b/lib/lp/registry/tests/test_personset.py
@@ -72,6 +72,7 @@ from lp.testing import (
     TestCase,
     TestCaseWithFactory,
     )
+from lp.services.webapp.publisher import canonical_url
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
 
@@ -1132,3 +1133,50 @@ class TestPersonDeleteSSHKeyFromSSO(TestCaseWithFactory):
                 SSHKeyAdditionError,
                 getUtility(IPersonSet).deleteSSHKeyFromSSO,
                 self.sso, u'openid', 'badtype key comment', False)
+
+
+class TestGDPRUserRetrieval(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestGDPRUserRetrieval, self).setUp()
+        self.person_set = getUtility(IPersonSet)
+
+    def test_no_data(self):
+        with admin_logged_in():
+            result = self.person_set.getUserData(u"no@xxxxxxxxxxx")
+        self.assertDictEqual({"status": "no data held"}, result)
+
+    def test_account_data(self):
+        person = self.factory.makePerson(email="test@xxxxxxxxxxx")
+        with admin_logged_in():
+            result = self.person_set.getUserData(u"test@xxxxxxxxxxx")
+        self.assertDictEqual({
+            "status": "account only; no other data",
+            "account": canonical_url(person)},
+            result)
+
+    def test_account_data_hidden(self):
+        person = self.factory.makePerson(email="test@xxxxxxxxxxx")
+        with person_logged_in(person):
+            person.hide_email_addresses = True
+        with admin_logged_in():
+            result = self.person_set.getUserData(u"test@xxxxxxxxxxx")
+        self.assertDictEqual({
+            "status": "account only; no other data",
+            "account": canonical_url(person)},
+            result)
+
+    def test_account_data_invalid_email_status(self):
+        person = self.factory.makePerson(email="test@xxxxxxxxxxx")
+        self.factory.makeEmail(
+            'new@xxxxxxxxxxx',
+            person,
+            email_status=EmailAddressStatus.NEW)
+        with admin_logged_in():
+            result = self.person_set.getUserData(u"new@xxxxxxxxxxx")
+        self.assertDictEqual({
+            "status": "account only; no other data",
+            "account": canonical_url(person)},
+            result)