launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26688
[Merge] ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master with ~pappacena/launchpad:ocirecipe-subscription-ui as a prerequisite.
Commit message:
Filtering private OCI recipes for users that doesn't have permission
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399886
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master.
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 801b005..5eee7e3 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -217,7 +217,8 @@ class OCIProjectRecipesView(LaunchpadView):
@property
def recipes(self):
- recipes = getUtility(IOCIRecipeSet).findByOCIProject(self.context)
+ recipes = getUtility(IOCIRecipeSet).findByOCIProject(
+ self.context, visible_by_user=self.user)
return recipes.order_by('name')
@property
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 4890679..143d553 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -2011,14 +2011,20 @@ class TestOCIProjectRecipesView(BaseTestOCIRecipeView):
pillar=self.distroseries.distribution,
ociprojectname="oci-project-name")
- def makeRecipes(self, count=1):
+ def makeRecipes(self, count=1, **kwargs):
with person_logged_in(self.person):
owner = self.factory.makePerson()
return [self.factory.makeOCIRecipe(
- registrant=owner, owner=owner, oci_project=self.oci_project)
+ registrant=owner, owner=owner, oci_project=self.oci_project,
+ **kwargs)
for _ in range(count)]
def test_shows_no_recipe(self):
+ """Should shows correct message when there are no visible recipes."""
+ # Create a private OCI recipe that should not be shown.
+ self.factory.makeOCIRecipe(
+ oci_project=self.oci_project,
+ information_type=InformationType.PRIVATESECURITY)
browser = self.getViewBrowser(
self.oci_project, "+recipes", user=self.person)
main_text = extract_text(find_main_content(browser.contents))
@@ -2031,10 +2037,17 @@ class TestOCIProjectRecipesView(BaseTestOCIRecipeView):
def test_paginates_recipes(self):
batch_size = 5
self.pushConfig("launchpad", default_batch_size=batch_size)
- recipes = self.makeRecipes(10)
+ # We will create 1 private recipe with proper permission in the
+ # list, and 9 others. This way, we should have 10 recipes in the list.
+ [private_recipe] = self.makeRecipes(
+ 1, information_type=InformationType.PRIVATESECURITY)
+ with admin_logged_in():
+ private_recipe.subscribe(self.person, private_recipe.owner)
+ recipes = self.makeRecipes(9)
+ recipes.append(private_recipe)
+
browser = self.getViewBrowser(
self.oci_project, "+recipes", user=self.person)
-
main_text = extract_text(find_main_content(browser.contents))
no_wrap_main_text = main_text.replace('\n', ' ')
with person_logged_in(self.person):
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 7ab9bcb..3d1fc88 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -539,7 +539,7 @@ class IOCIRecipeSet(Interface):
def findByOwner(owner):
"""Return all OCI Recipes with the given `owner`."""
- def findByOCIProject(oci_project):
+ def findByOCIProject(oci_project, visible_by_user=None):
"""Return all OCI recipes with the given `oci_project`."""
def preloadDataForOCIRecipes(recipes, user):
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 774e9f0..9364383 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
__all__ = [
+ 'get_ocirecipe_privacy_filter',
'OCIRecipe',
'OCIRecipeBuildRequest',
'OCIRecipeSet',
@@ -833,10 +834,12 @@ class OCIRecipeSet:
"""See `IOCIRecipeSet`."""
return IStore(OCIRecipe).find(OCIRecipe, OCIRecipe.owner == owner)
- def findByOCIProject(self, oci_project):
+ def findByOCIProject(self, oci_project, visible_by_user=None):
"""See `IOCIRecipeSet`."""
return IStore(OCIRecipe).find(
- OCIRecipe, OCIRecipe.oci_project == oci_project)
+ OCIRecipe,
+ OCIRecipe.oci_project == oci_project,
+ get_ocirecipe_privacy_filter(visible_by_user))
def findByGitRepository(self, repository, paths=None):
"""See `IOCIRecipeSet`."""
diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
index 7c3f719..2f07dd6 100644
--- a/lib/lp/registry/browser/ociproject.py
+++ b/lib/lp/registry/browser/ociproject.py
@@ -17,9 +17,7 @@ __all__ = [
from six.moves.urllib.parse import urlsplit
from zope.component import getUtility
-from zope.formlib import form
from zope.interface import implementer
-from zope.schema import Choice
from lp.app.browser.launchpadform import (
action,
@@ -199,7 +197,7 @@ class OCIProjectNavigationMenu(NavigationMenu):
def view_recipes(self):
enabled = not getUtility(IOCIRecipeSet).findByOCIProject(
- self.context).is_empty()
+ self.context, visible_by_user=self.user).is_empty()
return Link(
'+recipes', 'View all recipes', icon='info', enabled=enabled)
@@ -219,7 +217,7 @@ class OCIProjectContextMenu(ContextMenu):
def view_recipes(self):
enabled = not getUtility(IOCIRecipeSet).findByOCIProject(
- self.context).is_empty()
+ self.context, visible_by_user=self.user).is_empty()
return Link(
'+recipes', 'View all recipes', icon='info', enabled=enabled)
@@ -238,12 +236,18 @@ class OCIProjectIndexView(LaunchpadView):
return urlsplit(config.codehosting.git_ssh_root).hostname
@cachedproperty
+ def official_recipes(self):
+ return self.context.getOfficialRecipes(visible_by_user=self.user)
+
+ @cachedproperty
def official_recipe_count(self):
- return self.context.getOfficialRecipes().count()
+ return self.context.getOfficialRecipes(
+ visible_by_user=self.user).count()
@cachedproperty
def other_recipe_count(self):
- return self.context.getUnofficialRecipes().count()
+ return self.context.getUnofficialRecipes(
+ visible_by_user=self.user).count()
class OCIProjectEditView(LaunchpadEditFormView):
diff --git a/lib/lp/registry/browser/tests/test_ociproject.py b/lib/lp/registry/browser/tests/test_ociproject.py
index a63aa78..7c18245 100644
--- a/lib/lp/registry/browser/tests/test_ociproject.py
+++ b/lib/lp/registry/browser/tests/test_ociproject.py
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
-# Copyright 2019-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test OCI project views."""
@@ -14,6 +14,7 @@ from datetime import datetime
import pytz
from zope.security.proxy import removeSecurityProxy
+from lp.app.enums import InformationType
from lp.oci.tests.helpers import OCIConfigHelperMixin
from lp.registry.interfaces.ociproject import (
OCI_PROJECT_ALLOW_CREATE,
@@ -111,6 +112,49 @@ class TestOCIProjectView(OCIConfigHelperMixin, BrowserTestCase):
Name: oci-name
""", self.getMainText(oci_project))
+ def test_hides_recipes_link_if_no_recipe_is_present(self):
+ oci_project = self.factory.makeOCIProject(ociprojectname="oci-name")
+ browser = self.getViewBrowser(oci_project)
+ actions = extract_text(
+ find_tag_by_id(browser.contents, 'global-actions'))
+ expected_links = ["Create OCI recipe"]
+ self.assertEqual("\n".join(expected_links), actions)
+
+ def test_shows_recipes_link_if_public_recipe_is_present(self):
+ oci_project = self.factory.makeOCIProject(ociprojectname="oci-name")
+ self.factory.makeOCIRecipe(oci_project=oci_project)
+ browser = self.getViewBrowser(oci_project)
+ actions = extract_text(
+ find_tag_by_id(browser.contents, 'global-actions'))
+ expected_links = ["Create OCI recipe", "View all recipes"]
+ self.assertEqual("\n".join(expected_links), actions)
+
+ def test_hides_recipes_link_if_only_non_visible_recipe_exists(self):
+ oci_project = self.factory.makeOCIProject(ociprojectname="oci-name")
+ self.factory.makeOCIRecipe(
+ oci_project=oci_project,
+ information_type=InformationType.PRIVATESECURITY)
+ another_user = self.factory.makePerson()
+ browser = self.getViewBrowser(oci_project, user=another_user)
+ actions = extract_text(
+ find_tag_by_id(browser.contents, 'global-actions'))
+ expected_links = ["Create OCI recipe"]
+ self.assertEqual("\n".join(expected_links), actions)
+
+ def test_shows_recipes_link_if_user_has_access_to_private_recipe(self):
+ oci_project = self.factory.makeOCIProject(ociprojectname="oci-name")
+ recipe = self.factory.makeOCIRecipe(
+ oci_project=oci_project,
+ information_type=InformationType.PRIVATESECURITY)
+ another_user = self.factory.makePerson()
+ with admin_logged_in():
+ recipe.subscribe(another_user, recipe.owner)
+ browser = self.getViewBrowser(oci_project, user=another_user)
+ actions = extract_text(
+ find_tag_by_id(browser.contents, 'global-actions'))
+ expected_links = ["Create OCI recipe", "View all recipes"]
+ self.assertEqual("\n".join(expected_links), actions)
+
def test_git_repo_hint(self):
owner = self.factory.makePerson(name="a-usr")
pillar = self.factory.makeProduct(name="a-pillar")
@@ -188,6 +232,7 @@ class TestOCIProjectView(OCIConfigHelperMixin, BrowserTestCase):
pillar=distribution, ociprojectname="oci-name")
self.factory.makeOCIRecipe(oci_project=oci_project, official=False)
self.factory.makeOCIRecipe(oci_project=oci_project, official=False)
+
browser = self.getViewBrowser(
oci_project, view_name="+index", user=distribution.owner)
self.assertNotIn("Official recipes", browser.contents)
@@ -198,10 +243,48 @@ class TestOCIProjectView(OCIConfigHelperMixin, BrowserTestCase):
"There are no recipes registered for this OCI project.",
browser.contents)
+ def test_shows_private_recipes_with_proper_grants(self):
+ distribution = self.factory.makeDistribution(displayname="My Distro")
+ oci_project = self.factory.makeOCIProject(
+ pillar=distribution, ociprojectname="oci-name")
+ official_recipe = self.factory.makeOCIRecipe(
+ oci_project=oci_project, official=True,
+ information_type=InformationType.PRIVATESECURITY)
+ unofficial_recipe = self.factory.makeOCIRecipe(
+ oci_project=oci_project, official=False,
+ information_type=InformationType.PRIVATESECURITY)
+
+ granted_user = self.factory.makePerson()
+ with admin_logged_in():
+ unofficial_recipe.subscribe(granted_user, official_recipe.owner)
+ official_recipe.subscribe(granted_user, official_recipe.owner)
+ official_recipe_url = canonical_url(
+ official_recipe, force_local_path=True)
+ browser = self.getViewBrowser(oci_project, user=granted_user)
+
+ self.assertIn(
+ "There is <strong>1</strong> unofficial recipe.", browser.contents)
+ self.assertIn("<h3>Official recipes</h3>", browser.contents)
+
+ recipes_tag = find_tag_by_id(browser.contents, 'mirrors_list')
+ rows = recipes_tag.find_all('tr')
+ self.assertEqual(2, len(rows), 'We should have a header and 1 row')
+ self.assertIn(official_recipe_url, str(rows[1]))
+
def test_shows_no_recipes(self):
distribution = self.factory.makeDistribution(displayname="My Distro")
oci_project = self.factory.makeOCIProject(
pillar=distribution, ociprojectname="oci-name")
+
+ # Make sure we don't include private recipes that the visitor
+ # doesn't have access to.
+ self.factory.makeOCIRecipe(
+ oci_project=oci_project,
+ information_type=InformationType.PRIVATESECURITY)
+ self.factory.makeOCIRecipe(
+ oci_project=oci_project, official=True,
+ information_type=InformationType.PRIVATESECURITY)
+
browser = self.getViewBrowser(
oci_project, view_name="+index", user=distribution.owner)
self.assertNotIn("Official recipes", browser.contents)
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index b1d456b..00a6563 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""OCI Project interfaces."""
@@ -86,19 +86,19 @@ class IOCIProjectView(IHasGitRepositories, Interface):
def getSeriesByName(name):
"""Get an OCIProjectSeries for this OCIProject by series' name."""
- def getRecipeByNameAndOwner(recipe_name, owner_name):
+ def getRecipeByNameAndOwner(recipe_name, owner_name, visible_by_user=None):
"""Returns the exact match search for recipe_name AND owner_name."""
- def getRecipes():
+ def getRecipes(visible_by_user=None):
"""Returns the set of OCI recipes for this project."""
- def searchRecipes(query):
+ def searchRecipes(query, visible_by_user=None):
"""Searches for recipes in this OCI project."""
- def getOfficialRecipes():
+ def getOfficialRecipes(visible_by_user=None):
"""Gets the official recipes for this OCI project."""
- def getUnofficialRecipes():
+ def getUnofficialRecipes(visible_by_user=None):
"""Gets the unofficial recipes for this OCI project."""
def getDefaultGitRepository(person):
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index b034602..079df1e 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -198,40 +198,47 @@ class OCIProject(BugTargetBase, StormBase):
def getSeriesByName(self, name):
return self.series.find(OCIProjectSeries.name == name).one()
- def getRecipes(self):
+ def getRecipes(self, visible_by_user=None):
"""See `IOCIProject`."""
- from lp.oci.model.ocirecipe import OCIRecipe
+ from lp.oci.model.ocirecipe import (
+ OCIRecipe,
+ get_ocirecipe_privacy_filter,
+ )
rs = IStore(OCIRecipe).find(
OCIRecipe,
OCIRecipe.owner_id == Person.id,
- OCIRecipe.oci_project == self)
+ OCIRecipe.oci_project == self,
+ get_ocirecipe_privacy_filter(visible_by_user))
return rs.order_by(Person.name, OCIRecipe.name)
- def getRecipeByNameAndOwner(self, recipe_name, owner_name):
+ def getRecipeByNameAndOwner(self, recipe_name, owner_name,
+ visible_by_user=None):
"""See `IOCIProject`."""
from lp.oci.model.ocirecipe import OCIRecipe
- q = self.getRecipes().find(
+ q = self.getRecipes(visible_by_user=visible_by_user).find(
OCIRecipe.name == recipe_name,
Person.name == owner_name)
return q.one()
- def searchRecipes(self, query):
+ def searchRecipes(self, query, visible_by_user=None):
"""See `IOCIProject`."""
from lp.oci.model.ocirecipe import OCIRecipe
- q = self.getRecipes().find(
+ q = self.getRecipes(visible_by_user=visible_by_user).find(
OCIRecipe.name.contains_string(query) |
Person.name.contains_string(query))
return q.order_by(Person.name, OCIRecipe.name)
- def getOfficialRecipes(self):
+ def getOfficialRecipes(self, visible_by_user=None):
"""See `IOCIProject`."""
from lp.oci.model.ocirecipe import OCIRecipe
- return self.getRecipes().find(OCIRecipe._official == True)
+ return self.getRecipes(
+ visible_by_user=visible_by_user).find(OCIRecipe._official == True)
- def getUnofficialRecipes(self):
+ def getUnofficialRecipes(self, visible_by_user=None):
"""See `IOCIProject`."""
from lp.oci.model.ocirecipe import OCIRecipe
- return self.getRecipes().find(OCIRecipe._official == False)
+ return self.getRecipes(
+ visible_by_user=visible_by_user).find(OCIRecipe._official == False)
def setOfficialRecipeStatus(self, recipe, status):
"""See `IOCIProject`."""
diff --git a/lib/lp/registry/templates/ociproject-index.pt b/lib/lp/registry/templates/ociproject-index.pt
index 1f14e8e..75023b0 100644
--- a/lib/lp/registry/templates/ociproject-index.pt
+++ b/lib/lp/registry/templates/ociproject-index.pt
@@ -77,7 +77,7 @@
<th>Date created</th>
</tr>
- <tr tal:repeat="recipe context/getOfficialRecipes">
+ <tr tal:repeat="recipe view/official_recipes">
<td>
<a tal:content="recipe/name"
tal:attributes="href recipe/fmt:url" />
Follow ups
