[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 23: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 23 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Fri, 03 Feb 2023 03:57:40 + Gerrit-HasComments: No
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Reviewed-on: http://gerrit.cloudera.org:8080/18472 Tested-by: Kudu Jenkins Reviewed-by: Attila Bukor Reviewed-by: Wenzhe Zhou Reviewed-by: Alexey Serbin --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 371 insertions(+), 11 deletions(-) Approvals: Kudu Jenkins: Verified Attila Bukor: Looks good to me, but someone else must approve Wenzhe Zhou: Looks good to me, but someone else must approve Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 24 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 23: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 23 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 02 Feb 2023 18:22:53 + Gerrit-HasComments: No
[kudu-CR] jwt: determine discovery endpoint from token
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 23: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 23 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 02 Feb 2023 09:25:03 + Gerrit-HasComments: No
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 23:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util-test.cc@1089
PS20, Line 1089: n
> nit: why is this uppercase?
Done
http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc@960
PS20, Line 960:
> Right. The point was to shorten the time when the lock is held, and avoid
Ack
http://gerrit.cloudera.org:8080/#/c/18472/22/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/22/src/kudu/util/jwt-util.cc@1008
PS22, Line 1008:
: {
> This results either in IO which might take long time or in a remote call.
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 23
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Thu, 02 Feb 2023 08:50:43 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#23) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 371 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/23 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 23 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073
PS14, Line 1073: TEST(JwtUtilTest, VerifyJWKSDiscoveryEnpointMultipleClients) {
> Thanks for the clarification! I did some manual testing, by injecting asser
Cool, thanks for addressing this!
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 14
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Wed, 01 Feb 2023 20:41:19 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 22: (2 comments) http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc@960 PS20, Line 960: > A lock here is necessary as there might be a lookup here while another thre Right. The point was to shorten the time when the lock is held, and avoid makeing remote calls while holding the lock. http://gerrit.cloudera.org:8080/#/c/18472/22/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/22/src/kudu/util/jwt-util.cc@1008 PS22, Line 1008: RETURN_NOT_OK_PREPEND(new_helper->Init(jwks_uri, /*is_local_file*/ false), : "Error initializing JWT helper"); This results either in IO which might take long time or in a remote call. Holding a lock (especially a busy-waiting one) while making a remote call is a no-no. Once again: why not to limit the lifetime of the lock to the time interval when accessing the 'jwt_by_account_id_' dictionary? -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 22 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 01 Feb 2023 20:17:32 + Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#22) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 369 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/22 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 22 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 21: (2 comments) http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073 PS14, Line 1073: } > Ah, sure. I meant: what happens if, say, an assertion for VerifyToken() tr Thanks for the clarification! I did some manual testing, by injecting assertions to random iterations that would definitely fail and those did not trigger the expected failures. I've since changed those assertions to CHECKs, which work as expected. http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc@960 PS20, Line 960: > Is it really necessary to hold this lock even after finding the element in A lock here is necessary as there might be a lookup here while another thread might be writing at the same time, but I restructured the code a bit so the same lock is used, but separately in their own scope. -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 21 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Wed, 01 Feb 2023 15:38:39 + Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#21) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 369 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/21 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 21 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 20: (2 comments) http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073 PS14, Line 1073: } > I'm not sure I understand what you mean by injecting an assertio into the v Ah, sure. I meant: what happens if, say, an assertion for VerifyToken() triggers in one of the threads. Was curious how that'd behave in such a case. It's not a big deal, but was thinking whether the tests exists normally, reporting on the issue or it behaves some different way. http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util.cc@960 PS20, Line 960: std::lock_guard const l(jwt_by_account_id_map_lock_); Is it really necessary to hold this lock even after finding the element in the jwt_by_account_id_ dictionary? I'd expect that would be enough to guard just the access to the jwt_by_account_id_ dictionary, so those light-weight busy-wait locks would be very short in their lifetime. Especially given the fact that there are operations with EasyCurl below to make a remote call -- the latter might block for a really, really long time (especially in the context of busy-waiting synchronization primitives such as spinlocks). Ah, and another nit is that expressions like 'Type const x;' are valid, but don't follow the code style used in the project. -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 20 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Tue, 31 Jan 2023 01:55:08 + Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 20: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18472/20/src/kudu/util/jwt-util-test.cc@1089 PS20, Line 1089: N nit: why is this uppercase? -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 20 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Mon, 30 Jan 2023 22:39:58 + Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#20) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 365 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/20 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 20 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#19) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 365 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/19 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 19 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#18) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 8 files changed, 368 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/18 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 18 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 17:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073
PS14, Line 1073: }
> I'm curious how this test behaves if injecting an assertion into the 'verif
I'm not sure I understand what you mean by injecting an assertio into the
verify functor. Could you clarify a bit?
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1087
PS14, Line 1087:.set
> As per parallel execution, it make sense to gate all the threads with Count
Done
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1093
PS14, Line 1093: :atom
> nit: might be constexr ?
Done
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1096
PS14, Line 1096: for (int i = 0; i < N; i++) {
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1099
PS14, Line 1099: o {
> nits: (a) there is 'auto' in C++, staring C++11 (b) per C++ style guide use
Done
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1101
PS14, Line 1101: ASSERT_OK(jwt_verifier.VerifyToken(valid_user_token,
&subject));
> Does this thread joining work as expected when an assertion is triggered in
Done
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 17
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Mon, 23 Jan 2023 17:17:18 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#17) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 371 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/17 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#16) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 377 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/16 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 16 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#15) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 376 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/15 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 15 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 14:
(5 comments)
Almost there! A few question on how the newly introduced test behaves if any
of the assertion triggers, and some nits.
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1073
PS14, Line 1073: TEST(JwtUtilTest, VerifyJWKSDiscoveryEnpointMultipleClients) {
I'm curious how this test behaves if injecting an assertion into the 'verify'
functor? Is that as expected?
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1087
PS14, Line 1087: verify
As per parallel execution, it make sense to gate all the threads with
CountDownLatch to make sure all the threads start executing this 'verify' task
at once. For example, you could take a look at
https://github.com/apache/kudu/blob/f64ed2aac40515ae46132a9bc3cdf7ad5b3f33de/src/kudu/integration-tests/tablet_copy-itest.cc#L344-L358
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1093
PS14, Line 1093: const
nit: might be constexr ?
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1099
PS14, Line 1099: std::thread &th
nits: (a) there is 'auto' in C++, staring C++11 (b) per C++ style guide used in
the project, the asterisk should stick to the type, not the variable
http://gerrit.cloudera.org:8080/#/c/18472/14/src/kudu/util/jwt-util-test.cc@1101
PS14, Line 1101: }
Does this thread joining work as expected when an assertion is triggered in the
'verify' function? If not, then maybe a good alternative might be joining
threads using SCOPED_CLEANUP, something similar to this:
https://github.com/apache/kudu/blob/f64ed2aac40515ae46132a9bc3cdf7ad5b3f33de/src/kudu/consensus/log_cache-test.cc#L393-L398
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 14
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Fri, 20 Jan 2023 22:18:12 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 14:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1086
PS12, Line 1086:
> Isn't a race to update the 'subject' from different threads when they apply
Ack
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1089
PS12, Line 1089: O
> Does it make sense to add an assertion on the expected result status of the
Done
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1091
PS12, Line 1091: };
> +1: return isn't needed there
Ack
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1094
PS12, Line 1094: std::vector threads;
: for (int i = 0; i < N; i++) {
> Could these be placed in a container? Also, having more threads than just
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h
File src/kudu/util/jwt-util.h:
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h@120
PS10, Line 120: explicit PerAccountKeyBasedJwtVerifier(std::string jwks_uri)
> nit: add a blank line after each declaration for readability
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h@131
PS10, Line 131: // Returns an error if the token doesn't contain the
appropriate fields.
> nit: capitalize the first letter and add a period at the end of the sentenc
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@952
PS10, Line 952: }
> nit: add a blank line after each closing curly brace
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@960
PS10, Line 960: const auto* unique_helper = FindOrNull(jwt_by_account_id_,
account_id);
> nit: add a blank line above this
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@982
PS10, Line 982: dst.push_back('\0');
> Should this be #undef-ed when it's not used anymore?
Done
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@994
PS10, Line 994: RETURN_INVALID_IF(!jwks_uri_member->value.IsString(),
"jwks_uri must be a string");
> nit: maybe change the todo to yours?
Done
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util.cc@997
PS12, Line 997:
> I'd think moving this to be after initting the newly created helper, to gua
This was moved here as the ASAN test throws an error as follows:
SUMMARY: AddressSanitizer: heap-use-after-free
../src/kudu/util/jwt-util.cc:826:3 in
kudu::JWTHelper::Verify(kudu::JWTHelper::JWTDecodedToken const*) const
So I thought there was some race there as well, however it seems that I was
wrong and the issue is still present.
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 14
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Fri, 20 Jan 2023 17:38:42 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#14) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 359 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/14 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 14 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#13) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Andrew Wong Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 359 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/13 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 13 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1086 PS12, Line 1086: string subject; Isn't a race to update the 'subject' from different threads when they apply the 'verify' functor. http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1089 PS12, Line 1089: s Does it make sense to add an assertion on the expected result status of the PerAccountKeyBasedJwtVerifier::VerifyToken() call? http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1091 PS12, Line 1091: return; > warning: redundant return statement at the end of a function with a void re +1: return isn't needed there http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util-test.cc@1094 PS12, Line 1094: std::thread first_client(verify); : std::thread second_client(verify); Could these be placed in a container? Also, having more threads than just two (maybe, something about 8?) might have higher chance of exposing races and bugs, if any. http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18472/12/src/kudu/util/jwt-util.cc@997 PS12, Line 997: std::lock_guard l(jwt_by_account_id_map_lock_); I'd think moving this to be after initting the newly created helper, to guard only the insertion into the jwt_by_account_id_ dictionary. Why to protect the initting of a new helper instance if every thread creates its own? Once the critical section is shorten, maybe we can start using just a lightweight spinlock instead of a heavy mutex to guard access to the jwt_by_account_id_ dictionary? -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 19 Jan 2023 21:32:57 + Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#12) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 348 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/12 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#11) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 348 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/11 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 10:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h
File src/kudu/util/jwt-util.h:
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h@120
PS10, Line 120: explicit PerAccountKeyBasedJwtVerifier(std::string jwks_uri)
nit: add a blank line after each declaration for readability
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.h@131
PS10, Line 131: // marked as mutable so that
PerAccountKeyBasedJwtVerifier::JWTHelperForToken is able to emplace
nit: capitalize the first letter and add a period at the end of the sentence
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@952
PS10, Line 952: }
nit: add a blank line after each closing curly brace
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@960
PS10, Line 960: if (unique_helper) {
nit: add a blank line above this
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@982
PS10, Line 982: #define RETURN_INVALID_IF(stmt, msg) \
Should this be #undef-ed when it's not used anymore?
http://gerrit.cloudera.org:8080/#/c/18472/10/src/kudu/util/jwt-util.cc@994
PS10, Line 994: // TODO(awong): this implementation expects there to be a
small number of
nit: maybe change the todo to yours?
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Mon, 16 Jan 2023 20:23:08 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 12 Jan 2023 22:27:05 + Gerrit-HasComments: No
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util-test.cc@1071
PS9, Line 1071:
> Does it make sense to add a test that calls PerAccountKeyBasedJwtVerifier::
Yes, with multiple concurrent clients it would make sense to cover this as
well. I'll add a new test in the next patchset.
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@969
PS9, Line 969:
kudu::MonoDelta::FromSeconds(static_cast(FLAGS_jwks_pulling_timeout_s)));
> Could this be just
Done
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@974
PS9, Line 974:
: if (dst.size() <= 0) {
: return Status::RuntimeError("Discovery Endpoint returned an
empty document");
: }
:
: dst.push_back('\0');
: Document endpoint_doc;
: endpoint_doc.Parse(reinterpret_cast(dst.data()));
: #define RETURN_INVALID_IF(stmt, msg) \
: if (PREDICT_FALSE(stmt)) { \
: return Status::InvalidArgument(msg); \
: }
: RETURN_INVALID_IF(endpoint_doc.HasParseError(),
GetParseError_En(endpoint_doc.GetParseError()));
: RETURN_INVALID_IF(!endpoint_doc.IsObject(), "root element must
be a JSON Object");
: auto jwks_uri_member = endpoint_doc.FindMember("jwks_uri");
: RETURN_INVALID_IF(jwks_uri_member == endpoint_doc.MemberEnd(),
"jwks_uri is required");
: RETURN_INVALID_IF(!jwks_uri_member->value.IsString(),
"jwks_uri must be a string");
: j
> readability nit: it might be a bit more readable when rewritten like
dst is a faststring, so it doesn't have an empty() method, but I'll switch the
branches
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@1001
PS9, Line 1001: *helper = new_helper.get();
> A few questions here.
The PerAccountKeyBasedJwtVerifier::JWTHelperForToken() is only called from the
same class' VerifyToken method. Which is only invoked when a server/client
negotiation is taking place. So if multiple clients are connecting at the same
time it is possible that this is going to be invoked by multiple threads at the
same time if I understand correctly.
I've added a lock to this method.
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Wed, 11 Jan 2023 17:38:11 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#10) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/integration-tests/security-itest.cc M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 8 files changed, 315 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/10 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/18472 )
Change subject: jwt: determine discovery endpoint from token
..
Patch Set 9:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/server/server_base.cc@261
PS9, Line 261: DEFINE_string(jwks_discovery_endpoint_base, "",
I guess it make sense to add the 'experimental' tag for this flag as well.
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util-test.cc@1071
PS9, Line 1071:
Does it make sense to add a test that calls
PerAccountKeyBasedJwtVerifier::VerifyToken() concurrently from multiple threads?
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@969
PS9, Line 969:
kudu::MonoDelta::FromMilliseconds(static_cast(FLAGS_jwks_pulling_timeout_s
* 1000)))
Could this be just
MonoDelta::FromSeconds(FLAGS_jwks_pulling_timeout_s)
?
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@974
PS9, Line 974: if (dst.size() > 0) {
: dst.push_back('\0');
: Document endpoint_doc;
: endpoint_doc.Parse(reinterpret_cast(dst.data()));
: #define RETURN_INVALID_IF(stmt, msg) \
: if (PREDICT_FALSE(stmt)) { \
: return Status::InvalidArgument(msg); \
: }
: RETURN_INVALID_IF(endpoint_doc.HasParseError(),
:
GetParseError_En(endpoint_doc.GetParseError()));
: RETURN_INVALID_IF(!endpoint_doc.IsObject(), "root element
must be a JSON Object");
: auto jwks_uri_member = endpoint_doc.FindMember("jwks_uri");
: RETURN_INVALID_IF(jwks_uri_member ==
endpoint_doc.MemberEnd(), "jwks_uri is required");
: RETURN_INVALID_IF(!jwks_uri_member->value.IsString(),
"jwks_uri must be a string");
: jwks_uri = string(jwks_uri_member->value.GetString());
: } else {
: return Status::RuntimeError("Discovery Endpoint returned an
empty document");
: }
readability nit: it might be a bit more readable when rewritten like
if (dst.empty()) {
return Status::RuntimeError("Discovery Endpoint returned an empty document";
}
...
http://gerrit.cloudera.org:8080/#/c/18472/9/src/kudu/util/jwt-util.cc@1001
PS9, Line 1001: EmplaceOrDie(&jwt_by_account_id_, account_id,
std::move(new_helper));
A few questions here.
1) Could PerAccountKeyBasedJwtVerifier::JWTHelperForToken() be called by
multiple threads concurrently? If yes, how do we protect against concurrent
access to the jwt_by_account_id_ map?
2) Instead of crashing here when an element is in the map already, could we use
LookupOrEmplace() instead and return the result helper regardless it was
present or not?
--
To view, visit http://gerrit.cloudera.org:8080/18472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Mon, 19 Dec 2022 19:17:35 +
Gerrit-HasComments: Yes
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#9) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/integration-tests/security-itest.cc M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 8 files changed, 308 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/9 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#8) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 307 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/8 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#7) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 303 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/7 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#6) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 281 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/6 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#5) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 309 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/5 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#4) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Co-authored-by: Zoltan Chovan Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 297 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/4 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Zoltan Chovan has uploaded a new patch set (#3) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18472 ) Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 295 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/3 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] jwt: determine discovery endpoint from token
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18472 Change subject: jwt: determine discovery endpoint from token .. jwt: determine discovery endpoint from token This patch introduces a JWT verifier that manages multiple JWKSs, depending on an account ID included in a given JWT's payload. The current implementation is somewhat crude, simply instantiating new JWTHelpers per account ID, thereby creating new threads per account. Follow-on work will be required to ensure scalability (though it's unclear how many account IDs to expect in a typical deployment). Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 --- M src/kudu/rpc/negotiation.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/server/server_base.cc M src/kudu/util/jwt-util-test.cc M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h 7 files changed, 300 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/18472/1 -- To view, visit http://gerrit.cloudera.org:8080/18472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I970bcc6d894c0206160196418d549b71c35ac973 Gerrit-Change-Number: 18472 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
