Skip to content

Commit

Permalink
(admin): Account recovery tooling (pypi#16266)
Browse files Browse the repository at this point in the history
* WIP

* surface all available urls

* add user observations

* check links using JS via camo

* update observation structure

* handle projects with no releases

* display (jankily) active recoveries on user detail

* display account recoveries on admin page

* add cancel/complete functionality for account recoveries

* lint

* support (but don't implement) alternate emails for verification

* patch up easy to fix tests

* allow overriding to email address for verification

* overridable from!

* fixup test coverage for account service and emails

* test coverage for account_recovery_{cancel,complete}

* coverage for GET of user_recover_account_initiate

* flesh out tests for account_recovery.initiate

* fix email templates

* admin: splitting email form from userform

* add an "is_support" role

* remove TODO

* admin js: optimize query selector for link checker

* mikes visual linting

* test_add_email_bypass_ratelimit: update test

* indents

* re-organize admin user-detail

* fix closing tag on account initiate recovery

* mildly repair admin ui for user detail

* be good at js, per mikes visual linter

---------

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
  • Loading branch information
ewdurbin and di authored Jul 15, 2024
1 parent 2876c73 commit c59d8bb
Show file tree
Hide file tree
Showing 27 changed files with 2,128 additions and 143 deletions.
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ services:

camo:
image: pypa/warehouse-camo:2.0.0
command: /bin/go-camo --listen=0.0.0.0:9000
command: "/bin/go-camo --listen=0.0.0.0:9000 --header 'Access-Control-Allow-Origin: http://localhost'"
ports:
- "9000:9000"
environment:
Expand Down
32 changes: 31 additions & 1 deletion tests/unit/accounts/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,18 @@ def test_acl(self, db_session):
(
Permissions.AdminUsersRead,
Permissions.AdminUsersWrite,
Permissions.AdminUsersEmailWrite,
Permissions.AdminUsersAccountRecoveryWrite,
Permissions.AdminDashboardSidebarRead,
),
),
(
"Allow",
"group:support",
(
Permissions.AdminUsersRead,
Permissions.AdminUsersEmailWrite,
Permissions.AdminUsersAccountRecoveryWrite,
Permissions.AdminDashboardSidebarRead,
),
),
Expand All @@ -184,16 +196,18 @@ def test_acl(self, db_session):
@pytest.mark.parametrize(
(
"is_superuser",
"is_support",
"is_moderator",
"is_psf_staff",
"expected",
),
[
(False, False, False, []),
(False, False, False, False, []),
(
True,
False,
False,
False,
[
"group:admins",
"group:moderators",
Expand All @@ -202,13 +216,25 @@ def test_acl(self, db_session):
],
),
(
False,
True,
False,
False,
[
"group:support",
"group:moderators",
],
),
(
False,
False,
True,
False,
["group:moderators"],
),
(
True,
False,
True,
False,
[
Expand All @@ -219,12 +245,14 @@ def test_acl(self, db_session):
],
),
(
False,
False,
False,
True,
["group:psf_staff"],
),
(
False,
False,
True,
True,
Expand All @@ -235,13 +263,15 @@ def test_acl(self, db_session):
def test_principals(
self,
is_superuser,
is_support,
is_moderator,
is_psf_staff,
expected,
):
user = User(
id=uuid.uuid4(),
is_superuser=is_superuser,
is_support=is_support,
is_moderator=is_moderator,
is_psf_staff=is_psf_staff,
)
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/accounts/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,24 @@ def test_add_email_rate_limited(self, user_service, metrics, remote_addr):
)
]

def test_add_email_bypass_ratelimit(self, user_service, metrics, remote_addr):
resets = pretend.stub()
limiter = pretend.stub(
hit=pretend.call_recorder(lambda ip: None),
test=pretend.call_recorder(lambda ip: False),
resets_in=pretend.call_recorder(lambda ip: resets),
)
user_service.ratelimiters["email.add"] = limiter

user = UserFactory.create()
new_email = user_service.add_email(user.id, "foo@example.com", ratelimit=False)

assert new_email.email == "foo@example.com"
assert not new_email.verified
assert limiter.test.calls == []
assert limiter.resets_in.calls == []
assert metrics.increment.calls == []

def test_update_user(self, user_service):
user = UserFactory.create()
new_name, password = "new username", "TestPa@@w0rd"
Expand Down
25 changes: 23 additions & 2 deletions tests/unit/admin/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ def test_includeme():
factory="warehouse.accounts.models:UserFactory",
traverse="/{username}",
),
pretend.call(
"admin.user.submit_email",
"/admin/users/{username}/emails/",
domain=warehouse,
factory="warehouse.accounts.models:UserFactory",
traverse="/{username}",
),
pretend.call(
"admin.user.add_email",
"/admin/users/{username}/add_email/",
Expand Down Expand Up @@ -91,8 +98,22 @@ def test_includeme():
traverse="/{username}",
),
pretend.call(
"admin.user.wipe_factors",
"/admin/users/{username}/wipe_factors/",
"admin.user.account_recovery.initiate",
"/admin/users/{username}/account_recovery/initiate/",
domain=warehouse,
factory="warehouse.accounts.models:UserFactory",
traverse="/{username}",
),
pretend.call(
"admin.user.account_recovery.cancel",
"/admin/users/{username}/account_recovery/cancel/",
domain=warehouse,
factory="warehouse.accounts.models:UserFactory",
traverse="/{username}",
),
pretend.call(
"admin.user.account_recovery.complete",
"/admin/users/{username}/account_recovery/complete/",
domain=warehouse,
factory="warehouse.accounts.models:UserFactory",
traverse="/{username}",
Expand Down
Loading

0 comments on commit c59d8bb

Please sign in to comment.