diff --git a/tests/unit/manage/test_forms.py b/tests/unit/manage/test_forms.py index 5c09dd5362fd..43513427ed75 100644 --- a/tests/unit/manage/test_forms.py +++ b/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() diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index abe717be5cd9..beda096d2fdd 100644 --- a/tests/unit/manage/test_views.py +++ b/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,7 +1767,7 @@ 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, @@ -1775,6 +1775,8 @@ def test_delete_macaroon_invalid_form(self, monkeypatch): }[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,13 +1792,16 @@ 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, @@ -1804,6 +1809,8 @@ def test_delete_macaroon_dangerous_redirect(self, monkeypatch): }[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, diff --git a/warehouse/manage/forms.py b/warehouse/manage/forms.py index 64dcf92ee79e..408fadd64d40 100644 --- a/warehouse/manage/forms.py +++ b/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): diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 720c76a5dd36..df49a3c0f270 100644 --- a/warehouse/manage/views.py +++ b/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): diff --git a/warehouse/static/js/warehouse/controllers/confirm_controller.js b/warehouse/static/js/warehouse/controllers/confirm_controller.js index a008a558ca8a..9c18c9349f37 100644 --- a/warehouse/static/js/warehouse/controllers/confirm_controller.js +++ b/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; diff --git a/warehouse/static/js/warehouse/controllers/confirm_password_controller.js b/warehouse/static/js/warehouse/controllers/confirm_password_controller.js new file mode 100644 index 000000000000..518989a25c96 --- /dev/null +++ b/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() === ""; + } +} diff --git a/warehouse/static/js/warehouse/controllers/modal_close_controller.js b/warehouse/static/js/warehouse/controllers/modal_close_controller.js new file mode 100644 index 000000000000..e8e4832d2b5d --- /dev/null +++ b/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; + } +} diff --git a/warehouse/static/sass/blocks/_modal.scss b/warehouse/static/sass/blocks/_modal.scss index 70962bce24d9..44e1309fcee9 100644 --- a/warehouse/static/sass/blocks/_modal.scss +++ b/warehouse/static/sass/blocks/_modal.scss @@ -100,6 +100,11 @@ &__form { label { font-weight: bold; + + input { + width: auto; + min-width: auto; + } } input { diff --git a/warehouse/templates/manage/account.html b/warehouse/templates/manage/account.html index 67646f9cbe0a..5c695c38fe94 100644 --- a/warehouse/templates/manage/account.html +++ b/warehouse/templates/manage/account.html @@ -202,8 +202,7 @@ {% set token_warning_text %}
Applications or scripts using this token will no longer have access to PyPI.
{% 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 #}You will not be able to re-upload a new distribution of the same type with the same version number. Consider making a new release or a post release instead.
{% endset %} - {{ confirm_modal(title, "Version", release.version, slug, index=loop.index, custom_warning_text=version_warning_text, action=action) }} + {{ confirm_modal(title, "Version", release.version, slug, custom_warning_text=version_warning_text, action=action) }} {% endfor %}