You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/10/11 16:09:05 UTC
[GitHub] [pulsar-client-cpp] merlimat opened a new pull request, #37: Added option to set the method name for Basic authentication
merlimat opened a new pull request, #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37
### Motivation
Added option to set the method name for Basic authentication
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] merlimat merged pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
merlimat merged PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] nodece commented on pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37#issuecomment-1275016795
Do we need to extend the `c_Authentication.cc`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] merlimat commented on pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
merlimat commented on PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37#issuecomment-1275080163
> Do we need to extend the c_Authentication.cc?
Yes, we can add it later. Just want to get this included in the release and python too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37#discussion_r992546592
##########
lib/auth/AuthBasic.cc:
##########
@@ -96,11 +113,17 @@ AuthenticationPtr AuthBasic::create(ParamMap& params) {
if (passwordIt == params.end()) {
throw std::runtime_error("No password provided for basic provider");
}
-
- return create(usernameIt->second, passwordIt->second);
+ auto methodIt = params.find("method");
Review Comment:
Could you add a test to cover this part that a param string is accepted? Like https://github.com/apache/pulsar-client-cpp/blob/829618d686c11217922c70b4e5ffba97559c7056/tests/AuthBasicTest.cc#L125
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] merlimat commented on a diff in pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37#discussion_r992549581
##########
lib/auth/AuthBasic.cc:
##########
@@ -96,11 +113,17 @@ AuthenticationPtr AuthBasic::create(ParamMap& params) {
if (passwordIt == params.end()) {
throw std::runtime_error("No password provided for basic provider");
}
-
- return create(usernameIt->second, passwordIt->second);
+ auto methodIt = params.find("method");
Review Comment:
👍 done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #37: Added option to set the method name for Basic authentication
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #37:
URL: https://github.com/apache/pulsar-client-cpp/pull/37#discussion_r992556502
##########
lib/auth/AuthBasic.cc:
##########
@@ -41,6 +41,14 @@ std::string base64_encode(const std::string& s) {
AuthDataBasic::AuthDataBasic(const std::string& username, const std::string& password) {
commandAuthToken_ = username + ":" + password;
httpAuthToken_ = base64_encode(commandAuthToken_);
+ methodName_ = DEFAULT_BASIC_METHOD_NAME;
+}
Review Comment:
```suggestion
AuthDataBasic::AuthDataBasic(const std::string& username, const std::string& password)
: AuthDataBasic(username, password, DEFAULT_BASIC_METHOD_NAME) {}
```
Reuse the other constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org