launchpad-reviewers team mailing list archive

[Quentin Debhi <mp+481029@xxxxxxxxxxxxxxxxxx>]

Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:sourcecraft-private-builds into launchpad:master.

Commit message:
Allow sourcecraft builds with private information

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/481029
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:sourcecraft-private-builds into launchpad:master.

diff --git a/lib/lp/crafts/model/craftrecipe.py b/lib/lp/crafts/model/craftrecipe.py
index 9f2071f..3500de0 100644
--- a/lib/lp/crafts/model/craftrecipe.py
+++ b/lib/lp/crafts/model/craftrecipe.py
@@ -34,7 +34,7 @@ from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import (
-    FREE_INFORMATION_TYPES,
+    PRIVATE_INFORMATION_TYPES,
     PUBLIC_INFORMATION_TYPES,
     InformationType,
 )
@@ -95,6 +95,7 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.product import Product
 from lp.registry.model.series import ACTIVE_STATUSES
+from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.bulk import load_related
 from lp.services.database.constants import DEFAULT, UTC_NOW
 from lp.services.database.decoratedresultset import DecoratedResultSet
@@ -324,9 +325,15 @@ class CraftRecipe(StormBase):
 
     def getAllowedInformationTypes(self, user):
         """See `ICraftRecipe`."""
-        # XXX ruinedyourlife 2024-09-24: Only allow free information types
-        # until we have more privacy infrastructure in place.
-        return FREE_INFORMATION_TYPES
+        # Allow both public and private information types
+        if user is None:
+            return PUBLIC_INFORMATION_TYPES
+
+        # If the user is the owner or in the owning team, allow private types
+        if user.inTeam(self.owner) or user == self.owner:
+            return PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES
+
+        return PUBLIC_INFORMATION_TYPES
 
     def visibleByUser(self, user):
         """See `ICraftRecipe`."""
@@ -1088,10 +1095,32 @@ class CraftRecipeBuildRequest:
 
 def get_craft_recipe_privacy_filter(user):
     """Return a Storm query filter to find craft recipes visible to `user`."""
+    from storm.expr import And, Exists, Or, Select
+
     public_filter = CraftRecipe.information_type.is_in(
         PUBLIC_INFORMATION_TYPES
     )
 
-    # XXX ruinedyourlife 2024-10-02: Flesh this out once we have more privacy
-    # infrastructure.
-    return [public_filter]
+    if user is None:
+        return [public_filter]
+
+    # Users can see private recipes they own or are part of the owning team
+    private_filter = And(
+        CraftRecipe.information_type.is_in(PRIVATE_INFORMATION_TYPES),
+        Or(
+            CraftRecipe.owner == user,
+            CraftRecipe.registrant == user,
+            # If the user is in the owning team
+            Exists(
+                Select(
+                    (TeamParticipation.team_id,),
+                    And(
+                        TeamParticipation.person == user.id,
+                        TeamParticipation.team == CraftRecipe.owner_id,
+                    ),
+                )
+            ),
+        ),
+    )
+
+    return [Or(public_filter, private_filter)]
diff --git a/lib/lp/crafts/tests/test_craftrecipe.py b/lib/lp/crafts/tests/test_craftrecipe.py
index 4e35b89..6a62ddd 100644
--- a/lib/lp/crafts/tests/test_craftrecipe.py
+++ b/lib/lp/crafts/tests/test_craftrecipe.py
@@ -26,7 +26,11 @@ from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
-from lp.app.enums import InformationType
+from lp.app.enums import (
+    PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
+    InformationType,
+)
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.builderproxy import FetchServicePolicy
 from lp.buildmaster.enums import (
@@ -574,6 +578,94 @@ class TestCraftRecipe(TestCaseWithFactory):
             FetchServicePolicy.STRICT, build.recipe.fetch_service_policy
         )
 
+    def test_getAllowedInformationTypes(self):
+        """Test that getAllowedInformationTypes returns correct types based
+        on user."""
+        [ref] = self.factory.makeGitRefs()
+        owner = self.factory.makePerson()
+        member = self.factory.makePerson()
+        team = self.factory.makeTeam(owner=owner, members=[member])
+        non_member = self.factory.makePerson()
+        recipe = self.factory.makeCraftRecipe(
+            registrant=owner,
+            owner=team,
+            git_ref=ref,
+            information_type=InformationType.PUBLIC,
+        )
+
+        # Anonymous users can only see public types
+        self.assertEqual(
+            PUBLIC_INFORMATION_TYPES, recipe.getAllowedInformationTypes(None)
+        )
+
+        # Team owner can see both public and private types
+        self.assertEqual(
+            list(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES),
+            list(recipe.getAllowedInformationTypes(owner)),
+        )
+
+        # Team member can see both public and private types
+        self.assertEqual(
+            list(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES),
+            list(recipe.getAllowedInformationTypes(member)),
+        )
+
+        # Non-members can only see public types
+        self.assertEqual(
+            PUBLIC_INFORMATION_TYPES,
+            recipe.getAllowedInformationTypes(non_member),
+        )
+
+    def test_visibleByUser(self):
+        """Test that visibleByUser correctly determines visibility."""
+        # Enable both feature flags
+        self.useFixture(
+            FeatureFixture(
+                {
+                    CRAFT_RECIPE_ALLOW_CREATE: "on",
+                    CRAFT_RECIPE_PRIVATE_FEATURE_FLAG: "on",
+                }
+            )
+        )
+
+        [ref] = self.factory.makeGitRefs()
+        owner = self.factory.makePerson()
+        member = self.factory.makePerson()
+        team = self.factory.makeTeam(owner=owner, members=[member])
+        non_member = self.factory.makePerson()
+
+        # Test public recipe
+        public_recipe = removeSecurityProxy(
+            self.factory.makeCraftRecipe(
+                registrant=owner,
+                owner=team,
+                git_ref=ref,
+                information_type=InformationType.PUBLIC,
+            )
+        )
+
+        # Public recipes are visible to everyone
+        self.assertTrue(public_recipe.visibleByUser(None))
+        self.assertTrue(public_recipe.visibleByUser(owner))
+        self.assertTrue(public_recipe.visibleByUser(member))
+        self.assertTrue(public_recipe.visibleByUser(non_member))
+
+        # Test private recipe
+        private_recipe = removeSecurityProxy(
+            self.factory.makeCraftRecipe(
+                registrant=owner,
+                owner=team,
+                git_ref=ref,
+                information_type=InformationType.PROPRIETARY,
+            )
+        )
+
+        # Private recipes are only visible to team members
+        self.assertFalse(private_recipe.visibleByUser(None))
+        self.assertTrue(private_recipe.visibleByUser(owner))
+        self.assertTrue(private_recipe.visibleByUser(member))
+        self.assertFalse(private_recipe.visibleByUser(non_member))
+
 
 class TestCraftRecipeSet(TestCaseWithFactory):