Skip to content

Commit 8751450

Browse files
authored
move all record_event instances to send a request (#13744)
* move all record_event instances to send a request * mandate it!!!! * neat * not used * mandate!!!
1 parent 92af409 commit 8751450

File tree

28 files changed

+314
-166
lines changed

28 files changed

+314
-166
lines changed

tests/conftest.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,9 @@ def user_service(db_session, metrics, remote_addr):
341341

342342

343343
@pytest.fixture
344-
def project_service(db_session, remote_addr, metrics, ratelimiters=None):
344+
def project_service(db_session, metrics, ratelimiters=None):
345345
return packaging_services.ProjectService(
346-
db_session, remote_addr, metrics, ratelimiters=ratelimiters
346+
db_session, metrics, ratelimiters=ratelimiters
347347
)
348348

349349

tests/unit/accounts/test_core.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ def test_with_invalid_password(self, pyramid_request, pyramid_services):
105105
pretend.call(
106106
tag=EventTag.Account.LoginFailure,
107107
ip_address="1.2.3.4",
108+
request=pyramid_request,
108109
additional={"reason": "invalid_password", "auth_method": "basic"},
109110
)
110111
]
@@ -245,6 +246,7 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service
245246
assert user.record_event.calls == [
246247
pretend.call(
247248
ip_address="1.2.3.4",
249+
request=pyramid_request,
248250
tag=EventTag.Account.LoginSuccess,
249251
additional={"auth_method": "basic"},
250252
)
@@ -267,7 +269,7 @@ def test_via_basic_auth_compromised(
267269
),
268270
is_disabled=pretend.call_recorder(lambda user_id: (False, None)),
269271
disable_password=pretend.call_recorder(
270-
lambda user_id, reason=None, ip_address="127.0.0.1": None
272+
lambda user_id, request, reason=None: None
271273
),
272274
)
273275
breach_service = pretend.stub(
@@ -301,7 +303,9 @@ def test_via_basic_auth_compromised(
301303
]
302304
assert service.disable_password.calls == [
303305
pretend.call(
304-
2, reason=DisableReason.CompromisedPassword, ip_address="1.2.3.4"
306+
2,
307+
pyramid_request,
308+
reason=DisableReason.CompromisedPassword,
305309
)
306310
]
307311
assert send_email.calls == [pretend.call(pyramid_request, user)]

tests/unit/accounts/test_forms.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ def test_validate_password_notok(self, db_session):
203203
pretend.call(
204204
tag=EventTag.Account.LoginFailure,
205205
ip_address=request.remote_addr,
206+
request=request,
206207
additional={"reason": "invalid_password"},
207208
)
208209
]
@@ -258,7 +259,7 @@ def test_password_breached(self, monkeypatch):
258259
get_user=lambda _: user,
259260
check_password=lambda userid, pw, tags=None: True,
260261
disable_password=pretend.call_recorder(
261-
lambda user_id, reason=None, ip_address="127.0.0.1": None
262+
lambda user_id, request, reason=None: None
262263
),
263264
is_disabled=lambda userid: (False, None),
264265
)
@@ -276,7 +277,9 @@ def test_password_breached(self, monkeypatch):
276277
assert form.password.errors.pop() == "Bad Password!"
277278
assert user_service.disable_password.calls == [
278279
pretend.call(
279-
1, reason=DisableReason.CompromisedPassword, ip_address="1.2.3.4"
280+
1,
281+
request,
282+
reason=DisableReason.CompromisedPassword,
280283
)
281284
]
282285
assert send_email.calls == [pretend.call(request, user)]
@@ -846,6 +849,7 @@ def test_totp_secret_exists(self, pyramid_config):
846849
pretend.call(
847850
tag=EventTag.Account.LoginFailure,
848851
ip_address=request.remote_addr,
852+
request=request,
849853
additional={"reason": "invalid_totp"},
850854
)
851855
]
@@ -944,6 +948,7 @@ def test_credential_invalid(self):
944948
pretend.call(
945949
tag=EventTag.Account.LoginFailure,
946950
ip_address=request.remote_addr,
951+
request=request,
947952
additional={"reason": "invalid_webauthn"},
948953
)
949954
]
@@ -1045,6 +1050,7 @@ def test_invalid_recovery_code(
10451050
pretend.call(
10461051
tag=EventTag.Account.LoginFailure,
10471052
ip_address=request.remote_addr,
1053+
request=request,
10481054
additional={"reason": expected_reason},
10491055
)
10501056
]

tests/unit/accounts/test_services.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
from warehouse.rate_limiting.interfaces import IRateLimiter
5050

5151
from ...common.db.accounts import EmailFactory, UserFactory
52+
from ...common.db.ip_addresses import IpAddressFactory
5253

5354

5455
class TestDatabaseUserService:
@@ -440,6 +441,10 @@ def test_get_admin_user(self, user_service):
440441
],
441442
)
442443
def test_disable_password(self, user_service, reason, expected):
444+
request = pretend.stub(
445+
remote_addr="127.0.0.1",
446+
ip_address=IpAddressFactory.create(),
447+
)
443448
user = UserFactory.create()
444449
user.record_event = pretend.call_recorder(lambda *a, **kw: None)
445450

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

450455
# Now we'll actually test our disable function.
451-
user_service.disable_password(user.id, reason=reason)
456+
user_service.disable_password(user.id, reason=reason, request=request)
452457
assert user.password == "!"
453458

454459
assert user.record_event.calls == [
455460
pretend.call(
456461
tag=EventTag.Account.PasswordDisabled,
457462
ip_address="127.0.0.1",
463+
request=request,
458464
additional={"reason": expected},
459465
)
460466
]
@@ -464,19 +470,29 @@ def test_disable_password(self, user_service, reason, expected):
464470
[(True, None), (True, DisableReason.CompromisedPassword), (False, None)],
465471
)
466472
def test_is_disabled(self, user_service, disabled, reason):
473+
request = pretend.stub(
474+
remote_addr="127.0.0.1",
475+
ip_address=IpAddressFactory.create(),
476+
)
467477
user = UserFactory.create()
468478
user_service.update_user(user.id, password="foo")
469479
if disabled:
470-
user_service.disable_password(user.id, reason=reason)
480+
user_service.disable_password(user.id, reason=reason, request=request)
471481
assert user_service.is_disabled(user.id) == (disabled, reason)
472482

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

477487
def test_updating_password_undisables(self, user_service):
488+
request = pretend.stub(
489+
remote_addr="127.0.0.1",
490+
ip_address=IpAddressFactory.create(),
491+
)
478492
user = UserFactory.create()
479-
user_service.disable_password(user.id, reason=DisableReason.CompromisedPassword)
493+
user_service.disable_password(
494+
user.id, reason=DisableReason.CompromisedPassword, request=request
495+
)
480496
assert user_service.is_disabled(user.id) == (
481497
True,
482498
DisableReason.CompromisedPassword,

tests/unit/accounts/test_views.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ def test_post_validate_redirects(
288288
pretend.call(
289289
tag=EventTag.Account.LoginSuccess,
290290
ip_address=pyramid_request.remote_addr,
291+
request=pyramid_request,
291292
additional={"two_factor_method": None, "two_factor_label": None},
292293
)
293294
]
@@ -351,6 +352,7 @@ def test_post_validate_no_redirects(
351352
pretend.call(
352353
tag=EventTag.Account.LoginSuccess,
353354
ip_address=pyramid_request.remote_addr,
355+
request=pyramid_request,
354356
additional={"two_factor_method": None, "two_factor_label": None},
355357
)
356358
]
@@ -714,6 +716,7 @@ def test_totp_auth(
714716
pretend.call(
715717
tag=EventTag.Account.LoginSuccess,
716718
ip_address=pyramid_request.remote_addr,
719+
request=pyramid_request,
717720
additional={"two_factor_method": "totp", "two_factor_label": "totp"},
718721
)
719722
]
@@ -1157,6 +1160,7 @@ def test_recovery_code_auth(self, monkeypatch, pyramid_request, redirect_url):
11571160
pretend.call(
11581161
tag=EventTag.Account.LoginSuccess,
11591162
ip_address=pyramid_request.remote_addr,
1163+
request=pyramid_request,
11601164
additional={
11611165
"two_factor_method": "recovery-code",
11621166
"two_factor_label": None,
@@ -1165,6 +1169,7 @@ def test_recovery_code_auth(self, monkeypatch, pyramid_request, redirect_url):
11651169
pretend.call(
11661170
tag=EventTag.Account.RecoveryCodesUsed,
11671171
ip_address=pyramid_request.remote_addr,
1172+
request=pyramid_request,
11681173
),
11691174
]
11701175
assert pyramid_request.session.flash.calls == [
@@ -1411,11 +1416,13 @@ def _find_service(service=None, name=None, context=None):
14111416
pretend.call(
14121417
tag=EventTag.Account.AccountCreate,
14131418
ip_address=db_request.remote_addr,
1419+
request=db_request,
14141420
additional={"email": "[email protected]"},
14151421
),
14161422
pretend.call(
14171423
tag=EventTag.Account.LoginSuccess,
14181424
ip_address=db_request.remote_addr,
1425+
request=db_request,
14191426
additional={"two_factor_method": None, "two_factor_label": None},
14201427
),
14211428
]
@@ -1524,6 +1531,7 @@ def test_request_password_reset(
15241531
pretend.call(
15251532
tag=EventTag.Account.PasswordResetRequest,
15261533
ip_address=pyramid_request.remote_addr,
1534+
request=pyramid_request,
15271535
)
15281536
]
15291537

@@ -1588,6 +1596,7 @@ def test_request_password_reset_with_email(
15881596
pretend.call(
15891597
tag=EventTag.Account.PasswordResetRequest,
15901598
ip_address=pyramid_request.remote_addr,
1599+
request=pyramid_request,
15911600
)
15921601
]
15931602
assert user_service.ratelimiters["password.reset"].test.calls == [
@@ -1663,6 +1672,7 @@ def test_request_password_reset_with_non_primary_email(
16631672
pretend.call(
16641673
tag=EventTag.Account.PasswordResetRequest,
16651674
ip_address=pyramid_request.remote_addr,
1675+
request=pyramid_request,
16661676
)
16671677
]
16681678
assert user_service.ratelimiters["password.reset"].test.calls == [
@@ -1762,6 +1772,7 @@ def test_password_reset_prohibited(
17621772
pretend.call(
17631773
tag=EventTag.Account.PasswordResetAttempt,
17641774
ip_address=pyramid_request.remote_addr,
1775+
request=pyramid_request,
17651776
)
17661777
]
17671778

@@ -3569,6 +3580,7 @@ def test_add_pending_github_oidc_publisher(self, monkeypatch, db_request):
35693580
pretend.call(
35703581
tag=EventTag.Account.PendingOIDCPublisherAdded,
35713582
ip_address="1.2.3.4",
3583+
request=db_request,
35723584
additional={
35733585
"project": "some-project-name",
35743586
"publisher": pending_publisher.publisher_name,
@@ -3802,6 +3814,7 @@ def test_delete_pending_oidc_publisher(self, monkeypatch, db_request):
38023814
pretend.call(
38033815
tag=EventTag.Account.PendingOIDCPublisherRemoved,
38043816
ip_address="1.2.3.4",
3817+
request=db_request,
38053818
additional={
38063819
"project": "some-project-name",
38073820
"publisher": "GitHub",

tests/unit/admin/views/test_users.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,9 @@ def test_resets_password(self, db_request, monkeypatch):
405405
db_request.user = UserFactory.create()
406406
service = pretend.stub(
407407
find_userid=pretend.call_recorder(lambda username: user.username),
408-
disable_password=pretend.call_recorder(lambda userid, reason: None),
408+
disable_password=pretend.call_recorder(
409+
lambda userid, request, reason: None
410+
),
409411
)
410412
db_request.find_service = pretend.call_recorder(lambda iface, context: service)
411413

@@ -419,7 +421,7 @@ def test_resets_password(self, db_request, monkeypatch):
419421
]
420422
assert send_email.calls == [pretend.call(db_request, user)]
421423
assert service.disable_password.calls == [
422-
pretend.call(user.id, reason=DisableReason.CompromisedPassword)
424+
pretend.call(user.id, db_request, reason=DisableReason.CompromisedPassword)
423425
]
424426
assert db_request.route_path.calls == [
425427
pretend.call("admin.user.detail", username=user.username)

tests/unit/email/test_init.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,11 @@ class FakeUser:
308308
def __init__(self):
309309
self.events = []
310310

311-
def record_event(self, tag, ip_address, additional):
311+
def record_event(self, tag, ip_address, request=None, additional=None):
312312
self.events.append(
313313
{
314314
"ip_address": ip_address,
315+
"request": request,
315316
"tag": tag,
316317
"additional": additional,
317318
}
@@ -382,6 +383,7 @@ def get_user(self, user_id):
382383
assert user_service.user.events == [
383384
{
384385
"tag": "account:email:sent",
386+
"request": request,
385387
"ip_address": request.remote_addr,
386388
"additional": {
387389
"from_": "[email protected]",

tests/unit/integration/github/test_utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,7 @@ def metrics_increment(key):
610610
pretend.call(
611611
tag=EventTag.Account.APITokenRemovedLeak,
612612
ip_address=request.remote_addr,
613+
request=request,
613614
additional={
614615
"macaroon_id": "12",
615616
"public_url": "http://example.com",

0 commit comments

Comments
 (0)