Skip to content

Commit

Permalink
Add ADMIN roles through config file (#3475)
Browse files Browse the repository at this point in the history
* 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.)
  • Loading branch information
dbutenhof authored Jul 5, 2023
1 parent 75d4d91 commit 9008b1e
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 28 deletions.
3 changes: 3 additions & 0 deletions jenkins/run-server-func-tests
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
1 change: 1 addition & 0 deletions lib/pbench/server/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion lib/pbench/server/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
56 changes: 40 additions & 16 deletions lib/pbench/test/functional/server/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand All @@ -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
44 changes: 44 additions & 0 deletions lib/pbench/test/functional/server/test_audit.py
Original file line number Diff line number Diff line change
@@ -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"]
82 changes: 71 additions & 11 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
"""
Expand All @@ -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:
Expand All @@ -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"
Expand All @@ -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."""
Expand All @@ -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"],
Expand All @@ -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
"""
Expand All @@ -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()"""

Expand All @@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions server/lib/config/pbench-server-default.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions server/pbenchinacan/etc/pbench-server/pbench-server.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 9008b1e

Please sign in to comment.