Skip to content

Commit

Permalink
fix client unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
npalaska committed Feb 15, 2023
1 parent 2a94b6f commit c0c8625
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 43 deletions.
28 changes: 15 additions & 13 deletions lib/pbench/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
7 changes: 3 additions & 4 deletions lib/pbench/client/oidc_admin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from http import HTTPStatus
import json
import os

import requests
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 10 additions & 5 deletions lib/pbench/server/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -72,6 +74,7 @@ def _method(
kwargs = dict(
params=kwargs,
data=data,
json=json,
headers=final_headers,
verify=self.verify,
)
Expand Down Expand Up @@ -123,12 +126,13 @@ def get(
Returns:
Response from the request.
"""
return self._method("GET", path, None, headers=headers, **kwargs)
return self._method("GET", path, None, None, headers=headers, **kwargs)

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:
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion lib/pbench/test/functional/server/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions lib/pbench/test/functional/server/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 13 additions & 1 deletion lib/pbench/test/unit/client/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down
9 changes: 8 additions & 1 deletion lib/pbench/test/unit/client/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -51,3 +57,4 @@ def test_connect(self):
assert endpoints["api"] == {}
assert endpoints["identification"] == "string"
assert endpoints["uri"] == {}
assert endpoints["openid"] == openid_dict
21 changes: 9 additions & 12 deletions lib/pbench/test/unit/client/test_login.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from http import HTTPStatus

import pytest
import requests
import responses


Expand All @@ -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
Expand All @@ -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
Expand Down
11 changes: 9 additions & 2 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ 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: Dict, json: Dict, **kwargs
) -> requests.Response:
args["method"] = method
args["path"] = path
args["data"] = data
args["json"] = json
args["kwargs"] = kwargs
return requests.Response()

Expand Down Expand Up @@ -114,6 +115,7 @@ def request(self, *args, **kwargs):
"HEAD",
"/this/that",
None,
None,
headers={"header1": "one"},
this="that",
that="this",
Expand All @@ -123,6 +125,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

Expand Down Expand Up @@ -160,6 +163,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):
Expand All @@ -171,10 +175,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", {"one": "two", "three": "four"}, None, 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"}

Expand Down
4 changes: 2 additions & 2 deletions server/pbenchinacan/load_keycloak.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit c0c8625

Please sign in to comment.