From 2fb85130fa09fca5248a4d75a9e168eac3659d3e Mon Sep 17 00:00:00 2001 From: Peter Hooper Date: Tue, 24 Oct 2023 15:42:15 +0100 Subject: [PATCH] Check a user is active on at least one org (#34) --- app/repository/app_user.py | 10 ++++++++++ app/service/authentication.py | 3 +++ unit_tests/mocks/repos/app_user_repo.py | 7 +++++++ unit_tests/service/test_authorisation_service.py | 11 +++++++++++ 4 files changed, 31 insertions(+) diff --git a/app/repository/app_user.py b/app/repository/app_user.py index ead7b045..4d50952c 100644 --- a/app/repository/app_user.py +++ b/app/repository/app_user.py @@ -10,6 +10,16 @@ def get_user_by_email(db: Session, email: str) -> MaybeAppUser: return db.query(AppUser).filter(AppUser.email == email).one() +def is_active(db: Session, email: str) -> bool: + return ( + db.query(OrganisationUser) + .filter(OrganisationUser.appuser_email == email) + .filter(OrganisationUser.is_active is True) + .count() + > 0 + ) + + def get_app_user_authorisation( db: Session, app_user: AppUser ) -> list[Tuple[OrganisationUser, Organisation]]: diff --git a/app/service/authentication.py b/app/service/authentication.py index 7a28da87..d280e99b 100644 --- a/app/service/authentication.py +++ b/app/service/authentication.py @@ -52,6 +52,9 @@ def authenticate_user(email: str, password: str) -> str: if user is None: raise RepositoryError(f"User not found for {email}") + if not app_user_repo.is_active(db, email): + raise AuthenticationError(f"User {email} is marked as not active.") + if not verify_password(password, str(user.hashed_password)): # TODO: Log failed login attempt? raise AuthenticationError(f"Could not verify password for {email}") diff --git a/unit_tests/mocks/repos/app_user_repo.py b/unit_tests/mocks/repos/app_user_repo.py index bbfe4a82..ddf8cb9d 100644 --- a/unit_tests/mocks/repos/app_user_repo.py +++ b/unit_tests/mocks/repos/app_user_repo.py @@ -12,6 +12,8 @@ def mock_app_user_repo(app_user_repo, monkeypatch: MonkeyPatch, mocker): + app_user_repo.user_active = True + def mock_get_app_user_authorisation( _, __ ) -> list[Tuple[OrganisationUser, Organisation]]: @@ -26,10 +28,15 @@ def mock_get_user_by_email(_, __) -> MaybeAppUser: is_superuser=True, ) + def mock_is_active(_, email: str) -> bool: + return app_user_repo.user_active + app_user_repo.error = False monkeypatch.setattr(app_user_repo, "get_user_by_email", mock_get_user_by_email) + monkeypatch.setattr(app_user_repo, "is_active", mock_is_active) monkeypatch.setattr( app_user_repo, "get_app_user_authorisation", mock_get_app_user_authorisation ) mocker.spy(app_user_repo, "get_user_by_email") + mocker.spy(app_user_repo, "is_active") mocker.spy(app_user_repo, "get_app_user_authorisation") diff --git a/unit_tests/service/test_authorisation_service.py b/unit_tests/service/test_authorisation_service.py index 266e96bf..63ba40be 100644 --- a/unit_tests/service/test_authorisation_service.py +++ b/unit_tests/service/test_authorisation_service.py @@ -44,6 +44,17 @@ def test_raises_when_no_password( assert app_user_repo_mock.get_user_by_email.call_count == 1 +def test_raises_when_inactive( + app_user_repo_mock, +): + app_user_repo_mock.user_active = False + with pytest.raises(AuthenticationError) as e: + auth_service.authenticate_user(VALID_USERNAME, PLAIN_PASSWORD) + + assert e.value.message == f"User {VALID_USERNAME} is marked as not active." + assert app_user_repo_mock.get_user_by_email.call_count == 1 + + def test_can_auth( app_user_repo_mock, ):