launchpad-reviewers team mailing list archive

[Colin Watson <mp+450356@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:export-person-id into launchpad:master.

Commit message:
Export Person.id for privileged users

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

IS automation needs access to an immutable identifier for users (see https://portal.admin.canonical.com/C158967).  Export this on the webservice API, readable only by those with `launchpad.Moderate` permission.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:export-person-id 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 d51c0d2..e0dfcb9 100644
--- a/lib/lp/registry/browser/tests/test_person_webservice.py
+++ b/lib/lp/registry/browser/tests/test_person_webservice.py
@@ -146,6 +146,40 @@ class TestPersonAccountStatus(TestCaseWithFactory):
             )
 
 
+class TestPersonExportedID(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_normal_user_cannot_see_id(self):
+        # A normal user cannot read the `id` field, not even their own.
+        person = self.factory.makePerson()
+        person_url = api_url(person)
+
+        body = (
+            webservice_for_person(
+                person, permission=OAuthPermission.WRITE_PRIVATE
+            )
+            .get(person_url, api_version="devel")
+            .jsonBody()
+        )
+        self.assertEqual("tag:launchpad.net:2008:redacted", body["id"])
+
+    def test_registry_can_see_id(self):
+        # A member of ~registry can read the `id` field.
+        person = self.factory.makePerson()
+        person_id = person.id
+        person_url = api_url(person)
+
+        body = (
+            webservice_for_person(
+                self.factory.makeRegistryExpert(),
+                permission=OAuthPermission.WRITE_PRIVATE,
+            )
+            .get(person_url, api_version="devel")
+            .jsonBody()
+        )
+        self.assertEqual(person_id, body["id"])
+
+
 class TestPersonRepresentation(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
diff --git a/lib/lp/registry/interfaces/person.py b/lib/lp/registry/interfaces/person.py
index 3d6db5d..ec14854 100644
--- a/lib/lp/registry/interfaces/person.py
+++ b/lib/lp/registry/interfaces/person.py
@@ -2197,6 +2197,23 @@ class IPersonModerate(IPersonSettingsModerate):
 class IPersonModerateRestricted(Interface):
     """IPerson attributes that require launchpad.Moderate permission."""
 
+    # Most API clients have no need for the ID, but some systems need it as
+    # a stable identifier for users even across username changes (see
+    # https://portal.admin.canonical.com/C158967).  Access to this is
+    # restricted to limit the scope of privacy issues.
+    exported_id = exported(
+        Int(
+            title=_("ID"),
+            description=_(
+                "Internal immutable identifier for this person.  Only visible "
+                "by privileged users."
+            ),
+            required=True,
+            readonly=True,
+        ),
+        exported_as="id",
+    )
+
     @call_with(user=REQUEST_USER)
     @operation_parameters(
         status=copy_field(IAccount["status"]),
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 5fb3c74..8aef249 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -457,6 +457,11 @@ class Person(
                 .one()
             )
 
+    @property
+    def exported_id(self):
+        """See `IPerson`."""
+        return self.id
+
     sortingColumns = SQL("person_sort_key(Person.displayname, Person.name)")
     # Redefine the default ordering into Storm syntax.
     _storm_sortingColumns = ("Person.displayname", "Person.name")