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 2021/10/08 14:53:27 UTC
[GitHub] [pulsar] BewareMyPower opened a new pull request #12305: [C++] Support configuring optional scope field for OAuth2 authentication
BewareMyPower opened a new pull request #12305:
URL: https://github.com/apache/pulsar/pull/12305
### Motivation
It's a C++ client catchup for https://github.com/apache/pulsar/pull/11931.
### Modifications
- Add a `scope_` field to `ClientCredentialFlow` and load it from `ParamMap` object whose key is `scope`.
- Refactor `ClientCredentialFlow` to simplify code and make it testable:
- Use only one constructor instead of two overloaded constructors that might look confused
- Add a `generateJsonBody` public method for generating JSON body for post fields in `authenticate` so that it can be tested.
- Add a `KeyFile` class like what Java client does to load client id and client secret from `ParamMap` or file.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added test `AuthPluginTest.testOauth2RequestBody` for the cases that scope exists or doesn't exist.
--
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] github-actions[bot] commented on pull request #12305: [C++] Support configuring optional scope field for OAuth2 authentication
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12305:
URL: https://github.com/apache/pulsar/pull/12305#issuecomment-953487540
@BewareMyPower:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] tuteng commented on a change in pull request #12305: [C++] Support configuring optional scope field for OAuth2 authentication
Posted by GitBox <gi...@apache.org>.
tuteng commented on a change in pull request #12305:
URL: https://github.com/apache/pulsar/pull/12305#discussion_r725417245
##########
File path: pulsar-client-cpp/tests/AuthPluginTest.cc
##########
@@ -388,3 +389,34 @@ TEST(AuthPluginTest, testOauth2CredentialFile) {
ASSERT_EQ(data->hasDataFromCommand(), true);
ASSERT_EQ(data->getCommandData().length(), expectedTokenLength);
}
+
+TEST(AuthPluginTest, testOauth2RequestBody) {
+ ParamMap params;
+ params["issuer_url"] = "https://dev-kt-aa9ne.us.auth0.com";
+ params["client_id"] = "Xd23RHsUnvUlP7wchjNYOaIfazgeHd9x";
+ params["client_secret"] = "rT7ps7WY8uhdVuBTKWZkttwLdQotmdEliaM5rLfmgNibvqziZ-g07ZH52N_poGAb";
Review comment:
This seems to use a real auth0 environment, not sure it's appropriate to put secret here
--
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] BewareMyPower merged pull request #12305: [C++] Support configuring optional scope field for OAuth2 authentication
Posted by GitBox <gi...@apache.org>.
BewareMyPower merged pull request #12305:
URL: https://github.com/apache/pulsar/pull/12305
--
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] BewareMyPower commented on a change in pull request #12305: [C++] Support configuring optional scope field for OAuth2 authentication
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on a change in pull request #12305:
URL: https://github.com/apache/pulsar/pull/12305#discussion_r725417659
##########
File path: pulsar-client-cpp/tests/AuthPluginTest.cc
##########
@@ -388,3 +389,34 @@ TEST(AuthPluginTest, testOauth2CredentialFile) {
ASSERT_EQ(data->hasDataFromCommand(), true);
ASSERT_EQ(data->getCommandData().length(), expectedTokenLength);
}
+
+TEST(AuthPluginTest, testOauth2RequestBody) {
+ ParamMap params;
+ params["issuer_url"] = "https://dev-kt-aa9ne.us.auth0.com";
+ params["client_id"] = "Xd23RHsUnvUlP7wchjNYOaIfazgeHd9x";
+ params["client_secret"] = "rT7ps7WY8uhdVuBTKWZkttwLdQotmdEliaM5rLfmgNibvqziZ-g07ZH52N_poGAb";
Review comment:
It's used for test only. See https://github.com/apache/pulsar/pull/7467/files#diff-8b92a461baa4a7ae3dbee13d3cfc90559c1ee8a46fef263cb76438ea83b598f8
--
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