From aa3cbfcee24713a7642414bb64398b1509fec641 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 2 Jul 2024 20:13:19 +0200 Subject: [PATCH] feat: config with option to allow only existing OR active users to login --- fence/auth.py | 49 +++++++++++++++++++++------------- fence/config-default.yaml | 10 +++++++ fence/config.py | 1 + tests/login/test_login_user.py | 33 +++++++++++++++++++++++ 4 files changed, 74 insertions(+), 19 deletions(-) diff --git a/fence/auth.py b/fence/auth.py index e23ada890..e7429a42b 100644 --- a/fence/auth.py +++ b/fence/auth.py @@ -100,26 +100,37 @@ def set_flask_session_values(user): user = query_for_user(session=current_app.scoped_session(), username=username) if user: - _update_users_email(user, email) - _update_users_id_from_idp(user, id_from_idp) - _update_users_last_auth(user) - - # This expression is relevant to those users who already have user and - # idp info persisted to the database. We return early to avoid - # unnecessarily re-saving that user and idp info. - if user.identity_provider and user.identity_provider.name == provider: - set_flask_session_values(user) - return + if user.active is False: + # Abort login if user.active is False (user.active is None or True are both + # considered active in this case): + raise Unauthorized( + "User is known but not authorized/activated in the system" + ) + else: + _update_users_email(user, email) + _update_users_id_from_idp(user, id_from_idp) + _update_users_last_auth(user) + + # This expression is relevant to those users who already have user and + # idp info persisted to the database. We return early to avoid + # unnecessarily re-saving that user and idp info. + if user.identity_provider and user.identity_provider.name == provider: + set_flask_session_values(user) + return else: - # we need a new user - user = User(username=username) - - if email: - user.email = email - - if id_from_idp: - user.id_from_idp = id_from_idp - # TODO: update iss_sub mapping table? + if not config["ALLOW_NEW_USER_ON_LOGIN"]: + # do not create new active users automatically + raise Unauthorized("New user is not yet authorized/activated in the system") + else: + # add the new user + user = User(username=username) + + if email: + user.email = email + + if id_from_idp: + user.id_from_idp = id_from_idp + # TODO: update iss_sub mapping table? # setup idp connection for new user (or existing user w/o it setup) idp = ( diff --git a/fence/config-default.yaml b/fence/config-default.yaml index a570989c0..721994bde 100755 --- a/fence/config-default.yaml +++ b/fence/config-default.yaml @@ -500,6 +500,16 @@ DEFAULT_BACKOFF_SETTINGS_MAX_TRIES: 3 # here. Something like: support@example.com SUPPORT_EMAIL_FOR_ERRORS: null +# ////////////////////////////////////////////////////////////////////////////////////// +# USER ACTIVATION +# ////////////////////////////////////////////////////////////////////////////////////// +# If you want new users (read: users that login for the first time) to automatically be +# allowed through and added to the Fence DB, set this to true. Otherwise, set this to false. +# Setting it to false will ensure the user will only be able to login after the user +# is added to the Fence DB via a separate process. This two-step process allows for +# a separate onboarding and user "approval" process, instead of the default automatic approval. +ALLOW_NEW_USER_ON_LOGIN: true + # ////////////////////////////////////////////////////////////////////////////////////// # SHIBBOLETH # - Support using `shibboleth` in LOGIN_OPTIONS diff --git a/fence/config.py b/fence/config.py index d981bfd38..097dc0fb9 100644 --- a/fence/config.py +++ b/fence/config.py @@ -47,6 +47,7 @@ def post_process(self): "WHITE_LISTED_GOOGLE_PARENT_ORGS", "CLIENT_CREDENTIALS_ON_DOWNLOAD_ENABLED", "DATA_UPLOAD_BUCKET", + "ALLOW_NEW_USER_ON_LOGIN", ] for default in defaults: self.force_default_if_none(default, default_cfg=default_config) diff --git a/tests/login/test_login_user.py b/tests/login/test_login_user.py index 2afb7b5e0..f1556d42a 100644 --- a/tests/login/test_login_user.py +++ b/tests/login/test_login_user.py @@ -1,8 +1,11 @@ import flask +import pytest from fence.auth import login_user, logout from fence.models import User, IdentityProvider import time from datetime import datetime +from fence.config import config +from fence.errors import Unauthorized def test_login_user_already_in_db(db_session): @@ -33,6 +36,22 @@ def test_login_user_already_in_db(db_session): assert flask.g.user == test_user +def test_login_failure_for_user_already_in_db_but_inactive(db_session): + """ + Test that if a user is already in the database, but is set to user.active == False, + and logs in, the login returns an Unauthorized error. + """ + email = "testuser@gmail.com" + provider = "Test Provider" + id_from_idp = "Provider_ID_0001" + + test_user = User(username=email, is_admin=False, active=False) + db_session.add(test_user) + db_session.commit() + with pytest.raises(Unauthorized): + login_user(email, provider, email=email, id_from_idp=id_from_idp) + + def test_login_user_with_idp_already_in_db(db_session): """ Test that if a user is already in the database, has identity_provider @@ -85,6 +104,20 @@ def test_login_new_user(db_session): assert flask.g.user == test_user +def test_login_new_user_not_allowed(db_session, monkeypatch): + """ + Test that when ALLOW_NEW_USER_ON_LOGIN config is False, + and a user that is not in the database logs in, an + Unauthorized error is returned. + """ + monkeypatch.setitem(config, "ALLOW_NEW_USER_ON_LOGIN", False) + email = "testuser@gmail.com" + provider = "Test Provider" + id_from_idp = "Provider_ID_0001" + with pytest.raises(Unauthorized): + login_user(email, provider, email=email, id_from_idp=id_from_idp) + + def test_last_auth_update_in_db(db_session): """ Test that the _last_auth field in the DB is updated when the user logs in.