From f0423d2186842f6a5bc2a7e65345bf0cdf6bf9a1 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 13 Oct 2022 19:00:55 +0800 Subject: [PATCH 1/3] Support auth param string for Basic authentication ### Motivation https://github.com/apache/pulsar/pull/17482 supported basic authentication for Python client, but the `AuthenticationBasic` class accepts two positional arguments as the username and password. It's not good for extension. We should accept an auth param string like `AuthenticationOauth2` so that no changes are needed if the upstream C++ client's implementation changed, like https://github.com/apache/pulsar-client-cpp/pull/37. ### Modifications To be compatible with the existing API, change the first two arguments to keyword arguments. Then, add the 3rd keyword argument to represent the auth param string. ### Verifications `test_basic_auth` and `test_invalid_basic_auth` are extended for this change. --- pulsar/__init__.py | 30 ++++++++++++++++++++++-------- src/authentication.cc | 11 ++++++++--- tests/pulsar_test.py | 23 ++++++++++++++++++----- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/pulsar/__init__.py b/pulsar/__init__.py index ef17844..76929f4 100644 --- a/pulsar/__init__.py +++ b/pulsar/__init__.py @@ -347,18 +347,32 @@ class AuthenticationBasic(Authentication): """ Basic Authentication implementation """ - def __init__(self, username, password): + def __init__(self, username=None, password=None, auth_params_string=None): """ Create the Basic authentication provider instance. - **Args** + For example, if you want to create a basic authentication instance whose + username is "my-user" and password is "my-pass", there are two ways: - * `username`: Used to authentication as username - * `password`: Used to authentication as password - """ - _check_type(str, username, 'username') - _check_type(str, password, 'password') - self.auth = _pulsar.AuthenticationBasic(username, password) + ``` + auth = AuthenticationBasic('my-user', 'my-pass') + auth = AuthenticationBasic(auth_params_string='{"username": "my-user", "password": "my-pass"}') + ``` + + **Args** + * username : str, optional + * password : str, optional + * auth_params_string : str, optional + The JSON presentation of username and password (default is None) + If it's not None, the parameters will be ignored + """ + if auth_params_string is not None: + _check_type(str, auth_params_string, 'auth_params_string') + self.auth = _pulsar.AuthenticationBasic('', '', auth_params_string) + else: + _check_type(str, username, 'username') + _check_type(str, password, 'password') + self.auth = _pulsar.AuthenticationBasic(username, password, '') class Client: """ diff --git a/src/authentication.cc b/src/authentication.cc index 7917498..8802b57 100644 --- a/src/authentication.cc +++ b/src/authentication.cc @@ -91,9 +91,14 @@ struct AuthenticationOauth2Wrapper : public AuthenticationWrapper { }; struct AuthenticationBasicWrapper : public AuthenticationWrapper { - AuthenticationBasicWrapper(const std::string& username, const std::string& password) + AuthenticationBasicWrapper(const std::string& username, const std::string& password, + const std::string& authParamsString) : AuthenticationWrapper() { - this->auth = AuthBasic::create(username, password); + if (authParamsString.empty()) { + this->auth = AuthBasic::create(username, password); + } else { + this->auth = AuthBasic::create(authParamsString); + } } }; @@ -115,5 +120,5 @@ void export_authentication() { init()); class_ >( - "AuthenticationBasic", init()); + "AuthenticationBasic", init()); } diff --git a/tests/pulsar_test.py b/tests/pulsar_test.py index 86abbfe..6b4b7c5 100755 --- a/tests/pulsar_test.py +++ b/tests/pulsar_test.py @@ -1301,12 +1301,10 @@ def _check_type_error(self, fun): with self.assertRaises(TypeError): fun() - def test_basic_auth(self): - username = "admin" - password = "123456" - client = Client(self.adminUrl, authentication=AuthenticationBasic(username, password)) + def _test_basic_auth(self, id, auth): + client = Client(self.adminUrl, authentication=auth) - topic = "persistent://private/auth/my-python-topic-basic-auth" + topic = "persistent://private/auth/my-python-topic-basic-auth-" + str(id) consumer = client.subscribe(topic, "my-sub", consumer_type=ConsumerType.Shared) producer = client.create_producer(topic) producer.send(b"hello") @@ -1316,6 +1314,14 @@ def test_basic_auth(self): self.assertEqual(msg.data(), b"hello") client.close() + def test_basic_auth(self): + username = "admin" + password = "123456" + self._test_basic_auth(0, AuthenticationBasic(username, password)) + self._test_basic_auth(1, AuthenticationBasic( + auth_params_string='{{"username": "{}","password": "{}"}}'.format(username, password) + )) + def test_invalid_basic_auth(self): username = "invalid" password = "123456" @@ -1323,6 +1329,13 @@ def test_invalid_basic_auth(self): topic = "persistent://private/auth/my-python-topic-invalid-basic-auth" with self.assertRaises(pulsar.ConnectError): client.subscribe(topic, "my-sub", consumer_type=ConsumerType.Shared) + client = Client(self.adminUrl, authentication=AuthenticationBasic( + auth_params_string='{{"username": "{}","password": "{}"}}'.format(username, password) + )) + with self.assertRaises(pulsar.ConnectError): + client.subscribe(topic, "my-sub", consumer_type=ConsumerType.Shared) + with self.assertRaises(RuntimeError): + AuthenticationBasic(auth_params_string='invalid auth params') if __name__ == "__main__": main() From a530fc156b4b32ee5e019f3f1d1e9739b7ec3796 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 14 Oct 2022 00:06:31 +0800 Subject: [PATCH 2/3] Add method argument and related tests --- pulsar/__init__.py | 16 +++++++++++----- src/authentication.cc | 7 ++++--- tests/pulsar_test.py | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/pulsar/__init__.py b/pulsar/__init__.py index 76929f4..4f19d13 100644 --- a/pulsar/__init__.py +++ b/pulsar/__init__.py @@ -347,7 +347,7 @@ class AuthenticationBasic(Authentication): """ Basic Authentication implementation """ - def __init__(self, username=None, password=None, auth_params_string=None): + def __init__(self, username=None, password=None, method='basic', auth_params_string=None): """ Create the Basic authentication provider instance. @@ -362,17 +362,23 @@ def __init__(self, username=None, password=None, auth_params_string=None): **Args** * username : str, optional * password : str, optional + * method : str, optional + The authentication method name (default is 'basic') * auth_params_string : str, optional - The JSON presentation of username and password (default is None) - If it's not None, the parameters will be ignored + The JSON presentation of all fields above (default is None) + If it's not None, the other parameters will be ignored. + Here is an example JSON presentation: + {"username": "my-user", "password": "my-pass", "method": "oms3.0"} + The `username` and `password` fields are required. If the "method" field is not set, + it will be "basic" by default. """ if auth_params_string is not None: _check_type(str, auth_params_string, 'auth_params_string') - self.auth = _pulsar.AuthenticationBasic('', '', auth_params_string) + self.auth = _pulsar.AuthenticationBasic('', '', '', auth_params_string) else: _check_type(str, username, 'username') _check_type(str, password, 'password') - self.auth = _pulsar.AuthenticationBasic(username, password, '') + self.auth = _pulsar.AuthenticationBasic(username, password, method, '') class Client: """ diff --git a/src/authentication.cc b/src/authentication.cc index 8802b57..1bec468 100644 --- a/src/authentication.cc +++ b/src/authentication.cc @@ -92,10 +92,10 @@ struct AuthenticationOauth2Wrapper : public AuthenticationWrapper { struct AuthenticationBasicWrapper : public AuthenticationWrapper { AuthenticationBasicWrapper(const std::string& username, const std::string& password, - const std::string& authParamsString) + const std::string& method, const std::string& authParamsString) : AuthenticationWrapper() { if (authParamsString.empty()) { - this->auth = AuthBasic::create(username, password); + this->auth = AuthBasic::create(username, password, method); } else { this->auth = AuthBasic::create(authParamsString); } @@ -120,5 +120,6 @@ void export_authentication() { init()); class_ >( - "AuthenticationBasic", init()); + "AuthenticationBasic", + init()); } diff --git a/tests/pulsar_test.py b/tests/pulsar_test.py index 6b4b7c5..8cc2c55 100755 --- a/tests/pulsar_test.py +++ b/tests/pulsar_test.py @@ -1322,6 +1322,20 @@ def test_basic_auth(self): auth_params_string='{{"username": "{}","password": "{}"}}'.format(username, password) )) + def test_basic_auth_method(self): + username = "admin" + password = "123456" + self._test_basic_auth(2, AuthenticationBasic(username, password, 'basic')) + with self.assertRaises(pulsar.AuthorizationError): + self._test_basic_auth(3, AuthenticationBasic(username, password, 'unknown')) + self._test_basic_auth(4, AuthenticationBasic( + auth_params_string='{{"username": "{}","password": "{}", "method": "basic"}}'.format(username, password) + )) + with self.assertRaises(pulsar.AuthorizationError): + self._test_basic_auth(5, AuthenticationBasic( + auth_params_string='{{"username": "{}","password": "{}", "method": "unknown"}}'.format(username, password) + )) + def test_invalid_basic_auth(self): username = "invalid" password = "123456" From 72c1d7aed41fe0bc14266462c3315713c4ca58d3 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 14 Oct 2022 00:07:25 +0800 Subject: [PATCH 3/3] Add type check for method arg --- pulsar/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pulsar/__init__.py b/pulsar/__init__.py index 4f19d13..1d29d90 100644 --- a/pulsar/__init__.py +++ b/pulsar/__init__.py @@ -378,6 +378,7 @@ def __init__(self, username=None, password=None, method='basic', auth_params_str else: _check_type(str, username, 'username') _check_type(str, password, 'password') + _check_type(str, method, 'method') self.auth = _pulsar.AuthenticationBasic(username, password, method, '') class Client: