launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:archive-auth-inactive-person into launchpad:master.

Commit message:
Refuse archive auth tokens from inactive accounts

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

It would be possible to do this in `HtaccessTokenGenerator._getInvalidTokens` as well, but we no longer use .htpasswd authentication on production and that script is on its way out as a result, so don't bother.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:archive-auth-inactive-person into launchpad:master.

diff --git a/lib/lp/soyuz/doc/archiveauthtoken.txt b/lib/lp/soyuz/doc/archiveauthtoken.txt
index c02db56..58aa513 100644
--- a/lib/lp/soyuz/doc/archiveauthtoken.txt
+++ b/lib/lp/soyuz/doc/archiveauthtoken.txt
@@ -184,6 +184,26 @@ multiple paths:
     ...     new_token.archive, "bradsmith").token)
     testtoken
 
+Tokens for inactive users are not returned.
+
+    >>> from lp.services.identity.interfaces.account import AccountStatus
+
+    >>> login("admin@xxxxxxxxxxxxx")
+    >>> new_token.person.setAccountStatus(
+    ...     AccountStatus.DEACTIVATED, None, 'Bye')
+    >>> _ = login_person(joe)
+
+    >>> print(token_set.getActiveTokenForArchiveAndPerson(
+    ...     new_token.archive, new_token.person))
+    None
+    >>> print(token_set.getActiveTokenForArchiveAndPersonName(
+    ...     new_token.archive, "bradsmith"))
+    None
+
+    >>> login("admin@xxxxxxxxxxxxx")
+    >>> new_token.person.setAccountStatus(
+    ...     AccountStatus.ACTIVE, None, 'Back')
+
 
 == Amending Tokens ==
 
diff --git a/lib/lp/soyuz/model/archiveauthtoken.py b/lib/lp/soyuz/model/archiveauthtoken.py
index 3d39143..8d4c974 100644
--- a/lib/lp/soyuz/model/archiveauthtoken.py
+++ b/lib/lp/soyuz/model/archiveauthtoken.py
@@ -11,9 +11,13 @@ __all__ = [
 
 from lazr.uri import URI
 import pytz
+from storm.expr import LeftJoin
 from storm.locals import (
+    And,
     DateTime,
     Int,
+    Join,
+    Or,
     Reference,
     Storm,
     Unicode,
@@ -24,6 +28,8 @@ from zope.interface import implementer
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
+from lp.services.identity.interfaces.account import AccountStatus
+from lp.services.identity.model.account import Account
 from lp.soyuz.enums import ArchiveSubscriberStatus
 from lp.soyuz.interfaces.archiveauthtoken import (
     IArchiveAuthToken,
@@ -89,21 +95,40 @@ class ArchiveAuthTokenSet:
 
     def getByArchive(self, archive, with_current_subscription=False):
         """See `IArchiveAuthTokenSet`."""
-        # Circular import.
+        # Circular imports.
+        from lp.registry.model.person import Person
         from lp.soyuz.model.archivesubscriber import ArchiveSubscriber
         store = Store.of(archive)
+        tables = [
+            ArchiveAuthToken,
+            LeftJoin(Person, ArchiveAuthToken.person == Person.id),
+            Join(Account, Person.account == Account.id),
+            ]
         clauses = [
             ArchiveAuthToken.archive == archive,
             ArchiveAuthToken.date_deactivated == None,
+            Or(
+                ArchiveAuthToken.person == None,
+                Account.status == AccountStatus.ACTIVE),
             ]
         if with_current_subscription:
-            clauses.extend([
-                ArchiveAuthToken.archive_id == ArchiveSubscriber.archive_id,
-                ArchiveSubscriber.status == ArchiveSubscriberStatus.CURRENT,
-                ArchiveSubscriber.subscriber_id == TeamParticipation.teamID,
-                TeamParticipation.personID == ArchiveAuthToken.person_id,
+            tables.extend([
+                Join(
+                    ArchiveSubscriber,
+                    ArchiveAuthToken.archive_id ==
+                        ArchiveSubscriber.archive_id),
+                Join(
+                    TeamParticipation,
+                    And(
+                        ArchiveSubscriber.subscriber_id ==
+                            TeamParticipation.teamID,
+                        TeamParticipation.personID ==
+                            ArchiveAuthToken.person_id)),
                 ])
-        return store.find(ArchiveAuthToken, *clauses).config(distinct=True)
+            clauses.append(
+                ArchiveSubscriber.status == ArchiveSubscriberStatus.CURRENT)
+        return store.using(*tables).find(ArchiveAuthToken, *clauses).config(
+            distinct=True)
 
     def getActiveTokenForArchiveAndPerson(self, archive, person):
         """See `IArchiveAuthTokenSet`."""