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.)

Author: dbutenhof

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"
 

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)

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)

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 = "[email protected]"
-PASSWORD: str = "123456"
-FIRST_NAME: str = "Test"
-LAST_NAME: str = "User"
+USER = {
+    "username": "tester",
+    "email": "[email protected]",
+    "password": "123456",
+    "first_name": "Test",
+    "last_name": "User",
+}
+
+ADMIN = {
+    "username": "testadmin",
+    "email": "[email protected]",
+    "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

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"]

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,36 +566,41 @@ 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")
         assert user is None
 
     @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,14 +621,15 @@ 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)
 
         assert user.id == "12345"
         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,14 +646,15 @@ 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)
 
         assert user.id == "12345"
         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,14 +734,15 @@ 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)
 
         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,14 +757,15 @@ 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
             user = Auth.verify_auth(pbench_drb_api_key.key)
         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

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
 

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