Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2fa disabling ask password #6408

Merged
merged 19 commits into from
Aug 24, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions tests/unit/manage/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand All @@ -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()
29 changes: 18 additions & 11 deletions tests/unit/manage/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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/"
Expand All @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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",
)

Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions warehouse/manage/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
16 changes: 12 additions & 4 deletions warehouse/manage/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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"))

Expand Down Expand Up @@ -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,
),
}

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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() === "";
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
5 changes: 5 additions & 0 deletions warehouse/static/sass/blocks/_modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@
&__form {
label {
font-weight: bold;

input {
width: auto;
min-width: auto;
}
}

input {
Expand Down
13 changes: 7 additions & 6 deletions warehouse/templates/manage/account.html
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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 %}
Expand Down
Loading