← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:team-edit-oci-registry-credentials into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:team-edit-oci-registry-credentials into launchpad:master.

Commit message:
Fix editing team-owned OCI registry credentials

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1891078 in Launchpad itself: "OCI recipe push rules only editable by the team owner"
  https://bugs.launchpad.net/launchpad/+bug/1891078

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

The permissions for Person:+edit-oci-registry-credentials are a bit awkward for teams.  It looks as though it should use launchpad.Edit, but for teams that's defined as requiring ownership of the team, while in fact this is effectively a bulk editing interface for OCIRegistryCredentials objects owned by the team in question and so should only require membership of the team.  Work around this by reducing the ZCML permissions to launchpad.View and doing additional permission checking in the view.

In the process of testing this, I noticed that PersonEditOCIRegistryCredentialsView.getEditFieldsRow sets up the owner field with the wrong default (it needs to be given the Person object, not its name), which led to OCIRegistryCredentials objects accidentally changing owner when edited.  Fix this too.

I also added a +oci-registry-credentials link for teams, now that it should be reasonably usable.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:team-edit-oci-registry-credentials into launchpad:master.
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 7c463d5..3dd764c 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -55,6 +55,9 @@ from lp.oci.interfaces.ocirecipe import (
     OCIRecipeFeatureDisabled,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
+from lp.oci.interfaces.ociregistrycredentials import (
+    user_can_edit_credentials_for_owner,
+    )
 from lp.services.features import getFeatureFlag
 from lp.services.helpers import english_list
 from lp.services.propertycache import cachedproperty
@@ -276,6 +279,11 @@ class OCIRecipeEditPushRulesView(LaunchpadEditFormView):
     def has_push_rules(self):
         return len(self.push_rules) > 0
 
+    @cachedproperty
+    def can_edit_credentials(self):
+        return user_can_edit_credentials_for_owner(
+            self.context.owner, self.user)
+
     def _getFieldName(self, name, rule_id):
         """Get the combined field name for an `OCIPushRule` ID.
 
diff --git a/lib/lp/oci/interfaces/ociregistrycredentials.py b/lib/lp/oci/interfaces/ociregistrycredentials.py
index 5c5a71a..08c1bdb 100644
--- a/lib/lp/oci/interfaces/ociregistrycredentials.py
+++ b/lib/lp/oci/interfaces/ociregistrycredentials.py
@@ -10,6 +10,7 @@ __all__ = [
     'IOCIRegistryCredentials',
     'IOCIRegistryCredentialsSet',
     'OCIRegistryCredentialsAlreadyExist',
+    'user_can_edit_credentials_for_owner',
     ]
 
 from lazr.restful.declarations import error_status
@@ -21,7 +22,10 @@ from zope.schema import (
     )
 
 from lp import _
-from lp.registry.interfaces.role import IHasOwner
+from lp.registry.interfaces.role import (
+    IHasOwner,
+    IPersonRoles,
+    )
 from lp.services.fields import (
     PersonChoice,
     URIField,
@@ -102,3 +106,15 @@ class IOCIRegistryCredentialsSet(Interface):
 
     def findByOwner(owner):
         """Find matching `IOCIRegistryCredentials` by owner."""
+
+
+def user_can_edit_credentials_for_owner(owner, user):
+    """Can `user` edit OCI registry credentials belonging to `owner`?"""
+    if user is None:
+        return False
+    # This must follow the same rules as
+    # ViewOCIRegistryCredentials.checkAuthenticated would apply if we were
+    # asking about a hypothetical OCIRegistryCredentials object owned by the
+    # context person or team.
+    roles = IPersonRoles(user)
+    return roles.inTeam(owner) or roles.in_admin
diff --git a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
index a1adb89..ccfe716 100644
--- a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
+++ b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
@@ -10,7 +10,7 @@
 <div metal:fill-slot="main">
     <div metal:use-macro="context/@@launchpad_form/form">
     <metal:formbody fill-slot="widgets">
-      <p condition="context/required:launchpad.Edit">
+      <p condition="view/can_edit_credentials">
         <a class="sprite edit" tal:attributes="href context/owner/fmt:url/+edit-oci-registry-credentials">Edit OCI registry credentials</a>
       </p>
       <table class="form">
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 3707a8e..a718640 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -1230,17 +1230,20 @@
         permission="launchpad.Edit"
         template="../templates/person-oauth-tokens.pt"
         />
+    <!-- Person:+oci-registry-credentials and
+         Person:+edit-oci-registry-credentials do their own additional
+         permission checking.  -->
     <browser:page
         name="+oci-registry-credentials"
         for="lp.registry.interfaces.person.IPerson"
         class="lp.registry.browser.person.PersonOCIRegistryCredentialsView"
-        permission="launchpad.Edit"
+        permission="launchpad.View"
         template="../templates/person-ociregistrycredentials.pt"
         />
     <browser:page
         for="lp.registry.interfaces.person.IPerson"
         class="lp.registry.browser.person.PersonEditOCIRegistryCredentialsView"
-        permission="launchpad.Edit"
+        permission="launchpad.View"
         name="+edit-oci-registry-credentials"
         template="../templates/person-edit-ociregistrycredentials.pt" />
     <browser:page
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index a8ef6fc..bd95db9 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -102,10 +102,7 @@ from zope.schema.vocabulary import (
     SimpleVocabulary,
     )
 from zope.security.interfaces import Unauthorized
-from zope.security.proxy import (
-    isinstance as zope_isinstance,
-    removeSecurityProxy,
-    )
+from zope.security.proxy import removeSecurityProxy
 
 from lp import _
 from lp.app.browser.launchpadform import (
@@ -145,8 +142,8 @@ from lp.oci.interfaces.ocirecipe import IOCIRecipe
 from lp.oci.interfaces.ociregistrycredentials import (
     IOCIRegistryCredentialsSet,
     OCIRegistryCredentialsAlreadyExist,
+    user_can_edit_credentials_for_owner,
     )
-from lp.oci.model.ocirecipe import OCIRecipe
 from lp.registry.browser import BaseRdfView
 from lp.registry.browser.branding import BrandingChangeView
 from lp.registry.browser.menu import (
@@ -194,6 +191,7 @@ from lp.registry.interfaces.product import (
     InvalidProductName,
     IProduct,
     )
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.ssh import (
     ISSHKeySet,
     SSHKeyAdditionError,
@@ -204,10 +202,7 @@ from lp.registry.interfaces.teammembership import (
     )
 from lp.registry.interfaces.wikiname import IWikiNameSet
 from lp.registry.mail.notification import send_direct_contact_email
-from lp.registry.model.person import (
-    get_recipients,
-    Person,
-    )
+from lp.registry.model.person import get_recipients
 from lp.services.config import config
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.sqlbase import flush_database_updates
@@ -764,6 +759,12 @@ class CommonMenuLinks:
         text = 'Structural subscriptions'
         return Link(target, text, icon='info')
 
+    def oci_registry_credentials(self):
+        target = '+oci-registry-credentials'
+        text = 'OCI registry credentials'
+        enabled = user_can_edit_credentials_for_owner(self.context, self.user)
+        return Link(target, text, enabled=enabled, icon='info')
+
 
 class PersonMenuMixin(CommonMenuLinks):
 
@@ -850,12 +851,6 @@ class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin,
         return Link(target, text, enabled=enabled, icon='info')
 
     @enabled_with_permission('launchpad.Edit')
-    def oci_registry_credentials(self):
-        target = '+oci-registry-credentials'
-        text = 'OCI registry credentials'
-        return Link(target, text, icon='info')
-
-    @enabled_with_permission('launchpad.Edit')
     def editlanguages(self):
         target = '+editlanguages'
         text = 'Set preferred languages'
@@ -3663,6 +3658,11 @@ class PersonOCIRegistryCredentialsView(LaunchpadView):
 
     page_title = "OCI registry credentials"
 
+    def initialize(self):
+        if not user_can_edit_credentials_for_owner(self.context, self.user):
+            raise Unauthorized
+        super(PersonOCIRegistryCredentialsView, self).initialize()
+
     @property
     def label(self):
         return "OCI registry credentials for %s" % self.context.display_name
@@ -3688,6 +3688,11 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
 
     schema = Interface
 
+    def initialize(self):
+        if not user_can_edit_credentials_for_owner(self.context, self.user):
+            raise Unauthorized
+        super(PersonEditOCIRegistryCredentialsView, self).initialize()
+
     def _getFieldName(self, name, credentials_id):
         """Get the combined field name for an `OCIRegistryCredentials` ID.
 
@@ -3702,7 +3707,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
             title=u'Owner',
             vocabulary=(
                 'AllUserTeamsParticipationPlusSelfSimpleDisplay'),
-            default=credentials.owner.name,
+            default=credentials.owner,
             __name__=self._getFieldName('owner', id))
 
         username = TextLine(
diff --git a/lib/lp/registry/browser/team.py b/lib/lp/registry/browser/team.py
index 416a8a4..c309d57 100644
--- a/lib/lp/registry/browser/team.py
+++ b/lib/lp/registry/browser/team.py
@@ -1650,6 +1650,7 @@ class TeamOverviewMenu(ApplicationMenu, TeamMenuMixin, HasRecipesMenuMixin,
         'subscriptions',
         'structural_subscriptions',
         'upcomingwork',
+        'oci_registry_credentials',
         ]
 
 
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index 208e6bf..c7167bd 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -13,14 +13,20 @@ import re
 from textwrap import dedent
 
 from fixtures import FakeLogger
+import six
 from six.moves.urllib.parse import urljoin
 import soupmatchers
 from storm.store import Store
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
 from testtools.matchers import (
     DocTestMatches,
     Equals,
     LessThan,
     MatchesDict,
+    MatchesStructure,
     Not,
     )
 from testtools.testcase import ExpectedException
@@ -392,14 +398,10 @@ class TestPersonIndexView(BrowserTestCase):
                 text='OCI registry credentials'))
         self.assertThat(markup, link_match)
 
-        link_match = soupmatchers.HTMLContains(
-            soupmatchers.Tag(
-                'OCIRegistryCredentials missing link', 'a',
-                text='OCI registry credentials'))
-
         login(ANONYMOUS)
         markup = self.get_markup(view, person)
-        self.assertNotIn(link_match, markup)
+        self.assertNotEqual('', markup)
+        self.assertThat(markup, Not(link_match))
 
     def test_ppas_query_count(self):
         owner = self.factory.makePerson()
@@ -1317,16 +1319,23 @@ class TestPersonRelatedProjectsView(TestCaseWithFactory):
         self.assertThat(view(), next_match)
 
 
-class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
-                                           OCIConfigHelperMixin):
+class TestPersonOCIRegistryCredentialsView(
+        WithScenarios, BrowserTestCase, OCIConfigHelperMixin):
 
     layer = DatabaseFunctionalLayer
 
+    scenarios = [
+        ('person', {'use_team': False}),
+        ('team', {'use_team': True}),
+        ]
+
     def setUp(self):
         super(TestPersonOCIRegistryCredentialsView, self).setUp()
         self.setConfig()
-        self.person = self.factory.makePerson(
-            name="test-person", displayname="Test Person")
+        if self.use_team:
+            self.owner = self.factory.makeTeam(members=[self.user])
+        else:
+            self.owner = self.user
         self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         self.distroseries = self.factory.makeDistroSeries(
             distribution=self.ubuntu, name="shiny", displayname="Shiny")
@@ -1338,37 +1347,33 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         oci_project = self.factory.makeOCIProject(
             pillar=self.distroseries.distribution)
         self.recipe = self.factory.makeOCIRecipe(
-            registrant=self.person, owner=self.person,
-            oci_project=oci_project)
+            registrant=self.owner, owner=self.owner, oci_project=oci_project)
 
-    def test_view_oci_registry_credentials_on_person_page(self):
+    def test_view_oci_registry_credentials(self):
         # Verify view helper attributes.
         url = unicode(self.factory.getUniqueURL())
         credentials = {'username': 'foo', 'password': 'bar'}
         getUtility(IOCIRegistryCredentialsSet).new(
-            owner=self.user,
-            url=url,
-            credentials=credentials)
-        view = create_initialized_view(self.user, '+oci-registry-credentials')
+            owner=self.owner, url=url, credentials=credentials)
+        login_person(self.user)
+        view = create_initialized_view(
+            self.owner, '+oci-registry-credentials', principal=self.user)
         self.assertEqual('OCI registry credentials', view.page_title)
-        with person_logged_in(self.user):
-            self.assertEqual(
-                credentials.get('username'),
-                view.oci_registry_credentials[0].getCredentials()['username'])
-            self.assertEqual(url, view.oci_registry_credentials[0].url)
+        self.assertEqual(
+            credentials.get('username'),
+            view.oci_registry_credentials[0].getCredentials()['username'])
+        self.assertEqual(url, view.oci_registry_credentials[0].url)
 
-    def test_edit_oci_registry_creds_on_person_page(self):
+    def test_edit_oci_registry_credentials(self):
         url = unicode(self.factory.getUniqueURL())
         newurl = unicode(self.factory.getUniqueURL())
         third_url = unicode(self.factory.getUniqueURL())
         credentials = {'username': 'foo', 'password': 'bar'}
         registry_credentials = getUtility(IOCIRegistryCredentialsSet).new(
-            owner=self.user,
-            url=url,
-            credentials=credentials)
+            owner=self.owner, url=url, credentials=credentials)
 
         browser = self.getViewBrowser(
-            self.user, view_name='+oci-registry-credentials', user=self.user)
+            self.owner, view_name='+oci-registry-credentials', user=self.user)
         browser.getLink("Edit OCI registry credentials").click()
 
         # Change only the username
@@ -1378,6 +1383,8 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         username_control.value = 'different_username'
         browser.getControl("Save").click()
         with person_logged_in(self.user):
+            self.assertThat(registry_credentials, MatchesStructure.byEquality(
+                owner=self.owner, url=url))
             self.assertThat(
                 registry_credentials.getCredentials(),
                 MatchesDict(
@@ -1386,25 +1393,32 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
 
         # change only the registry url
         browser = self.getViewBrowser(
-            self.user, view_name='+oci-registry-credentials', user=self.user)
+            self.owner, view_name='+oci-registry-credentials', user=self.user)
         browser.getLink("Edit OCI registry credentials").click()
         url_control = browser.getControl(
             name="field.url.%d" % registry_credentials_id)
         url_control.value = newurl
         browser.getControl("Save").click()
         with person_logged_in(self.user):
-            self.assertEqual(newurl, registry_credentials.url)
+            self.assertThat(registry_credentials, MatchesStructure.byEquality(
+                owner=self.owner, url=newurl))
+            self.assertThat(
+                registry_credentials.getCredentials(),
+                MatchesDict(
+                    {"username": Equals("different_username"),
+                     "password": Equals("bar")}))
 
         # change only the password
         browser = self.getViewBrowser(
-            self.user, view_name='+oci-registry-credentials', user=self.user)
+            self.owner, view_name='+oci-registry-credentials', user=self.user)
         browser.getLink("Edit OCI registry credentials").click()
         password_control = browser.getControl(
             name="field.password.%d" % registry_credentials_id)
         password_control.value = 'newpassword'
 
         browser.getControl("Save").click()
-        self.assertIn("Passwords do not match.", browser.contents)
+        self.assertIn(
+            "Passwords do not match.", six.ensure_text(browser.contents))
 
         # change all fields with one edit action
         username_control = browser.getControl(
@@ -1421,28 +1435,27 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         confirm_password_control.value = 'third_newpassword'
         browser.getControl("Save").click()
         with person_logged_in(self.user):
+            self.assertThat(registry_credentials, MatchesStructure.byEquality(
+                owner=self.owner, url=third_url))
             self.assertThat(
                 registry_credentials.getCredentials(),
                 MatchesDict(
                     {"username": Equals("third_different_username"),
                      "password": Equals("third_newpassword")}))
-            self.assertEqual(third_url, registry_credentials.url)
 
-    def test_add_oci_registry_creds_on_person_page(self):
+    def test_add_oci_registry_credentials(self):
         url = unicode(self.factory.getUniqueURL())
         credentials = {'username': 'foo', 'password': 'bar'}
         image_name = self.factory.getUniqueUnicode()
         registry_credentials = getUtility(IOCIRegistryCredentialsSet).new(
-            owner=self.user,
-            url=url,
-            credentials=credentials)
+            owner=self.owner, url=url, credentials=credentials)
         getUtility(IOCIPushRuleSet).new(
             recipe=self.recipe,
             registry_credentials=registry_credentials,
             image_name=image_name)
 
         browser = self.getViewBrowser(
-            self.user, view_name='+oci-registry-credentials', user=self.user)
+            self.owner, view_name='+oci-registry-credentials', user=self.user)
         browser.getLink("Edit OCI registry credentials").click()
 
         browser.getControl(name="field.add_url").value = url
@@ -1453,24 +1466,21 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         browser.getControl("Save").click()
 
         with person_logged_in(self.user):
-            creds = list(getUtility(
-                IOCIRegistryCredentialsSet).findByOwner(
-                self.user))
+            creds = list(
+                getUtility(IOCIRegistryCredentialsSet).findByOwner(self.owner))
             self.assertEqual(url, creds[1].url)
             self.assertThat(
                 removeSecurityProxy(creds[1]).getCredentials(),
                 MatchesDict({"username": Equals("new_username"),
                 "password": Equals("password")}))
 
-    def test_delete_oci_registry_creds_on_person_page(self):
-        # Test that we do not delete creds when there are
+    def test_delete_oci_registry_credentials(self):
+        # Test that we do not delete credentials when there are
         # push rules defined to use them
         url = unicode(self.factory.getUniqueURL())
         credentials = {'username': 'foo', 'password': 'bar'}
         registry_credentials = getUtility(IOCIRegistryCredentialsSet).new(
-            owner=self.person,
-            url=url,
-            credentials=credentials)
+            owner=self.owner, url=url, credentials=credentials)
         IStore(registry_credentials).flush()
         registry_credentials_id = removeSecurityProxy(registry_credentials).id
         image_name = self.factory.getUniqueUnicode()
@@ -1480,8 +1490,7 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
             image_name=image_name)
 
         browser = self.getViewBrowser(
-            self.person, view_name='+oci-registry-credentials',
-            user=self.person)
+            self.owner, view_name='+oci-registry-credentials', user=self.user)
         browser.getLink("Edit OCI registry credentials").click()
         # assert full rule is displayed
         self.assertEqual(url, browser.getControl(
@@ -1496,11 +1505,11 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         browser.getControl("Save").click()
         self.assertIn("These credentials cannot be deleted as there are "
                       "push rules defined that still use them.",
-                      browser.contents)
+                      six.ensure_text(browser.contents))
 
         # make sure we don't have any push rules defined to use
         # the credentials we want to remove
-        with person_logged_in(self.person):
+        with person_logged_in(self.user):
             removeSecurityProxy(push_rule).destroySelf()
 
         delete_control = browser.getControl(
@@ -1508,9 +1517,9 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
         delete_control.getControl('Delete').selected = True
         browser.getControl("Save").click()
         credentials_set = getUtility(IOCIRegistryCredentialsSet)
-        with person_logged_in(self.person):
+        with person_logged_in(self.user):
             self.assertEqual(
-                0, credentials_set.findByOwner(self.person).count())
+                0, credentials_set.findByOwner(self.owner).count())
 
 
 class TestPersonLiveFSView(BrowserTestCase):
@@ -2077,3 +2086,6 @@ class TestPersonRdfView(BrowserTestCase):
             content_disposition, browser.headers['Content-disposition'])
         self.assertEqual(
             'application/rdf+xml', browser.headers['Content-type'])
+
+
+load_tests = load_tests_apply_scenarios
diff --git a/lib/lp/registry/browser/tests/test_team.py b/lib/lp/registry/browser/tests/test_team.py
index 2056ecc..672c59d 100644
--- a/lib/lp/registry/browser/tests/test_team.py
+++ b/lib/lp/registry/browser/tests/test_team.py
@@ -8,6 +8,7 @@ import contextlib
 from lazr.restful.interfaces import IJSONRequestCache
 import simplejson
 import soupmatchers
+from testtools.matchers import Not
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -38,8 +39,11 @@ from lp.services.webapp.escaping import html_escape
 from lp.services.webapp.publisher import canonical_url
 from lp.soyuz.enums import ArchiveStatus
 from lp.testing import (
+    ANONYMOUS,
+    login,
     login_celebrity,
     login_person,
+    monkey_patch,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -874,6 +878,40 @@ class TestTeamIndexView(TestCaseWithFactory):
                     extract_text(document.find(True, id='maincontent')),
                     'The information in this page is not shared with you.')
 
+    @staticmethod
+    def get_markup(view, team):
+        def fake_method():
+            return canonical_url(team)
+        with monkey_patch(view, _getURL=fake_method):
+            markup = view.render()
+        return markup
+
+    def test_show_oci_registry_credentials_link(self):
+        member = self.factory.makePerson()
+        team = self.factory.makeTeam(members=[member])
+        view = create_initialized_view(team, '+index', principal=member)
+        with person_logged_in(member):
+            markup = self.get_markup(view, team)
+        link_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'OCIRegistryCredentials link', 'a',
+                attrs={
+                    'href':
+                        'http://launchpad.test/~%s/+oci-registry-credentials'
+                        % team.name},
+                text='OCI registry credentials'))
+        self.assertThat(markup, link_match)
+
+        login_person(self.factory.makePerson())
+        markup = self.get_markup(view, team)
+        self.assertNotEqual('', markup)
+        self.assertThat(markup, Not(link_match))
+
+        login(ANONYMOUS)
+        markup = self.get_markup(view, team)
+        self.assertNotEqual('', markup)
+        self.assertThat(markup, Not(link_match))
+
 
 class TestPersonIndexVisibilityView(TestCaseWithFactory):
 
diff --git a/lib/lp/registry/templates/person-ociregistrycredentials.pt b/lib/lp/registry/templates/person-ociregistrycredentials.pt
index bec71e1..d245757 100644
--- a/lib/lp/registry/templates/person-ociregistrycredentials.pt
+++ b/lib/lp/registry/templates/person-ociregistrycredentials.pt
@@ -12,7 +12,7 @@
         <span tal:replace="context/title"/>
         has not set any credentials yet.
     </p>
-    <p condition="context/required:launchpad.Edit">
+    <p>
         <a class="sprite edit" tal:attributes="href context/fmt:url/+edit-oci-registry-credentials">Edit OCI registry credentials</a>
     </p>
 
diff --git a/lib/lp/registry/templates/team-index.pt b/lib/lp/registry/templates/team-index.pt
index 4aa2215..25db8f5 100644
--- a/lib/lp/registry/templates/team-index.pt
+++ b/lib/lp/registry/templates/team-index.pt
@@ -72,6 +72,10 @@
         Upcoming work assigned to members of this team
       </a>
     </li>
+    <li
+      tal:define="link context/menu:overview/oci_registry_credentials"
+      tal:condition="link/enabled"
+      tal:content="structure link/fmt:link" />
   </ul>
 
   <div class="yui-g">
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 696c55d..c36a99b 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -3563,6 +3563,8 @@ class ViewOCIRegistryCredentials(AuthorizationBase):
     usedfor = IOCIRegistryCredentials
 
     def checkAuthenticated(self, user):
+        # This must be kept in sync with user_can_edit_credentials_for_owner
+        # in lp.oci.interfaces.ociregistrycredentials.
         return (
             user.isOwner(self.obj) or
             user.in_admin)