You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Zoltan Chovan (Code Review)" <ge...@cloudera.org> on 2022/10/19 17:00:43 UTC
[kudu-CR] jwt: Additional test
Zoltan Chovan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19156
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/util/jwt-util.cc
M src/kudu/util/jwt-util.h
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
12 files changed, 366 insertions(+), 73 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/1
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#7).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 502 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/7
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 3:
> Uploaded patch set 3.
This patch contains code/textual conflicts. I tried to re-base it via gerrit on top of 2a02969e5c186b22f1bf89555f184680eaf31ca0, but it reported an error.
I guess this patch includes some modifications that have been already committed as part of other patches. Please resolve the conflicts and re-submit.
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Mar 2023 22:34:28 +0000
Gerrit-HasComments: No
[kudu-CR] jwt: Additional test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 13: Code-Review+2
Thank you very much for a new test!
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Apr 2023 18:38:16 +0000
Gerrit-HasComments: No
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#3).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 386 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/3
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 6:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@515
PS6, Line 515: TEST_F(SecurityITest, TestJwtMiniCluster) {
Since this scenario runs over 3 seconds now, consider adding SKIP_IF_SLOW_NOT_ALLOWED()
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@540
PS6, Line 540: if (delay_ms != 0) {
: SleepFor(MonoDelta::FromMilliseconds(delay_ms));
: }
nit: could use SleepFor(MonoDelta::FromMilliseconds(0)) to simplify the code since scheduler anomalies could pop up elsewhere anyways.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@572
PS6, Line 572: ASSERT_FALSE(s.ok()) << s.ToString();
Here and elsewhere in this test scenario: this sort of non-OK assertion should be on a specific status type since KuduClientBuilder is a part of the public API, at least.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/mini-cluster/external_mini_cluster.cc@273
PS6, Line 273: std::make_shared<SimpleJwtVerifier>();
Is this still needed?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/client_negotiation.cc
File src/kudu/rpc/client_negotiation.cc:
PS6:
It's better to keep non-related changes out of the patch.
If you want to update the comments w.r.t. KUDU-1921, it's better to do so in a separate patch (BTW, there are other occurrences KUDU-1921 related TODO in the code).
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@127
PS6, Line 127: bool jwt;
Add a comment similar to the comment for 'token' above to clarify on the semantics for the server and the client side negotiators.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@242
PS6, Line 242: string jwks_file_name = "keys.jwks";
: string jwt_test_dir = GetTestPath("jwt");
: string jwt_data = kudu::CreateTestJWT(true);
nit here and elsewhere: could these be marked as constant?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@247
PS6, Line 247: true
nit for better readability: add a comment for parameter name to clarify on its semantics for readers; alternatively, consider introducing enum with meaningful names and use it here and elsewhere instead of a boolean parameter
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@995
PS6, Line 995: PkiConfig::EXTERNALLY_SIGNED,
nit: looks a bit strange that the indent has changed only for this block, but not for other blocks updated above
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1079
PS6, Line 1079:
nit: extra spaces?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1113
PS6, Line 1113: Status::OK(),
: Status::OK(),
: AuthenticationType::JWT,
Does this look like a valid result with an empty list of SASL mechanisms to use during negotiation?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1200
PS6, Line 1200:
nit: extra empty lines
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1202
PS6, Line 1202: ));
I think it's necessary to add more test sub-scenarios, at least to test how the negotiation fires when
* the client and the server differ in JWT capabilities
* TLS encryption is optional at least at one of the sides
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation.cc
File src/kudu/rpc/negotiation.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation.cc@178
PS6, Line 178: const auto jwt = (conn->credentials_policy() == CredentialsPolicy::PRIMARY_CREDENTIALS)
: ? std::nullopt : messenger->jwt();
IIUC, JWT is this context represents primary credentials, not secondary: JWT is a substitute for Kerberos authn there.
Secondary credentials are the credentials created and dispensed by Kudu itself, not something from independent authentication system.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/server_negotiation.cc@208
PS6, Line 208: and jwt_verifier
So, where is that CHECK then? I don't see it in the code below.
And since you are touching this: it seems this comment isn't exactly relevant in the isolated scope of this ServerNegotiation::Negotiate() method without PreflightCheckGSSAPI around.
Maybe, update this comment stating that those should be non-null since the connection negotiation process needs them from now on?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/server/server_base.cc@703
PS6, Line 703: = std::make_shared<SimpleJwtVerifier>();
Is this still needed?
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/CMakeLists.txt@369
PS6, Line 369: set (KUDU_JWT_UTIL_SRCS jwt-util.cc jwt_test_certs.cc)
: add_library(kudu_jwt_util ${KUDU_JWT_UTIL_SRCS})
What drives this change?
I don't think we want to have test certs linked into the code of kudu-master and kudu-tserver.
If keeping jwt_test_certs.cc in test-only mini_oidc library isn't a good fit, consider separating them into their own library and link it instead of mini_oidc where needed.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.h
File src/kudu/util/jwt_test_certs.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.h@22
PS6, Line 22: namespace kudu {
style nit: add an empty line before 'namespace kudu'
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.cc@373
PS6, Line 373: RETURN_NOT_OK(WriteStringToFile(Env::Default(), jwks_content,
: JoinPathSegments(dir, file_name)));
: return Status::OK();
nit: could be shorten into
return WriteStringToFile(...);
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Mar 2023 00:59:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc@375
PS8, Line 375:
> Done
missed
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 17:07:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#4).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
15 files changed, 390 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/4
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#10).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set expires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 516 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/10
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#12).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set expires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 516 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/12
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19156/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19156/8//COMMIT_MSG@11
PS8, Line 11: exires
typo: expires
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc@375
PS8, Line 375:
nit: indent
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 16:55:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#2).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
11 files changed, 381 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/2
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#8).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 511 insertions(+), 82 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/8
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 11:
(1 comment)
> Patch Set 10:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc@375
PS8, Line 375: JoinPat
> missed
Done
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 17:17:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#13).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set expires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 519 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/13
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 9:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@515
PS6, Line 515: TEST_F(SecurityITest, TestJwtMiniCluster) {
> Since this scenario runs over 3 seconds now, consider adding SKIP_IF_SLOW_N
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@540
PS6, Line 540: CHECK(pb.SerializeToString(&creds));
:
: S
> nit: could use SleepFor(MonoDelta::FromMilliseconds(0)) to simplify the cod
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/integration-tests/security-itest.cc@572
PS6, Line 572: ASSERT_TRUE(s.IsRuntimeError());
> Here and elsewhere in this test scenario: this sort of non-OK assertion sho
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/client_negotiation.cc
File src/kudu/rpc/client_negotiation.cc:
PS6:
> It's better to keep non-related changes out of the patch.
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@127
PS6, Line 127: vector<SaslMechanism::Type> sasl_mechs;
> Add a comment similar to the comment for 'token' above to clarify on the se
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@242
PS6, Line 242: if (desc.server.token) {
: ASSERT_OK(token_verifier.ImportKeys(token_signer.verifier().ExportKeys()));
: }
> nit here and elsewhere: could these be marked as constant?
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@247
PS6, Line 247:
> nit for better readability: add a comment for parameter name to clarify on
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@995
PS6, Line 995: NegotiationDescriptor {
> nit: looks a bit strange that the indent has changed only for this block, b
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1200
PS6, Line 1200: },
> nit: extra empty lines
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation-test.cc@1202
PS6, Line 1202: true,
> I think it's necessary to add more test sub-scenarios, at least to test how
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation.cc
File src/kudu/rpc/negotiation.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/negotiation.cc@178
PS6, Line 178: const auto jwt = messenger->jwt();
: ClientNegotiation client_negotiation(conn->release_sock
> IIUC, JWT is this context represents primary credentials, not secondary: JW
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/rpc-test-base.h@56
PS6, Line 56: #include "kudu/util/trace.h"
> add it in alphabet order
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/server_negotiation.cc@208
PS6, Line 208: are not null, si
> So, where is that CHECK then? I don't see it in the code below.
Sorry this was left in, the jwt_verifier actually can be null and is not necessary for the negotiation if the server doesn't support JWT, I'll update the comment.
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/server/server_base.cc@703
PS6, Line 703:
> Is this still needed?
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/CMakeLists.txt@369
PS6, Line 369: add_library(jwt_test_certs jwt_test_certs.cc)
: target_link_libraries(jwt_test_certs)
> What drives this change?
Ack
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.h
File src/kudu/util/jwt_test_certs.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.h@22
PS6, Line 22:
> style nit: add an empty line before 'namespace kudu'
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/jwt_test_certs.cc@373
PS6, Line 373:
: return WriteStringToFile(Env::Default(), jwks_content,
:
> nit: could be shorten into
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.h
File src/kudu/util/mini_oidc.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.h@78
PS6, Line 78: const
> indent
Done
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.cc
File src/kudu/util/mini_oidc.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.cc@24
PS6, Line 24: #include <vector>
> alphabet order
Done
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 16:57:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#9).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 516 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/9
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#11).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set expires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 516 insertions(+), 79 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/11
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#5).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
15 files changed, 393 insertions(+), 84 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/5
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Wenzhe Zhou,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19156
to look at the new patch set (#6).
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set exires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
15 files changed, 393 insertions(+), 84 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/19156/6
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] jwt: Additional test
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19156/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19156/6//COMMIT_MSG@11
PS6, Line 11: exires
nit: expires
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/rpc/rpc-test-base.h@56
PS6, Line 56: #include "kudu/util/jwt.h"
add it in alphabet order
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.h
File src/kudu/util/mini_oidc.h:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.h@78
PS6, Line 78:
indent
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.cc
File src/kudu/util/mini_oidc.cc:
http://gerrit.cloudera.org:8080/#/c/19156/6/src/kudu/util/mini_oidc.cc@24
PS6, Line 24: #include <chrono>
alphabet order
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Mar 2023 08:00:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19156/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19156/8//COMMIT_MSG@11
PS8, Line 11: exires
> typo: expires
Done
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc
File src/kudu/util/jwt_test_certs.cc:
http://gerrit.cloudera.org:8080/#/c/19156/8/src/kudu/util/jwt_test_certs.cc@375
PS8, Line 375:
> nit: indent
Done
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 16:59:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Mar 2023 17:17:40 +0000
Gerrit-HasComments: No
[kudu-CR] jwt: Additional test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19156/12/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/19156/12/src/kudu/util/CMakeLists.txt@381
PS12, Line 381: jwt_test_certs)
Why do we need to link in jwt_test_certs into kudu_jwt_util? Just because jwt-util-test needs those certs and also links in kudu_jwt_util?
This is a bit strange, and that's not desirable: kudu_jwt_util is then linked into the server_process library, per src/kudu/server/CMakeLists.txt
In release build, all these libraries are linked in statically, and by transitive dependencies the test-only code in jwt_test_certs.cc gets into kudu-master and kudu-tserver binaries, but that should not be like that.
Instead, maybe just link jwt_test_certs into jwt-util-test in addition to kudu_jwt_util, but don't link jwt_test_certs as a dependency into kudu_jwt_util?
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Apr 2023 04:04:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] jwt: Additional test
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19156 )
Change subject: jwt: Additional test
......................................................................
jwt: Additional test
* Added JWT scenarios to the negiotiation test
* Updated the security-itest with JWT expiration scenario
* Updated MiniOidc to set expires_at and not_before properties of the
generated JWTs
* Added methods to be able to manually create JWT and JWKS files
Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Reviewed-on: http://gerrit.cloudera.org:8080/19156
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/jwt_test_certs.cc
M src/kudu/util/jwt_test_certs.h
M src/kudu/util/mini_oidc.cc
M src/kudu/util/mini_oidc.h
14 files changed, 519 insertions(+), 81 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19156
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 14
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>