From 92cdb4431018e2ab211f6f4d6500904ef0f5f63c Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Wed, 23 Nov 2022 13:18:35 +0100 Subject: [PATCH 1/2] tests: do not follow redirects in login_with_fc (#71607) Because it will break when we introduce a redirection to FranceConnect to close the FranceConnect session on failure to link. --- tests/auth_fc/conftest.py | 2 +- tests/auth_fc/test_auth_fc.py | 31 ++++++++++++++++++++++--------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/auth_fc/conftest.py b/tests/auth_fc/conftest.py index afdbde47..8177a3e6 100644 --- a/tests/auth_fc/conftest.py +++ b/tests/auth_fc/conftest.py @@ -98,7 +98,7 @@ class FranceConnectMock: } response = response.follow() response = response.click(href='callback') - return self.handle_authorization(app, response.location, status=302).follow() + return self.handle_authorization(app, response.location, status=302) def access_token_response(self, url, request): if self.token_endpoint_response: diff --git a/tests/auth_fc/test_auth_fc.py b/tests/auth_fc/test_auth_fc.py index cd49e5bc..37ed0194 100644 --- a/tests/auth_fc/test_auth_fc.py +++ b/tests/auth_fc/test_auth_fc.py @@ -234,6 +234,7 @@ def test_unlink_after_login_with_fc(app, franceconnect, simple_user): models.FcAccount.objects.create(user=simple_user, sub=franceconnect.sub, user_info='{}') response = franceconnect.login_with_fc(app, path='/accounts/') + response = response.maybe_follow() response = response.click('Delete link') response.form.set('new_password1', 'ikKL1234') response.form.set('new_password2', 'ikKL1234') @@ -351,6 +352,7 @@ def test_can_change_password(settings, app, franceconnect): models.FcAccount.objects.create(user=user, sub=franceconnect.sub) response = franceconnect.login_with_fc(app, path='/accounts/') + response = response.maybe_follow() assert len(response.pyquery('[href*="password/change"]')) == 0 response = response.click('Logout') response = franceconnect.handle_logout(app, response.location).follow() @@ -367,6 +369,7 @@ def test_can_change_password(settings, app, franceconnect): # Relogin with FC response = franceconnect.login_with_fc(app, path='/accounts/') + response = response.maybe_follow() assert len(response.pyquery('[href*="password/change"]')) == 0 # Unlink @@ -463,8 +466,8 @@ def test_multiple_accounts_with_same_email(settings, app, franceconnect): User.objects.create(email=franceconnect.user_info['email'], ou=ou) response = franceconnect.login_with_fc(app, path='/accounts/') - response = response.follow() + response = response.maybe_follow() assert 'is already used by another' in response @@ -498,8 +501,10 @@ def test_resolve_authorization_code_http_400(app, franceconnect, caplog): 'content': json.dumps({'error': 'invalid_request'}), } - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert re.match(r'WARNING.*token request failed.*invalid_request', caplog.text) + + response = response.maybe_follow() assert 'invalid_request' in response @@ -509,8 +514,10 @@ def test_resolve_authorization_code_http_400_error_description(app, franceconnec 'content': json.dumps({'error': 'invalid_request', 'error_description': 'Requête invalide'}), } - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert re.match(r'WARNING.*token request failed.*invalid_request', caplog.text) + + response = response.maybe_follow() assert 'invalid_request' not in response assert 'Requête invalide' in response @@ -553,30 +560,34 @@ def test_fc_is_down(app, franceconnect, freezer, caplog): franceconnect.token_endpoint_response = {'status_code': 500, 'content': 'Internal server error'} # first error -> warning - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert len(caplog.records) == 1 assert caplog.records[-1].levelname == 'WARNING' + response = response.maybe_follow() assert 'Unable to connect to FranceConnect' in response # second error, four minutes later -> warning freezer.move_to(datetime.timedelta(seconds=+240)) - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert len(caplog.records) == 2 assert caplog.records[-1].levelname == 'WARNING' + response = response.maybe_follow() assert 'Unable to connect to FranceConnect' in response # after 5 minutes an error is logged freezer.move_to(datetime.timedelta(seconds=+240)) - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert len(caplog.records) == 4 assert caplog.records[-1].levelname == 'ERROR' + response = response.maybe_follow() assert 'Unable to connect to FranceConnect' in response # but only every 5 minutes freezer.move_to(datetime.timedelta(seconds=+60)) - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert len(caplog.records) == 5 assert caplog.records[-1].levelname == 'WARNING' + response = response.maybe_follow() assert 'Unable to connect to FranceConnect' in response # a success clear the down flag @@ -589,9 +600,10 @@ def test_fc_is_down(app, franceconnect, freezer, caplog): # such that 5 minutes later only a warning is emitted freezer.move_to(datetime.timedelta(seconds=310)) franceconnect.token_endpoint_response = {'status_code': 500, 'content': 'Internal server error'} - response = franceconnect.login_with_fc(app, path='/accounts/').follow() + response = franceconnect.login_with_fc(app, path='/accounts/') assert len(caplog.records) == 8 assert caplog.records[-1].levelname == 'WARNING' + response = response.maybe_follow() assert 'Unable to connect to FranceConnect' in response @@ -643,8 +655,9 @@ def test_same_email_different_sub(app, franceconnect): # change sub franceconnect.sub = '4567' - resp = franceconnect.login_with_fc_fixed_params(app).maybe_follow() + resp = franceconnect.login_with_fc_fixed_params(app) + resp = resp.maybe_follow() # email collision, sub is different, no new user created assert User.objects.count() == 1 assert 'another email address' in resp -- 2.37.2