From 7cfbdd5ddc807ce09804f1ead56feaffb4b3f9cc Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Wed, 17 May 2023 12:50:16 +0200 Subject: [PATCH 1/9] Use /login endpoint instead of basic login The returned sessionID is then sent in subsequent requests. Signed-off-by: Justin Cinkelj --- plugins/module_utils/client.py | 25 +++++++++++--- .../unit/plugins/module_utils/test_client.py | 34 ++++++++++--------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/plugins/module_utils/client.py b/plugins/module_utils/client.py index f4812c391..8d8f97e17 100644 --- a/plugins/module_utils/client.py +++ b/plugins/module_utils/client.py @@ -13,7 +13,7 @@ from typing import Any, Optional, Union from io import BufferedReader -from ansible.module_utils.urls import Request, basic_auth_header +from ansible.module_utils.urls import Request from .errors import ( AuthError, @@ -98,7 +98,23 @@ def _login(self) -> dict[str, bytes]: return self._login_username_password() def _login_username_password(self) -> dict[str, bytes]: - return dict(Authorization=basic_auth_header(self.username, self.password)) + headers = { + "Accept": "application/json", + "Content-type": "application/json", + } + resp = self._request( + "POST", + f"{self.host}/rest/v1/login", + data=json.dumps( + dict( + username=self.username, + password=self.password, + ) + ), + headers=headers, + timeout=self.timeout, + ) + return dict(Cookie=f"sessionID={resp.json['sessionID']}") def _request( self, @@ -108,9 +124,8 @@ def _request( headers: Optional[dict[Any, Any]] = None, timeout: Optional[float] = None, ) -> Response: - if ( - timeout is None - ): # If timeout from request is not specifically provided, take it from the Client. + if timeout is None: + # If timeout from request is not specifically provided, take it from the Client. timeout = self.timeout try: raw_resp = self._client.open( diff --git a/tests/unit/plugins/module_utils/test_client.py b/tests/unit/plugins/module_utils/test_client.py index 3f75333aa..94f1d2363 100644 --- a/tests/unit/plugins/module_utils/test_client.py +++ b/tests/unit/plugins/module_utils/test_client.py @@ -91,32 +91,26 @@ def test_valid_host(self, host): class TestClientAuthHeader: - def test_basic_auth(self): - c = client.Client("https://instance.com", "user", "pass", None) - assert c.auth_header == {"Authorization": b"Basic dXNlcjpwYXNz"} - - def test_oauth(self, mocker): + def test_basic_auth(self, mocker): resp_mock = mocker.MagicMock() resp_mock.status = 200 # Used when testing on Python 3 - resp_mock.getcode.return_value = 200 # Used when testing on Python 2 - resp_mock.read.return_value = '{"access_token": "token"}' + resp_mock.read.return_value = ( + '{"sessionID":"7e3a2a70-7130-41c4-9402-fc0953cc1d7b"}' + ) request_mock = mocker.patch.object(client, "Request").return_value request_mock.open.return_value = resp_mock - c = client.Client( - "https://instance.com", - "user", - "pass", - None, - ) - - assert c.auth_header == {"Authorization": b"Basic dXNlcjpwYXNz"} + c = client.Client("https://instance.com", "user", "pass", None) + assert c.auth_header == { + "Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b" + } class TestClientRequest: def test_request_without_data_success(self, mocker): c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, '{"returned": "data"}', headers=[("Content-type", "application/json")] ) @@ -136,6 +130,7 @@ def test_request_without_data_success(self, mocker): def test_request_with_data_success(self, mocker): c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, '{"returned": "data"}', headers=[("Content-type", "application/json")] ) @@ -151,7 +146,7 @@ def test_request_with_data_success(self, mocker): headers={ "Accept": "application/json", "Content-type": "application/json", - "Authorization": c.auth_header["Authorization"], + "Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b", }, timeout=None, ) @@ -162,6 +157,7 @@ def test_auth_error(self, mocker): request_mock.open.side_effect = HTTPError("", 401, "Unauthorized", {}, None) c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} with pytest.raises(errors.AuthError): c.request("GET", "api/rest/v1/some/path") @@ -172,6 +168,7 @@ def test_http_error(self, mocker): ) c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} resp = c.request("GET", "api/rest/v1/some/path") assert resp.status == 404 @@ -183,6 +180,7 @@ def test_url_error(self, mocker): request_mock.open.side_effect = URLError("some error") c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} with pytest.raises(errors.ScaleComputingError, match="some error"): c.request("GET", "api/rest/v1/some/path") @@ -193,6 +191,7 @@ def test_path_escaping(self, mocker): raw_request.read.return_value = "{}" c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some path") request_mock.open.assert_called_once() @@ -206,6 +205,7 @@ def test_path_without_query(self, mocker, query): raw_request.read.return_value = "{}" c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some/path", query=query) request_mock.open.assert_called_once() @@ -226,6 +226,7 @@ def test_path_with_query(self, mocker, query): raw_request.read.return_value = "{}" c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some/path", query=query) request_mock.open.assert_called_once() @@ -235,6 +236,7 @@ def test_path_with_query(self, mocker, query): def test_request_without_data_binary_success(self, mocker): c = client.Client("https://instance.com", "user", "pass", None) + c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, "data", headers=[("Content-type", "image/apng")] ) From 86537304a7acc63745bfd15f5c31f02b0b1a837b Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Wed, 17 May 2023 14:33:41 +0200 Subject: [PATCH 2/9] Optionally login with OIDC user Signed-off-by: Justin Cinkelj --- plugins/doc_fragments/cluster_instance.py | 12 ++++ plugins/module_utils/arguments.py | 7 +++ plugins/module_utils/client.py | 11 ++++ .../unit/plugins/module_utils/test_client.py | 56 +++++++++---------- .../test_rest_client_CachedRestClient.py | 6 +- 5 files changed, 61 insertions(+), 31 deletions(-) diff --git a/plugins/doc_fragments/cluster_instance.py b/plugins/doc_fragments/cluster_instance.py index 66fc175e2..a7900bba0 100644 --- a/plugins/doc_fragments/cluster_instance.py +++ b/plugins/doc_fragments/cluster_instance.py @@ -47,4 +47,16 @@ class ModuleDocFragment(object): variable will be used. required: false type: float + auth_method: + description: + - Select login method. + - If not set, the value of the C(SC_AUTH_METHOD) environment + variable will be used. + - Value I(local) - username/password is verified by the HyperCore server (the local users). + - Value I(oidc) - username/password is verified by the configured OIDC provider. + default: local + choices: + - local + - oidc + type: str """ diff --git a/plugins/module_utils/arguments.py b/plugins/module_utils/arguments.py index 76f894bb9..40095bb9f 100644 --- a/plugins/module_utils/arguments.py +++ b/plugins/module_utils/arguments.py @@ -10,6 +10,7 @@ from ansible.module_utils.basic import env_fallback from typing import Any +from ..module_utils.client import AuthMethod # TODO - env from /etc/environment is loaded # But when env is set in bash session, env seems to be lost on ssh connection to localhost. @@ -42,6 +43,12 @@ required=False, fallback=(env_fallback, ["SC_TIMEOUT"]), ), + auth_method=dict( + type="str", + default=AuthMethod.local, + choices=[am.value for am in AuthMethod], + fallback=(env_fallback, ["SC_AUTH_METHOD"]), + ), ), required_together=[("username", "password")], ), diff --git a/plugins/module_utils/client.py b/plugins/module_utils/client.py index 8d8f97e17..392e3a7a7 100644 --- a/plugins/module_utils/client.py +++ b/plugins/module_utils/client.py @@ -12,6 +12,7 @@ import ssl from typing import Any, Optional, Union from io import BufferedReader +import enum from ansible.module_utils.urls import Request @@ -29,6 +30,11 @@ DEFAULT_HEADERS = dict(Accept="application/json") +class AuthMethod(str, enum.Enum): + local = "local" + oidc = "oidc" + + class Response: # I have felling (but I'm not sure) we will always use # "Response(raw_resp.status, raw_resp.read(), raw_resp.headers)" @@ -64,6 +70,7 @@ def __init__( username: str, password: str, timeout: float, + auth_method: str, ): if not (host or "").startswith(("https://", "http://")): raise ScaleComputingError( @@ -75,6 +82,7 @@ def __init__( self.username = username self.password = password self.timeout = timeout + self.auth_method = auth_method self._auth_header: Optional[dict[str, bytes]] = None self._client = Request() @@ -86,6 +94,7 @@ def get_client(cls, cluster_instance: TypedClusterInstance) -> Client: cluster_instance["username"], cluster_instance["password"], cluster_instance["timeout"], + cluster_instance["auth_method"], ) @property @@ -102,6 +111,7 @@ def _login_username_password(self) -> dict[str, bytes]: "Accept": "application/json", "Content-type": "application/json", } + use_oidc = self.auth_method == AuthMethod.oidc.value resp = self._request( "POST", f"{self.host}/rest/v1/login", @@ -109,6 +119,7 @@ def _login_username_password(self) -> dict[str, bytes]: dict( username=self.username, password=self.password, + useOIDC=use_oidc, ) ), headers=headers, diff --git a/tests/unit/plugins/module_utils/test_client.py b/tests/unit/plugins/module_utils/test_client.py index 94f1d2363..85011009e 100644 --- a/tests/unit/plugins/module_utils/test_client.py +++ b/tests/unit/plugins/module_utils/test_client.py @@ -83,11 +83,11 @@ def test_invalid_host(self, host): with pytest.raises( errors.ScaleComputingError, match="Invalid instance host value" ): - client.Client(host, "user", "pass", None) + client.Client(host, "user", "pass", None, "local") @pytest.mark.parametrize("host", ["http://insecure.host", "https://secure.host"]) def test_valid_host(self, host): - client.Client(host, "user", "pass", None) + client.Client(host, "user", "pass", None, "local") class TestClientAuthHeader: @@ -101,7 +101,7 @@ def test_basic_auth(self, mocker): request_mock = mocker.patch.object(client, "Request").return_value request_mock.open.return_value = resp_mock - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") assert c.auth_header == { "Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b" } @@ -109,7 +109,7 @@ def test_basic_auth(self, mocker): class TestClientRequest: def test_request_without_data_success(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, '{"returned": "data"}', headers=[("Content-type", "application/json")] @@ -129,7 +129,7 @@ def test_request_without_data_success(self, mocker): assert resp == mock_response def test_request_with_data_success(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, '{"returned": "data"}', headers=[("Content-type", "application/json")] @@ -156,7 +156,7 @@ def test_auth_error(self, mocker): request_mock = mocker.patch.object(client, "Request").return_value request_mock.open.side_effect = HTTPError("", 401, "Unauthorized", {}, None) - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} with pytest.raises(errors.AuthError): c.request("GET", "api/rest/v1/some/path") @@ -167,7 +167,7 @@ def test_http_error(self, mocker): "", 404, "Not Found", {}, io.StringIO(to_text("My Error")) ) - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} resp = c.request("GET", "api/rest/v1/some/path") @@ -179,7 +179,7 @@ def test_url_error(self, mocker): request_mock = mocker.patch.object(client, "Request").return_value request_mock.open.side_effect = URLError("some error") - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} with pytest.raises(errors.ScaleComputingError, match="some error"): @@ -190,7 +190,7 @@ def test_path_escaping(self, mocker): raw_request = mocker.MagicMock(status=200) raw_request.read.return_value = "{}" - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some path") @@ -204,7 +204,7 @@ def test_path_without_query(self, mocker, query): raw_request = mocker.MagicMock(status=200) raw_request.read.return_value = "{}" - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some/path", query=query) @@ -225,7 +225,7 @@ def test_path_with_query(self, mocker, query): raw_request = mocker.MagicMock(status=200) raw_request.read.return_value = "{}" - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} c.request("GET", "api/rest/v1/some/path", query=query) @@ -235,7 +235,7 @@ def test_path_with_query(self, mocker, query): assert parsed_query == dict((k, [str(v)]) for k, v in query.items()) def test_request_without_data_binary_success(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") c._auth_header = {"Cookie": "sessionID=7e3a2a70-7130-41c4-9402-fc0953cc1d7b"} mock_response = client.Response( 200, "data", headers=[("Content-type", "image/apng")] @@ -263,7 +263,7 @@ def test_request_without_data_binary_success(self, mocker): class TestClientGet: def test_ok(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") mock_response = client.Response(200, '{"incident": 1}', None) request_mock = mocker.patch.object(c, "request") request_mock.return_value = mock_response @@ -274,7 +274,7 @@ def test_ok(self, mocker): assert resp.json == {"incident": 1} def test_ok_missing(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") mock_response = client.Response(404, "Not Found", None) request_mock = mocker.patch.object(c, "request") request_mock.return_value = mock_response @@ -284,7 +284,7 @@ def test_ok_missing(self, mocker): assert resp == mock_response def test_error(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(403, "forbidden") @@ -292,7 +292,7 @@ def test_error(self, mocker): c.get("api/rest/v1/table/incident/1") def test_query(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(200, '{"incident": 1}', None) @@ -305,7 +305,7 @@ def test_query(self, mocker): class TestClientPost: def test_ok(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") mock_response = client.Response(201, '{"incident": 1}') request_mock = mocker.patch.object(c, "request") request_mock.return_value = mock_response @@ -316,7 +316,7 @@ def test_ok(self, mocker): assert resp.json == {"incident": 1} def test_error(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(400, "bad request") @@ -324,7 +324,7 @@ def test_error(self, mocker): c.post("api/rest/v1/table/incident", {"some": "data"}) def test_query(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(201, '{"incident": 1}') @@ -341,7 +341,7 @@ def test_query(self, mocker): class TestClientPatch: def test_ok(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") mock_response = client.Response(200, '{"incident": 1}') request_mock = mocker.patch.object(c, "request") request_mock.return_value = mock_response @@ -352,7 +352,7 @@ def test_ok(self, mocker): assert resp.json == {"incident": 1} def test_error(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(400, "bad request") @@ -360,7 +360,7 @@ def test_error(self, mocker): c.patch("api/rest/v1/table/incident/1", {"some": "data"}) def test_query(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(200, '{"incident": 1}') @@ -377,7 +377,7 @@ def test_query(self, mocker): class TestClientPut: def test_ok(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") mock_response = client.Response(200, '{"incident": 1}') request_mock = mocker.patch.object(c, "request") request_mock.return_value = mock_response @@ -388,7 +388,7 @@ def test_ok(self, mocker): assert resp.json == {"incident": 1} def test_error(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(400, "bad request") @@ -396,7 +396,7 @@ def test_error(self, mocker): c.put("api/rest/v1/table/incident/1", {"some": "data"}) def test_query(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(200, '{"incident": 1}') @@ -415,14 +415,14 @@ def test_query(self, mocker): class TestClientDelete: def test_ok(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(204, {}) c.delete("api/rest/v1/table/resource/1") def test_error(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(404, "not found") @@ -430,7 +430,7 @@ def test_error(self, mocker): c.delete("api/rest/v1/table/resource/1") def test_query(self, mocker): - c = client.Client("https://instance.com", "user", "pass", None) + c = client.Client("https://instance.com", "user", "pass", None, "local") request_mock = mocker.patch.object(c, "request") request_mock.return_value = client.Response(204, {}) diff --git a/tests/unit/plugins/module_utils/test_rest_client_CachedRestClient.py b/tests/unit/plugins/module_utils/test_rest_client_CachedRestClient.py index 17cb398ab..0458e62c0 100644 --- a/tests/unit/plugins/module_utils/test_rest_client_CachedRestClient.py +++ b/tests/unit/plugins/module_utils/test_rest_client_CachedRestClient.py @@ -50,7 +50,7 @@ def test_list_records(self, mocker, swap_query_order): query1, query0 = query0, query1 expected_data1, expected_data0 = expected_data0, expected_data1 - client_obj = client.Client("https://thehost", "user", "pass", None) + client_obj = client.Client("https://thehost", "user", "pass", None, "local") cached_client = rest_client.CachedRestClient(client=client_obj) client_mock = mocker.patch.object(cached_client.client, "get") client_mock.return_value = client.Response(200, data, "") @@ -88,7 +88,7 @@ def mockup_client_get(path, timeout): else: return client.Response(200, data1, "") - client_obj = client.Client("https://thehost", "user", "pass", None) + client_obj = client.Client("https://thehost", "user", "pass", None, "local") cached_client = rest_client.CachedRestClient(client=client_obj) client_mock = mocker.patch.object(cached_client.client, "get") client_mock.side_effect = mockup_client_get @@ -118,7 +118,7 @@ def test_list_records_empty_response(self, mocker): endpoint = "vms" data = "[]" - client_obj = client.Client("https://thehost", "user", "pass", None) + client_obj = client.Client("https://thehost", "user", "pass", None, "local") cached_client = rest_client.CachedRestClient(client=client_obj) client_mock = mocker.patch.object(cached_client.client, "get") client_mock.return_value = client.Response(200, data, "") From 74c794d4399a561f99df772549e1e65930f46204 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Wed, 17 May 2023 13:45:35 +0200 Subject: [PATCH 3/9] Integration test for OIDC user login Signed-off-by: Justin Cinkelj --- tests/integration/integration_config.yml.j2 | 4 + .../targets/utils_login/tasks/main.yml | 75 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/integration/targets/utils_login/tasks/main.yml diff --git a/tests/integration/integration_config.yml.j2 b/tests/integration/integration_config.yml.j2 index e688e13a9..8f810e448 100644 --- a/tests/integration/integration_config.yml.j2 +++ b/tests/integration/integration_config.yml.j2 @@ -55,6 +55,10 @@ sc_config: shared_secret_default: "{{ oidc_client_secret }}" config_url_default: https://login.microsoftonline.com/76d4c62a-a9ca-4dc2-9187-e2cc4d9abe7f/v2.0/.well-known/openid-configuration scopes: "openid+profile" + # Users that can login when config_url_default is configured as OIDC provider + users: + - username: "{{ oidc_users_0_username }}" + password: "{{ oidc_users_0_password }}" client_id_test: ci-client-id shared_secret_test: ci-client-secret config_url_test: https://login.microsoftonline.com/76d4c62a-a9ca-4dc2-9187-e2cc4d9abe7f/v2.0/.well-known/openid-configuration diff --git a/tests/integration/targets/utils_login/tasks/main.yml b/tests/integration/targets/utils_login/tasks/main.yml new file mode 100644 index 000000000..67cb927fc --- /dev/null +++ b/tests/integration/targets/utils_login/tasks/main.yml @@ -0,0 +1,75 @@ +--- +# Test modules are able to login to Hypercore. +# The local or oidc user accounts are supported. + +# =================================================================== +# local user, from environ +- environment: + SC_HOST: "{{ sc_host }}" + SC_USERNAME: "{{ sc_config[sc_host].sc_username }}" + SC_PASSWORD: "{{ sc_config[sc_host].sc_password }}" + SC_TIMEOUT: "{{ sc_timeout }}" + SC_AUTH_METHOD: local + + block: + - name: Get HC3 ping - local user, from environ + scale_computing.hypercore.api: + action: get + endpoint: /rest/v1/ping + register: ping_result + - &assert_ping_result + name: Check ping_result + ansible.builtin.assert: + that: + - ping_result is not changed + - ping_result.record.0 == "status" + +# =================================================================== +# local user, from cluster_instance variable +- vars: + cluster_instance: + host: "{{ sc_host }}" + username: "{{ sc_config[sc_host].sc_username }}" + password: "{{ sc_config[sc_host].sc_password }}" + auth_method: local + block: + - name: Get HC3 ping - local user, from cluster_instance variable + scale_computing.hypercore.api: + action: get + endpoint: /rest/v1/ping + cluster_instance: "{{ cluster_instance }}" + register: ping_result + - *assert_ping_result + +# =================================================================== +# OIDC user, from environ +- environment: + SC_HOST: "{{ sc_host }}" + SC_USERNAME: "{{ sc_config[sc_host].oidc.users[0].username }}" + SC_PASSWORD: "{{ sc_config[sc_host].oidc.users[0].password }}" + SC_TIMEOUT: "{{ sc_timeout }}" + SC_AUTH_METHOD: oidc + block: + - name: Get HC3 ping - OIDC user, from environ + scale_computing.hypercore.api: + action: get + endpoint: /rest/v1/ping + register: ping_result + - *assert_ping_result + +# =================================================================== +# OIDC user, from cluster_instance variable +- vars: + cluster_instance: &cluster_instance_local + host: "{{ sc_host }}" + username: "{{ sc_config[sc_host].oidc.users[0].username }}" + password: "{{ sc_config[sc_host].oidc.users[0].password }}" + auth_method: oidc + block: + - name: Get HC3 ping - OIDC user, from cluster_instance variable + scale_computing.hypercore.api: + action: get + endpoint: /rest/v1/ping + cluster_instance: "{{ cluster_instance }}" + register: ping_result + - *assert_ping_result From 475a11986c651acd5df852e7117ef77b02011a91 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Thu, 18 May 2023 08:24:38 +0200 Subject: [PATCH 4/9] CI Test OIDC login only on .50 HC3 cluster The oidc_config was tested there a while ago, so we have OIDC configured. Signed-off-by: Justin Cinkelj --- .github/workflows/integ-test.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integ-test.yml b/.github/workflows/integ-test.yml index 41ccd7324..a68cdefea 100644 --- a/.github/workflows/integ-test.yml +++ b/.github/workflows/integ-test.yml @@ -14,6 +14,7 @@ on: # dns_config|time_server - NTP cannot be reconfigured if DNS is invalid # git_issues - slow, do not run on each push. TODO - run them only once a day # oidc_config - during reconfiguration API returns 500/502 errors for other requests + # utils_login - it uses OIDC user to login # smtp - email_alert test requires a configured SMTP # role_cluster_config - role cluster_config reconfigures DNS, SMTP, OIDC. And it is slow. # version_update_single_node - role would change version, VSNS system cannot be updated. @@ -23,7 +24,7 @@ on: List integration tests to exclude. Use "*" to exclude all tests. Use regex like 'node|^git_issue|^dns_config$' to exclude only a subset. - default: "^dns_config$|^cluster_shutdown$|^version_update$|^oidc_config$|^smtp$|^role_cluster_config$|^role_version_update_single_node$" + default: "^dns_config$|^cluster_shutdown$|^version_update$|^oidc_config$|^smtp$|^role_cluster_config$|^role_version_update_single_node$|^utils_login$" examples_tests_include: type: string description: |- @@ -32,7 +33,7 @@ on: default: "iso_info" env: INTEG_TESTS_INCLUDE_SCHEDULE: "*" - INTEG_TESTS_EXCLUDE_SCHEDULE: "^dns_config$|^cluster_shutdown$|^version_update$|^oidc_config$|^smtp$|^role_cluster_config$|^role_version_update_single_node$" + INTEG_TESTS_EXCLUDE_SCHEDULE: "^dns_config$|^cluster_shutdown$|^version_update$|^oidc_config$|^smtp$|^role_cluster_config$|^role_version_update_single_node$|^utils_login$" EXAMPLES_TESTS_INCLUDE_SCHEDULE: "*" # ansible-test needs special directory structure. # WORKDIR is a subdir of GITHUB_WORKSPACE @@ -220,6 +221,8 @@ jobs: test_name: support_tunnel - sc_host: https://10.5.11.50 test_name: vm_clone__replicated + - sc_host: https://10.5.11.50 + test_name: utils_login exclude: # The VSNS were not configured with remote replication cluster. - sc_host: https://10.5.11.200 From f0f92d50fda72c1fbd9eef691befc5400aaf8160 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Thu, 18 May 2023 10:03:31 +0200 Subject: [PATCH 5/9] OIDC login for inventory plugin Signed-off-by: Justin Cinkelj --- .github/workflows/integ-test.yml | 4 +++ plugins/inventory/hypercore.py | 3 +- tests/integration/targets/inventory/runme.sh | 31 ++++++++++++++++++-- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/.github/workflows/integ-test.yml b/.github/workflows/integ-test.yml index a68cdefea..0c8e5d855 100644 --- a/.github/workflows/integ-test.yml +++ b/.github/workflows/integ-test.yml @@ -221,8 +221,12 @@ jobs: test_name: support_tunnel - sc_host: https://10.5.11.50 test_name: vm_clone__replicated + # test oidc login - sc_host: https://10.5.11.50 test_name: utils_login + # test inventory plugin with oidc login + - sc_host: https://10.5.11.50 + test_name: inventory exclude: # The VSNS were not configured with remote replication cluster. - sc_host: https://10.5.11.200 diff --git a/plugins/inventory/hypercore.py b/plugins/inventory/hypercore.py index f6d6426f3..88db7d76f 100644 --- a/plugins/inventory/hypercore.py +++ b/plugins/inventory/hypercore.py @@ -223,6 +223,7 @@ def parse(self, inventory, loader, path, cache=False): username = os.getenv("SC_USERNAME") password = os.getenv("SC_PASSWORD") timeout = os.getenv("SC_TIMEOUT") + auth_method = os.getenv("SC_AUTH_METHOD") if timeout: try: timeout = float(timeout) @@ -234,7 +235,7 @@ def parse(self, inventory, loader, path, cache=False): raise errors.ScaleComputingError( "Missing one or more parameters: sc_host, sc_username, sc_password." ) - client = Client(host, username, password, timeout) + client = Client(host, username, password, timeout, auth_method) rest_client = RestClient(client) vms = rest_client.list_records("/rest/v1/VirDomain") diff --git a/tests/integration/targets/inventory/runme.sh b/tests/integration/targets/inventory/runme.sh index dc05ad020..f6ba4b77a 100755 --- a/tests/integration/targets/inventory/runme.sh +++ b/tests/integration/targets/inventory/runme.sh @@ -12,6 +12,7 @@ print("export SC_HOST='{}'".format(sc_host)) print("export SC_TIMEOUT='{}'".format(sc_timeout)) print("export SC_USERNAME='{}'".format(data["sc_config"][sc_host]["sc_username"])) print("export SC_PASSWORD='{}'".format(data["sc_config"][sc_host]["sc_password"])) +# SC_AUTH_METHOD==local by default, leave it unset EOF )" @@ -45,8 +46,34 @@ ansible-playbook -i localhost, -i hypercore_inventory_ansible_enable.yml run_ans ansible-playbook -i localhost, -i hypercore_inventory_ansible_disable.yml run_ansible_disable_tests.yml ansible-playbook -i localhost, -i hypercore_inventory_ansible_both_true.yml run_ansible_both_true_tests.yml -unset SC_TIMEOUT # do one test without SC_TIMEOUT - +# do one test without SC_TIMEOUT +unset SC_TIMEOUT +ansible-playbook -i localhost, -i hypercore_inventory_ansible_both_false.yml run_ansible_both_false_tests.yml +# test with SC_AUTH_METHOD being set to "local" +export SC_AUTH_METHOD=local ansible-playbook -i localhost, -i hypercore_inventory_ansible_both_false.yml run_ansible_both_false_tests.yml +# test with OIDC user +eval "$(cat < Date: Thu, 18 May 2023 10:52:53 +0200 Subject: [PATCH 6/9] Fix bytes vs str for sessionID Signed-off-by: Justin Cinkelj --- plugins/module_utils/client.py | 8 ++++---- plugins/module_utils/typed_classes.py | 1 + tests/unit/plugins/module_utils/test_client.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/plugins/module_utils/client.py b/plugins/module_utils/client.py index 392e3a7a7..fa3294b2e 100644 --- a/plugins/module_utils/client.py +++ b/plugins/module_utils/client.py @@ -84,7 +84,7 @@ def __init__( self.timeout = timeout self.auth_method = auth_method - self._auth_header: Optional[dict[str, bytes]] = None + self._auth_header: Optional[dict[str, str]] = None self._client = Request() @classmethod @@ -98,15 +98,15 @@ def get_client(cls, cluster_instance: TypedClusterInstance) -> Client: ) @property - def auth_header(self) -> dict[str, bytes]: + def auth_header(self) -> dict[str, str]: if not self._auth_header: self._auth_header = self._login() return self._auth_header - def _login(self) -> dict[str, bytes]: + def _login(self) -> dict[str, str]: return self._login_username_password() - def _login_username_password(self) -> dict[str, bytes]: + def _login_username_password(self) -> dict[str, str]: headers = { "Accept": "application/json", "Content-type": "application/json", diff --git a/plugins/module_utils/typed_classes.py b/plugins/module_utils/typed_classes.py index fa7a9dd1d..7da5d5c86 100644 --- a/plugins/module_utils/typed_classes.py +++ b/plugins/module_utils/typed_classes.py @@ -21,6 +21,7 @@ class TypedClusterInstance(TypedDict): username: str password: str timeout: float + auth_method: str # Registration to ansible return dict. diff --git a/tests/unit/plugins/module_utils/test_client.py b/tests/unit/plugins/module_utils/test_client.py index 85011009e..e87c63c99 100644 --- a/tests/unit/plugins/module_utils/test_client.py +++ b/tests/unit/plugins/module_utils/test_client.py @@ -95,7 +95,7 @@ def test_basic_auth(self, mocker): resp_mock = mocker.MagicMock() resp_mock.status = 200 # Used when testing on Python 3 resp_mock.read.return_value = ( - '{"sessionID":"7e3a2a70-7130-41c4-9402-fc0953cc1d7b"}' + '{"sessionID":"7e3a2a70-7130-41c4-9402-fc0953cc1d7b"}'.encode("utf-8") ) request_mock = mocker.patch.object(client, "Request").return_value From ce5c01753ccec2d00a5cc2396a99f81dd0bfb760 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Thu, 18 May 2023 12:46:19 +0200 Subject: [PATCH 7/9] CI fix missing password for OIDC user Signed-off-by: Justin Cinkelj --- .github/actions/make-integ-config/action.yml | 5 +++++ .github/workflows/integ-test.yml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/.github/actions/make-integ-config/action.yml b/.github/actions/make-integ-config/action.yml index 59d126def..211045398 100644 --- a/.github/actions/make-integ-config/action.yml +++ b/.github/actions/make-integ-config/action.yml @@ -19,6 +19,9 @@ inputs: oidc_client_secret: description: 'oidc_client_secret' required: true + oidc_users_0_password: + description: 'oidc_users_0_password' + required: true runs: using: "composite" steps: @@ -35,6 +38,8 @@ runs: smb_share: /cidata oidc_client_id: d2298ec0-0596-49d2-9554-840a2fe20603 oidc_client_secret: ${{ inputs.oidc_client_secret }} + oidc_users_0_username: xlabciuser@scalemsdnscalecomputing.onmicrosoft.com + oidc_users_0_password: ${{ inputs.oidc_users_0_password }} EOF cat integ_config_vars.yml | jinja2 --strict tests/integration/integration_config.yml.j2 > tests/integration/integration_config.yml echo "sc_host: ${{ inputs.sc_host }}" >> tests/integration/integration_config.yml diff --git a/.github/workflows/integ-test.yml b/.github/workflows/integ-test.yml index 0c8e5d855..289d11494 100644 --- a/.github/workflows/integ-test.yml +++ b/.github/workflows/integ-test.yml @@ -74,6 +74,7 @@ jobs: sc_password_50: ${{ secrets.CI_CONFIG_HC_IP50_SC_PASSWORD }} smb_password: ${{ secrets.CI_CONFIG_HC_IP50_SMB_PASSWORD }} oidc_client_secret: ${{ secrets.OIDC_CLIENT_SECRET }} + oidc_users_0_password: ${{ secrets.OIDC_USERS_0_PASSWORD }} working_directory: ${{ env.WORKDIR }} - run: ansible-playbook tests/integration/prepare/prepare_iso.yml - run: ansible-playbook tests/integration/prepare/prepare_vm.yml @@ -178,6 +179,7 @@ jobs: sc_password_50: ${{ secrets.CI_CONFIG_HC_IP50_SC_PASSWORD }} smb_password: ${{ secrets.CI_CONFIG_HC_IP50_SMB_PASSWORD }} oidc_client_secret: ${{ secrets.OIDC_CLIENT_SECRET }} + oidc_users_0_password: ${{ secrets.OIDC_USERS_0_PASSWORD }} working_directory: ${{ env.WORKDIR }} - run: | pwd @@ -262,6 +264,7 @@ jobs: sc_password_50: ${{ secrets.CI_CONFIG_HC_IP50_SC_PASSWORD }} smb_password: ${{ secrets.CI_CONFIG_HC_IP50_SMB_PASSWORD }} oidc_client_secret: ${{ secrets.OIDC_CLIENT_SECRET }} + oidc_users_0_password: ${{ secrets.OIDC_USERS_0_PASSWORD }} working_directory: ${{ env.WORKDIR }} - run: ansible-test integration --local ${{ matrix.test_name }} @@ -289,6 +292,7 @@ jobs: sc_password_50: ${{ secrets.CI_CONFIG_HC_IP50_SC_PASSWORD }} smb_password: ${{ secrets.CI_CONFIG_HC_IP50_SMB_PASSWORD }} oidc_client_secret: ${{ secrets.OIDC_CLIENT_SECRET }} + oidc_users_0_password: ${{ secrets.OIDC_USERS_0_PASSWORD }} working_directory: ${{ env.WORKDIR }} - run: ansible-galaxy collection install community.general - run: ansible-playbook tests/integration/cleanup/ci_replica_cleanup.yml @@ -313,6 +317,7 @@ jobs: sc_password_50: ${{ secrets.CI_CONFIG_HC_IP50_SC_PASSWORD }} smb_password: ${{ secrets.CI_CONFIG_HC_IP50_SMB_PASSWORD }} oidc_client_secret: ${{ secrets.OIDC_CLIENT_SECRET }} + oidc_users_0_password: ${{ secrets.OIDC_USERS_0_PASSWORD }} working_directory: ${{ env.WORKDIR }} - run: | cd tests/integration/cleanup && ./smb_cleanup.sh \ From 3fd189c5c9841dfa8c8249285acbf835855cdb8a Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Thu, 18 May 2023 13:01:32 +0200 Subject: [PATCH 8/9] CI fix cleanup Do not change SC_AUTH_METHOD to oidc if OIDC is not configured. Cleanup will fail if we do this. Signed-off-by: Justin Cinkelj --- tests/integration/targets/inventory/runme.sh | 26 ++++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/integration/targets/inventory/runme.sh b/tests/integration/targets/inventory/runme.sh index f6ba4b77a..2d04dd1e0 100755 --- a/tests/integration/targets/inventory/runme.sh +++ b/tests/integration/targets/inventory/runme.sh @@ -54,23 +54,23 @@ export SC_AUTH_METHOD=local ansible-playbook -i localhost, -i hypercore_inventory_ansible_both_false.yml run_ansible_both_false_tests.yml # test with OIDC user -eval "$(cat < Date: Mon, 22 May 2023 08:06:34 +0200 Subject: [PATCH 9/9] Minor changes after code rivew Signed-off-by: Justin Cinkelj --- plugins/doc_fragments/cluster_instance.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/doc_fragments/cluster_instance.py b/plugins/doc_fragments/cluster_instance.py index a7900bba0..b0f86c0ef 100644 --- a/plugins/doc_fragments/cluster_instance.py +++ b/plugins/doc_fragments/cluster_instance.py @@ -50,13 +50,11 @@ class ModuleDocFragment(object): auth_method: description: - Select login method. - - If not set, the value of the C(SC_AUTH_METHOD) environment + If not set, the value of the C(SC_AUTH_METHOD) environment variable will be used. - Value I(local) - username/password is verified by the HyperCore server (the local users). - Value I(oidc) - username/password is verified by the configured OIDC provider. default: local - choices: - - local - - oidc + choices: [local, oidc] type: str """