2fa disabling ask password -- Take 2

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

@@ -666,9 +666,15 @@ def test_delete_account(self, monkeypatch, db_request):
         jid = JournalEntryFactory.create(submitted_by=user).id
 
         db_request.user = user
-        db_request.params = {"confirm_username": user.username}
+        db_request.params = {"confirm_password": user.password}
         db_request.find_service = lambda *a, **kw: pretend.stub()
 
+        confirm_password_obj = pretend.stub(validate=lambda: True)
+        confirm_password_cls = pretend.call_recorder(
+            lambda *a, **kw: confirm_password_obj
+        )
+        monkeypatch.setattr(views, "ConfirmPasswordForm", confirm_password_cls)
+
         monkeypatch.setattr(
             views.ManageAccountViews, "default_response", pretend.stub()
         )
@@ -697,7 +703,7 @@ def test_delete_account(self, monkeypatch, db_request):
 
     def test_delete_account_no_confirm(self, monkeypatch):
         request = pretend.stub(
-            params={"confirm_username": ""},
+            params={"confirm_password": ""},
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: pretend.stub(),
         )
@@ -715,12 +721,18 @@ def test_delete_account_no_confirm(self, monkeypatch):
 
     def test_delete_account_wrong_confirm(self, monkeypatch):
         request = pretend.stub(
-            params={"confirm_username": "invalid"},
+            params={"confirm_password": "invalid"},
             user=pretend.stub(username="username"),
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: pretend.stub(),
         )
 
+        confirm_password_obj = pretend.stub(validate=lambda: False)
+        confirm_password_cls = pretend.call_recorder(
+            lambda *a, **kw: confirm_password_obj
+        )
+        monkeypatch.setattr(views, "ConfirmPasswordForm", confirm_password_cls)
+
         monkeypatch.setattr(
             views.ManageAccountViews, "default_response", pretend.stub()
         )
@@ -730,19 +742,25 @@ def test_delete_account_wrong_confirm(self, monkeypatch):
         assert view.delete_account() == view.default_response
         assert request.session.flash.calls == [
             pretend.call(
-                "Could not delete account - 'invalid' is not the same as " "'username'",
+                "Could not delete account - Invalid credentials. Please try again.",
                 queue="error",
             )
         ]
 
     def test_delete_account_has_active_projects(self, monkeypatch):
         request = pretend.stub(
-            params={"confirm_username": "username"},
+            params={"confirm_password": "password"},
             user=pretend.stub(username="username"),
             session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
             find_service=lambda *a, **kw: pretend.stub(),
         )
 
+        confirm_password_obj = pretend.stub(validate=lambda: True)
+        confirm_password_cls = pretend.call_recorder(
+            lambda *a, **kw: confirm_password_obj
+        )
+        monkeypatch.setattr(views, "ConfirmPasswordForm", confirm_password_cls)
+
         monkeypatch.setattr(
             views.ManageAccountViews, "default_response", pretend.stub()
         )
@@ -1080,7 +1098,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 +1141,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 +1169,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 +1180,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 +1487,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 +1785,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 +1810,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 +1858,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 +1867,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 +1917,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,15 +87,20 @@ def __init__(self, *args, user_service, **kwargs):
         self.user_service = user_service
 
 
-class DeleteTOTPForm(UsernameMixin, forms.Form):
+class ConfirmPasswordForm(UsernameMixin, PasswordMixin, forms.Form):
 
-    __params__ = ["confirm_username"]
+    __params__ = ["confirm_password"]
 
     def __init__(self, *args, user_service, **kwargs):
         super().__init__(*args, **kwargs)
         self.user_service = user_service
 
 
+class DeleteTOTPForm(ConfirmPasswordForm):
+    # TODO: delete?
+    pass
+
+
 class ProvisionTOTPForm(TOTPValueMixin, forms.Form):
 
     __params__ = ["totp_value"]
@@ -246,15 +251,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

@@ -43,6 +43,7 @@
     AddEmailForm,
     ChangePasswordForm,
     ChangeRoleForm,
+    ConfirmPasswordForm,
     CreateMacaroonForm,
     CreateRoleForm,
     DeleteMacaroonForm,
@@ -304,18 +305,22 @@ def change_password(self):
 
         return {**self.default_response, "change_password_form": form}
 
-    @view_config(request_method="POST", request_param=["confirm_username"])
+    @view_config(request_method="POST", request_param=DeleteTOTPForm.__params__)
     def delete_account(self):
-        username = self.request.params.get("confirm_username")
-
-        if not username:
+        confirm_password = self.request.params.get("confirm_password")
+        if not confirm_password:
             self.request.session.flash("Confirm the request", queue="error")
             return self.default_response
 
-        if username != self.request.user.username:
+        form = ConfirmPasswordForm(
+            password=confirm_password,
+            username=self.request.user.username,
+            user_service=self.user_service,
+        )
+
+        if not form.validate():
             self.request.session.flash(
-                f"Could not delete account - {username!r} is not the same as "
-                f"{self.request.user.username!r}",
+                f"Could not delete account - Invalid credentials. Please try again.",
                 queue="error",
             )
             return self.default_response
@@ -470,7 +475,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 +494,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 +636,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 +706,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 +741,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):