From 2bde04f544963cc0720404bd7ee074413c9af4e8 Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Thu, 13 Apr 2023 10:35:52 -0400
Subject: [PATCH] [oidc] Highlight publisher errors (#13421)

* Jump to errors if present

* Flash error if publisher could not be registered
---
 warehouse/accounts/views.py                   |   4 +
 warehouse/locale/messages.pot                 |  24 ++--
 warehouse/manage/views/__init__.py            | 122 +++++++++---------
 .../templates/manage/account/publishing.html  |   2 +-
 .../templates/manage/project/publishing.html  |   2 +-
 5 files changed, 83 insertions(+), 71 deletions(-)

diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py
index d214ef39b200..c2629d9085a7 100644
--- a/warehouse/accounts/views.py
+++ b/warehouse/accounts/views.py
@@ -1460,6 +1460,10 @@ def add_pending_github_oidc_publisher(self):
         form = response["pending_github_publisher_form"]
 
         if not form.validate():
+            self.request.session.flash(
+                self.request._("The trusted publisher could not be registered"),
+                queue="error",
+            )
             return response
 
         publisher_already_exists = (
diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot
index d9cfbfe2cda9..11f541db26d6 100644
--- a/warehouse/locale/messages.pot
+++ b/warehouse/locale/messages.pot
@@ -274,7 +274,11 @@ msgid ""
 "again later."
 msgstr ""
 
-#: warehouse/accounts/views.py:1479
+#: warehouse/accounts/views.py:1464 warehouse/manage/views/__init__.py:1258
+msgid "The trusted publisher could not be registered"
+msgstr ""
+
+#: warehouse/accounts/views.py:1483
 msgid ""
 "This trusted publisher has already been registered. Please contact PyPI's"
 " admins if this wasn't intentional."
@@ -368,43 +372,43 @@ msgstr ""
 msgid "Generating new recovery codes will invalidate your existing codes."
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2035
+#: warehouse/manage/views/__init__.py:2039
 msgid "Team '${team_name}' already has ${role_name} role for project"
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2143
+#: warehouse/manage/views/__init__.py:2147
 msgid "User '${username}' already has ${role_name} role for project"
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2211
+#: warehouse/manage/views/__init__.py:2215
 msgid "${username} is now ${role} of the '${project_name}' project."
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2243
+#: warehouse/manage/views/__init__.py:2247
 msgid ""
 "User '${username}' does not have a verified primary email address and "
 "cannot be added as a ${role_name} for project"
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2256
+#: warehouse/manage/views/__init__.py:2260
 #: warehouse/manage/views/organizations.py:896
 msgid "User '${username}' already has an active invite. Please try again later."
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2322
+#: warehouse/manage/views/__init__.py:2326
 #: warehouse/manage/views/organizations.py:961
 msgid "Invitation sent to '${username}'"
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2355
+#: warehouse/manage/views/__init__.py:2359
 msgid "Could not find role invitation."
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2366
+#: warehouse/manage/views/__init__.py:2370
 msgid "Invitation already expired."
 msgstr ""
 
-#: warehouse/manage/views/__init__.py:2399
+#: warehouse/manage/views/__init__.py:2403
 #: warehouse/manage/views/organizations.py:1148
 msgid "Invitation revoked from '${username}'."
 msgstr ""
diff --git a/warehouse/manage/views/__init__.py b/warehouse/manage/views/__init__.py
index 8d81a42bf696..0e69b4f47ddd 100644
--- a/warehouse/manage/views/__init__.py
+++ b/warehouse/manage/views/__init__.py
@@ -1253,74 +1253,78 @@ def add_github_oidc_publisher(self):
         response = self.default_response
         form = response["github_publisher_form"]
 
-        if form.validate():
-            # GitHub OIDC publishers are unique on the tuple of
-            # (repository_name, repository_owner, workflow_filename, environment),
-            # so we check for an already registered one before creating.
-            publisher = (
-                self.request.db.query(GitHubPublisher)
-                .filter(
-                    GitHubPublisher.repository_name == form.repository.data,
-                    GitHubPublisher.repository_owner == form.normalized_owner,
-                    GitHubPublisher.workflow_filename == form.workflow_filename.data,
-                    GitHubPublisher.environment == form.normalized_environment,
-                )
-                .one_or_none()
-            )
-            if publisher is None:
-                publisher = GitHubPublisher(
-                    repository_name=form.repository.data,
-                    repository_owner=form.normalized_owner,
-                    repository_owner_id=form.owner_id,
-                    workflow_filename=form.workflow_filename.data,
-                    environment=form.normalized_environment,
-                )
-
-                self.request.db.add(publisher)
-
-            # Each project has a unique set of OIDC publishers; the same
-            # publisher can't be registered to the project more than once.
-            if publisher in self.project.oidc_publishers:
-                self.request.session.flash(
-                    f"{publisher} is already registered with {self.project.name}",
-                    queue="error",
-                )
-                return response
-
-            for user in self.project.users:
-                send_trusted_publisher_added_email(
-                    self.request,
-                    user,
-                    project_name=self.project.name,
-                    publisher=publisher,
-                )
-
-            self.project.oidc_publishers.append(publisher)
+        if not form.validate():
+            self.request.session.flash(
+                self.request._("The trusted publisher could not be registered"),
+                queue="error",
+            )
+            return response
 
-            self.project.record_event(
-                tag=EventTag.Project.OIDCPublisherAdded,
-                ip_address=self.request.remote_addr,
-                additional={
-                    "publisher": publisher.publisher_name,
-                    "id": str(publisher.id),
-                    "specifier": str(publisher),
-                    "url": publisher.publisher_url,
-                    "submitted_by": self.request.user.username,
-                },
+        # GitHub OIDC publishers are unique on the tuple of
+        # (repository_name, repository_owner, workflow_filename, environment),
+        # so we check for an already registered one before creating.
+        publisher = (
+            self.request.db.query(GitHubPublisher)
+            .filter(
+                GitHubPublisher.repository_name == form.repository.data,
+                GitHubPublisher.repository_owner == form.normalized_owner,
+                GitHubPublisher.workflow_filename == form.workflow_filename.data,
+                GitHubPublisher.environment == form.normalized_environment,
             )
+            .one_or_none()
+        )
+        if publisher is None:
+            publisher = GitHubPublisher(
+                repository_name=form.repository.data,
+                repository_owner=form.normalized_owner,
+                repository_owner_id=form.owner_id,
+                workflow_filename=form.workflow_filename.data,
+                environment=form.normalized_environment,
+            )
+
+            self.request.db.add(publisher)
 
+        # Each project has a unique set of OIDC publishers; the same
+        # publisher can't be registered to the project more than once.
+        if publisher in self.project.oidc_publishers:
             self.request.session.flash(
-                f"Added {publisher} to {self.project.name}",
-                queue="success",
+                f"{publisher} is already registered with {self.project.name}",
+                queue="error",
             )
+            return response
 
-            self.metrics.increment(
-                "warehouse.oidc.add_publisher.ok", tags=["publisher:GitHub"]
+        for user in self.project.users:
+            send_trusted_publisher_added_email(
+                self.request,
+                user,
+                project_name=self.project.name,
+                publisher=publisher,
             )
 
-            return HTTPSeeOther(self.request.path)
+        self.project.oidc_publishers.append(publisher)
 
-        return response
+        self.project.record_event(
+            tag=EventTag.Project.OIDCPublisherAdded,
+            ip_address=self.request.remote_addr,
+            additional={
+                "publisher": publisher.publisher_name,
+                "id": str(publisher.id),
+                "specifier": str(publisher),
+                "url": publisher.publisher_url,
+                "submitted_by": self.request.user.username,
+            },
+        )
+
+        self.request.session.flash(
+            f"Added {publisher} to {self.project.name}",
+            queue="success",
+        )
+
+        self.metrics.increment(
+            "warehouse.oidc.add_publisher.ok", tags=["publisher:GitHub"]
+        )
+
+        return HTTPSeeOther(self.request.path)
 
     @view_config(
         request_method="POST",
diff --git a/warehouse/templates/manage/account/publishing.html b/warehouse/templates/manage/account/publishing.html
index c6b16a3fde16..24344bcabeb5 100644
--- a/warehouse/templates/manage/account/publishing.html
+++ b/warehouse/templates/manage/account/publishing.html
@@ -107,7 +107,7 @@ <h2>{% trans %}Add a new pending publisher{% endtrans %}</h2>
   <h3>GitHub</h3>
 
   {{ form_error_anchor(pending_github_publisher_form) }}
-  <form method="POST" action="{{ request.route_path('manage.account.publishing') }}">
+  <form method="POST" action="{{ request.route_path('manage.account.publishing') }}#errors">
     <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
     {{ form_errors(pending_github_publisher_form) }}
     <div class="form-group">
diff --git a/warehouse/templates/manage/project/publishing.html b/warehouse/templates/manage/project/publishing.html
index d50dbf2dc876..71951c6c7159 100644
--- a/warehouse/templates/manage/project/publishing.html
+++ b/warehouse/templates/manage/project/publishing.html
@@ -68,7 +68,7 @@ <h3>GitHub</h3>
     </p>
 
     {{ form_error_anchor(github_publisher_form) }}
-    <form method="POST" action="{{ request.route_path('manage.project.settings.publishing', project_name=project.name) }}">
+    <form method="POST" action="{{ request.route_path('manage.project.settings.publishing', project_name=project.name) }}#errors">
       <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
       {{ form_errors(github_publisher_form) }}
       <div class="form-group">