Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the scope for oauthbearer OIDC #3912

Merged
merged 2 commits into from
Jul 14, 2022
Merged

Conversation

jliunyu
Copy link
Contributor

@jliunyu jliunyu commented Jul 14, 2022

There is a regression for oauthbearer oidc related to scope.

when malloc post_fields, the size is post_fields_size + 1, but when copy the string, the +1 missed. which is different than before. If the scope is “kirk-scope”, token provider will receive “kirk-scop”, the last character will be missed.

Local test with AP 3.1
ailing with expired JWT: PASS (10.09s) ]
[0126_oauthbearer_oidc       / 30.075s] 0126_oauthbearer_oidc: duration 30074.752ms
[0126_oauthbearer_oidc       / 30.075s] ================= Test 0126_oauthbearer_oidc PASSED =================
[<MAIN>                      / 30.099s] ALL-TESTS: duration 30098.795ms
TEST 20220714092549 (bare, scenario default) SUMMARY
#==================================================================#
| <MAIN>                                   |     PASSED |  30.099s |
| 0126_oauthbearer_oidc                    |     PASSED |  30.075s |
#==================================================================#

Unit test:

RDUT: INFO: rdkafka_sasl_oauthbearer_oidc.c:442: ut_sasl_oauthbearer_oidc_should_succeed: BEGIN: 
RDUT: PASS: rdkafka_sasl_oauthbearer_oidc.c:481: ut_sasl_oauthbearer_oidc_should_succeed
RDUT: INFO: rdkafka_sasl_oauthbearer_oidc.c:497: ut_sasl_oauthbearer_oidc_with_empty_key: BEGIN: 
RDUT: PASS: rdkafka_sasl_oauthbearer_oidc.c:527: ut_sasl_oauthbearer_oidc_with_empty_key
RDUT: INFO: rdkafka_sasl_oauthbearer_oidc.c:545: ut_sasl_oauthbearer_oidc_post_fields: BEGIN: 
RDUT: PASS: rdkafka_sasl_oauthbearer_oidc.c:560: ut_sasl_oauthbearer_oidc_post_fields
RDUT: INFO: rdkafka_sasl_oauthbearer_oidc.c:578: ut_sasl_oauthbearer_oidc_post_fields_with_empty_scope: BEGIN: 
RDUT: PASS: rdkafka_sasl_oauthbearer_oidc.c:593: ut_sasl_oauthbearer_oidc_post_fields_with_empty_scope
RDUT: INFO: rdunittest.c:502: rd_unittest: unittest: sasl_oauthbearer_oidc: PASS
[0000_unittests              /  3.443s] 0000_unittests: duration 3442.826ms
[0000_unittests              /  3.443s] ================= Test 0000_unittests PASSED =================
[<MAIN>                      /  4.016s] ALL-TESTS: duration 4015.695ms
TEST 20220714092931 (bare, scenario default) SUMMARY

@jliunyu jliunyu requested review from mhowlett and emasab July 14, 2022 16:39
* @brief Build post_fields with \p scope.
* The format of the post_fields is
* `grant_type=client_credentials&scope=scope`
* The post_fields will be returned in \p *post_fields.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe document these two as @param i think?

*post_fields_size =
strlen("grant_type=client_credentials&scope=") + scope_size;
*post_fields = rd_malloc(*post_fields_size + 1);
rd_snprintf(*post_fields, *post_fields_size + 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jliunyu jliunyu merged commit 4faeb81 into confluentinc:master Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants