Skip to content

Commit

Permalink
refactor test code
Browse files Browse the repository at this point in the history
  • Loading branch information
johanlundberg committed Sep 27, 2024
1 parent c42d35e commit 90597ba
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 36 deletions.
49 changes: 32 additions & 17 deletions src/eduid/webapp/bankid/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,18 +264,6 @@ def generate_auth_response(

return resp.encode("utf-8")

def _setup_faked_login(self, session: EduidSession, credentials_used: list[ElementKey]) -> None:
logger.debug(f"Test setting credentials used for login in session {session}: {credentials_used}")
authn_req = SP_AuthnRequest(
post_authn_action=AuthnAcsAction.login,
credentials_used=credentials_used,
authn_instant=utc_now(),
frontend_action=FrontendAction.LOGIN,
finish_url="https://example.com/ext-return/{app_name}/{authn_id}",
)

session.authn.sp.authns = {authn_req.authn_id: authn_req}

@staticmethod
def _get_request_id_from_session(session: EduidSession) -> tuple[str, AuthnRequestRef]:
"""extract the (probable) SAML request ID from the session"""
Expand Down Expand Up @@ -410,14 +398,20 @@ def _call_endpoint_and_saml_acs(

assert isinstance(browser, CSRFTestClient)

browser_with_session_cookie = self.session_cookie(browser, eppn)
if not logged_in:
if logged_in:
browser_with_session_cookie = self.session_cookie(browser, eppn)
self.set_authn_action(
eppn=eppn,
frontend_action=FrontendAction.LOGIN,
credentials_used=credentials_used,
)
else:
browser_with_session_cookie = self.session_cookie_anon(browser)

with browser_with_session_cookie as browser:
if credentials_used:
with browser.session_transaction() as sess:
self._setup_faked_login(session=sess, credentials_used=credentials_used)
# if credentials_used:
# with browser.session_transaction() as sess:
# self._setup_faked_login(session=sess, credentials_used=credentials_used)

Check failure on line 414 in src/eduid/webapp/bankid/tests/test_app.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (ERA001)

src/eduid/webapp/bankid/tests/test_app.py:414:13: ERA001 Found commented-out code

if logged_in is False:
with browser.session_transaction() as sess:
Expand Down Expand Up @@ -514,6 +508,27 @@ def test_webauthn_token_verify(self, mock_request_user_sync: MagicMock):

self._verify_user_parameters(eppn, token_verified=True, num_proofings=1)

@patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync")
def test_webauthn_token_verify_signup_authn(self, mock_request_user_sync: MagicMock):
mock_request_user_sync.side_effect = self.request_user_sync

eppn = self.test_user.eppn

credential = self.add_security_key_to_user(eppn, "test", "webauthn")

self._verify_user_parameters(eppn)

self.verify_token(
endpoint="/verify-credential",
frontend_action=FrontendAction.VERIFY_CREDENTIAL,
eppn=eppn,
expect_msg=BankIDMsg.credential_verify_success,
credentials_used=[credential.key, ElementKey("other_id")],
verify_credential=credential.key,
)

self._verify_user_parameters(eppn, token_verified=True, num_proofings=1)

def test_mfa_token_verify_wrong_verified_nin(self):
eppn = self.test_user.eppn
nin = self.test_user_wrong_nin
Expand Down
10 changes: 8 additions & 2 deletions src/eduid/webapp/common/api/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def set_authn_action(
post_authn_action: AuthnAcsAction = AuthnAcsAction.login,
age: timedelta = timedelta(seconds=30),
finish_url: str | None = None,
force_mfa: bool = False,
mock_mfa: bool = False,
credentials_used: list[ElementKey] | None = None,
):
if not finish_url:
Expand All @@ -335,7 +335,7 @@ def set_authn_action(
if credentials_used is None:
credentials_used = []

if force_mfa:
if mock_mfa:
credentials_used = [ElementKey("mock_credential_one"), ElementKey("mock_credential_two")]

with self.session_cookie(self.browser, eppn) as client:
Expand All @@ -350,6 +350,12 @@ def set_authn_action(
)
sess.authn.sp.authns[sp_authn_req.authn_id] = sp_authn_req

def setup_signup_authn(self):
# mock recent account creation from signup
with self.session_cookie(self.browser, self.test_user_eppn) as client:
with client.session_transaction() as sess:
sess.signup.user_created = True

def add_security_key_to_user(self, eppn: str, keyhandle: str, token_type: str = "webauthn") -> U2F | Webauthn:
user = self.app.central_userdb.get_user_by_eppn(eppn)
mfa_token: U2F | Webauthn
Expand Down
4 changes: 2 additions & 2 deletions src/eduid/webapp/security/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ def test_authn_status_no_mfa(self):

def test_authn_status_credential_not_existing(self):
frontend_action = FrontendAction.VERIFY_CREDENTIAL
self.set_authn_action(eppn=self.test_user_eppn, frontend_action=frontend_action, force_mfa=True)
self.set_authn_action(eppn=self.test_user_eppn, frontend_action=frontend_action, mock_mfa=True)
response = self._get_authn_status(frontend_action=frontend_action, credential_id="none_existing_credential_id")
self._check_error_response(
response=response,
Expand All @@ -625,7 +625,7 @@ def test_authn_status_credential_not_existing(self):
def test_authn_status_credential_not_used(self):
frontend_action = FrontendAction.VERIFY_CREDENTIAL
credential = self.add_security_key_to_user(self.test_user_eppn, keyhandle="keyhandle_1")
self.set_authn_action(eppn=self.test_user_eppn, frontend_action=frontend_action, force_mfa=True)
self.set_authn_action(eppn=self.test_user_eppn, frontend_action=frontend_action, mock_mfa=True)
response = self._get_authn_status(frontend_action=frontend_action, credential_id=credential.key)
self._check_success_response(
response=response,
Expand Down
55 changes: 41 additions & 14 deletions src/eduid/webapp/security/tests/test_webauthn.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ def _begin_register_key(
existing_legacy_token: bool = False,
csrf: str | None = None,
check_session: bool = True,
set_authn_action: bool = True,
):
"""
Start process to register a webauthn token for the test user,
Expand All @@ -216,11 +217,12 @@ def _begin_register_key(
# Fake that user used the other security key to authenticate
force_mfa = True

self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN,
force_mfa=force_mfa,
)
if set_authn_action:
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN,
mock_mfa=force_mfa,
)

if existing_legacy_token:
self._add_u2f_token_to_user(self.test_user_eppn)
Expand Down Expand Up @@ -256,6 +258,7 @@ def _finish_register_key(
cred_id: bytes,
existing_legacy_token: bool = False,
csrf: str | None = None,
set_authn_action: bool = True,
):
"""
Finish registering a webauthn token.
Expand All @@ -274,11 +277,12 @@ def _finish_register_key(
# Fake that user used the other security key to authenticate
force_mfa = True

self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN,
force_mfa=force_mfa,
)
if set_authn_action:
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN,
mock_mfa=force_mfa,
)

if existing_legacy_token:
self._add_u2f_token_to_user(self.test_user_eppn)
Expand Down Expand Up @@ -425,6 +429,16 @@ def test_begin_register_2nd_device_ater_ctap2_with_legacy_token(self):
data = self._begin_register_key(other="ctap2", authenticator="platform", existing_legacy_token=True)
self._check_registration_begun(data)

def test_begin_register_first_key_signup_authn(self):
self.setup_signup_authn()
data = self._begin_register_key(set_authn_action=False)
self._check_registration_begun(data)

def test_begin_register_2nd_key_ater_ctap2_signup_authn(self):
self.setup_signup_authn()
data = self._begin_register_key(other="ctap2", set_authn_action=False)
self._check_registration_begun(data)

def test_begin_register_wrong_csrf_token(self):
data = self._begin_register_key(csrf="wrong-token", check_session=False)
self.assertEqual(data["type"], "POST_WEBAUTHN_WEBAUTHN_REGISTER_BEGIN_FAIL")
Expand Down Expand Up @@ -470,6 +484,19 @@ def test_finish_register_ctap2_with_legacy_token(self):
# check that a proofing element was written as the token is mfa capable
assert self.app.proofing_log.db_count() == 1

def test_finish_register_ctap2_signup_authn(self):
self.setup_signup_authn()
data = self._finish_register_key(
set_authn_action=False,
client_data=CLIENT_DATA_JSON_2,
attestation=ATTESTATION_OBJECT_2,
state=STATE_2,
cred_id=CREDENTIAL_ID_2,
)
self._check_registration_complete(data)
# check that a proofing element was written as the token is mfa capable
assert self.app.proofing_log.db_count() == 1

def test_finish_register_wrong_csrf(self):
data = self._finish_register_key(
client_data=CLIENT_DATA_JSON,
Expand All @@ -485,7 +512,7 @@ def test_remove_ctap1(self):
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.REMOVE_SECURITY_KEY_AUTHN,
force_mfa=True,
mock_mfa=True,
)

user_token, data = self._remove(
Expand All @@ -502,7 +529,7 @@ def test_remove_ctap1_with_legacy_token(self):
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.REMOVE_SECURITY_KEY_AUTHN,
force_mfa=True,
mock_mfa=True,
)

user_token, data = self._remove(
Expand All @@ -520,7 +547,7 @@ def test_remove_ctap2(self):
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.REMOVE_SECURITY_KEY_AUTHN,
force_mfa=True,
mock_mfa=True,
)

user_token, data = self._remove(
Expand All @@ -537,7 +564,7 @@ def test_remove_ctap2_legacy_token(self):
self.set_authn_action(
eppn=self.test_user_eppn,
frontend_action=FrontendAction.REMOVE_SECURITY_KEY_AUTHN,
force_mfa=True,
mock_mfa=True,
)

user_token, data = self._remove(
Expand Down
2 changes: 1 addition & 1 deletion src/eduid/webapp/security/views/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,6 @@ def check_authn_status(user: User, frontend_action: str, credential_id: ElementK
return error_response(message=SecurityMsg.frontend_action_not_supported)

authn_status = validate_authn_for_action(
config=current_app.conf, frontend_action=_frontend_action, user=user, credential_used=credential
config=current_app.conf, frontend_action=_frontend_action, user=user, credential_requested=credential
)
return success_response(payload={"authn_status": authn_status.value})

0 comments on commit 90597ba

Please sign in to comment.