From 9008b1e7d3c4dd0515ab8d44e242f4bef9e44d1f Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Wed, 5 Jul 2023 16:48:59 -0400 Subject: [PATCH] Add ADMIN roles through config file (#3475) * Add ADMIN roles through config file PBENCH-1197 With the change to SSO, we've lost our own private realm and the ability to manage roles within it. We may be able to restore roles through Keycloak and LDAP groups, but this provides a temporary "quick and dirty" mechanism to define ADMIN roles for a server based on the username provided and cached from OIDC tokens. We define a simple `admin-role` config variable which can be defined to a comma-separated list of usernames to be granted ADMIN role. This value is processed by the authorization code when it caches local `User` objects for validation. I've added a functional test for the `audit` API, which requires ADMIN role, both to prove that it works and to provide a long-delayed minimal validation of auditing. (I decided not to merge this into the overloaded "datasets" test file, which means it can't easily be run last: this makes it a less rigorous "audit test", but that can be addressed later and it provides an "ADMIN role test" that's necessary now.) --- jenkins/run-server-func-tests | 3 + lib/pbench/server/api/__init__.py | 1 + lib/pbench/server/auth/auth.py | 8 +- lib/pbench/test/functional/server/conftest.py | 56 +++++++++---- .../test/functional/server/test_audit.py | 44 ++++++++++ lib/pbench/test/unit/server/auth/test_auth.py | 82 ++++++++++++++++--- server/lib/config/pbench-server-default.cfg | 7 ++ .../etc/pbench-server/pbench-server.cfg | 1 + server/pbenchinacan/run-pbench-in-a-can | 1 + 9 files changed, 175 insertions(+), 28 deletions(-) create mode 100644 lib/pbench/test/functional/server/test_audit.py diff --git a/jenkins/run-server-func-tests b/jenkins/run-server-func-tests index a52c7ee132..8cb71ad258 100755 --- a/jenkins/run-server-func-tests +++ b/jenkins/run-server-func-tests @@ -8,6 +8,9 @@ export PB_SERVER_IMAGE_TAG=${PB_SERVER_IMAGE_TAG:-"$(cat jenkins/branch.name)"} export PB_POD_NAME=${PB_POD_NAME:-"pbench-in-a-can_${PB_SERVER_IMAGE_TAG}"} export PB_SERVER_CONTAINER_NAME=${PB_SERVER_CONTAINER_NAME:-"${PB_POD_NAME}-pbenchserver"} +# For functional testing, assign ADMIN role to the testadmin username +export PB_ADMIN_NAMES=${PB_ADMIN_NAMES:-testadmin} + SERVER_URL="https://localhost:8443" SERVER_API_ENDPOINTS="${SERVER_URL}/api/v1/endpoints" diff --git a/lib/pbench/server/api/__init__.py b/lib/pbench/server/api/__init__.py index 54ab0ee834..6b5a3cbb86 100644 --- a/lib/pbench/server/api/__init__.py +++ b/lib/pbench/server/api/__init__.py @@ -214,6 +214,7 @@ def shutdown_session(exception=None): app = Flask(__name__.split(".")[0]) CORS(app, resources={r"/api/*": {"origins": "*"}}) + app.server_config = server_config app.logger = get_pbench_logger(__name__, server_config) Auth.setup_app(app, server_config) diff --git a/lib/pbench/server/auth/auth.py b/lib/pbench/server/auth/auth.py index ede0ec46ef..bdaaff39c3 100644 --- a/lib/pbench/server/auth/auth.py +++ b/lib/pbench/server/auth/auth.py @@ -8,7 +8,7 @@ from pbench.server import PbenchServerConfig from pbench.server.auth import OpenIDClient from pbench.server.database.models.api_keys import APIKey -from pbench.server.database.models.users import User +from pbench.server.database.models.users import Roles, User # Module public token_auth = HTTPTokenAuth("Bearer") @@ -159,6 +159,12 @@ def verify_auth_oidc(auth_token: str) -> Optional[User]: audiences = token_payload.get("resource_access", {}) pb_aud = audiences.get(oidc_client.client_id, {}) roles = pb_aud.get("roles", []) + if Roles.ADMIN.name not in roles: + admin_users = current_app.server_config.get( + "pbench-server", "admin-role", fallback="" + ) + if username in admin_users.split(","): + roles.append(Roles.ADMIN.name) # Create or update the user in our cache user = User.query(id=user_id) diff --git a/lib/pbench/test/functional/server/conftest.py b/lib/pbench/test/functional/server/conftest.py index cc2181f61e..b341deed9c 100644 --- a/lib/pbench/test/functional/server/conftest.py +++ b/lib/pbench/test/functional/server/conftest.py @@ -5,13 +5,24 @@ from pbench.client import PbenchServerClient from pbench.client.oidc_admin import OIDCAdmin +from pbench.client.types import JSONOBJECT from pbench.server.auth import OpenIDClientError -USERNAME: str = "tester" -EMAIL: str = "tester@gmail.com" -PASSWORD: str = "123456" -FIRST_NAME: str = "Test" -LAST_NAME: str = "User" +USER = { + "username": "tester", + "email": "tester@gmail.com", + "password": "123456", + "first_name": "Test", + "last_name": "User", +} + +ADMIN = { + "username": "testadmin", + "email": "testadmin@gmail.com", + "password": "123456", + "first_name": "Admin", + "last_name": "Tester", +} @pytest.fixture(scope="session") @@ -41,17 +52,9 @@ def oidc_admin(server_client: PbenchServerClient): return OIDCAdmin(server_url=server_client.endpoints["openid"]["server"]) -@pytest.fixture(scope="session") -def register_test_user(oidc_admin: OIDCAdmin): - """Create a test user for functional tests.""" +def register_user(oidc_admin: OIDCAdmin, user: JSONOBJECT): try: - response = oidc_admin.create_new_user( - username=USERNAME, - email=EMAIL, - password=PASSWORD, - first_name=FIRST_NAME, - last_name=LAST_NAME, - ) + response = oidc_admin.create_new_user(**user) except OpenIDClientError as e: # To allow testing outside our transient CI containers, allow the tester # user to already exist. @@ -62,10 +65,31 @@ def register_test_user(oidc_admin: OIDCAdmin): assert response.ok, f"Register failed with {response.json()}" +@pytest.fixture(scope="session") +def register_test_user(oidc_admin: OIDCAdmin): + """Create a test user for functional tests.""" + register_user(oidc_admin, USER) + + +@pytest.fixture(scope="session") +def register_admintest_user(oidc_admin: OIDCAdmin): + """Create a test user matching the configured Pbench admin.""" + register_user(oidc_admin, ADMIN) + + @pytest.fixture def login_user(server_client: PbenchServerClient, register_test_user): """Log in the test user and return the authentication token""" - server_client.login(USERNAME, PASSWORD) + server_client.login(USER["username"], USER["password"]) + assert server_client.auth_token + yield + server_client.auth_token = None + + +@pytest.fixture +def login_admin(server_client: PbenchServerClient, register_admintest_user): + """Log in the test user and return the authentication token""" + server_client.login(ADMIN["username"], ADMIN["password"]) assert server_client.auth_token yield server_client.auth_token = None diff --git a/lib/pbench/test/functional/server/test_audit.py b/lib/pbench/test/functional/server/test_audit.py new file mode 100644 index 0000000000..f5aa6ac4c0 --- /dev/null +++ b/lib/pbench/test/functional/server/test_audit.py @@ -0,0 +1,44 @@ +from pbench.client import API, PbenchServerClient + + +class TestAudit: + def test_get_all(self, server_client: PbenchServerClient, login_admin): + """ + Verify that we can retrieve the Pbench Server audit log. + + This relies on a "testadmin" user which has been granted ADMIN role + via the pbench-server.cfg file for functional testing. The audit API + should succeed without permissions failure, and we'll validate the + audit fields of the records we see. + """ + response = server_client.get(API.SERVER_AUDIT, {}) + json = response.json() + assert ( + response.ok + ), f"Reading audit log failed {response.status_code},{json['message']}" + assert isinstance(json, list) + print(f" ... read {len(json)} audit records") + for audit in json: + assert isinstance(audit["id"], int) + assert audit["name"] + assert audit["operation"] in ("CREATE", "READ", "UPDATE", "DELETE") + assert audit["reason"] in (None, "PERMISSION", "INTERNAL", "CONSISTENCY") + assert "root_id" in audit + if audit["root_id"]: + assert isinstance(audit["root_id"], int) + assert audit["status"] in ("BEGIN", "SUCCESS", "FAILURE", "WARNING") + assert audit["timestamp"] + assert audit["attributes"] + assert audit["object_type"] in ( + "API_KEY", + "CONFIG", + "DATASET", + "NONE", + "TEMPLATE", + ) + if audit["object_type"] != "NONE": + assert audit["object_name"] + if audit["object_type"] == "DATASET": + assert audit["object_id"] + if audit["user_name"] not in (None, "BACKGROUND"): + assert audit["user_id"] diff --git a/lib/pbench/test/unit/server/auth/test_auth.py b/lib/pbench/test/unit/server/auth/test_auth.py index fb1f0d90d4..2884d23ea7 100644 --- a/lib/pbench/test/unit/server/auth/test_auth.py +++ b/lib/pbench/test/unit/server/auth/test_auth.py @@ -10,7 +10,7 @@ from requests.structures import CaseInsensitiveDict import responses -from pbench.server import JSON +from pbench.server import JSON, PbenchServerConfig import pbench.server.auth from pbench.server.auth import Connection, OpenIDClient, OpenIDClientError import pbench.server.auth.auth as Auth @@ -529,7 +529,9 @@ def test_get_auth_token_succ(self, monkeypatch, make_logger): "headers", [{"Authorization": "not-bearer my-token"}, {"Authorization": "no-space"}, {}], ) - def test_get_auth_token_fail(self, monkeypatch, make_logger, headers): + def test_get_auth_token_fail( + self, monkeypatch, server_config, make_logger, headers + ): """Verify error handling fetching the authorization token from HTTP headers """ @@ -548,6 +550,7 @@ def record_abort(code: int, message: str = ""): app = Flask("test-get-auth-token-fail") app.logger = make_logger + app.server_config = server_config with app.app_context(): monkeypatch.setattr(Auth, "request", MockRequest(headers=headers)) try: @@ -563,28 +566,33 @@ def record_abort(code: int, message: str = ""): ) assert record["code"] == expected_code - def test_verify_auth(self, make_logger, pbench_drb_token): + def test_verify_auth(self, server_config, make_logger, pbench_drb_token): """Verify success path of verify_auth""" app = Flask("test-verify-auth") app.logger = make_logger + app.server_config = server_config with app.app_context(): current_app.secret_key = jwt_secret user = Auth.verify_auth(pbench_drb_token) assert user.id == DRB_USER_ID - def test_verify_auth_invalid(self, make_logger, pbench_drb_token_invalid): + def test_verify_auth_invalid( + self, server_config, make_logger, pbench_drb_token_invalid + ): """Verify handling of an invalid (expired) token in verify_auth""" app = Flask("test-verify-auth-invalid") app.logger = make_logger + app.server_config = server_config with app.app_context(): current_app.secret_key = jwt_secret user = Auth.verify_auth(pbench_drb_token_invalid) assert user is None - def test_verify_auth_invsig(self, make_logger, pbench_drb_token): + def test_verify_auth_invsig(self, server_config, make_logger, pbench_drb_token): """Verify handling of a token with an invalid signature""" app = Flask("test-verify-auth-invsig") app.logger = make_logger + app.server_config = server_config with app.app_context(): current_app.secret_key = jwt_secret user = Auth.verify_auth(pbench_drb_token + "1") @@ -592,7 +600,7 @@ def test_verify_auth_invsig(self, make_logger, pbench_drb_token): @pytest.mark.parametrize("roles", [["ROLE"], ["ROLE1", "ROLE2"], [], None]) def test_verify_auth_oidc( - self, monkeypatch, db_session, rsa_keys, make_logger, roles + self, monkeypatch, server_config, db_session, rsa_keys, make_logger, roles ): """Verify OIDC token offline verification success path""" client_id = "us" @@ -613,6 +621,7 @@ def test_verify_auth_oidc( app = Flask("test-verify-auth-oidc") app.logger = make_logger + app.server_config = server_config with app.app_context(): user = Auth.verify_auth(token) @@ -620,7 +629,7 @@ def test_verify_auth_oidc( assert user.roles == (roles if roles else []) def test_verify_auth_oidc_user_update( - self, monkeypatch, db_session, rsa_keys, make_logger + self, monkeypatch, server_config, db_session, rsa_keys, make_logger ): """Verify we update our internal user database when we get updated user payload from the OIDC token for an existing user.""" @@ -637,6 +646,7 @@ def test_verify_auth_oidc_user_update( app = Flask("test-verify-auth-oidc-user-update") app.logger = make_logger + app.server_config = server_config with app.app_context(): user = Auth.verify_auth(token) @@ -644,7 +654,7 @@ def test_verify_auth_oidc_user_update( assert user.roles == [] assert user.username == "dummy" - # Generate a new token with a role for the same user + # Generate a token with a new username for the same UUID token, expected_payload = gen_rsa_token( client_id, rsa_keys["private_key"], @@ -657,7 +667,54 @@ def test_verify_auth_oidc_user_update( assert user.roles == ["ROLE"] assert user.username == "new_dummy" - def test_verify_auth_oidc_invalid(self, monkeypatch, rsa_keys, make_logger): + def test_verify_auth_oidc_user_admin( + self, + monkeypatch, + server_config: PbenchServerConfig, + db_session, + rsa_keys, + make_logger, + ): + """Verify we update our internal user database when we get updated user + payload from the OIDC token for an existing user.""" + client_id = "us" + token, expected_payload = gen_rsa_token(client_id, rsa_keys["private_key"]) + + # Mock the Connection object and generate an OpenIDClient object, + # installing it as Auth module's OIDC client. + config = mock_connection( + monkeypatch, client_id, public_key=rsa_keys["public_key"] + ) + oidc_client = OpenIDClient.construct_oidc_client(config) + monkeypatch.setattr(Auth, "oidc_client", oidc_client) + server_config._conf.set("pbench-server", "admin-role", "friend,dummy,admin") + + app = Flask("test-verify-auth-oidc-user-admin") + app.logger = make_logger + app.server_config = server_config + with app.app_context(): + user = Auth.verify_auth(token) + + assert user.id == "12345" + assert user.roles == ["ADMIN"] + assert user.username == "dummy" + + # Generate a token with a role and new username for the same UUID + token, expected_payload = gen_rsa_token( + client_id, + rsa_keys["private_key"], + username="friend", + oidc_client_roles=["ROLE"], + ) + with app.app_context(): + user = Auth.verify_auth(token) + assert user.id == "12345" + assert user.roles == ["ROLE", "ADMIN"] + assert user.username == "friend" + + def test_verify_auth_oidc_invalid( + self, monkeypatch, server_config, rsa_keys, make_logger + ): """Verify OIDC token offline verification via Auth.verify_auth() fails gracefully with an invalid token """ @@ -677,6 +734,7 @@ def tio_exc(token: str) -> JSON: app = Flask("test-verify-auth-oidc-invalid") app.logger = make_logger + app.server_config = server_config with app.app_context(): monkeypatch.setattr(oidc_client, "token_introspect", tio_exc) user = Auth.verify_auth(token) @@ -684,7 +742,7 @@ def tio_exc(token: str) -> JSON: assert user is None def test_verify_auth_api_key( - self, monkeypatch, rsa_keys, make_logger, pbench_drb_api_key + self, monkeypatch, server_config, rsa_keys, make_logger, pbench_drb_api_key ): """Verify api_key verification via Auth.verify_auth()""" @@ -699,6 +757,7 @@ def tio_exc(token: str) -> JSON: app = Flask("test_verify_auth_api_key") app.logger = make_logger + app.server_config = server_config with app.app_context(): monkeypatch.setattr(oidc_client, "token_introspect", tio_exc) current_app.secret_key = jwt_secret @@ -706,7 +765,7 @@ def tio_exc(token: str) -> JSON: assert user.id == DRB_USER_ID def test_verify_auth_api_key_invalid( - self, monkeypatch, rsa_keys, make_logger, pbench_invalid_api_key + self, monkeypatch, server_config, rsa_keys, make_logger, pbench_invalid_api_key ): """Verify api_key verification via Auth.verify_auth() fails gracefully with an invalid token @@ -722,6 +781,7 @@ def tio_exc(token: str) -> JSON: app = Flask("test_verify_auth_api_key_invalid") app.logger = make_logger + app.server_config = server_config with app.app_context(): monkeypatch.setattr(oidc_client, "token_introspect", tio_exc) current_app.secret_key = jwt_secret diff --git a/server/lib/config/pbench-server-default.cfg b/server/lib/config/pbench-server-default.cfg index d4f231a92a..909fdabdec 100644 --- a/server/lib/config/pbench-server-default.cfg +++ b/server/lib/config/pbench-server-default.cfg @@ -25,6 +25,13 @@ admin-email=%(user)s@localhost mailto=%(admin-email)s mailfrom=%(user)s@localhost +# A comma-separated list of OIDC usernames with no spaces. These usernames +# will be granted ADMIN access on the server. These are OIDC ID provider +# usernames matched against decrypted authorization tokens. If no usernames +# are specified, no users have ADMIN access. NOTE: this is a temporary measure +# until we work out Keycloak / LDAP roles. +#admin-role=user1,user2 + # Token expiration duration in minutes, can be overridden in the main config file, defaults to 60 mins token_expiration_duration = 60 diff --git a/server/pbenchinacan/etc/pbench-server/pbench-server.cfg b/server/pbenchinacan/etc/pbench-server/pbench-server.cfg index 5e2c98cfc9..d72ede682d 100644 --- a/server/pbenchinacan/etc/pbench-server/pbench-server.cfg +++ b/server/pbenchinacan/etc/pbench-server/pbench-server.cfg @@ -12,6 +12,7 @@ realhost = pbenchinacan maximum-dataset-retention-days = 36500 default-dataset-retention-days = 730 roles = pbench-results +admin-role = ##ADMIN_NAMES## [Indexing] index_prefix = container-pbench diff --git a/server/pbenchinacan/run-pbench-in-a-can b/server/pbenchinacan/run-pbench-in-a-can index 0bdbd4616b..4461a5eb98 100755 --- a/server/pbenchinacan/run-pbench-in-a-can +++ b/server/pbenchinacan/run-pbench-in-a-can @@ -67,6 +67,7 @@ sed -Ei \ -e "/^ *realhost/ s/=.*/= $(hostname -f)/" \ -e "s//${KEYCLOAK_REALM}/" \ -e "s//${KEYCLOAK_CLIENT}/" \ + -e "s/##ADMIN_NAMES##/${PB_ADMIN_NAMES}/" \ ${PB_DEPLOY_FILES}/pbench-server.cfg # Set up the /srv/pbench file tree