Persist create token form params on error

Author: Luka Sterbic

tests/unit/manage/test_forms.py

@@ -340,7 +340,7 @@ def test_creation(self):
 
     def test_validate_description_missing(self):
         form = forms.CreateMacaroonForm(
-            data={"token_scope": "scope:user"},
+            data={"token_scopes": ["scope:user"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(),
             project_names=pretend.stub(),
@@ -351,7 +351,7 @@ def test_validate_description_missing(self):
 
     def test_validate_description_in_use(self):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": "scope:user"},
+            data={"description": "dummy", "token_scopes": ["scope:user"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(
                 get_macaroon_by_description=lambda *a: pretend.stub()
@@ -371,47 +371,61 @@ def test_validate_token_scope_missing(self):
         )
 
         assert not form.validate()
-        assert form.token_scope.errors.pop() == "Specify the token scope"
+        assert form.token_scopes.errors.pop() == "Specify the token scope"
 
     def test_validate_token_scope_unspecified(self):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": "scope:unspecified"},
+            data={"description": "dummy", "token_scopes": ["scope:unspecified"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
             project_names=pretend.stub(),
         )
 
         assert not form.validate()
-        assert form.token_scope.errors.pop() == "Specify the token scope"
+        assert form.token_scopes.errors.pop() == "Specify the token scope"
 
     @pytest.mark.parametrize(
         ("scope"), ["not a real scope", "scope:project", "scope:foo:bar"]
     )
     def test_validate_token_scope_invalid_format(self, scope):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": scope},
+            data={"description": "dummy", "token_scopes": [scope]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
             project_names=pretend.stub(),
         )
 
         assert not form.validate()
-        assert form.token_scope.errors.pop() == f"Unknown token scope: {scope}"
+        assert form.token_scopes.errors.pop() == f"Unknown token scope: {scope}"
 
     def test_validate_token_scope_invalid_project(self):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": "scope:project:foo"},
+            data={"description": "dummy", "token_scopes": ["scope:project:foo"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
             project_names=["bar"],
         )
 
         assert not form.validate()
-        assert form.token_scope.errors.pop() == "Unknown or invalid project name: foo"
+        assert form.token_scopes.errors.pop() == "Unknown or invalid project name: foo"
+
+    def test_validate_token_scope_user_and_project(self):
+        form = forms.CreateMacaroonForm(
+            data={
+                "description": "dummy",
+                "token_scopes": ["scope:project:bar", "scope:user"],
+            },
+            user_id=pretend.stub(),
+            macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
+            project_names=["bar"],
+        )
+
+        assert not form.validate()
+        assert form.token_scopes.errors.pop() == "Mixed user and project scopes"
 
     def test_validate_token_scope_valid_user(self):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": "scope:user"},
+            data={"description": "dummy", "token_scopes": ["scope:user"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
             project_names=pretend.stub(),
@@ -421,14 +435,27 @@ def test_validate_token_scope_valid_user(self):
 
     def test_validate_token_scope_valid_project(self):
         form = forms.CreateMacaroonForm(
-            data={"description": "dummy", "token_scope": "scope:project:foo"},
+            data={"description": "dummy", "token_scopes": ["scope:project:foo"]},
             user_id=pretend.stub(),
             macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
             project_names=["foo"],
         )
 
         assert form.validate()
 
+    def test_validate_token_scope_valid_projects(self):
+        form = forms.CreateMacaroonForm(
+            data={
+                "description": "dummy",
+                "token_scopes": ["scope:project:foo", "scope:project:bar"],
+            },
+            user_id=pretend.stub(),
+            macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
+            project_names=["foo", "bar"],
+        )
+
+        assert form.validate()
+
 
 class TestDeleteMacaroonForm:
     def test_creation(self):

tests/unit/manage/test_views.py

@@ -1427,7 +1427,7 @@ def test_create_macaroon_invalid_form(self, monkeypatch):
             create_macaroon=pretend.call_recorder(lambda *a, **kw: pretend.stub())
         )
         request = pretend.stub(
-            POST={},
+            POST=MultiDict({"description": "dummy"}),
             user=pretend.stub(id=pretend.stub(), has_primary_verified_email=True),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,
@@ -1468,7 +1468,7 @@ def test_create_macaroon(self, monkeypatch):
             )
         )
         request = pretend.stub(
-            POST={},
+            POST=MultiDict({"description": "dummy"}),
             domain=pretend.stub(),
             user=pretend.stub(id=pretend.stub(), has_primary_verified_email=True),
             find_service=lambda interface, **kw: {
@@ -1523,7 +1523,7 @@ def test_delete_macaroon_invalid_form(self, monkeypatch):
             delete_macaroon=pretend.call_recorder(lambda id: pretend.stub())
         )
         request = pretend.stub(
-            POST={},
+            POST=MultiDict({"description": "dummy"}),
             route_path=pretend.call_recorder(lambda x: pretend.stub()),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,

warehouse/manage/forms.py

@@ -186,7 +186,7 @@ def validate_label(self, field):
 
 
 class CreateMacaroonForm(forms.Form):
-    __params__ = ["description", "token_scope"]
+    __params__ = ["description", "token_scopes"]
 
     def __init__(self, *args, user_id, macaroon_service, project_names, **kwargs):
         super().__init__(*args, **kwargs)
@@ -203,8 +203,13 @@ def __init__(self, *args, user_id, macaroon_service, project_names, **kwargs):
         ]
     )
 
-    token_scope = wtforms.StringField(
-        validators=[wtforms.validators.DataRequired(message="Specify the token scope")]
+    token_scopes = wtforms.FieldList(
+        wtforms.StringField(
+            "scope",
+            validators=[
+                wtforms.validators.DataRequired(message="Specify the token scope")
+            ],
+        )
     )
 
     def validate_description(self, field):
@@ -216,34 +221,41 @@ def validate_description(self, field):
         ):
             raise wtforms.validators.ValidationError("API token name already in use")
 
-    def validate_token_scope(self, field):
-        scope = field.data
+    def validate_token_scopes(self, field):
+        scopes = field.data
+        if not scopes:
+            raise wtforms.ValidationError(f"Specify the token scope")
 
-        try:
-            _, scope_kind = scope.split(":", 1)
-        except ValueError:
-            raise wtforms.ValidationError(f"Unknown token scope: {scope}")
+        self.validated_scope = {"projects": []}
 
-        if scope_kind == "unspecified":
-            raise wtforms.ValidationError(f"Specify the token scope")
+        for scope in scopes:
+            try:
+                _, scope_kind = scope.split(":", 1)
+            except ValueError:
+                raise wtforms.ValidationError(f"Unknown token scope: {scope}")
 
-        if scope_kind == "user":
-            self.validated_scope = scope_kind
-            return
+            if scope_kind == "unspecified":
+                raise wtforms.ValidationError(f"Specify the token scope")
 
-        try:
-            scope_kind, scope_value = scope_kind.split(":", 1)
-        except ValueError:
-            raise wtforms.ValidationError(f"Unknown token scope: {scope}")
-
-        if scope_kind != "project":
-            raise wtforms.ValidationError(f"Unknown token scope: {scope}")
-        if scope_value not in self.project_names:
-            raise wtforms.ValidationError(
-                f"Unknown or invalid project name: {scope_value}"
-            )
+            if scope_kind == "user":
+                if len(scopes) != 1:
+                    raise wtforms.ValidationError(f"Mixed user and project scopes")
+                self.validated_scope = scope_kind
+                return
+
+            try:
+                scope_kind, scope_value = scope_kind.split(":", 1)
+            except ValueError:
+                raise wtforms.ValidationError(f"Unknown token scope: {scope}")
+
+            if scope_kind != "project":
+                raise wtforms.ValidationError(f"Unknown token scope: {scope}")
+            if scope_value not in self.project_names:
+                raise wtforms.ValidationError(
+                    f"Unknown or invalid project name: {scope_value}"
+                )
 
-        self.validated_scope = {"projects": [scope_value]}
+            self.validated_scope["projects"].append(scope_value)
 
 
 class DeleteMacaroonForm(forms.Form):

warehouse/manage/views.py

@@ -585,7 +585,8 @@ def create_macaroon(self):
             return HTTPSeeOther(self.request.route_path("manage.account"))
 
         form = CreateMacaroonForm(
-            **self.request.POST,
+            description=self.request.POST.getone("description"),
+            token_scopes=self.request.POST.getall("token_scopes"),
             user_id=self.request.user.id,
             macaroon_service=self.macaroon_service,
             project_names=self.project_names,

warehouse/static/sass/blocks/_form-group.scss

@@ -44,6 +44,11 @@
     max-width: 100%;
   }
 
+  &__multiselect {
+    height: 200px;
+    overflow: auto;
+  }
+
   &__text {
     font-size: 20px;
     padding: 4px 0 8px;

warehouse/templates/manage/token.html

@@ -35,7 +35,9 @@ <h2>Token for "{{ macaroon.description }}"</h2>
       {% if macaroon.caveats.permissions == "user" %}
       <strong>Scope:</strong> Entire account (all projects)
       {% else %}
-      <strong>Scope:</strong> Project "{{ macaroon.caveats.permissions.projects[0] }}"
+      {% for project in macaroon.caveats.permissions.projects %}
+        <strong>Scope:</strong> Project "{{ project }}"<br>
+      {% endfor %}
       {% endif %}
     </p>
     <p>For instructions on how to use this token, <a href="/help#apitoken">visit the PyPI help page</a>.</p>
@@ -85,16 +87,20 @@ <h2>Add another token</h2>
         <p class="form-group__text">Upload packages</p>
       </div>
       <div class="form-group">
-        <label for="token_scope" class="form-group__label">Scope</label>
-        <select name="token_scope" id="token_scope" class="form-group__input" aria-describedby="token_scope-errors">
-          <option disabled {{ "" if create_macaroon_form.token_scope.data else "selected" }} value="scope:unspecified">Select scope...</option>
-          <option value="scope:user">Entire account (all projects)</option>
+        <label for="token_scope" class="form-group__label">
+          Scope
+          {% if create_macaroon_form.token_scope.flags.required %}
+          <span class="form-group__required">(required)</span>
+          {% endif %}
+        <select multiple name="token_scopes" id="token_scopes" class="form-group__input form-group__multiselect" aria-describedby="token_scopes-errors">
+          <option disabled {{ "" if create_macaroon_form.token_scopes.data else "selected" }} value="scope:unspecified">Select scope...</option>
+          <option {{ "selected" if "scope:user" in create_macaroon_form.token_scopes.data else "" }} value="scope:user">Entire account (all projects)</option>
         {% for project in project_names %}
           {% set project_value = 'scope:project:' + project %}
-          <option value="{{ project_value }}" {{ "selected" if project_value == create_macaroon_form.token_scope.data else "" }}>Project: {{ project }}</option>
+          <option value="{{ project_value }}" {{ "selected" if project_value in create_macaroon_form.token_scopes.data else "" }}>Project: {{ project }}</option>
         {% endfor %}
         </select>
-        {{ field_errors(create_macaroon_form.token_scope) }}
+        {{ field_errors(create_macaroon_form.token_scopes) }}
       </div>
       <div id="api-token-scope-warning" class="callout-block callout-block--warning" hidden>
         <h3 class="callout-block__heading">Proceed with caution!</h3>