Merge ~ilasc/launchpad:person-oci-registry-credentials-edit into launchpad:master

[Ioana Lasc]

Add edit screen for OCIRegistryCredentials on person page

[Thiago F. Pappacena (pappacena)]

Really good job!

I've just added a few comments, mostly about coding style that could improve a bit code readability.

[Ioana Lasc (ilasc)]

Thanks Thiago, I added replies to each inline comment and will commit all changes in a few minutes - I agree with all comments and addressed accordingly in the code.

[Thiago F. Pappacena (pappacena)]

Added a reply, but other than that it LGTM.

Maybe twom would like to take a quick look here too.

review: Approve

[Ioana Lasc (ilasc)]

Thiago that last suggestion was good - agreed and added.

[Tom Wardill (twom)]

Some comments inline, mostly indentation and such.

The use of removeSecurityProxy in both the tests and the especially the view context seems a bit strange to me, is it required?
I can't see why you don't have the right privileges to update the objects without it. That said, my understanding of how all that works is a bit hazy!

review: Needs Information

[Ioana Lasc (ilasc)]

Thanks Tom! Indeed there were a couple of places on the view where access to the person object in the context didn't require removeSecurityProxy (setting owner=self.context) but it is necessary everywhere else.

[Tom Wardill (twom)]

Thanks!

review: Approve

diff --git a/lib/lp/oci/interfaces/ocipushrule.py b/lib/lp/oci/interfaces/ocipushrule.py
index b889cdd..45aeb3b 100644
--- a/lib/lp/oci/interfaces/ocipushrule.py
+++ b/lib/lp/oci/interfaces/ocipushrule.py
@@ -127,5 +127,8 @@ class IOCIPushRuleSet(Interface):
     def new(recipe, registry_credentials, image_name):
         """Create an `IOCIPushRule`."""
 
+    def findByRegistryCredentials(self, credentials):
+        """Find matching `IOCIPushRule` by credentials."""
+
     def getByID(id):
         """Get a single `IOCIPushRule` by its ID."""
diff --git a/lib/lp/oci/model/ocipushrule.py b/lib/lp/oci/model/ocipushrule.py
index dbdb22e..0354aad 100644
--- a/lib/lp/oci/model/ocipushrule.py
+++ b/lib/lp/oci/model/ocipushrule.py
@@ -69,7 +69,7 @@ class OCIPushRule(Storm):
 
     def destroySelf(self):
         """See `IOCIPushRule`."""
-        IStore(OCIPushRule).get(self.id).remove()
+        IStore(OCIPushRule).remove(self)
 
 
 @implementer(IOCIPushRuleSet)
@@ -90,3 +90,9 @@ class OCIPushRuleSet:
     def getByID(self, id):
         """See `IOCIPushRuleSet`."""
         return IStore(OCIPushRule).get(OCIPushRule, id)
+
+    def findByRegistryCredentials(self, credentials):
+        store = IStore(OCIPushRule)
+        return store.find(
+            OCIPushRule,
+            OCIPushRule.registry_credentials == credentials)
diff --git a/lib/lp/oci/model/ociregistrycredentials.py b/lib/lp/oci/model/ociregistrycredentials.py
index 49c740f..d86b140 100644
--- a/lib/lp/oci/model/ociregistrycredentials.py
+++ b/lib/lp/oci/model/ociregistrycredentials.py
@@ -122,11 +122,13 @@ class OCIRegistryCredentials(Storm):
     def username(self):
         return self._credentials.get('username')
 
+    @username.setter
+    def username(self, value):
+        self._credentials['username'] = value
+
     def destroySelf(self):
         """See `IOCIRegistryCredentials`."""
-        store = IStore(OCIRegistryCredentials)
-        store.find(
-            OCIRegistryCredentials, OCIRegistryCredentials.id == self).remove()
+        IStore(OCIRegistryCredentials).remove(self)
 
 
 @implementer(IOCIRegistryCredentialsSet)
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index b8f00d5..3707a8e 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -1238,6 +1238,12 @@
         template="../templates/person-ociregistrycredentials.pt"
         />
     <browser:page
+        for="lp.registry.interfaces.person.IPerson"
+        class="lp.registry.browser.person.PersonEditOCIRegistryCredentialsView"
+        permission="launchpad.Edit"
+        name="+edit-oci-registry-credentials"
+        template="../templates/person-edit-ociregistrycredentials.pt" />
+    <browser:page
         name="+livefs"
         for="lp.registry.interfaces.person.IPerson"
         class="lp.registry.browser.person.PersonLiveFSView"
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index 535cf4d..e46cce4 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -7,6 +7,7 @@ __metaclass__ = type
 __all__ = [
     'BeginTeamClaimView',
     'CommonMenuLinks',
+    'PersonEditOCIRegistryCredentialsView',
     'EmailToPersonView',
     'PeopleSearchView',
     'PersonAccountAdministerView',
@@ -90,7 +91,9 @@ from zope.interface import (
 from zope.interface.exceptions import Invalid
 from zope.publisher.interfaces import NotFound
 from zope.schema import (
+    Bool,
     Choice,
+    Password,
     Text,
     TextLine,
     )
@@ -134,8 +137,10 @@ from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
 from lp.code.errors import InvalidNamespace
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
 from lp.code.interfaces.gitlookup import IGitTraverser
+from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet
 from lp.oci.interfaces.ociregistrycredentials import (
     IOCIRegistryCredentialsSet,
+    OCIRegistryCredentialsAlreadyExist,
     )
 from lp.registry.browser import BaseRdfView
 from lp.registry.browser.branding import BrandingChangeView
@@ -3659,6 +3664,299 @@ class PersonOCIRegistryCredentialsView(LaunchpadView):
         return len(self.oci_registry_credentials) > 0
 
 
+class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
+    """View for Person:+edit-oci-registry-credentials."""
+
+    @cachedproperty
+    def oci_registry_credentials(self):
+        return list(getUtility(
+            IOCIRegistryCredentialsSet).findByOwner(self.context))
+
+    schema = Interface
+
+    def _getFieldName(self, name, credentials_id):
+        """Get the combined field name for an `OCIRegistryCredentials` ID.
+
+        In order to be able to render a table, we encode the credentials ID
+        in the form field name.
+        """
+        return "%s.%d" % (name, credentials_id)
+
+    def getEditFieldsRow(self, credentials=None):
+        id = getattr(credentials, 'id', None)
+        owner = Choice(
+            title=u'Owner',
+            vocabulary=(
+                'AllUserTeamsParticipationPlusSelfSimpleDisplay'),
+            default=credentials.owner.name,
+            __name__=self._getFieldName('owner', id))
+
+        username = TextLine(
+            title=u'Username',
+            __name__=self._getFieldName('username', id),
+            default=credentials.username,
+            required=False, readonly=False)
+
+        password = Password(
+            title=u'Password',
+            __name__=self._getFieldName('password', id),
+            default=None,
+            required=False, readonly=False)
+
+        confirm_password = Password(
+            title=u'Confirm password',
+            __name__=self._getFieldName('confirm_password', id),
+            default=None,
+            required=False, readonly=False)
+
+        url = TextLine(
+            title=u'Registry URL',
+            __name__=self._getFieldName('url', id),
+            default=credentials.url,
+            required=True, readonly=False)
+
+        delete = Bool(
+            title=u'Delete',
+            __name__=self._getFieldName('delete', id),
+            default=False,
+            required=True, readonly=False)
+
+        return owner, username, password, confirm_password, url, delete
+
+    def getAddFieldsRow(self):
+        add_url = TextLine(
+            title=u'Registry URL',
+            __name__=u'add_url',
+            required=False, readonly=False)
+        add_username = TextLine(
+            title=u'Username',
+            __name__=u'add_username',
+            required=False, readonly=False)
+        add_password = Password(
+            title=u'Password',
+            __name__=u'add_password',
+            required=False, readonly=False)
+        add_confirm_password = Password(
+            title=u'Confirm password',
+            __name__=u'add_confirm_password',
+            required=False, readonly=False)
+
+        return add_url, add_username, add_password, add_confirm_password
+
+    def _parseFieldName(self, field_name):
+        """Parse a combined field name as described in `_getFieldName`.
+
+        :raises UnexpectedFormData: if the field name cannot be parsed or
+            the `OCIRegistryCredentials` cannot be found.
+        """
+        field_bits = field_name.split(".")
+        if len(field_bits) != 2:
+            raise UnexpectedFormData(
+                "Cannot parse field name: %s" % field_name)
+        field_type = field_bits[0]
+        try:
+            credentials_id = int(field_bits[1])
+        except ValueError:
+            raise UnexpectedFormData(
+                "Cannot parse field name: %s" % field_name)
+        return field_type, credentials_id
+
+    def setUpFields(self):
+        """See `LaunchpadFormView`."""
+        LaunchpadFormView.setUpFields(self)
+
+        for elem in self.oci_registry_credentials:
+            fields = self.getEditFieldsRow(elem)
+            self.form_fields += FormFields(*fields)
+
+        add_fields = self.getAddFieldsRow()
+        self.form_fields += FormFields(*add_fields)
+
+    @property
+    def label(self):
+        return 'Edit OCI registry credentials'
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)
+
+    def getCredentialsWidgets(self, credentials):
+        widgets_by_name = {widget.name: widget for widget in self.widgets}
+        owner_field_name = (
+                "field." + self._getFieldName("owner", credentials.id))
+        username_field_name = (
+                "field." + self._getFieldName("username", credentials.id))
+        password_field_name = (
+                "field." + self._getFieldName("password", credentials.id))
+        confirm_password_field_name = (
+                "field." + self._getFieldName("confirm_password",
+                                              credentials.id))
+        url_field_name = "field." + self._getFieldName("url", credentials.id)
+        delete_field_name = (
+                "field." + self._getFieldName("delete", credentials.id))
+        return {
+            "owner": widgets_by_name[owner_field_name],
+            "username": widgets_by_name[username_field_name],
+            "password": widgets_by_name[password_field_name],
+            "confirm_password": widgets_by_name[confirm_password_field_name],
+            "url": widgets_by_name[url_field_name],
+            "delete": widgets_by_name[delete_field_name]
+        }
+
+    def parseData(self, data):
+        """Rearrange form data to make it easier to process."""
+        parsed_data = {}
+        add_url = data["add_url"]
+        add_username = data["add_username"]
+        add_password = data["add_password"]
+        add_confirm_password = data["add_confirm_password"]
+        if add_url or add_username or add_password or add_confirm_password:
+            parsed_data.setdefault(None, {
+                "username": add_username,
+                "password": add_password,
+                "confirm_password": add_confirm_password,
+                "url": add_url,
+                "action": "add",
+            })
+        for field_name in (
+                name for name in data if name.split(".")[0] == "owner"):
+            _, credentials_id = self._parseFieldName(field_name)
+            owner_field_name = self._getFieldName(
+                "owner", credentials_id)
+            username_field_name = self._getFieldName(
+                "username", credentials_id)
+            password_field_name = self._getFieldName(
+                "password", credentials_id)
+            confirm_password_field_name = self._getFieldName(
+                "confirm_password", credentials_id)
+            url_field_name = self._getFieldName("url", credentials_id)
+            delete_field_name = self._getFieldName("delete", credentials_id)
+            if data.get(delete_field_name):
+                action = "delete"
+            else:
+                action = "change"
+            parsed_data.setdefault(credentials_id, {
+                "username": data.get(username_field_name),
+                "password": data.get(password_field_name),
+                "confirm_password": data.get(confirm_password_field_name),
+                "url": data.get(url_field_name),
+                "owner": data.get(owner_field_name),
+                "action": action,
+            })
+
+        return parsed_data
+
+    def changeCredentials(self, parsed_credentials, credentials):
+        username = parsed_credentials["username"]
+        password = parsed_credentials["password"]
+        confirm_password = parsed_credentials["confirm_password"]
+        owner = parsed_credentials["owner"]
+        if password or confirm_password:
+            if password != confirm_password:
+                self.setFieldError(
+                    self._getFieldName(
+                        "confirm_password", credentials.id),
+                    "Passwords do not match.")
+            else:
+                credentials.setCredentials(
+                    {"username": username,
+                     "password": password})
+                credentials.url = parsed_credentials["url"]
+        elif username != credentials.username:
+            removeSecurityProxy(credentials).username = username
+            credentials.url = parsed_credentials["url"]
+        elif parsed_credentials["url"] != credentials.url:
+            credentials.url = parsed_credentials["url"]
+        if owner != credentials.owner:
+            credentials.owner = owner
+
+    def deleteCredentials(self, credentials):
+        push_rule_set = getUtility(IOCIPushRuleSet)
+        if not push_rule_set.findByRegistryCredentials(
+                credentials).is_empty():
+            self.setFieldError(
+                self._getFieldName(
+                    "delete", credentials.id),
+                "These credentials cannot be deleted as there are "
+                "push rules defined that still use them.")
+        else:
+            credentials.destroySelf()
+
+    def addCredentials(self, parsed_add_credentials):
+        url = parsed_add_credentials["url"]
+        password = parsed_add_credentials["password"]
+        confirm_password = parsed_add_credentials["confirm_password"]
+        username = parsed_add_credentials["username"]
+        if url:
+            if password or confirm_password:
+                if not password == confirm_password:
+                    self.setFieldError(
+                        "add_password",
+                        "Please make sure the new "
+                        "password matches the "
+                        "confirm password field.")
+                    return
+
+                credentials = {
+                    'username': username,
+                    'password': password}
+                try:
+                    getUtility(IOCIRegistryCredentialsSet).new(
+                        owner=self.context,
+                        url=url,
+                        credentials=credentials)
+                except OCIRegistryCredentialsAlreadyExist:
+                    self.setFieldError(
+                        "add_url",
+                        "Credentials already exist "
+                        "with the same URL and "
+                        "username.")
+            else:
+                credentials = {'username': username}
+                try:
+                    getUtility(IOCIRegistryCredentialsSet).new(
+                        owner=self.context,
+                        url=url,
+                        credentials=credentials)
+                except OCIRegistryCredentialsAlreadyExist:
+                    self.setFieldError(
+                        "add_url",
+                        "Credentials already exist "
+                        "with the same URL and username.")
+        else:
+            self.setFieldError(
+                "add_url",
+                "Registry URL cannot be empty.")
+
+    def updateCredentialsFromData(self, parsed_data):
+        credentials_map = {
+            credentials.id: credentials
+            for credentials in self.oci_registry_credentials}
+
+        for credentials_id, parsed_credentials in parsed_data.items():
+            credentials = credentials_map.get(credentials_id)
+            action = parsed_credentials["action"]
+
+            if action == "change":
+                self.changeCredentials(parsed_credentials, credentials)
+            elif action == "delete":
+                self.deleteCredentials(credentials)
+            elif action == "add":
+                parsed_add_credentials = parsed_data[credentials]
+                self.addCredentials(parsed_add_credentials)
+            else:
+                raise AssertionError("unknown action: %s" % action)
+
+    @action("Save")
+    def save(self, action, data):
+        parsed_data = self.parseData(data)
+        self.updateCredentialsFromData(parsed_data)
+
+        if not self.errors:
+            self.request.response.addNotification("Saved credentials")
+            self.next_url = canonical_url(self.context)
+
+
 class PersonLiveFSView(LaunchpadView):
     """Default view for the list of live filesystems owned by a person."""
     page_title = 'LiveFS'
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index 1264643..cc35863 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -19,6 +19,7 @@ from testtools.matchers import (
     DocTestMatches,
     Equals,
     LessThan,
+    MatchesDict,
     Not,
     )
 from testtools.testcase import ExpectedException
@@ -34,6 +35,7 @@ from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.blueprints.enums import SpecificationImplementationStatus
 from lp.buildmaster.enums import BuildStatus
+from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet
 from lp.oci.interfaces.ocirecipe import OCI_RECIPE_ALLOW_CREATE
 from lp.oci.interfaces.ociregistrycredentials import (
     IOCIRegistryCredentialsSet,
@@ -53,6 +55,7 @@ from lp.registry.model.karma import KarmaCategory
 from lp.registry.model.milestone import milestone_sort_key
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
 from lp.services.config import config
+from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
@@ -1350,6 +1353,161 @@ class TestPersonOCIRegistryCredentialsView(BrowserTestCase,
                 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):
+        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)
+
+        browser = self.getViewBrowser(
+            self.user, view_name='+oci-registry-credentials', user=self.user)
+        browser.getLink("Edit OCI registry credentials").click()
+
+        # Change only the username
+        registry_credentials_id = removeSecurityProxy(registry_credentials).id
+        username_control = browser.getControl(
+            name="field.username.%d" % registry_credentials_id)
+        username_control.value = 'different_username'
+        browser.getControl("Save").click()
+        with person_logged_in(self.user):
+            self.assertThat(
+                registry_credentials.getCredentials(),
+                MatchesDict(
+                    {"username": Equals("different_username"),
+                     "password": Equals("bar")}))
+
+        # change only the registry url
+        browser = self.getViewBrowser(
+            self.user, 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)
+
+        # change only the password
+        browser = self.getViewBrowser(
+            self.user, 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)
+
+        # change all fields with one edit action
+        username_control = browser.getControl(
+            name="field.username.%d" % registry_credentials_id)
+        username_control.value = 'third_different_username'
+        url_control = browser.getControl(
+            name="field.url.%d" % registry_credentials_id)
+        url_control.value = third_url
+        password_control = browser.getControl(
+            name="field.password.%d" % registry_credentials_id)
+        password_control.value = 'third_newpassword'
+        confirm_password_control = browser.getControl(
+            name="field.confirm_password.%d" % registry_credentials_id)
+        confirm_password_control.value = 'third_newpassword'
+        browser.getControl("Save").click()
+        with person_logged_in(self.user):
+            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):
+        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)
+        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)
+        browser.getLink("Edit OCI registry credentials").click()
+
+        browser.getControl(name="field.add_url").value = url
+        browser.getControl(name="field.add_username").value = "new_username"
+        browser.getControl(name="field.add_password").value = "password"
+        browser.getControl(
+            name="field.add_confirm_password").value = "password"
+        browser.getControl("Save").click()
+
+        with person_logged_in(self.user):
+            creds = list(getUtility(
+                IOCIRegistryCredentialsSet).findByOwner(
+                self.user))
+            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
+        # 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)
+        IStore(registry_credentials).flush()
+        registry_credentials_id = removeSecurityProxy(registry_credentials).id
+        image_name = self.factory.getUniqueUnicode()
+        push_rule = getUtility(IOCIPushRuleSet).new(
+            recipe=self.recipe,
+            registry_credentials=registry_credentials,
+            image_name=image_name)
+
+        browser = self.getViewBrowser(
+            self.person, view_name='+oci-registry-credentials',
+            user=self.person)
+        browser.getLink("Edit OCI registry credentials").click()
+        # assert full rule is displayed
+        self.assertEqual(url, browser.getControl(
+            name="field.url.%d" % registry_credentials_id).value)
+        self.assertEqual(credentials.get('username'), browser.getControl(
+            name="field.username.%d" % registry_credentials_id).value)
+
+        # mark one line of credentials for delete
+        delete_control = browser.getControl(
+            name="field.delete.%d" % registry_credentials_id)
+        delete_control.getControl('Delete').selected = True
+        browser.getControl("Save").click()
+        self.assertIn("These credentials cannot be deleted as there are "
+                      "push rules defined that still use them.",
+                      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):
+            removeSecurityProxy(push_rule).destroySelf()
+
+        delete_control = browser.getControl(
+            name="field.delete.%d" % registry_credentials_id)
+        delete_control.getControl('Delete').selected = True
+        browser.getControl("Save").click()
+        credentials_set = getUtility(IOCIRegistryCredentialsSet)
+        with person_logged_in(self.person):
+            self.assertEqual(
+                0, credentials_set.findByOwner(self.person).count())
+
 
 class TestPersonLiveFSView(BrowserTestCase):
     layer = DatabaseFunctionalLayer
diff --git a/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
new file mode 100644
index 0000000..89483da
--- /dev/null
+++ b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
@@ -0,0 +1,59 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+<body>
+
+<div metal:fill-slot="main">
+    <div metal:use-macro="context/@@launchpad_form/form">
+    <metal:formbody fill-slot="widgets">
+
+    <table class="form">
+        <tr tal:repeat="credentials view/oci_registry_credentials">
+            <tal:credentials_widgets
+              define="credentials_widgets python:view.getCredentialsWidgets(credentials);
+                      parity python:'even' if repeat['credentials'].even() else 'odd'">
+                <td tal:define="widget nocall:credentials_widgets/url">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+                <td tal:define="widget nocall:credentials_widgets/owner">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+                <td tal:define="widget nocall:credentials_widgets/username">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+                <td tal:define="widget nocall:credentials_widgets/password">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+                <td tal:define="widget nocall:credentials_widgets/confirm_password">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+                <td tal:define="widget nocall:credentials_widgets/delete">
+                    <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+                </td>
+            </tal:credentials_widgets>
+        </tr>
+        <tr>
+            <td tal:define="widget nocall:view/widgets/add_url">
+                <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td tal:define="widget nocall:view/widgets/add_username">
+                <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td tal:define="widget nocall:view/widgets/add_password">
+                <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+            <td tal:define="widget nocall:view/widgets/add_confirm_password">
+                <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+            </td>
+        </tr>
+    </table>
+
+    </metal:formbody>
+    </div>
+</div>
+</body>
+</html>
diff --git a/lib/lp/registry/templates/person-ociregistrycredentials.pt b/lib/lp/registry/templates/person-ociregistrycredentials.pt
index 612056e..bec71e1 100644
--- a/lib/lp/registry/templates/person-ociregistrycredentials.pt
+++ b/lib/lp/registry/templates/person-ociregistrycredentials.pt
@@ -12,6 +12,10 @@
         <span tal:replace="context/title"/>
         has not set any credentials yet.
     </p>
+    <p condition="context/required:launchpad.Edit">
+        <a class="sprite edit" tal:attributes="href context/fmt:url/+edit-oci-registry-credentials">Edit OCI registry credentials</a>
+    </p>
+
     <table id="oci-credentials" class="listing" tal:condition="view/has_credentials">
     <thead>
         <tr>