refactor: separate editing from publishing permissions

Also no need any more for committer permissions to be configurable.
It is not used in practice.

Author: fricklerhandwerk

infra/production.nix

@@ -95,7 +95,6 @@ in
       GH_SECURITY_TEAM = "security";
       GH_COMMITTERS_TEAM = "nixpkgs-committers";
       GH_ISSUES_LABELS = [ "1.severity: security" ];
-      GH_ISSUES_COMMITTERS_ONLY = true;
       EMAIL_BACKEND = "django.core.mail.backends.smtp.EmailBackend";
       EMAIL_HOST = "umbriel.nixos.org";
       EMAIL_PORT = 465;

src/project/settings.py

@@ -121,15 +121,6 @@ class DjangoSettings(BaseModel):
             This is used as a safety measure during development. Set to True in production.
             """
         )
-        GH_ISSUES_COMMITTERS_ONLY: bool = Field(
-            description="""
-            When set to True, only committers and admins can publish
-            suggestions to create GitHub issues. This also affects who can
-            create suggestions and add/remove maintainers.
-            Setting this to False, will also allow maintainers to create issues.
-            """,
-            default=True,
-        )
         GH_ORGANIZATION: str = Field(
             description="""
             The GitHub organisation from which to get team membership.

src/shared/auth/utils.py

@@ -20,9 +20,11 @@ def ismaintainer(user: Any) -> bool:
     ).exists()
 
 
-def can_publish_github_issue(user: Any) -> bool:
-    return (
-        isadmin(user)
-        or iscommitter(user)
-        or (not settings.GH_ISSUES_COMMITTERS_ONLY and ismaintainer(user))
-    )
+def can_edit_suggestion(user: Any) -> bool:
+    # XXX(@fricklerhandwerk): A subset of editing operations could be opened up to maintainers
+    return isadmin(user) or iscommitter(user)
+
+
+def can_publish_issue(user: Any) -> bool:
+    # XXX(@fricklerhandwerk): This could be staff only if needed
+    return isadmin(user) or iscommitter(user)

src/webview/suggestions/views/base.py

@@ -7,7 +7,7 @@
 from django.urls import resolve
 from django.views.generic import TemplateView
 
-from shared.auth import can_publish_github_issue
+from shared.auth import can_edit_suggestion
 from shared.models.linkage import (
     CVEDerivationClusterProposal,
 )
@@ -106,12 +106,13 @@ def __init__(self, response: HttpResponse) -> None:
     def _check_access_rights_and_get_suggestion(
         self, request: HttpRequest, suggestion_id: int
     ) -> tuple[CVEDerivationClusterProposal, SuggestionContext]:
-        if not request.user or not can_publish_github_issue(request.user):
+        can_edit = can_edit_suggestion(self.request.user)
+
+        if not request.user or not can_edit:
             raise self.ForbiddenOperationError(HttpResponseForbidden())
 
         # Get suggestion context
         suggestion = fetch_suggestion(suggestion_id)
-        can_edit = can_publish_github_issue(self.request.user)
         suggestion_context = get_suggestion_context(suggestion, can_edit=can_edit)
 
         # Validate that the suggestion status allows package editing

src/webview/suggestions/views/detail.py

@@ -5,7 +5,7 @@
 from django.urls import reverse
 from django.views.generic import DetailView, View
 
-from shared.auth import can_publish_github_issue
+from shared.auth import can_edit_suggestion
 from shared.models.issue import NixpkgsIssue
 from shared.models.linkage import (
     CVEDerivationClusterProposal,
@@ -21,7 +21,7 @@ class SuggestionDetailView(DetailView):
 
     def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
         context = super().get_context_data(**kwargs)
-        can_edit = can_publish_github_issue(self.request.user)
+        can_edit = can_edit_suggestion(self.request.user)
         context.update(
             {
                 "suggestion_context": get_suggestion_context(

src/webview/suggestions/views/lists.py

@@ -7,7 +7,7 @@
 from django.http import Http404
 from django.views.generic import ListView
 
-from shared.auth import can_publish_github_issue
+from shared.auth import can_edit_suggestion
 from shared.models.linkage import (
     CVEDerivationClusterProposal,
 )
@@ -61,7 +61,7 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
 
         # Convert suggestions to SuggestionContext objects for the current page
         suggestion_contexts = []
-        can_edit = can_publish_github_issue(self.request.user)
+        can_edit = can_edit_suggestion(self.request.user)
         # FIXME(@fricklerhandwerk): This is very slow (scales with number of events in the activity log), it should batch all related queries and do the wiring in Python.
         for suggestion in page_obj.object_list:
             suggestion_context = get_suggestion_context(suggestion, can_edit=can_edit)

src/webview/suggestions/views/status.py

@@ -2,7 +2,7 @@
 from django.http import HttpRequest, HttpResponse, HttpResponseForbidden
 from django.urls import reverse
 
-from shared.auth import can_publish_github_issue
+from shared.auth import can_edit_suggestion, can_publish_issue
 from shared.github import create_gh_issue
 from shared.models import (
     NixpkgsIssue,
@@ -29,7 +29,7 @@ class UpdateSuggestionStatusView(SuggestionBaseView):
 
     def post(self, request: HttpRequest, suggestion_id: int) -> HttpResponse:
         """Handle status change requests."""
-        can_edit = can_publish_github_issue(request.user)
+        can_edit = can_edit_suggestion(request.user)
         if not request.user or not can_edit:
             return HttpResponseForbidden()
 
@@ -69,6 +69,11 @@ def post(self, request: HttpRequest, suggestion_id: int) -> HttpResponse:
             if new_status == "accepted":
                 suggestion.status = CVEDerivationClusterProposal.Status.ACCEPTED
             elif new_status == "published":
+                # FIXME(@fricklerhandwerk): The UI should make hitting this code path impossible, and indicate the permission situation.
+                # Not a real issue at the moment since `can_edit` and `can_publish` happen to coincide.
+                if not can_publish_issue(request.user):
+                    return HttpResponseForbidden()
+
                 try:
                     with transaction.atomic():
                         tracker_issue = NixpkgsIssue.create_nixpkgs_issue(suggestion)