Skip to content
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

Signup auth #699

Merged
merged 16 commits into from
Nov 13, 2024
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
6 changes: 6 additions & 0 deletions src/eduid/common/config/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ class AuthnParameters(BaseModel):
same_user: bool = True # the same user was required to log in, such as when entering the security center
max_age: timedelta = timedelta(minutes=5) # the maximum age of the authentication
allow_login_auth: bool = False # allow login authentication as substitute action
allow_signup_auth: bool = False # allow during signup
finish_url: str # str as we want to use unformatted parts as {app_name} and {authn_id}


Expand All @@ -384,6 +385,7 @@ class FrontendActionMixin(BaseModel):
force_authn=True,
high_security=True,
allow_login_auth=True,
allow_signup_auth=True,
finish_url="https://eduid.se/profile/ext-return/{app_name}/{authn_id}",
),
FrontendAction.CHANGE_PW_AUTHN: AuthnParameters(
Expand Down Expand Up @@ -421,6 +423,7 @@ class FrontendActionMixin(BaseModel):
FrontendAction.VERIFY_IDENTITY: AuthnParameters(
force_authn=True,
allow_login_auth=True,
allow_signup_auth=True,
finish_url="https://eduid.se/profile/ext-return/{app_name}/{authn_id}",
),
FrontendAction.TERMINATE_ACCOUNT_AUTHN: AuthnParameters(
Expand All @@ -433,6 +436,7 @@ class FrontendActionMixin(BaseModel):
force_authn=True,
force_mfa=True,
allow_login_auth=True,
allow_signup_auth=True,
finish_url="https://eduid.se/profile/ext-return/{app_name}/{authn_id}",
),
FrontendAction.REMOVE_IDENTITY: AuthnParameters(
Expand All @@ -443,6 +447,8 @@ class FrontendActionMixin(BaseModel):
),
}
)
# slack for recent signup validity as an authn action
signup_auth_slack: timedelta = timedelta(minutes=3)


class ProofingConfigMixin(FrontendActionMixin):
Expand Down
1 change: 0 additions & 1 deletion src/eduid/satosa/scimapi/static_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def _build_static(self, requester: str, vidp: str, existing_attributes: dict) ->
else:
static_attributes[attr_name] = fmt


logger.debug(f"Appending static attribute {attr_name}: {fmt} for requester {requester} or {vidp}")

return static_attributes
Expand Down
7 changes: 5 additions & 2 deletions src/eduid/webapp/bankid/acs_actions.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
from eduid.userdb import User
from eduid.userdb.credentials.fido import FidoCredential
from eduid.webapp.bankid.app import current_bankid_app as current_app
from eduid.webapp.bankid.helpers import BankIDMsg, check_reauthn
from eduid.webapp.bankid.helpers import BankIDMsg
from eduid.webapp.bankid.proofing import get_proofing_functions
from eduid.webapp.common.api.decorators import require_user
from eduid.webapp.common.api.messages import AuthnStatusMsg
from eduid.webapp.common.authn.acs_enums import BankIDAcsAction
from eduid.webapp.common.authn.acs_registry import ACSArgs, ACSResult, acs_action
from eduid.webapp.common.authn.utils import check_reauthn
from eduid.webapp.common.proofing.messages import ProofingMsg
from eduid.webapp.common.proofing.methods import ProofingMethodSAML
from eduid.webapp.common.proofing.saml_helpers import authn_ctx_to_loa, is_required_loa, is_valid_authn_instant
Expand Down Expand Up @@ -105,7 +106,9 @@ def verify_credential_action(user: User, args: ACSArgs) -> ACSResult:
return ACSResult(message=BankIDMsg.credential_not_found)

# Check (again) if token was used to authenticate this session and that the auth is not stale.
_need_reauthn = check_reauthn(frontend_action=args.authn_req.frontend_action, user=user, credential_used=credential)
_need_reauthn = check_reauthn(
frontend_action=args.authn_req.frontend_action, user=user, credential_requested=credential
)
if _need_reauthn:
current_app.logger.error(f"User needs to authenticate: {_need_reauthn}")
return ACSResult(message=AuthnStatusMsg.must_authenticate)
Expand Down
23 changes: 0 additions & 23 deletions src/eduid/webapp/bankid/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,10 @@
from saml2.client import Saml2Client
from saml2.typing import SAMLHttpArgs

from eduid.common.config.base import FrontendAction
from eduid.userdb import User
from eduid.userdb.credentials import Credential
from eduid.userdb.credentials.external import TrustFramework
from eduid.webapp.bankid.app import current_bankid_app as current_app
from eduid.webapp.common.api.messages import TranslatableMsg
from eduid.webapp.common.api.schemas.authn_status import AuthnActionStatus
from eduid.webapp.common.authn.cache import OutstandingQueriesCache
from eduid.webapp.common.authn.utils import validate_authn_for_action
from eduid.webapp.common.session import session
from eduid.webapp.common.session.namespaces import AuthnRequestRef

Expand Down Expand Up @@ -94,21 +89,3 @@ def create_authn_info(
oq_cache = OutstandingQueriesCache(session.bankid.sp.pysaml2_dicts)
oq_cache.set(session_id, authn_ref)
return info


def check_reauthn(
frontend_action: FrontendAction, user: User, credential_used: Credential | None = None
) -> AuthnActionStatus | None:
"""Check if a re-authentication has been performed recently enough for this action"""

authn_status = validate_authn_for_action(
config=current_app.conf, frontend_action=frontend_action, credential_used=credential_used, user=user
)
current_app.logger.debug(f"check_reauthn called with authn status {authn_status}")
if authn_status != AuthnActionStatus.OK:
if authn_status == AuthnActionStatus.STALE:
# count stale authentications to monitor if users need more time
current_app.stats.count(name=f"{frontend_action.value}_stale_reauthn", value=1)
return authn_status
current_app.stats.count(name=f"{frontend_action.value}_successful_reauthn", value=1)
return None
132 changes: 107 additions & 25 deletions src/eduid/webapp/bankid/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import unittest
from collections.abc import Mapping
from datetime import timedelta
from typing import Any
from unittest.mock import MagicMock, patch

Expand All @@ -18,12 +19,11 @@
from eduid.webapp.bankid.helpers import BankIDMsg
from eduid.webapp.common.api.messages import AuthnStatusMsg, TranslatableMsg
from eduid.webapp.common.api.testing import CSRFTestClient
from eduid.webapp.common.authn.acs_enums import AuthnAcsAction
from eduid.webapp.common.authn.cache import OutstandingQueriesCache
from eduid.webapp.common.proofing.messages import ProofingMsg
from eduid.webapp.common.proofing.testing import ProofingTests
from eduid.webapp.common.session import EduidSession
from eduid.webapp.common.session.namespaces import AuthnRequestRef, SP_AuthnRequest
from eduid.webapp.common.session.namespaces import AuthnRequestRef

__author__ = "lundberg"

Expand Down Expand Up @@ -199,6 +199,7 @@ def update_config(self, config: dict[str, Any]) -> dict[str, Any]:
"force_authn": True,
"force_mfa": True,
"allow_login_auth": True,
"allow_signup_auth": True,
"finish_url": "http://test.localhost/testing-verify-credential/{app_name}/{authn_id}",
},
},
Expand Down Expand Up @@ -265,18 +266,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 @@ -329,6 +318,7 @@ def verify_token(
expect_error: bool = False,
expect_saml_error: bool = False,
identity: NinIdentity | None = None,
logged_in: bool = True,
method: str = "bankid",
response_template: str | None = None,
verify_credential: ElementKey | None = None,
Expand All @@ -344,6 +334,7 @@ def verify_token(
expect_saml_error=expect_saml_error,
frontend_action=frontend_action,
identity=identity,
logged_in=logged_in,
method=method,
response_template=response_template,
verify_credential=verify_credential,
Expand All @@ -355,9 +346,10 @@ def _get_authn_redirect_url(
endpoint: str,
method: str,
frontend_action: FrontendAction,
expect_success: bool = True,
verify_credential: ElementKey | None = None,
frontend_state: str | None = None,
) -> str:
) -> str | None:
with browser.session_transaction() as sess:
csrf_token = sess.get_csrf_token()

Expand All @@ -373,10 +365,16 @@ def _get_authn_redirect_url(
req["frontend_action"] = frontend_action.value
assert browser
response = browser.post(endpoint, json=req)
self._check_success_response(response, type_=None, payload={"csrf_token": csrf_token})

loc = self.get_response_payload(response).get("location")
assert loc is not None, "Location in header is missing"
if expect_success:
self._check_success_response(response, type_=None, payload={"csrf_token": csrf_token})
loc = self.get_response_payload(response).get("location")
assert loc is not None, "Location in header is missing"
else:
loc = None
payload = {"csrf_token": csrf_token}
if verify_credential:
payload["credential_description"] = "test"
self._check_error_response(response, type_=None, payload=payload, msg=AuthnStatusMsg.must_authenticate)
return loc

def _call_endpoint_and_saml_acs(
Expand Down Expand Up @@ -411,15 +409,17 @@ 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 logged_in is False:
with browser.session_transaction() as sess:
# the user is at least partially logged in at this stage
Expand Down Expand Up @@ -515,6 +515,54 @@ def test_webauthn_token_verify(self, mock_request_user_sync: MagicMock) -> None:

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) -> None:
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.setup_signup_authn(eppn=eppn)

self.verify_token(
endpoint="/verify-credential",
frontend_action=FrontendAction.VERIFY_CREDENTIAL,
eppn=eppn,
expect_msg=BankIDMsg.credential_verify_success,
verify_credential=credential.key,
logged_in=False,
)

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_token_to_old(self, mock_request_user_sync: MagicMock) -> None:
mock_request_user_sync.side_effect = self.request_user_sync

eppn = self.test_user.eppn
credential = self.add_security_key_to_user(
eppn=eppn, keyhandle="test", token_type="webauthn", created_ts=utc_now() + timedelta(minutes=6)
)
self._verify_user_parameters(eppn)

self.setup_signup_authn(eppn=eppn)

with self.session_cookie(self.browser, eppn) as browser:
location = self._get_authn_redirect_url(
browser=browser,
endpoint="/verify-credential",
method="bankid",
frontend_action=FrontendAction.VERIFY_CREDENTIAL,
verify_credential=credential.key,
expect_success=False,
)
assert location is None

self._verify_user_parameters(eppn, token_verified=False, num_proofings=0)

def test_mfa_token_verify_wrong_verified_nin(self) -> None:
eppn = self.test_user.eppn
nin = self.test_user_wrong_nin
Expand Down Expand Up @@ -722,6 +770,40 @@ def test_nin_verify(self, mock_request_user_sync: MagicMock) -> None:
assert doc["given_name"] == "Ûlla"
assert doc["surname"] == "Älm"

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

eppn = self.test_unverified_user_eppn
self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False)

self.setup_signup_authn(eppn=eppn)

self.reauthn(
"/verify-identity",
frontend_action=FrontendAction.VERIFY_IDENTITY,
expect_msg=BankIDMsg.identity_verify_success,
eppn=eppn,
logged_in=False,
)
user = self.app.central_userdb.get_user_by_eppn(eppn)
self._verify_user_parameters(
eppn,
num_mfa_tokens=0,
identity_verified=True,
num_proofings=1,
locked_identity=user.identities.nin,
proofing_method=IdentityProofingMethod.BANKID,
proofing_version=self.app.conf.bankid_proofing_version,
)
# check names
assert user.given_name == "Ûlla"
assert user.surname == "Älm"
# check proofing log
doc = self.app.proofing_log._get_documents_by_attr(attr="eduPersonPrincipalName", value=eppn)[0]
assert doc["given_name"] == "Ûlla"
assert doc["surname"] == "Älm"

@patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync")
def test_mfa_login(self, mock_request_user_sync: MagicMock) -> None:
mock_request_user_sync.side_effect = self.request_user_sync
Expand Down
14 changes: 4 additions & 10 deletions src/eduid/webapp/bankid/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from eduid.userdb.credentials import FidoCredential
from eduid.userdb.element import ElementKey
from eduid.webapp.bankid.app import current_bankid_app as current_app
from eduid.webapp.bankid.helpers import BankIDMsg, check_reauthn, create_authn_info
from eduid.webapp.bankid.helpers import BankIDMsg, create_authn_info
from eduid.webapp.common.api.decorators import MarshalWith, UnmarshalWith, require_user
from eduid.webapp.common.api.errors import EduidErrorsContext, goto_errors_response
from eduid.webapp.common.api.helpers import check_magic_cookie
Expand All @@ -18,15 +18,14 @@
FluxData,
TranslatableMsg,
error_response,
need_authentication_response,
success_response,
)
from eduid.webapp.common.api.schemas.authn_status import StatusRequestSchema, StatusResponseSchema
from eduid.webapp.common.api.schemas.csrf import EmptyResponse
from eduid.webapp.common.authn.acs_enums import BankIDAcsAction
from eduid.webapp.common.authn.acs_registry import ACSArgs, get_action
from eduid.webapp.common.authn.eduid_saml2 import process_assertion
from eduid.webapp.common.authn.utils import get_location
from eduid.webapp.common.authn.utils import check_reauthn, get_location
from eduid.webapp.common.proofing.methods import ProofingMethodSAML, get_proofing_method
from eduid.webapp.common.proofing.saml_helpers import create_metadata
from eduid.webapp.common.session import session
Expand Down Expand Up @@ -94,14 +93,9 @@ def verify_credential(
current_app.logger.error(f"Can't find credential with id: {credential_id}")
return error_response(message=BankIDMsg.credential_not_found)

_need_reauthn = check_reauthn(frontend_action=_frontend_action, user=user, credential_used=credential)
_need_reauthn = check_reauthn(frontend_action=_frontend_action, user=user, credential_requested=credential)
if _need_reauthn:
current_app.logger.debug(f"Need re-authentication for credential: {credential_id}")
return need_authentication_response(
frontend_action=_frontend_action,
authn_status=_need_reauthn,
payload={"credential_description": credential.description}, # type: ignore[attr-defined]
)
return _need_reauthn

result = _authn(
BankIDAcsAction.verify_credential,
Expand Down
Loading
Loading