From 9f96d38413046d7aab54dcad07f5b12c8591d644 Mon Sep 17 00:00:00 2001 From: npalaska Date: Tue, 14 Feb 2023 19:00:00 -0500 Subject: [PATCH] fix client unit tests --- lib/pbench/client/__init__.py | 28 ++++++++++--------- lib/pbench/client/oidc_admin.py | 7 ++--- lib/pbench/server/auth/__init__.py | 13 ++++++--- lib/pbench/test/functional/server/conftest.py | 1 - .../test/functional/server/test_connect.py | 4 +-- 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 ++++++-------- server/pbenchinacan/load_keycloak.sh | 4 +-- 9 files changed, 61 insertions(+), 40 deletions(-) diff --git a/lib/pbench/client/__init__.py b/lib/pbench/client/__init__.py index b40b8ef4dd..5a6e9c146a 100644 --- a/lib/pbench/client/__init__.py +++ b/lib/pbench/client/__init__.py @@ -10,6 +10,7 @@ from pbench.client.oidc_admin import OIDCAdmin from pbench.client.types import Dataset, JSONOBJECT +from pbench.server.auth import OpenIDClientError class PbenchClientError(Exception): @@ -309,8 +310,8 @@ def connect(self, headers: Optional[dict[str, str]] = None) -> None: 1. Connect to the Pbench Server host using the endpoints API to be sure that it responds, and cache the endpoints response payload. - 2. Creates an OIDCAdmin object that server client can use to - perform admin privileged actions on OIDC server. + 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 @@ -330,25 +331,26 @@ def connect(self, headers: Optional[dict[str, str]] = None) -> None: # Create an OIDCAdmin object and confirm the connection was successful self.oidc_admin = OIDCAdmin(server_url=self.endpoints["openid"]["server"]) - assert self.oidc_admin.get_admin_token()["access_token"] def login(self, user: str, password: str): - """Login to a specified username with the password on OIDC server, + """Log into the OIDC server using the specified username and password, and store the resulting authentication token. Args: user: Account username password: Account password """ - response = self.oidc_admin.user_login( - client_id=self.endpoints["openid"]["client"], - username=user, - password=password, - ) - assert response["access_token"] - auth_token = response["access_token"] - self.username = user - self.auth_token = auth_token + try: + response = self.oidc_admin.user_login( + client_id=self.endpoints["openid"]["client"], + username=user, + password=password, + ) + except OpenIDClientError: + self.auth_token = None + else: + 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 index 23ce4761de..a1064ddc15 100644 --- a/lib/pbench/client/oidc_admin.py +++ b/lib/pbench/client/oidc_admin.py @@ -1,5 +1,4 @@ from http import HTTPStatus -import json import os import requests @@ -9,8 +8,8 @@ class OIDCAdmin(Connection): OIDC_REALM = os.getenv("OIDC_REALM", "pbench-server") - ADMIN_USERNAME = os.getenv("ADMIN_USERNAME", "admin") - ADMIN_PASSWORD = os.getenv("ADMIN_PASSWORD", "admin") + 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) @@ -79,7 +78,7 @@ def create_new_user( {"type": "password", "value": password, "temporary": False} ], } - response = self.post(path=url_path, data=json.dumps(data), headers=headers) + response = self.post(path=url_path, json=data, headers=headers) return response def user_login(self, client_id: str, username: str, password: str) -> dict: diff --git a/lib/pbench/server/auth/__init__.py b/lib/pbench/server/auth/__init__.py index d3b5207b37..90665326b4 100644 --- a/lib/pbench/server/auth/__init__.py +++ b/lib/pbench/server/auth/__init__.py @@ -51,6 +51,7 @@ def _method( method: str, path: str, data: Union[Dict, str, None], + json: Optional[Dict] = None, headers: Optional[Dict] = None, **kwargs, ) -> requests.Response: @@ -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 : 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: @@ -72,6 +74,7 @@ def _method( kwargs = dict( params=kwargs, data=data, + json=json, headers=final_headers, verify=self.verify, ) @@ -128,7 +131,8 @@ def get( def post( self, path: str, - data: Union[Dict, str], + data: Union[Dict, str] = None, + json: Optional[Dict] = None, headers: Optional[Dict] = None, **kwargs, ) -> requests.Response: @@ -137,14 +141,15 @@ def post( Args: path : Path for the request - data : JSON request body + data : Request 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 diff --git a/lib/pbench/test/functional/server/conftest.py b/lib/pbench/test/functional/server/conftest.py index 0a61d83757..060f7b257b 100644 --- a/lib/pbench/test/functional/server/conftest.py +++ b/lib/pbench/test/functional/server/conftest.py @@ -63,4 +63,3 @@ def login_user(server_client: PbenchServerClient, register_test_user): """Log in the test user and return the authentication token""" server_client.login(USERNAME, PASSWORD) assert server_client.auth_token - yield diff --git a/lib/pbench/test/functional/server/test_connect.py b/lib/pbench/test/functional/server/test_connect.py index d29a2f0886..f58857fe8f 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"} - assert set(endpoints["openid"]) >= expected + expected = {"server", "client", "realm"} + assert set(endpoints["openid"]) == expected 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..94747439e7 100644 --- a/lib/pbench/test/unit/client/test_login.py +++ b/lib/pbench/test/unit/client/test_login.py @@ -1,7 +1,5 @@ from http import HTTPStatus -import pytest -import requests import responses @@ -11,11 +9,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 +28,17 @@ 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): - connect.login("user", "password") - + 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/server/pbenchinacan/load_keycloak.sh b/server/pbenchinacan/load_keycloak.sh index 9fbab02ee0..7c23c5a186 100755 --- a/server/pbenchinacan/load_keycloak.sh +++ b/server/pbenchinacan/load_keycloak.sh @@ -23,8 +23,8 @@ ADMIN_USERNAME=${ADMIN_USERNAME:-"admin"} ADMIN_PASSWORD=${ADMIN_PASSWORD:-"admin"} # These values must match the options "realm" and "client in the # "openid-connect" section of the pbench server configuration file. -REALM=${KEYCLOAK_REALM:-"pbench-server-test"} -CLIENT=${KEYCLOAK_CLIENT:-"pbench-dashboard-test"} +REALM=${KEYCLOAK_REALM:-"pbench-server"} +CLIENT=${KEYCLOAK_CLIENT:-"pbench-dashboard"} end_in_epoch_secs=$(date --date "2 minutes" +%s)