Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/19910 )
Change subject: [jwt] switching JWT verification to KeyBasedJwtVerifier ...................................................................... Patch Set 9: (8 comments) http://gerrit.cloudera.org:8080/#/c/19910/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19910/6//COMMIT_MSG@10 PS6, Line 10: OIDC > nit: OIDC (since using JWT, not jwt) Done http://gerrit.cloudera.org:8080/#/c/19910/6//COMMIT_MSG@11 PS6, Line 11: jwks > nit: JWKS Done http://gerrit.cloudera.org:8080/#/c/19910/6//COMMIT_MSG@15 PS6, Line 15: not be a > not be able ? Done http://gerrit.cloudera.org:8080/#/c/19910/6//COMMIT_MSG@16 PS6, Line 16: URL > nit: URL Done http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/jwt-util.h File src/kudu/util/jwt-util.h: http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/jwt-util.h@107 PS6, Line 107: explicit KeyBasedJwtVerifier(const std::string& jwks_file_path) : : jwt_(JWTHelper::GetInstance()), : jwks_uri_(std::move(jwks_file_path)), : is_local_file_(true), : jwks_verify_server_certificate_(false), : jwks_ca_certificate_("") { : } : > Maybe, make the signature of the constructors more tailored to the usage pa Done http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/mini_oidc.h File src/kudu/util/mini_oidc.h: http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/mini_oidc.h@93 PS6, Line 93: } > nit: add a DCHECK() to make sure this method is called only when jwks_url_ Done http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/mini_oidc.cc File src/kudu/util/mini_oidc.cc: http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/mini_oidc.cc@141 PS6, Line 141: "RS256", : kRsaPubKeyJwkN, : kRsaPubKeyJwkE, kKid2, : "RS256", : kRsaInvalidPubKeyJwkN, : kRsaPubKeyJwkE), > nit: wrong indent? Done http://gerrit.cloudera.org:8080/#/c/19910/6/src/kudu/util/mini_oidc.cc@147 PS6, Line 147: resp > nit: wrong indent? Done -- To view, visit http://gerrit.cloudera.org:8080/19910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1f166807bfcf7051bda7843e186eacfbe379eba Gerrit-Change-Number: 19910 Gerrit-PatchSet: 9 Gerrit-Owner: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Wed, 31 May 2023 15:11:41 +0000 Gerrit-HasComments: Yes
