refactor: replace permission strings with enums (pypi#15205)

tests/unit/accounts/test_models.py

@@ -18,6 +18,7 @@
 from pyramid.authorization import Authenticated
 
 from warehouse.accounts.models import Email, RecoveryCode, User, UserFactory, WebAuthn
+from warehouse.authnz import Permissions
 from warehouse.utils.security_policy import principals_for
 
 from ...common.db.accounts import (
@@ -164,8 +165,20 @@ def test_has_no_burned_recovery_codes(self, db_session):
     def test_acl(self, db_session):
         user = DBUserFactory.create()
         assert user.__acl__() == [
-            ("Allow", "group:admins", "admin"),
-            ("Allow", "group:moderators", "moderator"),
+            (
+                "Allow",
+                "group:admins",
+                (
+                    Permissions.AdminUsersRead,
+                    Permissions.AdminUsersWrite,
+                    Permissions.AdminDashboardSidebarRead,
+                ),
+            ),
+            (
+                "Allow",
+                "group:moderators",
+                (Permissions.AdminUsersRead, Permissions.AdminDashboardSidebarRead),
+            ),
         ]
 
     @pytest.mark.parametrize(

tests/unit/organizations/test_models.py

@@ -17,6 +17,7 @@
 from pyramid.httpexceptions import HTTPPermanentRedirect
 from pyramid.location import lineage
 
+from warehouse.authnz import Permissions
 from warehouse.organizations.models import (
     OrganizationFactory,
     OrganizationRoleType,
@@ -115,8 +116,15 @@ def test_acl(self, db_session):
             acls.extend(acl)
 
         assert acls == [
-            (Allow, "group:admins", "admin"),
-            (Allow, "group:moderators", "moderator"),
+            (
+                Allow,
+                "group:admins",
+                (
+                    Permissions.AdminOrganizationsRead,
+                    Permissions.AdminOrganizationsWrite,
+                ),
+            ),
+            (Allow, "group:moderators", Permissions.AdminOrganizationsRead),
         ] + sorted(
             [
                 (
@@ -299,8 +307,15 @@ def test_acl(self, db_session):
             acls.extend(acl)
 
         assert acls == [
-            (Allow, "group:admins", "admin"),
-            (Allow, "group:moderators", "moderator"),
+            (
+                Allow,
+                "group:admins",
+                (
+                    Permissions.AdminOrganizationsRead,
+                    Permissions.AdminOrganizationsWrite,
+                ),
+            ),
+            (Allow, "group:moderators", Permissions.AdminOrganizationsRead),
         ] + sorted(
             [
                 (

tests/unit/packaging/test_models.py

@@ -18,6 +18,7 @@
 from pyramid.authorization import Allow
 from pyramid.location import lineage
 
+from warehouse.authnz import Permissions
 from warehouse.organizations.models import TeamProjectRoleType
 from warehouse.packaging.models import File, ProjectFactory, ReleaseURL
 
@@ -158,8 +159,33 @@ def test_acl(self, db_session):
             acls.extend(acl)
 
         assert acls == [
-            (Allow, "group:admins", "admin"),
-            (Allow, "group:moderators", "moderator"),
+            (
+                Allow,
+                "group:admins",
+                (
+                    Permissions.AdminDashboardSidebarRead,
+                    Permissions.AdminObservationsRead,
+                    Permissions.AdminObservationsWrite,
+                    Permissions.AdminProhibitedProjectsWrite,
+                    Permissions.AdminProjectsDelete,
+                    Permissions.AdminProjectsRead,
+                    Permissions.AdminProjectsWrite,
+                    Permissions.AdminRoleAdd,
+                    Permissions.AdminRoleDelete,
+                ),
+            ),
+            (
+                Allow,
+                "group:moderators",
+                (
+                    Permissions.AdminDashboardSidebarRead,
+                    Permissions.AdminObservationsRead,
+                    Permissions.AdminObservationsWrite,
+                    Permissions.AdminProjectsRead,
+                    Permissions.AdminRoleAdd,
+                    Permissions.AdminRoleDelete,
+                ),
+            ),
         ] + sorted(
             [(Allow, f"oidc:{publisher.id}", ["upload"])], key=lambda x: x[1]
         ) + sorted(
@@ -415,8 +441,33 @@ def test_acl(self, db_session):
             acls.extend(acl)
 
         assert acls == [
-            (Allow, "group:admins", "admin"),
-            (Allow, "group:moderators", "moderator"),
+            (
+                Allow,
+                "group:admins",
+                (
+                    Permissions.AdminDashboardSidebarRead,
+                    Permissions.AdminObservationsRead,
+                    Permissions.AdminObservationsWrite,
+                    Permissions.AdminProhibitedProjectsWrite,
+                    Permissions.AdminProjectsDelete,
+                    Permissions.AdminProjectsRead,
+                    Permissions.AdminProjectsWrite,
+                    Permissions.AdminRoleAdd,
+                    Permissions.AdminRoleDelete,
+                ),
+            ),
+            (
+                Allow,
+                "group:moderators",
+                (
+                    Permissions.AdminDashboardSidebarRead,
+                    Permissions.AdminObservationsRead,
+                    Permissions.AdminObservationsWrite,
+                    Permissions.AdminProjectsRead,
+                    Permissions.AdminRoleAdd,
+                    Permissions.AdminRoleDelete,
+                ),
+            ),
         ] + sorted(
             [
                 (Allow, f"user:{owner1.user.id}", ["manage:project", "upload"]),

tests/unit/test_config.py

@@ -24,6 +24,7 @@
 from pyramid.tweens import EXCVIEW
 
 from warehouse import config
+from warehouse.authnz import Permissions
 from warehouse.utils.wsgi import ProxyFixer, VhmRootRemover
 
 
@@ -453,11 +454,69 @@ def test_root_factory_access_control_list():
     acl = config.RootFactory.__acl__
 
     assert acl == [
-        (Allow, "group:admins", "admin"),
-        (Allow, "group:admins", "admin_dashboard_access"),
-        (Allow, "group:moderators", "moderator"),
-        (Allow, "group:moderators", "admin_dashboard_access"),
-        (Allow, "group:psf_staff", "psf_staff"),
-        (Allow, "group:psf_staff", "admin_dashboard_access"),
+        (
+            Allow,
+            "group:admins",
+            (
+                Permissions.AdminBannerRead,
+                Permissions.AdminBannerWrite,
+                Permissions.AdminDashboardRead,
+                Permissions.AdminDashboardSidebarRead,
+                Permissions.AdminEmailsRead,
+                Permissions.AdminEmailsWrite,
+                Permissions.AdminFlagsRead,
+                Permissions.AdminFlagsWrite,
+                Permissions.AdminIpAddressesRead,
+                Permissions.AdminJournalRead,
+                Permissions.AdminMacaroonsRead,
+                Permissions.AdminMacaroonsWrite,
+                Permissions.AdminObservationsRead,
+                Permissions.AdminObservationsWrite,
+                Permissions.AdminOrganizationsRead,
+                Permissions.AdminOrganizationsWrite,
+                Permissions.AdminProhibitedProjectsRead,
+                Permissions.AdminProhibitedProjectsWrite,
+                Permissions.AdminProjectsDelete,
+                Permissions.AdminProjectsRead,
+                Permissions.AdminProjectsWrite,
+                Permissions.AdminRoleAdd,
+                Permissions.AdminRoleDelete,
+                Permissions.AdminSponsorsRead,
+                Permissions.AdminUsersRead,
+                Permissions.AdminUsersWrite,
+            ),
+        ),
+        (
+            Allow,
+            "group:moderators",
+            (
+                Permissions.AdminBannerRead,
+                Permissions.AdminDashboardRead,
+                Permissions.AdminDashboardSidebarRead,
+                Permissions.AdminEmailsRead,
+                Permissions.AdminFlagsRead,
+                Permissions.AdminJournalRead,
+                Permissions.AdminObservationsRead,
+                Permissions.AdminObservationsWrite,
+                Permissions.AdminOrganizationsRead,
+                Permissions.AdminProhibitedProjectsRead,
+                Permissions.AdminProjectsRead,
+                Permissions.AdminRoleAdd,
+                Permissions.AdminRoleDelete,
+                Permissions.AdminSponsorsRead,
+                Permissions.AdminUsersRead,
+            ),
+        ),
+        (
+            Allow,
+            "group:psf_staff",
+            (
+                Permissions.AdminBannerRead,
+                Permissions.AdminBannerWrite,
+                Permissions.AdminDashboardRead,
+                Permissions.AdminSponsorsRead,
+                Permissions.AdminSponsorsWrite,
+            ),
+        ),
         (Allow, Authenticated, "manage:user"),
     ]

warehouse/accounts/models.py

@@ -35,6 +35,7 @@
 from sqlalchemy.orm import Mapped, mapped_column
 
 from warehouse import db
+from warehouse.authnz import Permissions
 from warehouse.events.models import HasEvents
 from warehouse.observations.models import HasObserversMixin
 from warehouse.sitemap.models import SitemapMixin
@@ -253,9 +254,25 @@ def __principals__(self) -> list[str]:
         return principals
 
     def __acl__(self):
+        # TODO: This ACL is duplicating permissions set in RootFactory.__acl__
+        #   If nothing else, setting the ACL on the model is more restrictive
+        #   than RootFactory.__acl__, which is why we duplicate
+        #   AdminDashboardSidebarRead here, otherwise the sidebar is not displayed.
         return [
-            (Allow, "group:admins", "admin"),
-            (Allow, "group:moderators", "moderator"),
+            (
+                Allow,
+                "group:admins",
+                (
+                    Permissions.AdminUsersRead,
+                    Permissions.AdminUsersWrite,
+                    Permissions.AdminDashboardSidebarRead,
+                ),
+            ),
+            (
+                Allow,
+                "group:moderators",
+                (Permissions.AdminUsersRead, Permissions.AdminDashboardSidebarRead),
+            ),
         ]
 
     def __lt__(self, other):

warehouse/admin/templates/admin/banners/edit.html

@@ -82,12 +82,12 @@ <h3 class="card-title">Create banner</h3>
 
 
             <div class="form-group">
-              <button type="submit" class="btn btn-danger" title="{{ 'Submitting requires staff privileges' if not request.has_permission('psf_staff') }}" {{ "disabled" if not request.has_permission('psf_staff') }}>Submit</button>
+              <button type="submit" class="btn btn-danger" title="{{ 'Submitting requires staff privileges' if not request.has_permission(Permissions.AdminBannerWrite) }}" {{ "disabled" if not request.has_permission(Permissions.AdminBannerWrite) }}>Submit</button>
 
               {% if banner %}
               <a class="btn btn-info" href="{{ request.route_path('admin.banner.preview', banner_id=banner.id) }}" rel="nofollow noopener noreferrer" target="_blank"><i class="icon fa fa-eye"></i> Preview Saved</a>
 
-              <button type="button" class="btn btn-danger float-right" data-toggle="modal" data-target="#deleteBanner" {{ "disabled" if not request.has_permission('psf_staff') }} value=>
+              <button type="button" class="btn btn-danger float-right" data-toggle="modal" data-target="#deleteBanner" {{ "disabled" if not request.has_permission(Permissions.AdminBannerWrite) }} value=>
                 <i class="icon fa fa-trash"></i> Delete banner
               </button>
               {% endif %}

warehouse/admin/templates/admin/banners/list.html

@@ -22,7 +22,7 @@
 {% block content %}
 
 <div class="card card-primary card-outline">
-  {% if request.has_permission('psf_staff') %}
+  {% if request.has_permission(Permissions.AdminBannerWrite) %}
   <div class="card-header">
     <a class="btn btn-primary" href="{{ request.route_path('admin.banner.create') }}"><i class="fa fa-plus"></i> Create Banner</a>
   </div>

warehouse/admin/templates/admin/base.html

@@ -111,7 +111,8 @@
       <!-- Sidebar Menu -->
       <nav class="mt-2">
         <ul class="nav nav-pills nav-sidebar flex-column" data-widget="treeview" role="menu" data-accordion="false">
-          {% if request.has_permission('moderator') %}
+          {# TODO: Is there a cleaner way to show/hide the allowed sections? #}
+          {% if request.has_permission(Permissions.AdminDashboardSidebarRead) %}
           <li class="nav-item">
             <a href="{{ request.route_path('admin.organization_application.list') }}" class="nav-link">
               <i class="fa fa-sitemap nav-icon"></i> <p>Organization Applications <span class="right badge badge-danger">New</span></p>
@@ -163,6 +164,7 @@
             </a>
           </li>
           {% endif %}
+          {# TODO: This is implicitly allowed for PSF Staff #}
           <li class="nav-item">
             <a href="{{ request.route_path('admin.banner.list') }}" class="nav-link">
               <i class="fa fa-quote-right nav-icon"></i> <p>Banners</p>

warehouse/admin/templates/admin/emails/list.html

@@ -36,6 +36,7 @@
 {% endblock %}
 
 {% block content %}
+{% if request.has_permission(Permissions.AdminEmailsWrite) %}
 <form method="POST" action={{ request.route_path('admin.emails.mass') }} enctype="multipart/form-data">
   <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
   <div class="card card-primary card-outline">
@@ -58,6 +59,7 @@ <h3 class="card-title">Send mass emails</h3>
     </div>
   </div>
 </form>
+{% endif %}
 
 <div class="card card-primary card-outline">
   <div class="card-header">

warehouse/admin/templates/admin/flags/index.html

@@ -46,10 +46,10 @@ <h3 class="card-title">Edit Flags</h3>
           <input name="csrf_token" type="hidden" value="{{ csrf_token }}">
           <input name="id" type="hidden" value="{{ flag.id }}">
           <td><code>{{ flag.id }}</code></td>
-          <td><input name="description" size="50" value="{{ flag.description }}" {{ "disabled" if not request.has_permission('admin') }}></td>
+          <td><input name="description" size="50" value="{{ flag.description }}" {{ "disabled" if not request.has_permission(Permissions.AdminFlagsWrite) }}></td>
           <td>{{ flag.notify }}</td>
-          <td><input name="enabled" type="checkbox" {{ "disabled" if not request.has_permission('admin') }} {{ 'checked' if flag.enabled else '' }}></td>
-          <td><input type="submit" title="{{ 'Flag changes require superuser privileges' if not request.has_permission('admin') }}" value="Save" {{ "disabled" if not request.has_permission('admin') }}></td>
+          <td><input name="enabled" type="checkbox" {{ "disabled" if not request.has_permission(Permissions.AdminFlagsWrite) }} {{ 'checked' if flag.enabled else '' }}></td>
+          <td><input type="submit" title="{{ 'Flag changes require superuser privileges' if not request.has_permission(Permissions.AdminFlagsWrite) }}" value="Save" {{ "disabled" if not request.has_permission(Permissions.AdminFlagsWrite) }}></td>
         </form>
       </tr>
       {% endfor %}

warehouse/admin/templates/admin/macaroons/detail.html

@@ -70,7 +70,7 @@
   </div>
   <div class="card-footer">
     <div class="form-group">
-      <button type="button" class="btn btn-danger float-right" data-toggle="modal" data-target="#deleteMacaroon" {{ "disabled" if not request.has_permission('admin') }} value=>
+      <button type="button" class="btn btn-danger float-right" data-toggle="modal" data-target="#deleteMacaroon" {{ "disabled" if not request.has_permission(Permissions.AdminMacaroonsWrite) }} value=>
         <i class="icon fa fa-trash"></i> Delete Macaroon
       </button>
     </div>

warehouse/admin/templates/admin/organization_applications/detail.html

@@ -13,24 +13,6 @@
 -#}
 {% extends "admin/base.html" %}
 
-{% macro render_field(label, field, input_id, placeholder=None, class=None) %}
-<div class="form-group{% if field.errors %} has-error{% endif %}">
-  <label for="{{ input_id }}" class="col-sm-2 control-label">{{ label }}</label>
-
-  <div class="col-sm-10">
-    {{ field(id=input_id, class=class, placeholder=placeholder, disabled=(not request.has_permission('admin')))}}
-
-    {% if field.errors %}
-    <div class="help-block">
-    {% for error in field.errors %}
-      <div>{{ error }}</div>
-    {% endfor %}
-    </div>
-    {% endif %}
-  </div>
-</div>
-{% endmacro %}
-
 {% block title %}Application for {{ organization_application.name }}{% endblock %}
 
 {% block breadcrumb %}
@@ -63,10 +45,10 @@ <h2 class="card-title">Actions</h2>
             Approve or decline request for organization named <strong>{{ organization_application.name }}</strong>?
           </p>
           <div class="form-group">
-            <button type="button" class="btn {{ 'btn-primary' if organization_application.is_approved != True else '' }} btn-block" data-toggle="modal" data-target="#approveModal" {{ 'disabled' if not request.has_permission('admin') or organization_application.is_approved == True }} title="{{ 'Admin permission required to approve organization request' if not request.has_permission('admin') else 'Organization request already approved' if organization_application.is_approved == True else 'Approve organization request' }}">
+            <button type="button" class="btn {{ 'btn-primary' if organization_application.is_approved != True else '' }} btn-block" data-toggle="modal" data-target="#approveModal" {{ 'disabled' if not request.has_permission(Permissions.AdminOrganizationsWrite) or organization_application.is_approved == True }} title="{{ 'Admin permission required to approve organization request' if not request.has_permission(Permissions.AdminOrganizationsWrite) else 'Organization request already approved' if organization_application.is_approved == True else 'Approve organization request' }}">
               <i class="icon fa fa-check"></i> Approve
             </button>
-            <button type="button" class="btn {{ 'btn-danger' if organization_application.is_approved != False else '' }} btn-block" data-toggle="modal" data-target="#declineModal" {{ 'disabled' if not request.has_permission('admin') or organization_application.is_approved == False }} title="{{ 'Admin permission required to decline organization request' if not request.has_permission('admin') else 'Organization request already declined' if organization_application.is_approved == False else 'Decline organization request' }}">
+            <button type="button" class="btn {{ 'btn-danger' if organization_application.is_approved != False else '' }} btn-block" data-toggle="modal" data-target="#declineModal" {{ 'disabled' if not request.has_permission(Permissions.AdminOrganizationsWrite) or organization_application.is_approved == False }} title="{{ 'Admin permission required to decline organization request' if not request.has_permission(Permissions.AdminOrganizationsWrite) else 'Organization request already declined' if organization_application.is_approved == False else 'Decline organization request' }}">
               <i class="icon fa fa-times"></i> Decline
             </button>
             {% if user %}