Ask for password when disabling 2FA and API tokens(#6408)

tests/unit/manage/test_forms.py

@@ -167,6 +167,19 @@ def test_creation(self):
 
         assert form.user_service is user_service
 
+    def test_validate_confirm_password(self):
+        user_service = pretend.stub(
+            find_userid=pretend.call_recorder(lambda userid: 1),
+            check_password=pretend.call_recorder(
+                lambda userid, password, tags=None: True
+            ),
+        )
+        form = forms.DeleteTOTPForm(
+            username="username", user_service=user_service, password="password"
+        )
+
+        assert form.validate()
+
 
 class TestProvisionWebAuthnForm:
     def test_creation(self):
@@ -433,16 +446,26 @@ def test_validate_token_scope_valid_project(self):
 class TestDeleteMacaroonForm:
     def test_creation(self):
         macaroon_service = pretend.stub()
-        form = forms.DeleteMacaroonForm(macaroon_service=macaroon_service)
+        user_service = pretend.stub()
+        form = forms.DeleteMacaroonForm(
+            macaroon_service=macaroon_service, user_service=user_service
+        )
 
         assert form.macaroon_service is macaroon_service
+        assert form.user_service is user_service
 
     def test_validate_macaroon_id_invalid(self):
         macaroon_service = pretend.stub(
             find_macaroon=pretend.call_recorder(lambda id: None)
         )
+        user_service = pretend.stub(
+            find_userid=lambda *a, **kw: 1, check_password=lambda *a, **kw: True
+        )
         form = forms.DeleteMacaroonForm(
-            data={"macaroon_id": pretend.stub()}, macaroon_service=macaroon_service
+            data={"macaroon_id": pretend.stub(), "password": "password"},
+            macaroon_service=macaroon_service,
+            user_service=user_service,
+            username="username",
         )
 
         assert not form.validate()
@@ -452,8 +475,14 @@ def test_validate_macaroon_id(self):
         macaroon_service = pretend.stub(
             find_macaroon=pretend.call_recorder(lambda id: pretend.stub())
         )
+        user_service = pretend.stub(
+            find_userid=lambda *a, **kw: 1, check_password=lambda *a, **kw: True
+        )
         form = forms.DeleteMacaroonForm(
-            data={"macaroon_id": pretend.stub()}, macaroon_service=macaroon_service
+            data={"macaroon_id": pretend.stub(), "password": "password"},
+            macaroon_service=macaroon_service,
+            username="username",
+            user_service=user_service,
         )
 
         assert form.validate()

tests/unit/manage/test_views.py

@@ -1080,7 +1080,7 @@ def test_delete_totp(self, monkeypatch, db_request):
             record_event=pretend.call_recorder(lambda *a, **kw: None),
         )
         request = pretend.stub(
-            POST={"confirm_username": pretend.stub()},
+            POST={"confirm_password": pretend.stub()},
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: user_service,
             user=pretend.stub(
@@ -1123,13 +1123,13 @@ def test_delete_totp(self, monkeypatch, db_request):
             )
         ]
 
-    def test_delete_totp_bad_username(self, monkeypatch, db_request):
+    def test_delete_totp_bad_password(self, monkeypatch, db_request):
         user_service = pretend.stub(
             get_totp_secret=lambda id: b"secret",
             update_user=pretend.call_recorder(lambda *a, **kw: None),
         )
         request = pretend.stub(
-            POST={"confirm_username": pretend.stub()},
+            POST={"confirm_password": pretend.stub()},
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: user_service,
             user=pretend.stub(
@@ -1151,7 +1151,7 @@ def test_delete_totp_bad_username(self, monkeypatch, db_request):
 
         assert user_service.update_user.calls == []
         assert request.session.flash.calls == [
-            pretend.call("Invalid credentials", queue="error")
+            pretend.call("Invalid credentials. Try again", queue="error")
         ]
         assert isinstance(result, HTTPSeeOther)
         assert result.headers["Location"] == "/foo/bar/"
@@ -1162,7 +1162,7 @@ def test_delete_totp_not_provisioned(self, monkeypatch, db_request):
             update_user=pretend.call_recorder(lambda *a, **kw: None),
         )
         request = pretend.stub(
-            POST={"confirm_username": pretend.stub()},
+            POST={"confirm_password": pretend.stub()},
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: user_service,
             user=pretend.stub(
@@ -1469,7 +1469,7 @@ def test_default_response(self, monkeypatch):
         )
 
         request = pretend.stub(
-            user=pretend.stub(id=pretend.stub()),
+            user=pretend.stub(id=pretend.stub(), username=pretend.stub()),
             find_service=lambda interface, **kw: {
                 IMacaroonService: pretend.stub(),
                 IUserService: pretend.stub(),
@@ -1767,14 +1767,16 @@ def test_delete_macaroon_invalid_form(self, monkeypatch):
             delete_macaroon=pretend.call_recorder(lambda id: pretend.stub())
         )
         request = pretend.stub(
-            POST={},
+            POST={"confirm_password": "password", "macaroon_id": "macaroon_id"},
             route_path=pretend.call_recorder(lambda x: pretend.stub()),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,
                 IUserService: pretend.stub(),
             }[interface],
             referer="/fake/safe/route",
             host=None,
+            user=pretend.stub(username=pretend.stub()),
+            session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
         )
 
         delete_macaroon_obj = pretend.stub(validate=lambda: False)
@@ -1790,20 +1792,25 @@ def test_delete_macaroon_invalid_form(self, monkeypatch):
         assert isinstance(result, HTTPSeeOther)
         assert result.location == "/fake/safe/route"
         assert macaroon_service.delete_macaroon.calls == []
+        assert request.session.flash.calls == [
+            pretend.call("Invalid credentials. Try again", queue="error")
+        ]
 
     def test_delete_macaroon_dangerous_redirect(self, monkeypatch):
         macaroon_service = pretend.stub(
             delete_macaroon=pretend.call_recorder(lambda id: pretend.stub())
         )
         request = pretend.stub(
-            POST={},
+            POST={"confirm_password": "password", "macaroon_id": "macaroon_id"},
             route_path=pretend.call_recorder(lambda x: "/safe/route"),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,
                 IUserService: pretend.stub(),
             }[interface],
             referer="http://google.com/",
             host=None,
+            user=pretend.stub(username=pretend.stub()),
+            session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
         )
 
         delete_macaroon_obj = pretend.stub(validate=lambda: False)
@@ -1833,7 +1840,7 @@ def test_delete_macaroon(self, monkeypatch):
         )
         user_service = pretend.stub(record_event=record_event)
         request = pretend.stub(
-            POST={},
+            POST={"confirm_password": "password", "macaroon_id": "macaroon_id"},
             route_path=pretend.call_recorder(lambda x: pretend.stub()),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,
@@ -1842,7 +1849,7 @@ def test_delete_macaroon(self, monkeypatch):
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             referer="/fake/safe/route",
             host=None,
-            user=pretend.stub(id=pretend.stub()),
+            user=pretend.stub(id=pretend.stub(), username=pretend.stub()),
             remote_addr="0.0.0.0",
         )
 
@@ -1892,7 +1899,7 @@ def test_delete_macaroon_records_events_for_each_project(self, monkeypatch):
         )
         user_service = pretend.stub(record_event=record_event)
         request = pretend.stub(
-            POST={},
+            POST={"confirm_password": pretend.stub(), "macaroon_id": pretend.stub()},
             route_path=pretend.call_recorder(lambda x: pretend.stub()),
             find_service=lambda interface, **kw: {
                 IMacaroonService: macaroon_service,

warehouse/manage/forms.py

@@ -87,9 +87,9 @@ def __init__(self, *args, user_service, **kwargs):
         self.user_service = user_service
 
 
-class DeleteTOTPForm(UsernameMixin, forms.Form):
+class DeleteTOTPForm(UsernameMixin, PasswordMixin, forms.Form):
 
-    __params__ = ["confirm_username"]
+    __params__ = ["confirm_password"]
 
     def __init__(self, *args, user_service, **kwargs):
         super().__init__(*args, **kwargs)
@@ -246,15 +246,16 @@ def validate_token_scope(self, field):
         self.validated_scope = {"projects": [scope_value]}
 
 
-class DeleteMacaroonForm(forms.Form):
-    __params__ = ["macaroon_id"]
+class DeleteMacaroonForm(UsernameMixin, PasswordMixin, forms.Form):
+    __params__ = ["confirm_password", "macaroon_id"]
 
     macaroon_id = wtforms.StringField(
         validators=[wtforms.validators.DataRequired(message="Identifier required")]
     )
 
-    def __init__(self, *args, macaroon_service, **kwargs):
+    def __init__(self, *args, macaroon_service, user_service, **kwargs):
         super().__init__(*args, **kwargs)
+        self.user_service = user_service
         self.macaroon_service = macaroon_service
 
     def validate_macaroon_id(self, field):

warehouse/manage/views.py

@@ -470,7 +470,7 @@ def delete_totp(self):
             return HTTPSeeOther(self.request.route_path("manage.account"))
 
         form = DeleteTOTPForm(
-            **self.request.POST,
+            password=self.request.POST["confirm_password"],
             username=self.request.user.username,
             user_service=self.user_service,
         )
@@ -489,7 +489,7 @@ def delete_totp(self):
                 queue="success",
             )
         else:
-            self.request.session.flash("Invalid credentials", queue="error")
+            self.request.session.flash("Invalid credentials. Try again", queue="error")
 
         return HTTPSeeOther(self.request.route_path("manage.account"))
 
@@ -631,7 +631,9 @@ def default_response(self):
                 project_names=self.project_names,
             ),
             "delete_macaroon_form": DeleteMacaroonForm(
-                macaroon_service=self.macaroon_service
+                username=self.request.user.username,
+                user_service=self.user_service,
+                macaroon_service=self.macaroon_service,
             ),
         }
 
@@ -699,7 +701,11 @@ def create_macaroon(self):
     @view_config(request_method="POST", request_param=DeleteMacaroonForm.__params__)
     def delete_macaroon(self):
         form = DeleteMacaroonForm(
-            **self.request.POST, macaroon_service=self.macaroon_service
+            password=self.request.POST["confirm_password"],
+            macaroon_id=self.request.POST["macaroon_id"],
+            macaroon_service=self.macaroon_service,
+            username=self.request.user.username,
+            user_service=self.user_service,
         )
 
         if form.validate():
@@ -730,6 +736,8 @@ def delete_macaroon(self):
             self.request.session.flash(
                 f"Deleted API token '{macaroon.description}'.", queue="success"
             )
+        else:
+            self.request.session.flash("Invalid credentials. Try again", queue="error")
 
         redirect_to = self.request.referer
         if not is_safe_url(redirect_to, host=self.request.host):

warehouse/static/js/warehouse/controllers/confirm_controller.js

@@ -22,14 +22,6 @@ export default class extends Controller {
     this.buttonTarget.disabled = true;
   }
 
-  cancel() {
-    // Cancel button is a button (not an `a`) so we need to do close the
-    // modal manually
-    window.location.href = "#modal-close";
-    this.inputTarget.value = "";
-    this.buttonTarget.disabled = true;
-  }
-
   check() {
     if (this.inputTarget.value == this.buttonTarget.dataset.expected) {
       this.buttonTarget.disabled = false;

warehouse/static/js/warehouse/controllers/confirm_password_controller.js

@@ -0,0 +1,33 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+import { Controller } from "stimulus";
+
+export default class extends Controller {
+  static targets = [ "button", "password", "showPassword" ]
+
+  connect() {
+    this.buttonTarget.disabled = true;
+    this.setPasswordVisibility();
+  }
+
+  setPasswordVisibility() {
+    this.passwordTarget.type = this.showPasswordTarget.checked ? "text" : "password";
+  }
+
+  check() {
+    this.buttonTarget.disabled = this.passwordTarget.value.trim() === "";
+  }
+}

warehouse/static/js/warehouse/controllers/modal_close_controller.js

@@ -0,0 +1,28 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+import { Controller } from "stimulus";
+
+export default class extends Controller {
+  static targets = [ "input", "button" ]
+
+  cancel() {
+    // Cancel button is a button (not an `a`) so we need to do close the
+    // modal manually
+    window.location.href = "#modal-close";
+    this.inputTarget.value = "";
+    this.buttonTarget.disabled = true;
+  }
+}

warehouse/static/sass/blocks/_modal.scss

@@ -100,6 +100,11 @@
   &__form {
     label {
       font-weight: bold;
+
+      input {
+        width: auto;
+        min-width: auto;
+      }
     }
 
     input {

warehouse/templates/manage/account.html

@@ -202,8 +202,7 @@
       {% set token_warning_text %}
         <p>Applications or scripts using this token will no longer have access to PyPI.</p>
       {% endset %}
-
-      {{ confirm_modal(title=title, confirm_name="Username", confirm_string=user.username, confirm_button_label=confirm_button_label, slug=slug, extra_fields=extra_fields, action=action, custom_warning_text=token_warning_text, confirm_string_in_title=False) }}
+      {{ confirm_password_modal(title=title, confirm_button_label=confirm_button_label, slug=slug, extra_fields=extra_fields, action=action, custom_warning_text=token_warning_text) }}
 
       {# modal to view token ID #}
       <div id="view-identifier--{{ macaroon.id }}" class="modal">
@@ -421,10 +420,12 @@ <h2 class="sub-title">Two factor authentication (2FA)</h2>
             <th scope="row">Authentication application (TOTP)</th>
             <td class="table__action">
               <a href="#disable-totp" class="button button--primary">Remove</a>
-              {% set title="Remove authentication application" %}
-              {% set confirm_button_label="Remove application" %}
-              {% set action="/manage/account/totp-provision" %}
-              {{ confirm_modal(title=title, confirm_name="Username", confirm_string=user.username, confirm_button_label=confirm_button_label, slug="disable-totp", action=action, warning=False, confirm_string_in_title=False) }}
+              {# modal to remove TOTP #}
+              {% set slug="disable-totp" %}
+              {% set title="Disable 2FA by TOTP application?" %}
+              {% set action=request.route_path('manage.account.totp-provision') %}
+              {% set confirm_button_label="Disable TOTP application" %}
+              {{ confirm_password_modal(slug=slug, title=title, action=action, confirm_button_label=confirm_button_label) }}
             </td>
           </tr>
           {% endif %}