From 07edb987f968d52ca09fe07eab9cc391b807cbd6 Mon Sep 17 00:00:00 2001 From: Nikhil Palaskar Date: Mon, 20 Feb 2023 23:52:20 -0500 Subject: [PATCH] Add OIDC user to the functional test (#3235) * Use OIDC user for the functional test Functional tests should move to using OIDC tokens by making an Admin REST API request to the OIDC server (Keycloak) directly as the server won't be providing Register/Login capabilities on its endpoints. - Create a new scope in the Keycloak to add OIDC client name in aud claim (required for the authentication) - Functional test user registration happens directly with the OIDC server using the Admin token - Functional test user makes a REST API call to get the OIDC token PBENCH-1070 --- lib/pbench/client/__init__.py | 46 ++-- lib/pbench/client/oidc_admin.py | 141 ++++++++++++ lib/pbench/server/api/resources/__init__.py | 20 +- lib/pbench/server/auth/__init__.py | 37 +-- lib/pbench/test/functional/server/conftest.py | 43 ++-- .../test/functional/server/test_connect.py | 2 +- .../test/functional/server/test_user.py | 214 ------------------ lib/pbench/test/unit/client/conftest.py | 14 +- lib/pbench/test/unit/client/test_connect.py | 9 +- lib/pbench/test/unit/client/test_login.py | 21 +- lib/pbench/test/unit/server/auth/test_auth.py | 18 +- .../test/unit/server/test_authorization.py | 6 +- .../unit/server/test_datasets_inventory.py | 3 +- .../etc/pbench-server/pbench-server.cfg | 3 +- server/pbenchinacan/load_keycloak.sh | 38 +++- 15 files changed, 310 insertions(+), 305 deletions(-) create mode 100644 lib/pbench/client/oidc_admin.py delete mode 100644 lib/pbench/test/functional/server/test_user.py diff --git a/lib/pbench/client/__init__.py b/lib/pbench/client/__init__.py index a9e21a4be5..014a72ea78 100644 --- a/lib/pbench/client/__init__.py +++ b/lib/pbench/client/__init__.py @@ -6,7 +6,8 @@ import requests from requests.structures import CaseInsensitiveDict -from pbench.client.types import Dataset, JSONMap, JSONOBJECT +from pbench.client.oidc_admin import OIDCAdmin +from pbench.client.types import Dataset, JSONOBJECT class PbenchClientError(Exception): @@ -83,6 +84,7 @@ def __init__(self, host: str): self.auth_token: Optional[str] = None self.session: Optional[requests.Session] = None self.endpoints: Optional[JSONOBJECT] = None + self.oidc_admin: Optional[OIDCAdmin] = None def _headers( self, user_headers: Optional[dict[str, str]] = None @@ -299,8 +301,13 @@ def delete( return response def connect(self, headers: Optional[dict[str, str]] = None) -> None: - """Connect to the Pbench Server host using the endpoints API to be sure - that it responds, and cache the endpoints response payload. + """Performs some pre-requisite actions to make server client usable. + + 1. Connect to the Pbench Server host using the endpoints API to be + sure that it responds, and cache the endpoints response payload. + + 2. Create an OIDCAdmin object that a server client can use to + perform privileged actions on an OIDC server. This also allows the client to add default HTTP headers to the session which will be used for all operations unless overridden for specific @@ -318,31 +325,24 @@ def connect(self, headers: Optional[dict[str, str]] = None) -> None: self.endpoints = response.json() assert self.endpoints - def login(self, user: str, password: str) -> JSONMap: - """Login to a specified username with the password, and store the - resulting authentication token. + # Create an OIDCAdmin object and confirm the connection was successful + self.oidc_admin = OIDCAdmin(server_url=self.endpoints["openid"]["server"]) + + def login(self, user: str, password: str): + """Log into the OIDC server using the specified username and password, + and store the resulting authentication token. Args: user: Account username password: Account password - - Returns: - The login response - """ - response = self.post(API.LOGIN, json={"username": user, "password": password}) - response.raise_for_status() - json = response.json() - self.username = json["username"] - self.auth_token = json["auth_token"] - return JSONMap(json) - - def logout(self) -> None: - """Logout the currently authenticated user and remove the - authentication token. """ - self.post(API.LOGOUT) - self.username = None - self.auth_token = None + response = self.oidc_admin.user_login( + client_id=self.endpoints["openid"]["client"], + username=user, + password=password, + ) + self.username = user + self.auth_token = response["access_token"] def upload(self, tarball: Path, **kwargs) -> requests.Response: """Upload a tarball to the server. diff --git a/lib/pbench/client/oidc_admin.py b/lib/pbench/client/oidc_admin.py new file mode 100644 index 0000000000..d21a6cb0d9 --- /dev/null +++ b/lib/pbench/client/oidc_admin.py @@ -0,0 +1,141 @@ +from http import HTTPStatus +import os + +import requests + +from pbench.server.auth import Connection + + +class OIDCAdmin(Connection): + OIDC_REALM = os.getenv("OIDC_REALM", "pbench-server") + ADMIN_USERNAME = os.getenv("OIDC_ADMIN_USERNAME", "admin") + ADMIN_PASSWORD = os.getenv("OIDC_ADMIN_PASSWORD", "admin") + + def __init__(self, server_url: str): + super().__init__(server_url, verify=False) + + def get_admin_token(self) -> dict: + """pbench-server realm admin user login. + + Returns: + access_token json payload + + { 'access_token': , + 'expires_in': 60, 'refresh_expires_in': 1800, + 'refresh_token': , + 'token_type': 'Bearer', + 'not-before-policy': 0, + 'session_state': '8f558797-50e7-496d-bb45-3b5ac9fdcddb', + 'scope': 'profile email'} + + """ + url_path = "/realms/master/protocol/openid-connect/token" + data = { + "grant_type": "password", + "client_id": "admin-cli", + "username": self.ADMIN_USERNAME, + "password": self.ADMIN_PASSWORD, + } + return self.post(path=url_path, data=data).json() + + def create_new_user( + self, + username: str, + email: str, + password: str, + first_name: str = "", + last_name: str = "", + ) -> requests.Response: + """Creates a new user under the OIDC_REALM. + + Note: This involves a REST API call to the + OIDC server to create a new user. + + Args: + username: username to register, + email: user email address, + password: user password, + first_name: Optional first name of the user, + last_name: Optional first name of the user, + + Returns: + Response from the request. + """ + admin_token = self.get_admin_token().get("access_token") + url_path = f"/admin/realms/{self.OIDC_REALM}/users" + headers = {"Authorization": f"Bearer {admin_token}"} + data = { + "username": username, + "email": email, + "emailVerified": True, + "enabled": True, + "firstName": first_name, + "lastName": last_name, + "credentials": [ + {"type": "password", "value": password, "temporary": False} + ], + } + response = self.post(path=url_path, json=data, headers=headers) + return response + + def user_login(self, client_id: str, username: str, password: str) -> dict: + """pbench-server realm user login on a specified client. + + Args: + client_id: client_name to use in the request + username: username of the user logging in + password: OIDC password + + Returns: + access_token json payload + + { 'access_token': , + 'expires_in': 60, 'refresh_expires_in': 1800, + 'refresh_token': , + 'token_type': 'Bearer', + 'not-before-policy': 0, + 'session_state': '8f558797-50e7-496d-bb45-3b5ac9fdcddb', + 'scope': 'profile email'} + + """ + url_path = f"/realms/{self.OIDC_REALM}/protocol/openid-connect/token" + data = { + "client_id": client_id, + "grant_type": "password", + "scope": "profile email", + "username": username, + "password": password, + } + return self.post(path=url_path, data=data).json() + + def get_user(self, username: str, token: str) -> dict: + """Get the OIDC user representation dict. + + Args: + username: username to query + token: access_token string to validate + + Returns: + User dict representation + + {'id': '37117992-a3de-43f7-b844-e6ee178e9965', + 'createdTimestamp': 1675981768951, + 'username': 'admin', + 'enabled': True, + 'totp': False, + 'emailVerified': False, + 'disableableCredentialTypes': [], + 'requiredActions': [], + 'notBefore': 0, + 'access': {'manageGroupMembership': True, 'view': True, 'mapRoles': True, 'impersonate': True, 'manage': True} + ... + } + """ + response = self.get( + f"admin/realms/{self.OIDC_REALM}/users", + headers={"Authorization": f"Bearer {token}"}, + username=username, + ) + if response.status_code == HTTPStatus.OK: + return response.json()[0] + return {} diff --git a/lib/pbench/server/api/resources/__init__.py b/lib/pbench/server/api/resources/__init__.py index b5ea4999aa..e1a3d92f96 100644 --- a/lib/pbench/server/api/resources/__init__.py +++ b/lib/pbench/server/api/resources/__init__.py @@ -1326,13 +1326,6 @@ def _check_authorization(self, mode: ApiAuthorization): user_id = mode.user role = mode.role authorized_user: User = Auth.token_auth.current_user() - username = "none" - if user_id: - user = User.query(id=user_id) - if user: - username = user.username - else: - current_app.logger.error("User ID {} not found", user_id) # The ADMIN authorization doesn't involve a target resource owner or # access, so take care of that first as a special case. If there is @@ -1355,10 +1348,9 @@ def _check_authorization(self, mode: ApiAuthorization): access = mode.access current_app.logger.debug( - "Authorizing {} access for {} to user {} ({}) with access {} using {}", + "Authorizing {} access for {} to user (user id: {}) with access {} using {}", role, authorized_user, - username, user_id, mode.access, mode.type, @@ -1387,12 +1379,12 @@ def _check_authorization(self, mode: ApiAuthorization): # An unauthenticated user is never allowed to access private # data nor to perform an potential mutation of data: REJECT current_app.logger.warning( - "Attempt to {} user {} data without login", role, username + "Attempt to {} user {} data without login", role, user_id ) raise UnauthorizedAccess( authorized_user, role, - username, + user_id, access, HTTPStatus.UNAUTHORIZED, ) @@ -1405,7 +1397,7 @@ def _check_authorization(self, mode: ApiAuthorization): role, ) raise UnauthorizedAccess( - authorized_user, role, username, access, HTTPStatus.FORBIDDEN + authorized_user, role, user_id, access, HTTPStatus.FORBIDDEN ) elif ( user_id @@ -1419,10 +1411,10 @@ def _check_authorization(self, mode: ApiAuthorization): "Unauthorized attempt by {} to {} user {} data", authorized_user, role, - username, + user_id, ) raise UnauthorizedAccess( - authorized_user, role, username, access, HTTPStatus.FORBIDDEN + authorized_user, role, user_id, access, HTTPStatus.FORBIDDEN ) else: # We have determined that there is an authenticated user with diff --git a/lib/pbench/server/auth/__init__.py b/lib/pbench/server/auth/__init__.py index 878c9bd234..c8292f26f3 100644 --- a/lib/pbench/server/auth/__init__.py +++ b/lib/pbench/server/auth/__init__.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from http import HTTPStatus import logging -from typing import Dict, Optional, Union +from typing import Any, Optional from urllib.parse import urljoin import jwt @@ -38,7 +38,7 @@ class Connection: def __init__( self, server_url: str, - headers: Optional[Dict[str, str]] = None, + headers: Optional[dict[str, str]] = None, verify: bool = True, ): self.server_url = server_url @@ -50,8 +50,9 @@ def _method( self, method: str, path: str, - data: Union[Dict, None], - headers: Optional[Dict] = None, + data: Optional[Any] = None, + json: Optional[dict[str, Any]] = None, + headers: Optional[dict[str, str]] = None, **kwargs, ) -> requests.Response: """Common frontend for the HTTP operations on OIDC client connection. @@ -59,7 +60,8 @@ def _method( Args: method : The API HTTP method path : Path for the request. - data : Json data to send with the request in case of the POST + data : Form data to send with the request in case of the POST + json : JSON data to send with the request in case of the POST kwargs : Additional keyword args Returns: @@ -69,14 +71,15 @@ def _method( if headers is not None: final_headers.update(headers) url = urljoin(self.server_url, path) - kwargs = dict( + request_dict = dict( params=kwargs, data=data, + json=json, headers=final_headers, verify=self.verify, ) try: - response = self._connection.request(method, url, **kwargs) + response = self._connection.request(method, url, **request_dict) except requests.exceptions.ConnectionError as exc: raise OpenIDClientError( http_status=HTTPStatus.BAD_GATEWAY, @@ -110,7 +113,7 @@ def _method( return response def get( - self, path: str, headers: Optional[Dict] = None, **kwargs + self, path: str, headers: Optional[dict[str, str]] = None, **kwargs ) -> requests.Response: """GET wrapper to handle an authenticated GET operation on the Resource at a given path. @@ -123,24 +126,30 @@ def get( Returns: Response from the request. """ - return self._method("GET", path, None, headers=headers, **kwargs) + return self._method("GET", path, headers=headers, **kwargs) def post( - self, path: str, data: Dict, headers: Optional[Dict] = None, **kwargs + self, + path: str, + data: Optional[Any] = None, + json: Optional[dict[str, Any]] = None, + headers: Optional[dict[str, str]] = None, + **kwargs, ) -> requests.Response: """POST wrapper to handle an authenticated POST operation on the Resource at a given path. Args: path : Path for the request - data : JSON request body + data : Optional HTML form body to attach + json : JSON request body headers : Additional headers to add to the request kwargs : Additional keyword args to be added as URL parameters Returns: Response from the request. """ - return self._method("POST", path, data, headers=headers, **kwargs) + return self._method("POST", path, data, json, headers=headers, **kwargs) @dataclass @@ -323,7 +332,7 @@ def __init__( client_id: str, realm_name: str, verify: bool = True, - headers: Optional[Dict[str, str]] = None, + headers: Optional[dict[str, str]] = None, ): """Initialize an OpenID Connect Client object. @@ -404,7 +413,7 @@ def token_introspect(self, token: str) -> JSON: token, self._pem_public_key, algorithms=[self._TOKEN_ALG], - audience=self.client_id, + audience=[self.client_id], options={ "verify_signature": True, "verify_aud": True, diff --git a/lib/pbench/test/functional/server/conftest.py b/lib/pbench/test/functional/server/conftest.py index 953afc4f93..060f7b257b 100644 --- a/lib/pbench/test/functional/server/conftest.py +++ b/lib/pbench/test/functional/server/conftest.py @@ -3,7 +3,14 @@ import pytest -from pbench.client import API, PbenchServerClient +from pbench.client import PbenchServerClient +from pbench.client.oidc_admin import OIDCAdmin + +USERNAME: str = "tester" +EMAIL: str = "tester@gmail.com" +PASSWORD: str = "123456" +FIRST_NAME: str = "Test" +LAST_NAME: str = "User" @pytest.fixture(scope="module") @@ -25,32 +32,34 @@ def server_client(): @pytest.fixture(scope="module") -def register_test_user(server_client: PbenchServerClient): - """Create a test user for functional tests.""" +def oidc_admin(server_client: PbenchServerClient): + """ + Used by Pbench Server functional tests to get admin access + on OIDC server. + """ + return OIDCAdmin(server_url=server_client.endpoints["openid"]["server"]) - response = server_client.post( - API.REGISTER, - json={ - "username": "tester", - "first_name": "Test", - "last_name": "User", - "password": "123456", - "email": "tester@gmail.com", - }, - raise_error=False, + +@pytest.fixture(scope="module") +def register_test_user(oidc_admin: OIDCAdmin): + """Create a test user for functional tests.""" + response = oidc_admin.create_new_user( + username=USERNAME, + email=EMAIL, + password=PASSWORD, + first_name=FIRST_NAME, + last_name=LAST_NAME, ) # To allow testing outside our transient CI containers, allow the tester # user to already exist. assert ( - response.ok or response.status_code == HTTPStatus.FORBIDDEN + response.ok or response.status_code == HTTPStatus.CONFLICT ), f"Register failed with {response.json()}" @pytest.fixture def login_user(server_client: PbenchServerClient, register_test_user): """Log in the test user and return the authentication token""" - server_client.login("tester", "123456") + server_client.login(USERNAME, PASSWORD) assert server_client.auth_token - yield - server_client.logout() diff --git a/lib/pbench/test/functional/server/test_connect.py b/lib/pbench/test/functional/server/test_connect.py index d29a2f0886..b0f0664121 100644 --- a/lib/pbench/test/functional/server/test_connect.py +++ b/lib/pbench/test/functional/server/test_connect.py @@ -29,5 +29,5 @@ def test_connect(self, server_client: PbenchServerClient): # verify all the required openid-connect fields are present if "openid" in endpoints: - expected = {"server", "client", "realm", "secret"} + expected = {"client", "realm", "server"} assert set(endpoints["openid"]) >= expected diff --git a/lib/pbench/test/functional/server/test_user.py b/lib/pbench/test/functional/server/test_user.py deleted file mode 100644 index a5ecd2b30e..0000000000 --- a/lib/pbench/test/functional/server/test_user.py +++ /dev/null @@ -1,214 +0,0 @@ -from http import HTTPStatus - -import pytest -from requests import HTTPError - -from pbench.client import API, PbenchServerClient - -USERNAME1 = "functional" -USERNAME2 = "nonfunctional" -PASSWORD = "tests" - - -class TestUser: - """ - Pbench Server functional tests for basic "user" operations. - - NOTE: The functional test system is intended to run on a pristine - containerized Pbench Server. We could check for some pre-conditions if we - had a known Administrative user (e.g., admin/admin). - """ - - def test_login_fail(self, server_client: PbenchServerClient): - """ - Trying to log in to a non-existent user should fail. - - NOTE: This will fail if "nouser" exists. - """ - with pytest.raises( - HTTPError, - match=f"UNAUTHORIZED for url: {server_client.scheme}://.*?/api/v1/login", - ): - server_client.login("nouser", "nopassword") - - @pytest.mark.parametrize( - "json", - [ - { - "username": USERNAME1, - "first_name": "Test", - "last_name": "Account", - "password": PASSWORD, - "email": f"{USERNAME1}@gmail.com", - }, - { - "username": USERNAME2, - "first_name": "Tester", - "last_name": "Accountant", - "password": PASSWORD, - "email": f"{USERNAME2}@gmail.com", - }, - ], - ) - def test_register(self, server_client: PbenchServerClient, json): - """ - Test that we can register a new user. - - NOTE: This will fail if the username already exists. - """ - response = server_client.post(API.REGISTER, json=json, raise_error=False) - assert ( - response.status_code == HTTPStatus.CREATED - ), f"Register failed with {response.json()}" - - def test_register_redux(self, server_client: PbenchServerClient): - """ - Test that an attempt to register an existing user fails. - """ - json = { - "username": USERNAME1, - "first_name": "Repeat", - "last_name": "Redux", - "password": PASSWORD, - "email": f"{USERNAME1}@gmail.com", - } - with pytest.raises( - HTTPError, - match=f"FORBIDDEN for url: {server_client.scheme}://.*?/api/v1/register", - ): - server_client.post(API.REGISTER, json=json) - - def test_profile_noauth(self, server_client: PbenchServerClient): - """ - Test that we can't access a user profile without authentication. - """ - with pytest.raises( - HTTPError, - match=f"UNAUTHORIZED for url: {server_client.scheme}://.*?/api/v1/user/{USERNAME1}", - ): - server_client.get(API.USER, {"target_username": USERNAME1}) - - def test_login(self, server_client: PbenchServerClient): - """ - Test that we can log in using our new user. - - NOTE: This assumes test cases will be run in order. There are Pytest - plugins to explicitly control test case order, but Pytest generally - does run tests within a single class in order. Is it worth taking on - another dependency to make this explicit? - """ - server_client.login(USERNAME1, PASSWORD) - assert server_client.auth_token - - def test_profile_bad_auth(self, server_client: PbenchServerClient): - """ - Test that we can't access a user profile with an invalid authentication - token. - """ - with pytest.raises( - HTTPError, - match=f"UNAUTHORIZED for url: {server_client.scheme}://.*?/api/v1/user/{USERNAME1}", - ): - server_client.get( - API.USER, - {"target_username": USERNAME1}, - headers={"Authorization": "Bearer of bad tokens"}, - ) - - def test_profile_not_self(self, server_client: PbenchServerClient): - """ - Test that we can't access another user's profile. - """ - with pytest.raises( - HTTPError, - match=f"FORBIDDEN for url: {server_client.scheme}://.*?/api/v1/user/{USERNAME2}", - ): - server_client.get(API.USER, {"target_username": USERNAME2}) - - def test_profile(self, server_client: PbenchServerClient): - """ - Test that we can retrieve our own user profile once logged in. - - NOTE: This assumes test cases will be run in order. - """ - response = server_client.get(API.USER, {"target_username": USERNAME1}) - assert response.json()["username"] == USERNAME1 - - def test_update(self, server_client: PbenchServerClient): - """ - Test that we can update our own user profile once logged in. - - NOTE: This assumes test cases will be run in order. - """ - response = server_client.get(API.USER, {"target_username": USERNAME1}) - user = response.json() - assert user["first_name"] != "Mycroft" - user["first_name"] = "Mycroft" - - # Remove fields that PUT will reject, but use a copy so that we can - # compare the new value against what we expect. - # - # This is unfortunate: PUT should IGNORE unmodifiable fields to allow a - # standard REST GET/update/PUT sequence. - payload = user.copy() - del payload["registered_on"] - - response = server_client.put( - API.USER, {"target_username": USERNAME1}, json=payload - ) - put_response = response.json() - - response = server_client.get(API.USER, {"target_username": USERNAME1}) - updated = response.json() - assert user == updated - assert put_response == user - - def test_delete_other(self, server_client: PbenchServerClient): - """ - Test that we can't delete another user. - - NOTE: This assumes test cases will be run in order. - """ - with pytest.raises( - HTTPError, - match=f"FORBIDDEN for url: {server_client.scheme}://.*?/api/v1/user/{USERNAME2}", - ): - server_client.delete(API.USER, {"target_username": USERNAME2}) - - def test_delete_nologin(self, server_client: PbenchServerClient): - """ - Test that we can't delete a user profile when not logged in. - - NOTE: This assumes test cases will be run in order. - """ - server_client.logout() - with pytest.raises( - HTTPError, - match=f"UNAUTHORIZED for url: {server_client.scheme}://.*?/api/v1/user/{USERNAME1}", - ): - server_client.delete(API.USER, {"target_username": USERNAME1}) - - def test_delete(self, server_client: PbenchServerClient): - """ - Test that we can delete our own user profile once logged in. - - NOTE: This assumes test cases will be run in order. - """ - server_client.login(USERNAME1, PASSWORD) - server_client.delete(API.USER, {"target_username": USERNAME1}) - - def test_logout(self, server_client: PbenchServerClient): - """ - Test that we can log out from our session. - - NOTE: This assumes test cases will be run in order. - """ - server_client.logout() - assert server_client.auth_token is None - - def test_cleanup_user2(self, server_client: PbenchServerClient): - """ - Remove the second test user to make the test idempotent. - """ - server_client.login(USERNAME2, PASSWORD) - server_client.delete(API.USER, {"target_username": USERNAME2}) diff --git a/lib/pbench/test/unit/client/conftest.py b/lib/pbench/test/unit/client/conftest.py index 9496cd9302..a3e87f3d21 100644 --- a/lib/pbench/test/unit/client/conftest.py +++ b/lib/pbench/test/unit/client/conftest.py @@ -16,8 +16,20 @@ def connect(): f"{pbench.url}/api/v1/endpoints", json={ "identification": "string", - "api": {"login": f"{pbench.url}/api/v1/login"}, + "api": {}, "uri": {}, + "openid": { + "server": "http://oidc_server", + "realm": "pbench-server", + "client": "pbench-client", + }, + }, + ) + responses.add( + responses.POST, + "http://oidc_server/realms/master/protocol/openid-connect/token", + json={ + "access_token": "admin_token", }, ) pbench.connect({"accept": "application/json"}) diff --git a/lib/pbench/test/unit/client/test_connect.py b/lib/pbench/test/unit/client/test_connect.py index 36e018deb5..0b9ef4d09a 100644 --- a/lib/pbench/test/unit/client/test_connect.py +++ b/lib/pbench/test/unit/client/test_connect.py @@ -22,12 +22,18 @@ def test_construct_url(self): def test_connect(self): pbench = PbenchServerClient("10.1.100.2") url = f"{pbench.url}/api/v1/endpoints" + openid_dict = {"server": "http://oidc_server", "client": "pbench_client"} with responses.RequestsMock() as rsp: rsp.add( responses.GET, url, - json={"identification": "string", "api": {}, "uri": {}}, + json={ + "identification": "string", + "api": {}, + "uri": {}, + "openid": openid_dict, + }, ) pbench.connect({"accept": "application/json"}) assert len(rsp.calls) == 1 @@ -51,3 +57,4 @@ def test_connect(self): assert endpoints["api"] == {} assert endpoints["identification"] == "string" assert endpoints["uri"] == {} + assert endpoints["openid"] == openid_dict diff --git a/lib/pbench/test/unit/client/test_login.py b/lib/pbench/test/unit/client/test_login.py index 6f23192845..650b52d9a9 100644 --- a/lib/pbench/test/unit/client/test_login.py +++ b/lib/pbench/test/unit/client/test_login.py @@ -1,9 +1,10 @@ from http import HTTPStatus import pytest -import requests import responses +from pbench.server.auth import OpenIDClientError + class TestLogin: def test_login(self, connect): @@ -11,11 +12,11 @@ def test_login(self, connect): Confirm that a successful Pbench Server login captures the 'username' and 'auth_token' in the client object. """ - url = f"{connect.url}/api/v1/login" + oidc_server = connect.endpoints["openid"]["server"] + oidc_realm = connect.endpoints["openid"]["realm"] + url = f"{oidc_server}/realms/{oidc_realm}/protocol/openid-connect/token" with responses.RequestsMock() as rsp: - rsp.add( - responses.POST, url, json={"username": "user", "auth_token": "foobar"} - ) + rsp.add(responses.POST, url, json={"access_token": "foobar"}) connect.login("user", "password") assert len(rsp.calls) == 1 assert rsp.calls[0].request.url == url @@ -30,18 +31,18 @@ def test_bad_login(self, connect): handled by the client library, and does not set the client 'username` and 'auth_token' properties. """ - url = f"{connect.url}/api/v1/login" + oidc_server = connect.endpoints["openid"]["server"] + oidc_realm = connect.endpoints["openid"]["realm"] + url = f"{oidc_server}/realms/{oidc_realm}/protocol/openid-connect/token" with responses.RequestsMock() as rsp: rsp.add( responses.POST, url, status=HTTPStatus.UNAUTHORIZED, - json={"username": "user", "auth_token": "foobar"}, + json={"error_description": "Invalid user credentials"}, ) - - with pytest.raises(requests.HTTPError): + with pytest.raises(OpenIDClientError): connect.login("user", "password") - assert len(rsp.calls) == 1 assert rsp.calls[0].request.url == url assert rsp.calls[0].response.status_code == 401 diff --git a/lib/pbench/test/unit/server/auth/test_auth.py b/lib/pbench/test/unit/server/auth/test_auth.py index 2aadca49d3..4559df6dac 100644 --- a/lib/pbench/test/unit/server/auth/test_auth.py +++ b/lib/pbench/test/unit/server/auth/test_auth.py @@ -1,7 +1,7 @@ import configparser from dataclasses import dataclass from http import HTTPStatus -from typing import Dict, Optional, Tuple, Union +from typing import Any, Dict, Optional, Tuple, Union from flask import current_app, Flask import jwt @@ -43,11 +43,17 @@ def fake_method(self, monkeypatch): args = {} def fake_method( - the_self, method: str, path: str, data: Dict, **kwargs + the_self, + method: str, + path: str, + data: Optional[Any] = None, + json: Optional[dict[str, Any]] = None, + **kwargs, ) -> requests.Response: args["method"] = method args["path"] = path args["data"] = data + args["json"] = json args["kwargs"] = kwargs return requests.Response() @@ -114,6 +120,7 @@ def request(self, *args, **kwargs): "HEAD", "/this/that", None, + None, headers={"header1": "one"}, this="that", that="this", @@ -123,6 +130,7 @@ def request(self, *args, **kwargs): assert response.args[1] == "https://example.com/this/that" assert response.kwargs["params"] == {"this": "that", "that": "this"} assert response.kwargs["data"] is None + assert response.kwargs["json"] is None assert response.kwargs["headers"] == {"header0": "zero", "header1": "one"} assert response.kwargs["verify"] is False @@ -160,6 +168,7 @@ def test_get(self, fake_method, conn): assert args["method"] == "GET" assert args["path"] == "foo/bar" assert args["data"] is None + assert args["json"] is None assert args["kwargs"] == {"headers": None, "this": "that", "then": "now"} def test_post(self, fake_method, conn): @@ -171,10 +180,13 @@ def test_post(self, fake_method, conn): conn : an existing Connection object to use for testing """ args = fake_method - response = conn.post("foo/bar", {"one": "two", "three": "four"}, five="six") + response = conn.post( + "foo/bar", data={"one": "two", "three": "four"}, five="six" + ) assert response is not None assert args["method"] == "POST" assert args["path"] == "foo/bar" + assert args["json"] is None assert args["data"] == {"one": "two", "three": "four"} assert args["kwargs"] == {"headers": None, "five": "six"} diff --git a/lib/pbench/test/unit/server/test_authorization.py b/lib/pbench/test/unit/server/test_authorization.py index d9a6ae3122..8639fa5ec3 100644 --- a/lib/pbench/test/unit/server/test_authorization.py +++ b/lib/pbench/test/unit/server/test_authorization.py @@ -70,7 +70,7 @@ def test_disallowed_admin(self, apibase, server_config, current_user_admin, ask) ApiAuthorizationType.USER_ACCESS, ask["role"], user, access ) ) - assert exc.value.owner == (ask["user"] if ask["user"] else "none") + assert exc.value.owner == (user if ask["user"] else None) assert exc.value.user == current_user_admin @pytest.mark.parametrize( @@ -128,7 +128,7 @@ def test_disallowed_auth( ApiAuthorizationType.USER_ACCESS, ask["role"], user, access ) ) - assert exc.value.owner == (ask["user"] if ask["user"] else "none") + assert exc.value.owner == (user if ask["user"] else None) assert exc.value.user == current_user_drb @pytest.mark.parametrize( @@ -176,7 +176,7 @@ def test_disallowed_noauth( ApiAuthorizationType.USER_ACCESS, ask["role"], user, access ) ) - assert exc.value.owner == (ask["user"] if ask["user"] else "none") + assert exc.value.owner == (user if ask["user"] else None) assert exc.value.user is None def test_admin_unauth(self, apibase, server_config, current_user_none): diff --git a/lib/pbench/test/unit/server/test_datasets_inventory.py b/lib/pbench/test/unit/server/test_datasets_inventory.py index 5248b9043a..1dc028f823 100644 --- a/lib/pbench/test/unit/server/test_datasets_inventory.py +++ b/lib/pbench/test/unit/server/test_datasets_inventory.py @@ -7,6 +7,7 @@ from pbench.server.cache_manager import CacheManager from pbench.server.database.models.datasets import Dataset, DatasetNotFound +from pbench.test.unit.server import TEST_USER_ID class TestDatasetsAccess: @@ -65,7 +66,7 @@ def test_dataset_not_present(self, query_get_as): def test_unauthorized_access(self, query_get_as): response = query_get_as("test", "metadata.log", HTTPStatus.FORBIDDEN) assert response.json == { - "message": "User drb is not authorized to READ a resource owned by test with private access" + "message": f"User drb is not authorized to READ a resource owned by {TEST_USER_ID} with private access" } def test_dataset_is_not_unpacked(self, query_get_as, monkeypatch): diff --git a/server/pbenchinacan/etc/pbench-server/pbench-server.cfg b/server/pbenchinacan/etc/pbench-server/pbench-server.cfg index c3ef8790a1..896d6c7665 100644 --- a/server/pbenchinacan/etc/pbench-server/pbench-server.cfg +++ b/server/pbenchinacan/etc/pbench-server/pbench-server.cfg @@ -24,7 +24,8 @@ uri = postgresql://pbenchcontainer:pbench@localhost:5432/pbenchcontainer secret-key = "pbench-in-a-can secret shhh" [openid-connect] -#server_url = http://localhost:8090 +server_url = http://localhost:8090 +secret = keycloak_secret realm = client = diff --git a/server/pbenchinacan/load_keycloak.sh b/server/pbenchinacan/load_keycloak.sh index 56dfba4e6b..7c23c5a186 100755 --- a/server/pbenchinacan/load_keycloak.sh +++ b/server/pbenchinacan/load_keycloak.sh @@ -65,10 +65,44 @@ else echo "Created ${REALM} realm" fi +# Create a client scope with custom mapper that will instruct Keycloak +# to include the (pbench-dashboard) when someone requests +# a token from Keycloak using a . +# Having in the aud claim of the token is essential for the token +# to be validated. +curl -si -f -X POST "${KEYCLOAK_HOST_PORT}/admin/realms/${REALM}/client-scopes" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}" \ + -H "Content-Type: application/json" \ + -d '{ + "name": "pbench", + "description": "", + "protocol": "openid-connect", + "attributes": { + "include.in.token.scope": "true", + "display.on.consent.screen": "true", + "gui.order": "", + "consent.screen.text": "" + }, + "protocolMappers": [ + { + "name": "pbench-mapper", + "protocol": "openid-connect", + "protocolMapper": "oidc-audience-mapper", + "consentRequired": false, + "config": { + "included.client.audience": "'${CLIENT}'", + "id.token.claim": "false", + "access.token.claim": "true" + } + } + ] + }' + + CLIENT_CONF=$(curl -si -f -X POST "${KEYCLOAK_HOST_PORT}/admin/realms/${REALM}/clients" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ - -d '{"clientId": "'${CLIENT}'", "publicClient": true, "directAccessGrantsEnabled": true, "enabled": true, "redirectUris": ["'${KEYCLOAK_REDIRECT_URI}'"]}') + -d '{"clientId": "'${CLIENT}'", "publicClient": true, "defaultClientScopes": ["pbench", "openid", "profile", "email"], "directAccessGrantsEnabled": true, "serviceAccountsEnabled": true, "enabled": true, "redirectUris": ["'${KEYCLOAK_REDIRECT_URI}'"]}') CLIENT_ID=$(grep -o -e 'http://[^[:space:]]*' <<< ${CLIENT_CONF} | sed -e 's|.*/||') if [[ -z "${CLIENT_ID}" ]]; then @@ -124,7 +158,7 @@ else echo "Assigned an 'ADMIN' client role to the user 'admin' created above" fi -# Verify that the user id has a role assigned to it +# Verify that the user id has an 'ADMIN' role assigned to it USER_ROLES=$(curl -s "${KEYCLOAK_HOST_PORT}/admin/realms/${REALM}/users/${USER_ID}/role-mappings/clients/${CLIENT_ID}" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" | jq -r '.[].name')