lidavidm commented on a change in pull request #8724:
URL: https://github.com/apache/arrow/pull/8724#discussion_r529089266
##########
File path: cpp/src/arrow/flight/client.cc
##########
@@ -1175,6 +1222,7 @@ class FlightClient::FlightClientImpl {
noop_auth_check_;
#endif
int64_t write_size_limit_bytes_;
+ GrpcClientInterceptorAdapterFactory* interceptor_pointer;
Review comment:
Please follow naming conventions (`interceptor_pointer_`)
##########
File path: cpp/src/arrow/flight/client.cc
##########
@@ -993,6 +1010,36 @@ class FlightClient::FlightClientImpl {
return Status::OK();
}
+ Status AuthenticateBasicToken(const std::string& username, const
std::string& password,
+ std::pair<std::string, std::string>*
bearer_token) {
+ // Add bearer token factory to middleware so it can intercept the bearer
token.
+ if (interceptor_pointer != NULLPTR) {
Review comment:
Honestly, instead of using interceptors, why not use ClientRpc.context
(a
[grpc::ClientContext](https://grpc.github.io/grpc/cpp/classgrpc_1_1_client_context.html))
to attach the headers (AddMetadata) and retrieve them
(GetServerInitialMetadata)? This isn't a general auth mechanism anyways, so I'd
rather we keep the implementation simple & tightly scoped in this case.
##########
File path: cpp/src/arrow/flight/client.cc
##########
@@ -993,6 +1010,36 @@ class FlightClient::FlightClientImpl {
return Status::OK();
}
+ Status AuthenticateBasicToken(const std::string& username, const
std::string& password,
+ std::pair<std::string, std::string>*
bearer_token) {
+ // Add bearer token factory to middleware so it can intercept the bearer
token.
+ if (interceptor_pointer != NULLPTR) {
+ interceptor_pointer->AddMiddlewareFactory(
+ std::make_shared<internal::ClientBearerTokenFactory>(bearer_token));
+ } else {
+ return MakeFlightError(FlightStatusCode::Internal,
+ "Connect must be called before
AuthenticateBasicToken.");
+ }
+ ClientRpc rpc({});
Review comment:
You may actually want to allow passing call options so things like
timeouts can be set.
##########
File path: cpp/src/arrow/flight/client.h
##########
@@ -191,6 +194,14 @@ class ARROW_FLIGHT_EXPORT FlightClient {
Status Authenticate(const FlightCallOptions& options,
std::unique_ptr<ClientAuthHandler> auth_handler);
+ /// \brief Authenticate to the server using basic authentication with base
64 encoding.
Review comment:
nit: base64 encoding is part of the definition of HTTP basic auth
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
