Skip to content

move all record_event instances to send a request #13744

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

Merged
merged 5 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,9 @@ def user_service(db_session, metrics, remote_addr):


@pytest.fixture
def project_service(db_session, remote_addr, metrics, ratelimiters=None):
def project_service(db_session, metrics, ratelimiters=None):
return packaging_services.ProjectService(
db_session, remote_addr, metrics, ratelimiters=ratelimiters
db_session, metrics, ratelimiters=ratelimiters
)


Expand Down
8 changes: 6 additions & 2 deletions tests/unit/accounts/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def test_with_invalid_password(self, pyramid_request, pyramid_services):
pretend.call(
tag=EventTag.Account.LoginFailure,
ip_address="1.2.3.4",
request=pyramid_request,
additional={"reason": "invalid_password", "auth_method": "basic"},
)
]
Expand Down Expand Up @@ -245,6 +246,7 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service
assert user.record_event.calls == [
pretend.call(
ip_address="1.2.3.4",
request=pyramid_request,
tag=EventTag.Account.LoginSuccess,
additional={"auth_method": "basic"},
)
Expand All @@ -267,7 +269,7 @@ def test_via_basic_auth_compromised(
),
is_disabled=pretend.call_recorder(lambda user_id: (False, None)),
disable_password=pretend.call_recorder(
lambda user_id, reason=None, ip_address="127.0.0.1": None
lambda user_id, request, reason=None: None
),
)
breach_service = pretend.stub(
Expand Down Expand Up @@ -301,7 +303,9 @@ def test_via_basic_auth_compromised(
]
assert service.disable_password.calls == [
pretend.call(
2, reason=DisableReason.CompromisedPassword, ip_address="1.2.3.4"
2,
pyramid_request,
reason=DisableReason.CompromisedPassword,
)
]
assert send_email.calls == [pretend.call(pyramid_request, user)]
Expand Down
10 changes: 8 additions & 2 deletions tests/unit/accounts/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ def test_validate_password_notok(self, db_session):
pretend.call(
tag=EventTag.Account.LoginFailure,
ip_address=request.remote_addr,
request=request,
additional={"reason": "invalid_password"},
)
]
Expand Down Expand Up @@ -258,7 +259,7 @@ def test_password_breached(self, monkeypatch):
get_user=lambda _: user,
check_password=lambda userid, pw, tags=None: True,
disable_password=pretend.call_recorder(
lambda user_id, reason=None, ip_address="127.0.0.1": None
lambda user_id, request, reason=None: None
),
is_disabled=lambda userid: (False, None),
)
Expand All @@ -276,7 +277,9 @@ def test_password_breached(self, monkeypatch):
assert form.password.errors.pop() == "Bad Password!"
assert user_service.disable_password.calls == [
pretend.call(
1, reason=DisableReason.CompromisedPassword, ip_address="1.2.3.4"
1,
request,
reason=DisableReason.CompromisedPassword,
)
]
assert send_email.calls == [pretend.call(request, user)]
Expand Down Expand Up @@ -846,6 +849,7 @@ def test_totp_secret_exists(self, pyramid_config):
pretend.call(
tag=EventTag.Account.LoginFailure,
ip_address=request.remote_addr,
request=request,
additional={"reason": "invalid_totp"},
)
]
Expand Down Expand Up @@ -944,6 +948,7 @@ def test_credential_invalid(self):
pretend.call(
tag=EventTag.Account.LoginFailure,
ip_address=request.remote_addr,
request=request,
additional={"reason": "invalid_webauthn"},
)
]
Expand Down Expand Up @@ -1045,6 +1050,7 @@ def test_invalid_recovery_code(
pretend.call(
tag=EventTag.Account.LoginFailure,
ip_address=request.remote_addr,
request=request,
additional={"reason": expected_reason},
)
]
Expand Down
22 changes: 19 additions & 3 deletions tests/unit/accounts/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
from warehouse.rate_limiting.interfaces import IRateLimiter

from ...common.db.accounts import EmailFactory, UserFactory
from ...common.db.ip_addresses import IpAddressFactory


class TestDatabaseUserService:
Expand Down Expand Up @@ -440,6 +441,10 @@ def test_get_admin_user(self, user_service):
],
)
def test_disable_password(self, user_service, reason, expected):
request = pretend.stub(
remote_addr="127.0.0.1",
ip_address=IpAddressFactory.create(),
)
user = UserFactory.create()
user.record_event = pretend.call_recorder(lambda *a, **kw: None)

Expand All @@ -448,13 +453,14 @@ def test_disable_password(self, user_service, reason, expected):
assert user.password != "!"

# Now we'll actually test our disable function.
user_service.disable_password(user.id, reason=reason)
user_service.disable_password(user.id, reason=reason, request=request)
assert user.password == "!"

assert user.record_event.calls == [
pretend.call(
tag=EventTag.Account.PasswordDisabled,
ip_address="127.0.0.1",
request=request,
additional={"reason": expected},
)
]
Expand All @@ -464,19 +470,29 @@ def test_disable_password(self, user_service, reason, expected):
[(True, None), (True, DisableReason.CompromisedPassword), (False, None)],
)
def test_is_disabled(self, user_service, disabled, reason):
request = pretend.stub(
remote_addr="127.0.0.1",
ip_address=IpAddressFactory.create(),
)
user = UserFactory.create()
user_service.update_user(user.id, password="foo")
if disabled:
user_service.disable_password(user.id, reason=reason)
user_service.disable_password(user.id, reason=reason, request=request)
assert user_service.is_disabled(user.id) == (disabled, reason)

def test_is_disabled_user_frozen(self, user_service):
user = UserFactory.create(is_frozen=True)
assert user_service.is_disabled(user.id) == (True, DisableReason.AccountFrozen)

def test_updating_password_undisables(self, user_service):
request = pretend.stub(
remote_addr="127.0.0.1",
ip_address=IpAddressFactory.create(),
)
user = UserFactory.create()
user_service.disable_password(user.id, reason=DisableReason.CompromisedPassword)
user_service.disable_password(
user.id, reason=DisableReason.CompromisedPassword, request=request
)
assert user_service.is_disabled(user.id) == (
True,
DisableReason.CompromisedPassword,
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/accounts/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ def test_post_validate_redirects(
pretend.call(
tag=EventTag.Account.LoginSuccess,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
additional={"two_factor_method": None, "two_factor_label": None},
)
]
Expand Down Expand Up @@ -351,6 +352,7 @@ def test_post_validate_no_redirects(
pretend.call(
tag=EventTag.Account.LoginSuccess,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
additional={"two_factor_method": None, "two_factor_label": None},
)
]
Expand Down Expand Up @@ -714,6 +716,7 @@ def test_totp_auth(
pretend.call(
tag=EventTag.Account.LoginSuccess,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
additional={"two_factor_method": "totp", "two_factor_label": "totp"},
)
]
Expand Down Expand Up @@ -1157,6 +1160,7 @@ def test_recovery_code_auth(self, monkeypatch, pyramid_request, redirect_url):
pretend.call(
tag=EventTag.Account.LoginSuccess,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
additional={
"two_factor_method": "recovery-code",
"two_factor_label": None,
Expand All @@ -1165,6 +1169,7 @@ def test_recovery_code_auth(self, monkeypatch, pyramid_request, redirect_url):
pretend.call(
tag=EventTag.Account.RecoveryCodesUsed,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
),
]
assert pyramid_request.session.flash.calls == [
Expand Down Expand Up @@ -1411,11 +1416,13 @@ def _find_service(service=None, name=None, context=None):
pretend.call(
tag=EventTag.Account.AccountCreate,
ip_address=db_request.remote_addr,
request=db_request,
additional={"email": "[email protected]"},
),
pretend.call(
tag=EventTag.Account.LoginSuccess,
ip_address=db_request.remote_addr,
request=db_request,
additional={"two_factor_method": None, "two_factor_label": None},
),
]
Expand Down Expand Up @@ -1524,6 +1531,7 @@ def test_request_password_reset(
pretend.call(
tag=EventTag.Account.PasswordResetRequest,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
)
]

Expand Down Expand Up @@ -1588,6 +1596,7 @@ def test_request_password_reset_with_email(
pretend.call(
tag=EventTag.Account.PasswordResetRequest,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
)
]
assert user_service.ratelimiters["password.reset"].test.calls == [
Expand Down Expand Up @@ -1663,6 +1672,7 @@ def test_request_password_reset_with_non_primary_email(
pretend.call(
tag=EventTag.Account.PasswordResetRequest,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
)
]
assert user_service.ratelimiters["password.reset"].test.calls == [
Expand Down Expand Up @@ -1762,6 +1772,7 @@ def test_password_reset_prohibited(
pretend.call(
tag=EventTag.Account.PasswordResetAttempt,
ip_address=pyramid_request.remote_addr,
request=pyramid_request,
)
]

Expand Down Expand Up @@ -3569,6 +3580,7 @@ def test_add_pending_github_oidc_publisher(self, monkeypatch, db_request):
pretend.call(
tag=EventTag.Account.PendingOIDCPublisherAdded,
ip_address="1.2.3.4",
request=db_request,
additional={
"project": "some-project-name",
"publisher": pending_publisher.publisher_name,
Expand Down Expand Up @@ -3802,6 +3814,7 @@ def test_delete_pending_oidc_publisher(self, monkeypatch, db_request):
pretend.call(
tag=EventTag.Account.PendingOIDCPublisherRemoved,
ip_address="1.2.3.4",
request=db_request,
additional={
"project": "some-project-name",
"publisher": "GitHub",
Expand Down
6 changes: 4 additions & 2 deletions tests/unit/admin/views/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,9 @@ def test_resets_password(self, db_request, monkeypatch):
db_request.user = UserFactory.create()
service = pretend.stub(
find_userid=pretend.call_recorder(lambda username: user.username),
disable_password=pretend.call_recorder(lambda userid, reason: None),
disable_password=pretend.call_recorder(
lambda userid, request, reason: None
),
)
db_request.find_service = pretend.call_recorder(lambda iface, context: service)

Expand All @@ -419,7 +421,7 @@ def test_resets_password(self, db_request, monkeypatch):
]
assert send_email.calls == [pretend.call(db_request, user)]
assert service.disable_password.calls == [
pretend.call(user.id, reason=DisableReason.CompromisedPassword)
pretend.call(user.id, db_request, reason=DisableReason.CompromisedPassword)
]
assert db_request.route_path.calls == [
pretend.call("admin.user.detail", username=user.username)
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/email/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,11 @@ class FakeUser:
def __init__(self):
self.events = []

def record_event(self, tag, ip_address, additional):
def record_event(self, tag, ip_address, request=None, additional=None):
self.events.append(
{
"ip_address": ip_address,
"request": request,
"tag": tag,
"additional": additional,
}
Expand Down Expand Up @@ -382,6 +383,7 @@ def get_user(self, user_id):
assert user_service.user.events == [
{
"tag": "account:email:sent",
"request": request,
"ip_address": request.remote_addr,
"additional": {
"from_": "[email protected]",
Expand Down
1 change: 1 addition & 0 deletions tests/unit/integration/github/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@ def metrics_increment(key):
pretend.call(
tag=EventTag.Account.APITokenRemovedLeak,
ip_address=request.remote_addr,
request=request,
additional={
"macaroon_id": "12",
"public_url": "http://example.com",
Expand Down
Loading