From 972648962ae4496eebaab8a64d1012c98ef9325b Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Fri, 7 Jan 2022 14:14:09 +0100 Subject: [PATCH 01/12] document and test stored proxy configuration --- qiskit_ibm_runtime/accounts/__init__.py | 2 +- qiskit_ibm_runtime/accounts/account.py | 23 +++++++++++++++++---- qiskit_ibm_runtime/accounts/management.py | 4 ++-- qiskit_ibm_runtime/api/client_parameters.py | 4 ++-- qiskit_ibm_runtime/ibm_runtime_service.py | 12 +++++------ 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/qiskit_ibm_runtime/accounts/__init__.py b/qiskit_ibm_runtime/accounts/__init__.py index ea424ef45..b94681093 100644 --- a/qiskit_ibm_runtime/accounts/__init__.py +++ b/qiskit_ibm_runtime/accounts/__init__.py @@ -14,5 +14,5 @@ Account management functionality related to the IBM Runtime Services. """ -from .account import Account, AccountType +from .account import Account, AccountType, ProxyConfigurationType from .management import AccountManager diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index 3f81ba962..c47e2fa53 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -16,13 +16,29 @@ from typing import Optional from urllib.parse import urlparse -from typing_extensions import Literal from requests.auth import AuthBase +from typing_extensions import Literal, TypedDict from ..api.auth import LegacyAuth, CloudAuth AccountType = Optional[Literal["cloud", "legacy"]] + +class ProxyConfigurationType(TypedDict, total=False): + """Dictionary type for custom proxy configuration. + + All items in the dictionary are optional. When ``urls`` are provided, they must contain a dictionary mapping + protocol or protocol and host to the URL of the proxy. Refer to + https://docs.python-requests.org/en/latest/api/#requests.Session.proxies for details and examples. + + NTLM user authentication can be enabled by setting ``username_ntlm`` and ``password_ntlm``. + """ + + urls: dict[str, str] + username_ntlm: str + password_ntlm: str + + LEGACY_API_URL = "https://auth.quantum-computing.ibm.com/api" CLOUD_API_URL = "https://us-east.quantum-computing.cloud.ibm.com" @@ -70,8 +86,7 @@ def __init__( token: str, url: Optional[str] = None, instance: Optional[str] = None, - # TODO: add validation for proxies input format - proxies: Optional[dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: Optional[bool] = True, ): """Account constructor. @@ -81,7 +96,7 @@ def __init__( token: Account token to use. url: Authentication URL. instance: Service instance to use. - proxies: Proxies to use. + proxies: Proxy configuration. verify: Whether to verify server's TLS certificate. """ _assert_valid_auth(auth) diff --git a/qiskit_ibm_runtime/accounts/management.py b/qiskit_ibm_runtime/accounts/management.py index d95d9778d..e57ad3b3d 100644 --- a/qiskit_ibm_runtime/accounts/management.py +++ b/qiskit_ibm_runtime/accounts/management.py @@ -15,7 +15,7 @@ import os from typing import Optional, Dict -from .account import Account, AccountType +from .account import Account, AccountType, ProxyConfigurationType from .storage import save_config, read_config, delete_config _DEFAULT_ACCOUNT_CONFIG_JSON_FILE = os.path.join( @@ -39,7 +39,7 @@ def save( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = _DEFAULT_ACCOUNT_NAME, - proxies: Optional[dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: Optional[bool] = None, ) -> None: """Save account on disk.""" diff --git a/qiskit_ibm_runtime/api/client_parameters.py b/qiskit_ibm_runtime/api/client_parameters.py index 1ad34ab74..cbf325bde 100644 --- a/qiskit_ibm_runtime/api/client_parameters.py +++ b/qiskit_ibm_runtime/api/client_parameters.py @@ -15,7 +15,7 @@ from typing import Dict, Optional, Any, Union from requests_ntlm import HttpNtlmAuth - +from ..accounts import ProxyConfigurationType from ..api.auth import LegacyAuth, CloudAuth TEMPLATE_IBM_HUBS = "{prefix}/Network/{hub}/Groups/{group}/Projects/{project}" @@ -31,7 +31,7 @@ def __init__( token: str, url: str = None, instance: Optional[str] = None, - proxies: Optional[Dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: bool = True, ) -> None: """ClientParameters constructor. diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py index 73bc8c055..b608a7f55 100644 --- a/qiskit_ibm_runtime/ibm_runtime_service.py +++ b/qiskit_ibm_runtime/ibm_runtime_service.py @@ -24,8 +24,8 @@ from qiskit.providers.exceptions import QiskitBackendNotFoundError from qiskit.providers.providerutils import filter_backends -from qiskit_ibm_runtime import ibm_backend # pylint: disable=unused-import -from .accounts import AccountManager, Account, AccountType +from qiskit_ibm_runtime import ibm_backend +from .accounts import AccountManager, Account, AccountType, ProxyConfigurationType from .accounts.exceptions import AccountsError from .api.clients import AuthClient, VersionClient from .api.clients.runtime import RuntimeClient @@ -117,7 +117,7 @@ def __init__( url: Optional[str] = None, name: Optional[str] = None, instance: Optional[str] = None, - proxies: Optional[dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: Optional[bool] = None, ) -> None: """IBMRuntimeService constructor @@ -143,7 +143,7 @@ def __init__( name: Name of the account to load. instance: The service instance to use. For cloud runtime, this is the Cloud Resource Name (CRN). For legacy runtime, this is the hub/group/project in that format. - proxies: Proxy configuration for the server. + proxies: Proxy configuration. verify: Whether to verify the server's TLS certificate. Returns: @@ -536,7 +536,7 @@ def save_account( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = None, - proxies: Optional[dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: Optional[bool] = None, ) -> None: """Save the account to disk for future use. @@ -549,7 +549,7 @@ def save_account( instance: The CRN (cloud) or hub/group/project (legacy). auth: Authentication type. `cloud` or `legacy`. name: Name of the account to save. - proxies: Proxy configuration for the server. + proxies: Proxy configuration. verify: Verify the server's TLS certificate. """ From 2ad0e98b6c7d540dd03ff7d4449e1e1cf849c385 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Fri, 7 Jan 2022 15:05:49 +0100 Subject: [PATCH 02/12] document and test stored proxy configuration --- qiskit_ibm_runtime/accounts/account.py | 56 ++++++++++++++++---- test/test_account.py | 73 ++++++++++++++++++++++++-- 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index c47e2fa53..2b12605b0 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -18,7 +18,7 @@ from requests.auth import AuthBase from typing_extensions import Literal, TypedDict - +from ..utils.hgp import from_instance_format from ..api.auth import LegacyAuth, CloudAuth AccountType = Optional[Literal["cloud", "legacy"]] @@ -47,7 +47,7 @@ def _assert_valid_auth(auth: AccountType) -> None: """Assert that the auth parameter is valid.""" if not (auth in ["cloud", "legacy"]): raise ValueError( - f"Inappropriate `auth` value. Expected one of ['cloud', 'legacy'], got '{auth}'." + f"Invalid `auth` value. Expected one of ['cloud', 'legacy'], got '{auth}'." ) @@ -55,7 +55,7 @@ def _assert_valid_token(token: str) -> None: """Assert that the token is valid.""" if not (isinstance(token, str) and len(token) > 0): raise ValueError( - f"Inappropriate `token` value. Expected a non-empty string, got '{token}'." + f"Invalid `token` value. Expected a non-empty string, got '{token}'." ) @@ -64,17 +64,51 @@ def _assert_valid_url(url: str) -> None: try: urlparse(url) except: - raise ValueError(f"Inappropriate `url` value. Failed to parse '{url}' as URL.") + raise ValueError(f"Invalid `url` value. Failed to parse '{url}' as URL.") + + +def _assert_valid_proxies(config: ProxyConfigurationType) -> None: + """Assert that the proxy configuration is valid.""" + + if config is None: + return + + # verify NTLM authentication configuration + ntlm_user = config.get("username_ntlm") + ntlm_pass = config.get("password_ntlm") + if not any( + [ + type(ntlm_user) == str and type(ntlm_pass) == str, + ntlm_user is None and ntlm_pass is None, + ] + ): + raise ValueError( + f"Invalid proxy configuration for NTLM authentication. None or both of username and password must be " + f"provided. Got username_ntlm={ntlm_user}, password_ntlm={ntlm_pass}." + ) + + # verify proxy configuration + urls = config.get("urls") + if urls is not None and not type(urls) is dict: + raise ValueError( + f"Invalid proxy configuration. Expected `urls` to contain a dictionary mapping protocol " + f"or protocol and host to the URL of the proxy. Got {urls}" + ) def _assert_valid_instance(auth: AccountType, instance: str) -> None: """Assert that the instance name is valid for the given account type.""" if auth == "cloud": - if not (isinstance(instance, str) and len(instance) > 0): - raise ValueError( - f"Inappropriate `instance` value. Expected a non-empty string." - ) - # TODO: add validation for legacy instance when adding test coverage + if not (isinstance(instance, str) and instance.find("crn:") != -1): + raise ValueError(f"Invalid `instance` value. Expected a non-empty CRN.") + if auth == "legacy": + if instance is not None: + try: + from_instance_format(instance) + except: + raise ValueError( + f"Invalid `instance` value. Expected hub/group/project format, got {instance}" + ) class Account: @@ -109,8 +143,12 @@ def __init__( _assert_valid_url(resolved_url) self.url = resolved_url + _assert_valid_instance(auth, instance) self.instance = instance + + _assert_valid_proxies(proxies) self.proxies = proxies + self.verify = verify def to_saved_format(self) -> dict: diff --git a/test/test_account.py b/test/test_account.py index 91affd44b..695c8508e 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -13,15 +13,15 @@ """Tests for the account functions.""" import json -import uuid import logging import os +import uuid +from typing import Any from unittest import skipIf -from qiskit_ibm_runtime.accounts.account import CLOUD_API_URL, LEGACY_API_URL from qiskit_ibm_runtime.accounts import AccountManager, Account, management +from qiskit_ibm_runtime.accounts.account import CLOUD_API_URL, LEGACY_API_URL from qiskit_ibm_runtime.exceptions import IBMInputValueError - from .ibm_test_case import IBMTestCase from .mock.fake_runtime_service import FakeRuntimeService from .utils.account import ( @@ -46,6 +46,73 @@ ) +class TestAccount(IBMTestCase): + """Tests for Account class.""" + + dummy_token = "123" + dummy_cloud_url = "https://us-east.quantum-computing.cloud.ibm.com" + dummy_legacy_url = "https://auth.quantum-computing.ibm.com/api" + + def test_invalid_auth(self): + with self.assertRaises(ValueError) as err: + invalid_auth: Any = "phantom" + Account(auth=invalid_auth, token=self.dummy_token, url=self.dummy_cloud_url) + self.assertIn("Invalid `auth` value.", str(err.exception)) + + def test_invalid_token(self): + invalid_tokens = [1, None, ""] + for token in invalid_tokens: + with self.subTest(token=token): + with self.assertRaises(ValueError) as err: + Account(auth="cloud", token=token, url=self.dummy_cloud_url) + self.assertIn("Invalid `token` value.", str(err.exception)) + + def test_invalid_url(self): + subtests = [ + {"auth": "cloud", "url": 123}, + ] + for params in subtests: + with self.subTest(params=params): + with self.assertRaises(ValueError) as err: + Account(**params, token=self.dummy_token) + self.assertIn("Invalid `url` value.", str(err.exception)) + + def test_invalid_instance(self): + subtests = [ + {"auth": "cloud", "instance": "no-valid-crn"}, + {"auth": "cloud"}, + {"auth": "legacy", "instance": "no-hgp-format"}, + ] + for params in subtests: + with self.subTest(params=params): + with self.assertRaises(ValueError) as err: + Account(**params, token=self.dummy_token, url=self.dummy_cloud_url) + self.assertIn("Invalid `instance` value.", str(err.exception)) + + def test_invalid_proxy_config(self): + subtests = [ + { + "proxies": {"username_ntlm": "user-only"}, + }, + { + "proxies": {"password_ntlm": "password-only"}, + }, + { + "proxies": {"urls": ""}, + }, + ] + for params in subtests: + with self.subTest(params=params): + with self.assertRaises(ValueError) as err: + Account( + **params, + auth="cloud", + token=self.dummy_token, + url=self.dummy_cloud_url, + ) + self.assertIn("Invalid proxy configuration", str(err.exception)) + + # NamedTemporaryFiles not supported in Windows @skipIf(os.name == "nt", "Test not supported in Windows") class TestAccountManager(IBMTestCase): From 02273167e78132925771cbc867b86c52278df37e Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Fri, 7 Jan 2022 15:19:06 +0100 Subject: [PATCH 03/12] document and test stored proxy configuration --- qiskit_ibm_runtime/accounts/account.py | 16 ++++++++-------- qiskit_ibm_runtime/api/client_parameters.py | 2 +- qiskit_ibm_runtime/ibm_runtime_service.py | 2 +- test/test_account.py | 12 +++++++++++- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index 2b12605b0..a2919d865 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -13,7 +13,7 @@ """Account related classes and functions.""" -from typing import Optional +from typing import Optional, Dict from urllib.parse import urlparse from requests.auth import AuthBase @@ -27,14 +27,14 @@ class ProxyConfigurationType(TypedDict, total=False): """Dictionary type for custom proxy configuration. - All items in the dictionary are optional. When ``urls`` are provided, they must contain a dictionary mapping - protocol or protocol and host to the URL of the proxy. Refer to + All items in the dictionary are optional. When ``urls`` are provided, they must contain a dictionary + mapping protocol or protocol and host to the URL of the proxy. Refer to https://docs.python-requests.org/en/latest/api/#requests.Session.proxies for details and examples. NTLM user authentication can be enabled by setting ``username_ntlm`` and ``password_ntlm``. """ - urls: dict[str, str] + urls: Dict[str, str] username_ntlm: str password_ntlm: str @@ -78,18 +78,18 @@ def _assert_valid_proxies(config: ProxyConfigurationType) -> None: ntlm_pass = config.get("password_ntlm") if not any( [ - type(ntlm_user) == str and type(ntlm_pass) == str, + isinstance(ntlm_user, str) and isinstance(ntlm_pass, str), ntlm_user is None and ntlm_pass is None, ] ): raise ValueError( - f"Invalid proxy configuration for NTLM authentication. None or both of username and password must be " - f"provided. Got username_ntlm={ntlm_user}, password_ntlm={ntlm_pass}." + f"Invalid proxy configuration for NTLM authentication. None or both of username and " + f"password must be provided. Got username_ntlm={ntlm_user}, password_ntlm={ntlm_pass}." ) # verify proxy configuration urls = config.get("urls") - if urls is not None and not type(urls) is dict: + if urls is not None and not isinstance(urls, dict): raise ValueError( f"Invalid proxy configuration. Expected `urls` to contain a dictionary mapping protocol " f"or protocol and host to the URL of the proxy. Got {urls}" diff --git a/qiskit_ibm_runtime/api/client_parameters.py b/qiskit_ibm_runtime/api/client_parameters.py index cbf325bde..08c1fe5b5 100644 --- a/qiskit_ibm_runtime/api/client_parameters.py +++ b/qiskit_ibm_runtime/api/client_parameters.py @@ -64,7 +64,7 @@ def connection_parameters(self) -> Dict[str, Any]: expected by ``requests``. The following keys can be present: ``proxies``, ``verify``, and ``auth``. """ - request_kwargs = {"verify": self.verify} + request_kwargs: Any = {"verify": self.verify} if self.proxies: if "urls" in self.proxies: diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py index b608a7f55..2dd00841e 100644 --- a/qiskit_ibm_runtime/ibm_runtime_service.py +++ b/qiskit_ibm_runtime/ibm_runtime_service.py @@ -211,7 +211,7 @@ def _discover_account( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = None, - proxies: Optional[dict] = None, + proxies: Optional[ProxyConfigurationType] = None, verify: Optional[bool] = None, ) -> Account: """Discover account.""" diff --git a/test/test_account.py b/test/test_account.py index 695c8508e..e63789cdf 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -54,12 +54,16 @@ class TestAccount(IBMTestCase): dummy_legacy_url = "https://auth.quantum-computing.ibm.com/api" def test_invalid_auth(self): + """Test invalid values for auth parameter.""" + with self.assertRaises(ValueError) as err: invalid_auth: Any = "phantom" Account(auth=invalid_auth, token=self.dummy_token, url=self.dummy_cloud_url) self.assertIn("Invalid `auth` value.", str(err.exception)) def test_invalid_token(self): + """Test invalid values for token parameter.""" + invalid_tokens = [1, None, ""] for token in invalid_tokens: with self.subTest(token=token): @@ -68,6 +72,8 @@ def test_invalid_token(self): self.assertIn("Invalid `token` value.", str(err.exception)) def test_invalid_url(self): + """Test invalid values for url parameter.""" + subtests = [ {"auth": "cloud", "url": 123}, ] @@ -78,6 +84,8 @@ def test_invalid_url(self): self.assertIn("Invalid `url` value.", str(err.exception)) def test_invalid_instance(self): + """Test invalid values for instance parameter.""" + subtests = [ {"auth": "cloud", "instance": "no-valid-crn"}, {"auth": "cloud"}, @@ -90,6 +98,8 @@ def test_invalid_instance(self): self.assertIn("Invalid `instance` value.", str(err.exception)) def test_invalid_proxy_config(self): + """Test invalid values for proxy configuration.""" + subtests = [ { "proxies": {"username_ntlm": "user-only"}, @@ -106,7 +116,7 @@ def test_invalid_proxy_config(self): with self.assertRaises(ValueError) as err: Account( **params, - auth="cloud", + auth="legacy", token=self.dummy_token, url=self.dummy_cloud_url, ) From 90942c78d8a7164e96035dba03e58a8d9eb48b24 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Fri, 7 Jan 2022 16:18:20 +0100 Subject: [PATCH 04/12] fix failing test cases --- qiskit_ibm_runtime/accounts/account.py | 6 ++++-- test/test_account.py | 20 ++++++++++++-------- test/test_backend_retrieval.py | 4 ++-- test/utils/decorators.py | 4 ++-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index a2919d865..3af12903a 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -99,8 +99,10 @@ def _assert_valid_proxies(config: ProxyConfigurationType) -> None: def _assert_valid_instance(auth: AccountType, instance: str) -> None: """Assert that the instance name is valid for the given account type.""" if auth == "cloud": - if not (isinstance(instance, str) and instance.find("crn:") != -1): - raise ValueError(f"Invalid `instance` value. Expected a non-empty CRN.") + if not (isinstance(instance, str) and len(instance) > 0): + raise ValueError( + f"Invalid `instance` value. Expected a non-empty string, got '{instance}'." + ) if auth == "legacy": if instance is not None: try: diff --git a/test/test_account.py b/test/test_account.py index e63789cdf..def465b18 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -87,7 +87,7 @@ def test_invalid_instance(self): """Test invalid values for instance parameter.""" subtests = [ - {"auth": "cloud", "instance": "no-valid-crn"}, + {"auth": "cloud", "instance": ""}, {"auth": "cloud"}, {"auth": "legacy", "instance": "no-hgp-format"}, ] @@ -199,10 +199,10 @@ def test_list(self): "key1": _TEST_CLOUD_ACCOUNT.to_saved_format(), "key2": _TEST_LEGACY_ACCOUNT.to_saved_format(), management._DEFAULT_ACCOUNT_NAME_CLOUD: Account( - "cloud", "token-legacy" + "cloud", "token-cloud", instance="crn:123" ).to_saved_format(), management._DEFAULT_ACCOUNT_NAME_LEGACY: Account( - "legacy", "token-cloud" + "legacy", "token-legacy" ).to_saved_format(), } ), self.subTest("filtered list of accounts"): @@ -320,7 +320,7 @@ def test_enable_cloud_account_by_auth_token_url(self): for url in urls: with self.subTest(url=url), no_envs(["QISKIT_IBM_API_TOKEN"]): token = uuid.uuid4().hex - with self.assertRaises(IBMInputValueError) as err: + with self.assertRaises(ValueError) as err: _ = FakeRuntimeService(auth="cloud", token=token, url=url) self.assertIn("instance", str(err.exception)) @@ -397,7 +397,7 @@ def test_enable_account_by_env_auth(self): envs = { "QISKIT_IBM_API_TOKEN": token, "QISKIT_IBM_API_URL": url, - "QISKIT_IBM_INSTANCE": "my_crn", + "QISKIT_IBM_INSTANCE": "h/g/p" if auth == "legacy" else "crn:123", } with custom_envs(envs): service = FakeRuntimeService(auth=auth) @@ -504,7 +504,7 @@ def test_enable_account_by_name_input_instance(self): """Test initializing account by name and input instance.""" name = "foo" instance = uuid.uuid4().hex - with temporary_account_config_file(name=name, instance=""): + with temporary_account_config_file(name=name, instance="stored-instance"): service = FakeRuntimeService(name=name, instance=instance) self.assertTrue(service._account) self.assertEqual(service._account.instance, instance) @@ -512,7 +512,7 @@ def test_enable_account_by_name_input_instance(self): def test_enable_account_by_auth_input_instance(self): """Test initializing account by auth and input instance.""" instance = uuid.uuid4().hex - with temporary_account_config_file(auth="cloud", instance=""): + with temporary_account_config_file(auth="cloud", instance="bla"): service = FakeRuntimeService(auth="cloud", instance=instance) self.assertTrue(service._account) self.assertEqual(service._account.instance, instance) @@ -520,7 +520,11 @@ def test_enable_account_by_auth_input_instance(self): def test_enable_account_by_env_input_instance(self): """Test initializing account by env and input instance.""" instance = uuid.uuid4().hex - envs = {"QISKIT_IBM_API_TOKEN": "some_token", "QISKIT_IBM_API_URL": "some_url"} + envs = { + "QISKIT_IBM_API_TOKEN": "some_token", + "QISKIT_IBM_API_URL": "some_url", + "QISKIT_IBM_INSTANCE": "some_instance", + } with custom_envs(envs): service = FakeRuntimeService(auth="cloud", instance=instance) self.assertTrue(service._account) diff --git a/test/test_backend_retrieval.py b/test/test_backend_retrieval.py index 29f03bba4..37d56c0ce 100644 --- a/test/test_backend_retrieval.py +++ b/test/test_backend_retrieval.py @@ -157,7 +157,7 @@ def test_filter_by_hgp(self): legacy_service = FakeRuntimeService( auth="legacy", token="my_token", - instance="my_instance", + instance="h/g/p", test_options=test_options, ) backends = legacy_service.backends(instance="hub0/group0/project0") @@ -182,7 +182,7 @@ def _get_services(self, fake_backends): legacy_service = FakeRuntimeService( auth="legacy", token="my_token", - instance="my_instance", + instance="h/g/p", test_options=test_options, ) cloud_service = FakeRuntimeService( diff --git a/test/utils/decorators.py b/test/utils/decorators.py index 33e6cdfed..7c65a2c17 100644 --- a/test/utils/decorators.py +++ b/test/utils/decorators.py @@ -272,10 +272,10 @@ def run_legacy_and_cloud_fake(func): @wraps(func) def _wrapper(self, *args, **kwargs): legacy_service = FakeRuntimeService( - auth="legacy", token="my_token", instance="my_instance" + auth="legacy", token="my_token", instance="h/g/p" ) cloud_service = FakeRuntimeService( - auth="cloud", token="my_token", instance="my_instance" + auth="cloud", token="my_token", instance="crn:123" ) for service in [legacy_service, cloud_service]: with self.subTest(service=service.auth): From 4f8cfa801713aa45bbc97bf747ac359417e7b366 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Fri, 7 Jan 2022 16:53:19 +0100 Subject: [PATCH 05/12] fix linting issue due to unused import --- test/test_account.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_account.py b/test/test_account.py index def465b18..cdf7375ac 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -21,7 +21,6 @@ from qiskit_ibm_runtime.accounts import AccountManager, Account, management from qiskit_ibm_runtime.accounts.account import CLOUD_API_URL, LEGACY_API_URL -from qiskit_ibm_runtime.exceptions import IBMInputValueError from .ibm_test_case import IBMTestCase from .mock.fake_runtime_service import FakeRuntimeService from .utils.account import ( From 6bed2813079a47b54ab57cebdd77427e800b89cd Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Mon, 10 Jan 2022 14:01:32 +0100 Subject: [PATCH 06/12] address code review feedback --- qiskit_ibm_runtime/accounts/__init__.py | 2 +- qiskit_ibm_runtime/accounts/account.py | 60 ++++----------- qiskit_ibm_runtime/accounts/management.py | 5 +- qiskit_ibm_runtime/api/client_parameters.py | 13 +--- qiskit_ibm_runtime/api/clients/utils.py | 4 +- qiskit_ibm_runtime/ibm_runtime_service.py | 25 ++++-- qiskit_ibm_runtime/proxies/__init__.py | 17 +++++ qiskit_ibm_runtime/proxies/configuration.py | 84 +++++++++++++++++++++ test/test_account.py | 27 ++++--- test/test_client_parameters.py | 21 ++---- 10 files changed, 162 insertions(+), 96 deletions(-) create mode 100644 qiskit_ibm_runtime/proxies/__init__.py create mode 100644 qiskit_ibm_runtime/proxies/configuration.py diff --git a/qiskit_ibm_runtime/accounts/__init__.py b/qiskit_ibm_runtime/accounts/__init__.py index b94681093..ea424ef45 100644 --- a/qiskit_ibm_runtime/accounts/__init__.py +++ b/qiskit_ibm_runtime/accounts/__init__.py @@ -14,5 +14,5 @@ Account management functionality related to the IBM Runtime Services. """ -from .account import Account, AccountType, ProxyConfigurationType +from .account import Account, AccountType from .management import AccountManager diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index 3af12903a..d7d3665e0 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -13,32 +13,17 @@ """Account related classes and functions.""" -from typing import Optional, Dict +from typing import Optional from urllib.parse import urlparse from requests.auth import AuthBase -from typing_extensions import Literal, TypedDict +from typing_extensions import Literal +from ..proxies import ProxyConfiguration from ..utils.hgp import from_instance_format from ..api.auth import LegacyAuth, CloudAuth AccountType = Optional[Literal["cloud", "legacy"]] - -class ProxyConfigurationType(TypedDict, total=False): - """Dictionary type for custom proxy configuration. - - All items in the dictionary are optional. When ``urls`` are provided, they must contain a dictionary - mapping protocol or protocol and host to the URL of the proxy. Refer to - https://docs.python-requests.org/en/latest/api/#requests.Session.proxies for details and examples. - - NTLM user authentication can be enabled by setting ``username_ntlm`` and ``password_ntlm``. - """ - - urls: Dict[str, str] - username_ntlm: str - password_ntlm: str - - LEGACY_API_URL = "https://auth.quantum-computing.ibm.com/api" CLOUD_API_URL = "https://us-east.quantum-computing.cloud.ibm.com" @@ -67,33 +52,10 @@ def _assert_valid_url(url: str) -> None: raise ValueError(f"Invalid `url` value. Failed to parse '{url}' as URL.") -def _assert_valid_proxies(config: ProxyConfigurationType) -> None: +def _assert_valid_proxies(config: ProxyConfiguration) -> None: """Assert that the proxy configuration is valid.""" - - if config is None: - return - - # verify NTLM authentication configuration - ntlm_user = config.get("username_ntlm") - ntlm_pass = config.get("password_ntlm") - if not any( - [ - isinstance(ntlm_user, str) and isinstance(ntlm_pass, str), - ntlm_user is None and ntlm_pass is None, - ] - ): - raise ValueError( - f"Invalid proxy configuration for NTLM authentication. None or both of username and " - f"password must be provided. Got username_ntlm={ntlm_user}, password_ntlm={ntlm_pass}." - ) - - # verify proxy configuration - urls = config.get("urls") - if urls is not None and not isinstance(urls, dict): - raise ValueError( - f"Invalid proxy configuration. Expected `urls` to contain a dictionary mapping protocol " - f"or protocol and host to the URL of the proxy. Got {urls}" - ) + if config is not None: + config.validate() def _assert_valid_instance(auth: AccountType, instance: str) -> None: @@ -122,7 +84,7 @@ def __init__( token: str, url: Optional[str] = None, instance: Optional[str] = None, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[ProxyConfiguration] = None, verify: Optional[bool] = True, ): """Account constructor. @@ -155,17 +117,21 @@ def __init__( def to_saved_format(self) -> dict: """Returns a dictionary that represents how the account is saved on disk.""" - return {k: v for k, v in self.__dict__.items() if v is not None} + result = {k: v for k, v in self.__dict__.items() if v is not None} + if self.proxies: + result["proxies"] = self.proxies.to_dict() + return result @classmethod def from_saved_format(cls, data: dict) -> "Account": """Creates an account instance from data saved on disk.""" + proxies = data.get("proxies") return cls( auth=data.get("auth"), url=data.get("url"), token=data.get("token"), instance=data.get("instance"), - proxies=data.get("proxies"), + proxies=ProxyConfiguration(**proxies) if proxies else None, verify=data.get("verify", True), ) diff --git a/qiskit_ibm_runtime/accounts/management.py b/qiskit_ibm_runtime/accounts/management.py index e57ad3b3d..a38911251 100644 --- a/qiskit_ibm_runtime/accounts/management.py +++ b/qiskit_ibm_runtime/accounts/management.py @@ -15,7 +15,8 @@ import os from typing import Optional, Dict -from .account import Account, AccountType, ProxyConfigurationType +from .account import Account, AccountType +from ..proxies import ProxyConfiguration from .storage import save_config, read_config, delete_config _DEFAULT_ACCOUNT_CONFIG_JSON_FILE = os.path.join( @@ -39,7 +40,7 @@ def save( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = _DEFAULT_ACCOUNT_NAME, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[ProxyConfiguration] = None, verify: Optional[bool] = None, ) -> None: """Save account on disk.""" diff --git a/qiskit_ibm_runtime/api/client_parameters.py b/qiskit_ibm_runtime/api/client_parameters.py index 08c1fe5b5..69544eb09 100644 --- a/qiskit_ibm_runtime/api/client_parameters.py +++ b/qiskit_ibm_runtime/api/client_parameters.py @@ -14,9 +14,8 @@ from typing import Dict, Optional, Any, Union -from requests_ntlm import HttpNtlmAuth -from ..accounts import ProxyConfigurationType from ..api.auth import LegacyAuth, CloudAuth +from ..proxies import ProxyConfiguration TEMPLATE_IBM_HUBS = "{prefix}/Network/{hub}/Groups/{group}/Projects/{project}" """str: Template for creating an IBM Quantum URL with hub/group/project information.""" @@ -31,7 +30,7 @@ def __init__( token: str, url: str = None, instance: Optional[str] = None, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[ProxyConfiguration] = None, verify: bool = True, ) -> None: """ClientParameters constructor. @@ -67,12 +66,6 @@ def connection_parameters(self) -> Dict[str, Any]: request_kwargs: Any = {"verify": self.verify} if self.proxies: - if "urls" in self.proxies: - request_kwargs["proxies"] = self.proxies["urls"] - - if "username_ntlm" in self.proxies and "password_ntlm" in self.proxies: - request_kwargs["auth"] = HttpNtlmAuth( - self.proxies["username_ntlm"], self.proxies["password_ntlm"] - ) + request_kwargs.update(self.proxies.to_request_params()) return request_kwargs diff --git a/qiskit_ibm_runtime/api/clients/utils.py b/qiskit_ibm_runtime/api/clients/utils.py index b285c52ab..0804f391a 100644 --- a/qiskit_ibm_runtime/api/clients/utils.py +++ b/qiskit_ibm_runtime/api/clients/utils.py @@ -61,8 +61,8 @@ def ws_proxy_params(client_params: ClientParameters, ws_url: str) -> Dict: if "auth" in conn_data: out["http_proxy_auth"] = ( - client_params.proxies["username_ntlm"], - client_params.proxies["password_ntlm"], + client_params.proxies.username_ntlm, + client_params.proxies.password_ntlm, ) return out diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py index 2dd00841e..dcb0b4965 100644 --- a/qiskit_ibm_runtime/ibm_runtime_service.py +++ b/qiskit_ibm_runtime/ibm_runtime_service.py @@ -25,8 +25,9 @@ from qiskit.providers.providerutils import filter_backends from qiskit_ibm_runtime import ibm_backend -from .accounts import AccountManager, Account, AccountType, ProxyConfigurationType +from .accounts import AccountManager, Account, AccountType from .accounts.exceptions import AccountsError +from .proxies import ProxyConfiguration from .api.clients import AuthClient, VersionClient from .api.clients.runtime import RuntimeClient from .api.exceptions import RequestsApiError @@ -117,7 +118,7 @@ def __init__( url: Optional[str] = None, name: Optional[str] = None, instance: Optional[str] = None, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[dict] = None, verify: Optional[bool] = None, ) -> None: """IBMRuntimeService constructor @@ -143,7 +144,11 @@ def __init__( name: Name of the account to load. instance: The service instance to use. For cloud runtime, this is the Cloud Resource Name (CRN). For legacy runtime, this is the hub/group/project in that format. - proxies: Proxy configuration. + proxies: Proxy configuration. Supported optional keys are + ``urls`` (a dictionary mapping protocol or protocol and host to the URL of the proxy, + documented at https://docs.python-requests.org/en/latest/api/#requests.Session.proxies), + ```username_ntlm```, ```password_ntlm```(username and password to enable NTLM user + authentication) verify: Whether to verify the server's TLS certificate. Returns: @@ -160,7 +165,7 @@ def __init__( instance=instance, auth=auth, name=name, - proxies=proxies, + proxies=ProxyConfiguration(**proxies) if proxies else None, verify=verify, ) if self._account.auth == "cloud" and not self._account.instance: @@ -211,7 +216,7 @@ def _discover_account( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = None, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[ProxyConfiguration] = None, verify: Optional[bool] = None, ) -> Account: """Discover account.""" @@ -536,7 +541,7 @@ def save_account( instance: Optional[str] = None, auth: Optional[AccountType] = None, name: Optional[str] = None, - proxies: Optional[ProxyConfigurationType] = None, + proxies: Optional[dict] = None, verify: Optional[bool] = None, ) -> None: """Save the account to disk for future use. @@ -549,7 +554,11 @@ def save_account( instance: The CRN (cloud) or hub/group/project (legacy). auth: Authentication type. `cloud` or `legacy`. name: Name of the account to save. - proxies: Proxy configuration. + proxies: Proxy configuration. Supported optional keys are + ``urls`` (a dictionary mapping protocol or protocol and host to the URL of the proxy, + documented at https://docs.python-requests.org/en/latest/api/#requests.Session.proxies), + ```username_ntlm```, ```password_ntlm```(username and password to enable NTLM user + authentication) verify: Verify the server's TLS certificate. """ @@ -559,7 +568,7 @@ def save_account( instance=instance, auth=auth, name=name, - proxies=proxies, + proxies=ProxyConfiguration(**proxies) if proxies else None, verify=verify, ) diff --git a/qiskit_ibm_runtime/proxies/__init__.py b/qiskit_ibm_runtime/proxies/__init__.py new file mode 100644 index 000000000..c0212bfb0 --- /dev/null +++ b/qiskit_ibm_runtime/proxies/__init__.py @@ -0,0 +1,17 @@ +# This code is part of Qiskit. +# +# (C) Copyright IBM 2022. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +""" +Proxy configuration. +""" + +from .configuration import ProxyConfiguration diff --git a/qiskit_ibm_runtime/proxies/configuration.py b/qiskit_ibm_runtime/proxies/configuration.py new file mode 100644 index 000000000..2a14bac59 --- /dev/null +++ b/qiskit_ibm_runtime/proxies/configuration.py @@ -0,0 +1,84 @@ +# This code is part of Qiskit. +# +# (C) Copyright IBM 2022. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Proxy related classes and functions.""" + + +from dataclasses import dataclass +from typing import Optional, Dict +from requests_ntlm import HttpNtlmAuth + + +@dataclass +class ProxyConfiguration: + """Class for representing a proxy configuration. + + Args + urls: a dictionary mapping protocol or protocol and host to the URL of the proxy. Refer to + https://docs.python-requests.org/en/latest/api/#requests.Session.proxies for details. + username_ntlm: username used to enable NTLM user authentication. + password_ntlm: password used to enable NTLM user authentication. + """ + + urls: Optional[Dict[str, str]] = None + username_ntlm: Optional[str] = None + password_ntlm: Optional[str] = None + + def validate(self) -> None: + """Validate configuration. + + Raises: + ValueError: If configuration is invalid. + """ + if not any( + [ + isinstance(self.username_ntlm, str) + and isinstance(self.password_ntlm, str), + self.username_ntlm is None and self.password_ntlm is None, + ] + ): + raise ValueError( + f"Invalid proxy configuration for NTLM authentication. None or both of username and " + f"password must be provided. Got username_ntlm={self.username_ntlm}, " + f"password_ntlm={self.password_ntlm}." + ) + + if self.urls is not None and not isinstance(self.urls, dict): + raise ValueError( + f"Invalid proxy configuration. Expected `urls` to contain a dictionary mapping protocol " + f"or protocol and host to the URL of the proxy. Got {self.urls}" + ) + + def to_dict(self) -> dict: + """Transform configuration to dictionary.""" + + return {k: v for k, v in self.__dict__.items() if v is not None} + + def to_request_params(self) -> dict: + """Transform configuration to request parameters. + + Returns: + A dictionary with proxy configuration parameters in the format + expected by ``requests``. The following keys can be present: + ``proxies``and ``auth``. + """ + + request_kwargs = {} + if self.urls: + request_kwargs["proxies"] = self.urls + + if self.username_ntlm and self.password_ntlm: + request_kwargs["auth"] = HttpNtlmAuth( + self.username_ntlm, self.password_ntlm + ) + + return request_kwargs diff --git a/test/test_account.py b/test/test_account.py index cdf7375ac..c34f34dab 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -20,6 +20,7 @@ from unittest import skipIf from qiskit_ibm_runtime.accounts import AccountManager, Account, management +from qiskit_ibm_runtime.proxies import ProxyConfiguration from qiskit_ibm_runtime.accounts.account import CLOUD_API_URL, LEGACY_API_URL from .ibm_test_case import IBMTestCase from .mock.fake_runtime_service import FakeRuntimeService @@ -42,6 +43,9 @@ token="token-y", url="https://cloud.ibm.com", instance="crn:v1:bluemix:public:quantum-computing:us-east:a/...::", + proxies=ProxyConfiguration( + username_ntlm="bla", password_ntlm="blub", urls={"https": "127.0.0.1"} + ), ) @@ -101,13 +105,13 @@ def test_invalid_proxy_config(self): subtests = [ { - "proxies": {"username_ntlm": "user-only"}, + "proxies": ProxyConfiguration(**{"username_ntlm": "user-only"}), }, { - "proxies": {"password_ntlm": "password-only"}, + "proxies": ProxyConfiguration(**{"password_ntlm": "password-only"}), }, { - "proxies": {"urls": ""}, + "proxies": ProxyConfiguration(**{"urls": ""}), }, ] for params in subtests: @@ -252,6 +256,9 @@ def test_delete(self): self.assertTrue(len(AccountManager.list()) == 0) +MOCK_PROXY_CONFIG_DICT = { + "urls": {"https": "127.0.0.1", "username_ntlm": "", "password_ntlm": ""} +} # NamedTemporaryFiles not supported in Windows @skipIf(os.name == "nt", "Test not supported in Windows") class TestEnableAccount(IBMTestCase): @@ -441,10 +448,10 @@ def test_enable_account_by_name_pref(self): """Test initializing account by name and preferences.""" name = "foo" subtests = [ - {"proxies": "foo"}, + {"proxies": MOCK_PROXY_CONFIG_DICT}, {"verify": False}, {"instance": "bar"}, - {"proxies": "foo", "verify": False, "instance": "bar"}, + {"proxies": MOCK_PROXY_CONFIG_DICT, "verify": False, "instance": "bar"}, ] for extra in subtests: with self.subTest(extra=extra): @@ -458,10 +465,10 @@ def test_enable_account_by_name_pref(self): def test_enable_account_by_auth_pref(self): """Test initializing account by auth and preferences.""" subtests = [ - {"proxies": "foo"}, + {"proxies": MOCK_PROXY_CONFIG_DICT}, {"verify": False}, {"instance": "bar"}, - {"proxies": "foo", "verify": False, "instance": "bar"}, + {"proxies": MOCK_PROXY_CONFIG_DICT, "verify": False, "instance": "bar"}, ] for auth in ["cloud", "legacy"]: for extra in subtests: @@ -479,10 +486,10 @@ def test_enable_account_by_auth_pref(self): def test_enable_account_by_env_pref(self): """Test initializing account by environment variable and preferences.""" subtests = [ - {"proxies": "foo"}, + {"proxies": MOCK_PROXY_CONFIG_DICT}, {"verify": False}, {"instance": "bar"}, - {"proxies": "foo", "verify": False, "instance": "bar"}, + {"proxies": MOCK_PROXY_CONFIG_DICT, "verify": False, "instance": "bar"}, ] for extra in subtests: with self.subTest(extra=extra): @@ -531,7 +538,7 @@ def test_enable_account_by_env_input_instance(self): def _verify_prefs(self, prefs, account): if "proxies" in prefs: - self.assertEqual(account.proxies, prefs["proxies"]) + self.assertEqual(account.proxies, ProxyConfiguration(**prefs["proxies"])) if "verify" in prefs: self.assertEqual(account.verify, prefs["verify"]) if "instance" in prefs: diff --git a/test/test_client_parameters.py b/test/test_client_parameters.py index 1b294e7a8..3c83c6794 100644 --- a/test/test_client_parameters.py +++ b/test/test_client_parameters.py @@ -15,7 +15,7 @@ import uuid from requests_ntlm import HttpNtlmAuth - +from qiskit_ibm_runtime.proxies import ProxyConfiguration from qiskit_ibm_runtime.api.client_parameters import ClientParameters from qiskit_ibm_runtime.api.auth import CloudAuth, LegacyAuth @@ -43,7 +43,9 @@ def test_proxy_param(self) -> None: """Test using only proxy urls (no NTLM credentials).""" urls = {"http": "localhost:8080", "https": "localhost:8080"} proxies_only_expected_result = {"verify": True, "proxies": urls} - proxies_only_credentials = self._get_client_params(proxies={"urls": urls}) + proxies_only_credentials = self._get_client_params( + proxies=ProxyConfiguration(**{"urls": urls}) + ) result = proxies_only_credentials.connection_parameters() self.assertDictEqual(proxies_only_expected_result, result) @@ -61,7 +63,7 @@ def test_proxies_param_with_ntlm(self) -> None: "auth": HttpNtlmAuth("domain\\username", "password"), } proxies_with_ntlm_credentials = self._get_client_params( - proxies=proxies_with_ntlm_dict + proxies=ProxyConfiguration(**proxies_with_ntlm_dict) ) result = proxies_with_ntlm_credentials.connection_parameters() @@ -74,19 +76,6 @@ def test_proxies_param_with_ntlm(self) -> None: result.pop("auth") self.assertDictEqual(ntlm_expected_result, result) - def test_malformed_proxy_param(self) -> None: - """Test input with malformed nesting of the proxies dictionary.""" - urls = {"http": "localhost:8080", "https": "localhost:8080"} - malformed_nested_proxies_dict = {"proxies": urls} - malformed_nested_credentials = self._get_client_params( - proxies=malformed_nested_proxies_dict - ) - - # Malformed proxy entries should be ignored. - expected_result = {"verify": True} - result = malformed_nested_credentials.connection_parameters() - self.assertDictEqual(expected_result, result) - def test_malformed_ntlm_params(self) -> None: """Test input with malformed NTLM credentials.""" urls = {"http": "localhost:8080", "https": "localhost:8080"} From d94daf50f4e7b5a2b4698b14b1ba0cebfa72c205 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Mon, 10 Jan 2022 16:53:13 +0100 Subject: [PATCH 07/12] address code review feedback (part 2) --- qiskit_ibm_runtime/accounts/account.py | 104 ++++++++++---------- qiskit_ibm_runtime/api/clients/base.py | 7 +- qiskit_ibm_runtime/api/clients/utils.py | 68 ------------- qiskit_ibm_runtime/proxies/configuration.py | 50 +++++++++- 4 files changed, 105 insertions(+), 124 deletions(-) delete mode 100644 qiskit_ibm_runtime/api/clients/utils.py diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index d7d3665e0..08811310b 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -28,53 +28,6 @@ CLOUD_API_URL = "https://us-east.quantum-computing.cloud.ibm.com" -def _assert_valid_auth(auth: AccountType) -> None: - """Assert that the auth parameter is valid.""" - if not (auth in ["cloud", "legacy"]): - raise ValueError( - f"Invalid `auth` value. Expected one of ['cloud', 'legacy'], got '{auth}'." - ) - - -def _assert_valid_token(token: str) -> None: - """Assert that the token is valid.""" - if not (isinstance(token, str) and len(token) > 0): - raise ValueError( - f"Invalid `token` value. Expected a non-empty string, got '{token}'." - ) - - -def _assert_valid_url(url: str) -> None: - """Assert that the URL is valid.""" - try: - urlparse(url) - except: - raise ValueError(f"Invalid `url` value. Failed to parse '{url}' as URL.") - - -def _assert_valid_proxies(config: ProxyConfiguration) -> None: - """Assert that the proxy configuration is valid.""" - if config is not None: - config.validate() - - -def _assert_valid_instance(auth: AccountType, instance: str) -> None: - """Assert that the instance name is valid for the given account type.""" - if auth == "cloud": - if not (isinstance(instance, str) and len(instance) > 0): - raise ValueError( - f"Invalid `instance` value. Expected a non-empty string, got '{instance}'." - ) - if auth == "legacy": - if instance is not None: - try: - from_instance_format(instance) - except: - raise ValueError( - f"Invalid `instance` value. Expected hub/group/project format, got {instance}" - ) - - class Account: """Class that represents an account.""" @@ -97,20 +50,20 @@ def __init__( proxies: Proxy configuration. verify: Whether to verify server's TLS certificate. """ - _assert_valid_auth(auth) + self._assert_valid_auth(auth) self.auth = auth - _assert_valid_token(token) + self._assert_valid_token(token) self.token = token resolved_url = url or (LEGACY_API_URL if auth == "legacy" else CLOUD_API_URL) - _assert_valid_url(resolved_url) + self._assert_valid_url(resolved_url) self.url = resolved_url - _assert_valid_instance(auth, instance) + self._assert_valid_instance(auth, instance) self.instance = instance - _assert_valid_proxies(proxies) + self._assert_valid_proxies(proxies) self.proxies = proxies self.verify = verify @@ -155,3 +108,50 @@ def __eq__(self, other: object) -> bool: self.verify == other.verify, ] ) + + @staticmethod + def _assert_valid_auth(auth: AccountType) -> None: + """Assert that the auth parameter is valid.""" + if not (auth in ["cloud", "legacy"]): + raise ValueError( + f"Invalid `auth` value. Expected one of ['cloud', 'legacy'], got '{auth}'." + ) + + @staticmethod + def _assert_valid_token(token: str) -> None: + """Assert that the token is valid.""" + if not (isinstance(token, str) and len(token) > 0): + raise ValueError( + f"Invalid `token` value. Expected a non-empty string, got '{token}'." + ) + + @staticmethod + def _assert_valid_url(url: str) -> None: + """Assert that the URL is valid.""" + try: + urlparse(url) + except: + raise ValueError(f"Invalid `url` value. Failed to parse '{url}' as URL.") + + @staticmethod + def _assert_valid_proxies(config: ProxyConfiguration) -> None: + """Assert that the proxy configuration is valid.""" + if config is not None: + config.validate() + + @staticmethod + def _assert_valid_instance(auth: AccountType, instance: str) -> None: + """Assert that the instance name is valid for the given account type.""" + if auth == "cloud": + if not (isinstance(instance, str) and len(instance) > 0): + raise ValueError( + f"Invalid `instance` value. Expected a non-empty string, got '{instance}'." + ) + if auth == "legacy": + if instance is not None: + try: + from_instance_format(instance) + except: + raise ValueError( + f"Invalid `instance` value. Expected hub/group/project format, got {instance}" + ) diff --git a/qiskit_ibm_runtime/api/clients/base.py b/qiskit_ibm_runtime/api/clients/base.py index 2ada1850d..010063cb2 100644 --- a/qiskit_ibm_runtime/api/clients/base.py +++ b/qiskit_ibm_runtime/api/clients/base.py @@ -26,7 +26,6 @@ from ..client_parameters import ClientParameters from ..exceptions import WebsocketError, WebsocketTimeoutError -from .utils import ws_proxy_params logger = logging.getLogger(__name__) @@ -68,8 +67,10 @@ def __init__( message_queue: Queue used to hold received messages. """ self._websocket_url = websocket_url.rstrip("/") - self._proxy_params = ws_proxy_params( - client_params=client_params, ws_url=self._websocket_url + self._proxy_params = ( + client_params.proxies.to_ws_params(self._websocket_url) + if client_params.proxies + else {} ) self._access_token = client_params.token self._job_id = job_id diff --git a/qiskit_ibm_runtime/api/clients/utils.py b/qiskit_ibm_runtime/api/clients/utils.py deleted file mode 100644 index 0804f391a..000000000 --- a/qiskit_ibm_runtime/api/clients/utils.py +++ /dev/null @@ -1,68 +0,0 @@ -# This code is part of Qiskit. -# -# (C) Copyright IBM 2021. -# -# This code is licensed under the Apache License, Version 2.0. You may -# obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -# -# Any modifications or derivative works of this code must retain this -# copyright notice, and modified files need to carry a notice indicating -# that they have been altered from the originals. - -"""Utilities for working with IBM Quantum API.""" - -from typing import Dict -from urllib.parse import urlparse - -from ..client_parameters import ClientParameters - - -def ws_proxy_params(client_params: ClientParameters, ws_url: str) -> Dict: - """Extract proxy information for websocket. - - Args: - client_params: Parameters used for server connection. - ws_url: Websocket URL. - - Returns: - Proxy information to be used by the websocket client. - """ - conn_data = client_params.connection_parameters() - out = {} - - if "proxies" in conn_data: - proxies = conn_data["proxies"] - url_parts = urlparse(ws_url) - proxy_keys = [ - ws_url, - "wss", - "https://" + url_parts.hostname, - "https", - "all://" + url_parts.hostname, - "all", - ] - for key in proxy_keys: - if key in proxies: - proxy_parts = urlparse(proxies[key], scheme="http") - out["http_proxy_host"] = proxy_parts.hostname - out["http_proxy_port"] = proxy_parts.port - out["proxy_type"] = ( - "http" - if proxy_parts.scheme.startswith("http") - else proxy_parts.scheme - ) - if proxy_parts.username and proxy_parts.password: - out["http_proxy_auth"] = ( - proxy_parts.username, - proxy_parts.password, - ) - break - - if "auth" in conn_data: - out["http_proxy_auth"] = ( - client_params.proxies.username_ntlm, - client_params.proxies.password_ntlm, - ) - - return out diff --git a/qiskit_ibm_runtime/proxies/configuration.py b/qiskit_ibm_runtime/proxies/configuration.py index 2a14bac59..2e0ea25eb 100644 --- a/qiskit_ibm_runtime/proxies/configuration.py +++ b/qiskit_ibm_runtime/proxies/configuration.py @@ -14,7 +14,9 @@ from dataclasses import dataclass -from typing import Optional, Dict +from typing import Optional, Dict, Any +from urllib.parse import urlparse + from requests_ntlm import HttpNtlmAuth @@ -82,3 +84,49 @@ def to_request_params(self) -> dict: ) return request_kwargs + + def to_ws_params(self, ws_url: str) -> dict: + """Extract proxy information for websocket. + + Args: + ws_url: Websocket URL. + + Returns: + A dictionary with proxy configuration parameters in the format expected by websocket. + The following keys can be present: ``http_proxy_host``and ``http_proxy_port``, + ``proxy_type``, ``http_proxy_auth``. + """ + out: Any = {} + + if self.urls: + proxies = self.urls + url_parts = urlparse(ws_url) + proxy_keys = [ + ws_url, + "wss", + "https://" + url_parts.hostname, + "https", + "all://" + url_parts.hostname, + "all", + ] + for key in proxy_keys: + if key in proxies: + proxy_parts = urlparse(proxies[key], scheme="http") + out["http_proxy_host"] = proxy_parts.hostname + out["http_proxy_port"] = proxy_parts.port + out["proxy_type"] = ( + "http" + if proxy_parts.scheme.startswith("http") + else proxy_parts.scheme + ) + if proxy_parts.username and proxy_parts.password: + out["http_proxy_auth"] = ( + proxy_parts.username, + proxy_parts.password, + ) + break + + if self.username_ntlm and self.password_ntlm: + out["http_proxy_auth"] = (self.username_ntlm, self.password_ntlm) + + return out From ebf94474fffff6e74471bf47fd7b1943a5bb23aa Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Mon, 10 Jan 2022 17:13:14 +0100 Subject: [PATCH 08/12] fix doc build issue --- qiskit_ibm_runtime/ibm_runtime_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py index dcb0b4965..5dfd7a64e 100644 --- a/qiskit_ibm_runtime/ibm_runtime_service.py +++ b/qiskit_ibm_runtime/ibm_runtime_service.py @@ -147,7 +147,7 @@ def __init__( proxies: Proxy configuration. Supported optional keys are ``urls`` (a dictionary mapping protocol or protocol and host to the URL of the proxy, documented at https://docs.python-requests.org/en/latest/api/#requests.Session.proxies), - ```username_ntlm```, ```password_ntlm```(username and password to enable NTLM user + ``username_ntlm``, ``password_ntlm`` (username and password to enable NTLM user authentication) verify: Whether to verify the server's TLS certificate. @@ -557,7 +557,7 @@ def save_account( proxies: Proxy configuration. Supported optional keys are ``urls`` (a dictionary mapping protocol or protocol and host to the URL of the proxy, documented at https://docs.python-requests.org/en/latest/api/#requests.Session.proxies), - ```username_ntlm```, ```password_ntlm```(username and password to enable NTLM user + ``username_ntlm``, ``password_ntlm`` (username and password to enable NTLM user authentication) verify: Verify the server's TLS certificate. """ From de874a9d56f863f919202716b86ef6b7498aa315 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Tue, 11 Jan 2022 11:07:34 +0100 Subject: [PATCH 09/12] fix failing proxy integration test cases --- test/test_proxies.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_proxies.py b/test/test_proxies.py index b98c2b615..4ac90e4b7 100644 --- a/test/test_proxies.py +++ b/test/test_proxies.py @@ -12,17 +12,17 @@ """Tests for the proxy support.""" -import urllib import subprocess +import urllib from requests.exceptions import ProxyError from qiskit_ibm_runtime import IBMRuntimeService -from qiskit_ibm_runtime.api.clients import AuthClient, VersionClient -from qiskit_ibm_runtime.api.exceptions import RequestsApiError from qiskit_ibm_runtime.api.client_parameters import ClientParameters +from qiskit_ibm_runtime.api.clients import AuthClient, VersionClient from qiskit_ibm_runtime.api.clients.runtime import RuntimeClient - +from qiskit_ibm_runtime.api.exceptions import RequestsApiError +from qiskit_ibm_runtime.proxies import ProxyConfiguration from .ibm_test_case import IBMTestCase from .utils.decorators import requires_qe_access, requires_cloud_service @@ -120,7 +120,7 @@ def test_proxies_authclient(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": VALID_PROXIES}, + proxies=ProxyConfiguration(urls=VALID_PROXIES), ) _ = AuthClient(params) @@ -153,7 +153,7 @@ def test_invalid_proxy_port_runtime_client(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": INVALID_PORT_PROXIES}, + proxies=ProxyConfiguration(urls=INVALID_PORT_PROXIES), ) with self.assertRaises(RequestsApiError) as context_manager: client = RuntimeClient(params) @@ -167,7 +167,7 @@ def test_invalid_proxy_port_authclient(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": INVALID_PORT_PROXIES}, + proxies=ProxyConfiguration(urls=INVALID_PORT_PROXIES), ) with self.assertRaises(RequestsApiError) as context_manager: _ = AuthClient(params) @@ -191,7 +191,7 @@ def test_invalid_proxy_address_runtime_client(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": INVALID_ADDRESS_PROXIES}, + proxies=ProxyConfiguration(urls=INVALID_ADDRESS_PROXIES), ) with self.assertRaises(RequestsApiError) as context_manager: client = RuntimeClient(params) @@ -206,7 +206,7 @@ def test_invalid_proxy_address_authclient(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": INVALID_ADDRESS_PROXIES}, + proxies=ProxyConfiguration(urls=INVALID_ADDRESS_PROXIES), ) with self.assertRaises(RequestsApiError) as context_manager: _ = AuthClient(params) @@ -237,7 +237,7 @@ def test_proxy_urls(self, qe_token, qe_url): auth_type="legacy", token=qe_token, url=qe_url, - proxies={"urls": {"https": proxy_url}}, + proxies=ProxyConfiguration(urls={"https": proxy_url}), ) version_finder = VersionClient( params.url, **params.connection_parameters() From 68149286ec054183887b1abef6cfc688efcc2d4b Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Tue, 11 Jan 2022 11:49:18 +0100 Subject: [PATCH 10/12] fix failing proxy integration test cases (part 2) --- qiskit_ibm_runtime/accounts/account.py | 27 ++++++++++++++--------- qiskit_ibm_runtime/accounts/management.py | 4 +++- qiskit_ibm_runtime/ibm_runtime_service.py | 9 ++++---- test/test_account.py | 20 +++++++++++------ 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py index 08811310b..8f5e0bc65 100644 --- a/qiskit_ibm_runtime/accounts/account.py +++ b/qiskit_ibm_runtime/accounts/account.py @@ -50,22 +50,13 @@ def __init__( proxies: Proxy configuration. verify: Whether to verify server's TLS certificate. """ - self._assert_valid_auth(auth) - self.auth = auth + resolved_url = url or (LEGACY_API_URL if auth == "legacy" else CLOUD_API_URL) - self._assert_valid_token(token) + self.auth = auth self.token = token - - resolved_url = url or (LEGACY_API_URL if auth == "legacy" else CLOUD_API_URL) - self._assert_valid_url(resolved_url) self.url = resolved_url - - self._assert_valid_instance(auth, instance) self.instance = instance - - self._assert_valid_proxies(proxies) self.proxies = proxies - self.verify = verify def to_saved_format(self) -> dict: @@ -109,6 +100,20 @@ def __eq__(self, other: object) -> bool: ] ) + def validate(self) -> "Account": + """Validates the account instance. + + Raises: + ValueError: if the account is invalid + """ + + self._assert_valid_auth(self.auth) + self._assert_valid_token(self.token) + self._assert_valid_url(self.url) + self._assert_valid_instance(self.auth, self.instance) + self._assert_valid_proxies(self.proxies) + return self + @staticmethod def _assert_valid_auth(auth: AccountType) -> None: """Assert that the auth parameter is valid.""" diff --git a/qiskit_ibm_runtime/accounts/management.py b/qiskit_ibm_runtime/accounts/management.py index a38911251..ef149b970 100644 --- a/qiskit_ibm_runtime/accounts/management.py +++ b/qiskit_ibm_runtime/accounts/management.py @@ -56,7 +56,9 @@ def save( auth=auth, proxies=proxies, verify=verify, - ).to_saved_format(), + ) + # avoid storing invalid accounts + .validate().to_saved_format(), ) @staticmethod diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py index 66ce2f4e4..594c374c2 100644 --- a/qiskit_ibm_runtime/ibm_runtime_service.py +++ b/qiskit_ibm_runtime/ibm_runtime_service.py @@ -168,10 +168,6 @@ def __init__( proxies=ProxyConfiguration(**proxies) if proxies else None, verify=verify, ) - if self._account.auth == "cloud" and not self._account.instance: - raise IBMInputValueError( - f"Cloud account must have a service instance (CRN)." - ) self._client_params = ClientParameters( auth_type=self._account.auth, @@ -240,7 +236,7 @@ def _discover_account( instance=instance, proxies=proxies, verify=verify_, - ) + ).validate() if url: logger.warning( "Loading default %s account. Input 'url' is ignored.", auth @@ -264,6 +260,9 @@ def _discover_account( if verify is not None: account.verify = verify + # ensure account is valid, fail early if not + account.validate() + return account def _discover_cloud_backends(self) -> Dict[str, "ibm_backend.IBMBackend"]: diff --git a/test/test_account.py b/test/test_account.py index c34f34dab..cf3691458 100644 --- a/test/test_account.py +++ b/test/test_account.py @@ -61,7 +61,9 @@ def test_invalid_auth(self): with self.assertRaises(ValueError) as err: invalid_auth: Any = "phantom" - Account(auth=invalid_auth, token=self.dummy_token, url=self.dummy_cloud_url) + Account( + auth=invalid_auth, token=self.dummy_token, url=self.dummy_cloud_url + ).validate() self.assertIn("Invalid `auth` value.", str(err.exception)) def test_invalid_token(self): @@ -71,7 +73,9 @@ def test_invalid_token(self): for token in invalid_tokens: with self.subTest(token=token): with self.assertRaises(ValueError) as err: - Account(auth="cloud", token=token, url=self.dummy_cloud_url) + Account( + auth="cloud", token=token, url=self.dummy_cloud_url + ).validate() self.assertIn("Invalid `token` value.", str(err.exception)) def test_invalid_url(self): @@ -83,7 +87,7 @@ def test_invalid_url(self): for params in subtests: with self.subTest(params=params): with self.assertRaises(ValueError) as err: - Account(**params, token=self.dummy_token) + Account(**params, token=self.dummy_token).validate() self.assertIn("Invalid `url` value.", str(err.exception)) def test_invalid_instance(self): @@ -97,7 +101,9 @@ def test_invalid_instance(self): for params in subtests: with self.subTest(params=params): with self.assertRaises(ValueError) as err: - Account(**params, token=self.dummy_token, url=self.dummy_cloud_url) + Account( + **params, token=self.dummy_token, url=self.dummy_cloud_url + ).validate() self.assertIn("Invalid `instance` value.", str(err.exception)) def test_invalid_proxy_config(self): @@ -122,7 +128,7 @@ def test_invalid_proxy_config(self): auth="legacy", token=self.dummy_token, url=self.dummy_cloud_url, - ) + ).validate() self.assertIn("Invalid proxy configuration", str(err.exception)) @@ -467,8 +473,8 @@ def test_enable_account_by_auth_pref(self): subtests = [ {"proxies": MOCK_PROXY_CONFIG_DICT}, {"verify": False}, - {"instance": "bar"}, - {"proxies": MOCK_PROXY_CONFIG_DICT, "verify": False, "instance": "bar"}, + {"instance": "h/g/p"}, + {"proxies": MOCK_PROXY_CONFIG_DICT, "verify": False, "instance": "h/g/p"}, ] for auth in ["cloud", "legacy"]: for extra in subtests: From fbd7b7dc012834c382fe931a91c51695831cdbc7 Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Tue, 11 Jan 2022 13:08:11 +0100 Subject: [PATCH 11/12] fix failing proxy integration test cases (part 3) --- test/mock/proxy_server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/mock/proxy_server.py b/test/mock/proxy_server.py index 3f95d5abd..a50b8141b 100644 --- a/test/mock/proxy_server.py +++ b/test/mock/proxy_server.py @@ -15,6 +15,8 @@ import subprocess from contextlib import contextmanager +from qiskit_ibm_runtime.proxies import ProxyConfiguration + class MockProxyServer: """Local proxy server for testing.""" @@ -55,7 +57,7 @@ def stop(self): def use_proxies(service, proxies): """Context manager to set and restore proxies setting.""" try: - service._client_params.proxies = {"urls": proxies} + service._client_params.proxies = ProxyConfiguration(urls=proxies) yield finally: service._client_params.proxies = None From d942a81f1792c7a2e830dc99a8aab9c2ce30f4df Mon Sep 17 00:00:00 2001 From: Daniel Kaulen Date: Tue, 11 Jan 2022 14:16:30 +0100 Subject: [PATCH 12/12] fix failing proxy integration test cases (part 4) --- test/test_proxies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_proxies.py b/test/test_proxies.py index 4ac90e4b7..c2cc2dfa1 100644 --- a/test/test_proxies.py +++ b/test/test_proxies.py @@ -60,7 +60,7 @@ def test_proxies_cloud_runtime_client(self, service, instance): """Should reach the proxy using RuntimeClient.""" # pylint: disable=unused-argument params = service._client_params - params.proxies = {"urls": VALID_PROXIES} + params.proxies = ProxyConfiguration(urls=VALID_PROXIES) client = RuntimeClient(params) client.list_programs(limit=1) api_line = pproxy_desired_access_log_line(params.url)