Skip to content

Generalize trusted publishing emails #13872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 7, 2023
Merged

Generalize trusted publishing emails #13872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebb4ad8
oidc/models/google: add missing members
woodruffw Jun 6, 2023
c3cfc1b
tests, warehouse: general publishing emails
woodruffw Jun 6, 2023
f2c721a
warehouse: make publisher_url optional
woodruffw Jun 6, 2023
f817a6f
warehouse: `make translations`
woodruffw Jun 6, 2023
a7a7dd5
tests: fix google publisher_url test
woodruffw Jun 6, 2023
db3cffa
Merge branch 'main' into tob-google-publisher-emails
woodruffw Jun 6, 2023
90a9469
Merge branch 'main' into tob-google-publisher-emails
di Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/unit/accounts/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3627,7 +3627,7 @@ def test_add_pending_github_oidc_publisher(self, monkeypatch, db_request):

assert db_request.session.flash.calls == [
pretend.call(
"Registered a new publishing publisher to create "
"Registered a new pending publisher to create "
"the project 'some-project-name'.",
queue="success",
)
Expand Down
6 changes: 1 addition & 5 deletions tests/unit/email/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5819,11 +5819,7 @@ def test_trusted_publisher_emails(
assert result == {
"username": stub_user.username,
"project_name": project_name,
"publisher_name": "fakepublisher",
"publisher_workflow": "fakespecifier",
"publisher_repository_owner": "fakeowner",
"publisher_repository_name": "fakerepository",
"publisher_environment": "fakeenvironment",
"publisher": fakepublisher,
}
subject_renderer.assert_()
body_renderer.assert_(username=stub_user.username, project_name=project_name)
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/oidc/models/test_google.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ def test_lookup_strategies():


class TestGooglePublisher:
def test_publisher_name(self):
publisher = google.GooglePublisher(email="[email protected]")

assert publisher.publisher_name == "Google"

def test_publisher_url(self):
publisher = google.GooglePublisher(email="[email protected]")

assert publisher.publisher_url() is None

def test_stringifies_as_email(self):
publisher = google.GooglePublisher(email="[email protected]")

Expand Down
2 changes: 1 addition & 1 deletion warehouse/accounts/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ def add_pending_github_oidc_publisher(self):

self.request.session.flash(
self.request._(
"Registered a new publishing publisher to create "
"Registered a new pending publisher to create "
f"the project '{pending_publisher.project_name}'."
),
queue="success",
Expand Down
6 changes: 5 additions & 1 deletion warehouse/admin/templates/admin/projects/detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ <h4 class="modal-title" id="exampleModalLabel">Remove role for {{ role.user.user
{% for pub in oidc_publishers %}
<tr>
<td>{{ pub.publisher_name }}</td>
<td><a href="{{ pub.publisher_url() }}">{{ pub.repository }}</a></td>
{% if pub.publisher_url() %}
<td><a href="{{ pub.publisher_url() }}">{{ pub.publisher_url() }}</a></td>
{% else %}
<td>N/A</td>
{% endif %}
<td><code>{{ pub }}</code></td>
</tr>
{% endfor %}
Expand Down
6 changes: 5 additions & 1 deletion warehouse/admin/templates/admin/users/detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,11 @@ <h3 class="card-title">Pending OpenID Connect Publishers</h3>
<tr>
<td>{{ pub.project_name }}</td>
<td>{{ pub.publisher_name }}</td>
<td><a href="{{ pub.publisher_url() }}">{{ pub.repository }}</a></td>
{% if pub.publisher_url() %}
<td><a href="{{ pub.publisher_url() }}">{{ pub.publisher_url() }}</a></td>
{% else %}
<td>N/A</td>
{% endif %}
<td><code>{{ pub }}</code></td>
</tr>
{% endfor %}
Expand Down
12 changes: 2 additions & 10 deletions warehouse/email/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,11 +992,7 @@ def send_trusted_publisher_added_email(request, user, project_name, publisher):
return {
"username": request.user.username,
"project_name": project_name,
"publisher_name": publisher.publisher_name,
"publisher_workflow": str(publisher),
"publisher_repository_owner": publisher.repository_owner,
"publisher_repository_name": publisher.repository_name,
"publisher_environment": publisher.environment,
"publisher": publisher,
}


Expand All @@ -1006,11 +1002,7 @@ def send_trusted_publisher_removed_email(request, user, project_name, publisher)
return {
"username": request.user.username,
"project_name": project_name,
"publisher_name": publisher.publisher_name,
"publisher_workflow": str(publisher),
"publisher_repository_owner": publisher.repository_owner,
"publisher_repository_name": publisher.repository_name,
"publisher_environment": publisher.environment,
"publisher": publisher,
}


Expand Down
89 changes: 48 additions & 41 deletions warehouse/locale/messages.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ msgid ""
msgstr ""

#: warehouse/accounts/views.py:1586
msgid "Registered a new publishing publisher to create "
msgid "Registered a new pending publisher to create "
msgstr ""

#: warehouse/accounts/views.py:1623 warehouse/accounts/views.py:1636
Expand Down Expand Up @@ -2375,15 +2375,15 @@ msgstr ""
msgid "Publisher name"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:31
#: warehouse/templates/email/trusted-publisher-removed/body.html:29
#: warehouse/templates/email/trusted-publisher-added/body.html:32
#: warehouse/templates/email/trusted-publisher-removed/body.html:30
#: warehouse/templates/manage/account/publishing.html:173
#: warehouse/templates/manage/project/publishing.html:46
msgid "Workflow"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:32
#: warehouse/templates/email/trusted-publisher-removed/body.html:30
#: warehouse/templates/email/trusted-publisher-added/body.html:33
#: warehouse/templates/email/trusted-publisher-removed/body.html:31
#: warehouse/templates/includes/packaging/project-data.html:117
#: warehouse/templates/manage/account/publishing.html:47
#: warehouse/templates/manage/organization/roles.html:53
Expand All @@ -2399,27 +2399,38 @@ msgstr ""
msgid "Owner"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:33
#: warehouse/templates/email/trusted-publisher-removed/body.html:31
#: warehouse/templates/email/trusted-publisher-added/body.html:34
#: warehouse/templates/email/trusted-publisher-removed/body.html:32
#: warehouse/templates/manage/account/publishing.html:172
#: warehouse/templates/manage/project/publishing.html:45
msgid "Repository"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:35
#: warehouse/templates/email/trusted-publisher-removed/body.html:33
#: warehouse/templates/email/trusted-publisher-added/body.html:36
#: warehouse/templates/email/trusted-publisher-removed/body.html:34
msgid "Environment"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:39
#: warehouse/templates/email/trusted-publisher-removed/body.html:37
#: warehouse/templates/includes/accounts/profile-public-email.html:17
msgid "Email"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:41
#: warehouse/templates/email/trusted-publisher-removed/body.html:39
msgid "Subject"
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:48
msgid ""
"If you did not make this change and you think it was made maliciously, "
"you can remove it from the project via the \"Publishing\" tab on the "
"project's page."
msgstr ""

#: warehouse/templates/email/trusted-publisher-added/body.html:48
#: warehouse/templates/email/trusted-publisher-removed/body.html:46
#: warehouse/templates/email/trusted-publisher-added/body.html:55
#: warehouse/templates/email/trusted-publisher-removed/body.html:53
#, python-format
msgid ""
"If you are unable to revert the change and need to do so, you can email "
Expand All @@ -2434,7 +2445,7 @@ msgid ""
"from a project (<strong>%(project_name)s</strong>) that you manage."
msgstr ""

#: warehouse/templates/email/trusted-publisher-removed/body.html:39
#: warehouse/templates/email/trusted-publisher-removed/body.html:46
msgid ""
"If you did not make this change and you think it was made in error, you "
"can check the \"Security history\" tab on the project's page."
Expand Down Expand Up @@ -2676,10 +2687,6 @@ msgstr ""
msgid "%(username)s has not uploaded any projects to PyPI, yet."
msgstr ""

#: warehouse/templates/includes/accounts/profile-public-email.html:17
msgid "Email"
msgstr ""

#: warehouse/templates/includes/manage/manage-organization-menu.html:14
#, python-format
msgid "Navigation for managing %(organization)s"
Expand Down Expand Up @@ -3161,7 +3168,7 @@ msgstr ""

#: warehouse/templates/manage/account.html:504
#: warehouse/templates/manage/account.html:523
#: warehouse/templates/manage/project/history.html:268
#: warehouse/templates/manage/project/history.html:272
msgid "Reason:"
msgstr ""

Expand Down Expand Up @@ -3302,13 +3309,13 @@ msgstr ""

#: warehouse/templates/manage/account.html:620
#: warehouse/templates/manage/account.html:643
#: warehouse/templates/manage/project/history.html:259
#: warehouse/templates/manage/project/history.html:266
#: warehouse/templates/manage/project/history.html:263
#: warehouse/templates/manage/project/history.html:270
msgid "Token name:"
msgstr ""

#: warehouse/templates/manage/account.html:637
#: warehouse/templates/manage/project/history.html:261
#: warehouse/templates/manage/project/history.html:265
msgid "API token removed"
msgstr ""

Expand Down Expand Up @@ -3360,16 +3367,16 @@ msgstr ""

#: warehouse/templates/manage/account.html:683
#: warehouse/templates/manage/organization/history.html:201
#: warehouse/templates/manage/project/history.html:300
#: warehouse/templates/manage/project/history.html:304
#: warehouse/templates/manage/team/history.html:108
msgid "Event"
msgstr ""

#: warehouse/templates/manage/account.html:684
#: warehouse/templates/manage/organization/history.html:202
#: warehouse/templates/manage/organization/history.html:211
#: warehouse/templates/manage/project/history.html:301
#: warehouse/templates/manage/project/history.html:310
#: warehouse/templates/manage/project/history.html:305
#: warehouse/templates/manage/project/history.html:314
#: warehouse/templates/manage/team/history.html:109
#: warehouse/templates/manage/team/history.html:118
msgid "Time"
Expand All @@ -3392,7 +3399,7 @@ msgstr ""

#: warehouse/templates/manage/account.html:698
#: warehouse/templates/manage/organization/history.html:217
#: warehouse/templates/manage/project/history.html:316
#: warehouse/templates/manage/project/history.html:320
#: warehouse/templates/manage/team/history.html:124
msgid "Device Info"
msgstr ""
Expand Down Expand Up @@ -3728,7 +3735,7 @@ msgid "Submitted by:"
msgstr ""

#: warehouse/templates/manage/manage_base.html:546
#: warehouse/templates/manage/project/history.html:243
#: warehouse/templates/manage/project/history.html:247
msgid "Workflow:"
msgstr ""

Expand Down Expand Up @@ -4700,7 +4707,7 @@ msgid "Revoked by:"
msgstr ""

#: warehouse/templates/manage/organization/history.html:198
#: warehouse/templates/manage/project/history.html:297
#: warehouse/templates/manage/project/history.html:301
#: warehouse/templates/manage/team/history.html:105
#, python-format
msgid "Security history for %(source_name)s"
Expand Down Expand Up @@ -5304,13 +5311,13 @@ msgid "Short-lived API token created"
msgstr ""

#: warehouse/templates/manage/project/history.html:222
#: warehouse/templates/manage/project/history.html:248
#: warehouse/templates/manage/project/history.html:262
#: warehouse/templates/manage/project/history.html:252
#: warehouse/templates/manage/project/history.html:266
msgid "Permissions: Can upload to this project"
msgstr ""

#: warehouse/templates/manage/project/history.html:225
#: warehouse/templates/manage/project/history.html:256
#: warehouse/templates/manage/project/history.html:260
msgid "Expiration:"
msgstr ""

Expand All @@ -5323,47 +5330,47 @@ msgid "Expired"
msgstr ""

#: warehouse/templates/manage/project/history.html:237
msgid "Creator:"
msgid "Creator"
msgstr ""

#: warehouse/templates/manage/project/history.html:247
#: warehouse/templates/manage/project/history.html:251
msgid "API token created"
msgstr ""

#: warehouse/templates/manage/project/history.html:251
#: warehouse/templates/manage/project/history.html:264
#: warehouse/templates/manage/project/history.html:255
#: warehouse/templates/manage/project/history.html:268
msgid "Controlled by:"
msgstr ""

#: warehouse/templates/manage/project/history.html:273
#: warehouse/templates/manage/project/history.html:277
msgid "Trusted publisher added"
msgstr ""

#: warehouse/templates/manage/project/history.html:276
#: warehouse/templates/manage/project/history.html:280
msgid "Trusted publisher removed"
msgstr ""

#: warehouse/templates/manage/project/history.html:281
#: warehouse/templates/manage/project/history.html:285
msgid "2FA requirement enabled"
msgstr ""

#: warehouse/templates/manage/project/history.html:283
#: warehouse/templates/manage/project/history.html:287
msgid "Enabled by:"
msgstr ""

#: warehouse/templates/manage/project/history.html:286
#: warehouse/templates/manage/project/history.html:290
msgid "2FA requirement disabled"
msgstr ""

#: warehouse/templates/manage/project/history.html:288
#: warehouse/templates/manage/project/history.html:292
msgid "Disabled by:"
msgstr ""

#: warehouse/templates/manage/project/history.html:302
#: warehouse/templates/manage/project/history.html:306
msgid "Additional info"
msgstr ""

#: warehouse/templates/manage/project/history.html:314
#: warehouse/templates/manage/project/history.html:318
#: warehouse/templates/manage/team/history.html:122
msgid "Location info"
msgstr ""
Expand Down
2 changes: 1 addition & 1 deletion warehouse/oidc/models/_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ def publisher_name(self) -> str: # pragma: no cover
# Only concrete subclasses are constructed.
raise NotImplementedError

def publisher_url(self, claims=None) -> str: # pragma: no cover
def publisher_url(self, claims=None) -> str | None: # pragma: no cover
"""
NOTE: This is **NOT** a `@property` because we pass `claims` to it.
When calling, make sure to use `publisher_url()`
Expand Down
7 changes: 7 additions & 0 deletions warehouse/oidc/models/google.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ def __lookup_no_sub__(klass, signed_claims: SignedClaims) -> Query | None:
__lookup_no_sub__,
]

@property
def publisher_name(self):
return "Google"

def publisher_url(self, claims=None):
return None

@property
def email_verified(self):
# We don't consider a claim set valid unless `email_verified` is true;
Expand Down
19 changes: 13 additions & 6 deletions warehouse/templates/email/trusted-publisher-added/body.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,19 @@
<p>
{% trans %}Publisher information{% endtrans %}:
<ul>
<li>{% trans %}Publisher name{% endtrans %}: {{ publisher_name }}</li>
<li>{% trans %}Workflow{% endtrans %}: {{ publisher_workflow }}</li>
<li>{% trans %}Owner{% endtrans %}: {{ publisher_repository_owner }}</li>
<li>{% trans %}Repository{% endtrans %}: {{ publisher_repository_name }}</li>
{% if publisher_environment %}
<li>{% trans %}Environment{% endtrans %}: {{ publisher_environment }}</li>
<li>{% trans %}Publisher name{% endtrans %}: {{ publisher.publisher_name }}</li>
{% if publisher.publisher_name == "GitHub" %}
<li>{% trans %}Workflow{% endtrans %}: {{ publisher }}</li>
<li>{% trans %}Owner{% endtrans %}: {{ publisher.repository_owner }}</li>
<li>{% trans %}Repository{% endtrans %}: {{ publisher.repository_name }}</li>
{% if publisher.environment %}
<li>{% trans %}Environment{% endtrans %}: {{ publisher.environment }}</li>
{% endif %}
{% elif publisher.publisher_name == "Google" %}
<li>{% trans %}Email{% endtrans %}: {{ publisher.email }}</li>
{% if publisher.sub %}
<li>{% trans %}Subject{% endtrans %}: {{ publisher.sub }}</li>
{% endif %}
{% endif %}
</ul>
</p>
Expand Down
Loading