You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/21 21:25:33 UTC
[GitHub] [arrow] jduo commented on a change in pull request #8959: Arrow-10486: [Python] Header-based auth in client
jduo commented on a change in pull request #8959:
URL: https://github.com/apache/arrow/pull/8959#discussion_r545598826
##########
File path: python/pyarrow/tests/test_flight.py
##########
@@ -506,6 +505,95 @@ def get_token(self):
return self.token
+class NoopAuthHandler(ServerAuthHandler):
+ """A no-op auth handler."""
+
+ def authenticate(self, outgoing, incoming):
+ """Do nothing."""
+
+ def is_valid(self, token):
+ """Do nothing."""
+ return ""
+
+
+class HeaderAuthServerMiddlewareFactory(ServerMiddlewareFactory):
Review comment:
This class should not be part of test_flight.py to be used in production. It should also be changed to have some API for checking username/password/bearer rather than being hard coded.
Unless this PR is really for header-based auth in just clients, in which case the description is not really correct.
##########
File path: python/pyarrow/_flight.pyx
##########
@@ -1150,6 +1156,38 @@ cdef class FlightClient(_Weakrefable):
self.client.get().Authenticate(deref(c_options),
move(handler)))
+ def authenticateBasicToken(self, username, password,
+ options: FlightCallOptions = None):
+ """Authenticate to the server with header token authentication.
Review comment:
It's not really header token authentication, it's HTTP basic authentication
##########
File path: python/pyarrow/tests/test_flight.py
##########
@@ -506,6 +505,95 @@ def get_token(self):
return self.token
+class NoopAuthHandler(ServerAuthHandler):
+ """A no-op auth handler."""
+
+ def authenticate(self, outgoing, incoming):
+ """Do nothing."""
+
+ def is_valid(self, token):
+ """Do nothing."""
+ return ""
Review comment:
Better to return None than empty string?
##########
File path: python/pyarrow/includes/libarrow_flight.pxd
##########
@@ -307,6 +308,12 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil:
CStatus Authenticate(CFlightCallOptions& options,
unique_ptr[CClientAuthHandler] auth_handler)
+ # TODO: Add AuthenticateBasicToken
Review comment:
Remove TODO
##########
File path: python/pyarrow/tests/test_flight.py
##########
@@ -506,6 +505,95 @@ def get_token(self):
return self.token
+class NoopAuthHandler(ServerAuthHandler):
+ """A no-op auth handler."""
+
+ def authenticate(self, outgoing, incoming):
+ """Do nothing."""
+
+ def is_valid(self, token):
+ """Do nothing."""
+ return ""
+
+
+class HeaderAuthServerMiddlewareFactory(ServerMiddlewareFactory):
+ """Validates incoming username and password."""
+
+ def start_call(self, info, headers):
+ auth_header = headers.get('authorization')
Review comment:
This is test code, but this should be a case insensitive lookup.
##########
File path: python/pyarrow/tests/test_flight.py
##########
@@ -506,6 +505,95 @@ def get_token(self):
return self.token
+class NoopAuthHandler(ServerAuthHandler):
+ """A no-op auth handler."""
+
+ def authenticate(self, outgoing, incoming):
+ """Do nothing."""
+
+ def is_valid(self, token):
+ """Do nothing."""
+ return ""
+
+
+class HeaderAuthServerMiddlewareFactory(ServerMiddlewareFactory):
+ """Validates incoming username and password."""
+
+ def start_call(self, info, headers):
+ auth_header = headers.get('authorization')
+ values = auth_header[0].split(' ')
+ token = ''
+
+ if values[0] == 'Basic':
+ decoded = base64.b64decode(values[1])
+ pair = decoded.decode("utf-8").split(':')
+ if not (pair[0] == 'test' and pair[1] == 'password'):
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+ token = 'token1234'
+ elif values[0] == 'Bearer':
+ token = values[1]
+ if not token == 'token1234':
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+ else:
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+
+ return HeaderAuthServerMiddleware(token)
+
+
+class HeaderAuthServerMiddleware(ServerMiddleware):
+ """A ServerMiddleware that transports incoming username and passowrd."""
+
+ def __init__(self, token):
+ self.token = token
+
+ def sending_headers(self):
+ return {'authorization': 'Bearer ' + self.token}
+
+
+class HeaderAuthFlightServer(FlightServerBase):
+ """A Flight server that tests with basic token authentication. """
+
+ def do_action(self, context, action):
+ middleware = context.get_middleware("auth")
+ if middleware:
+ headers = middleware.sending_headers()
+ auth_header = headers.get('authorization')
+ values = auth_header.split(' ')
+ return [values[1].encode("utf-8")]
+ raise flight.FlightUnauthenticatedError(
Review comment:
This seems like it's an internal server error.
##########
File path: python/pyarrow/_flight.pyx
##########
@@ -1150,6 +1156,38 @@ cdef class FlightClient(_Weakrefable):
self.client.get().Authenticate(deref(c_options),
move(handler)))
+ def authenticateBasicToken(self, username, password,
+ options: FlightCallOptions = None):
+ """Authenticate to the server with header token authentication.
+
+ Parameters
+ ----------
+ username : string
+ Username to authenticate with
+ password : string
+ Password to authenticate with
+ options : FlightCallOptions
+ Options for this call
+
+ Returns
+ -------
+ pair : pair[string, string]
+ A pair representing the FlightCallOptions header
+ entry of a bearer token.
+ """
+ cdef:
+ CResult[pair[c_string, c_string]] result
+ CFlightCallOptions* c_options = FlightCallOptions.unwrap(options)
+ c_string user = tobytes(username)
+ c_string pw = tobytes(password)
+
+ with nogil:
+ result = self.client.get().AuthenticateBasicToken(deref(c_options),
Review comment:
Shouldn't there be a FlightCallOption implementation to do this? How do you use HTTP basic auth without using authenticateBasicToken? (eg in Java you can directly call getFlightInfo without calling an authenticate function and supply a FlightCallOption with the basic auth info).
##########
File path: python/pyarrow/tests/test_flight.py
##########
@@ -506,6 +505,95 @@ def get_token(self):
return self.token
+class NoopAuthHandler(ServerAuthHandler):
+ """A no-op auth handler."""
+
+ def authenticate(self, outgoing, incoming):
+ """Do nothing."""
+
+ def is_valid(self, token):
+ """Do nothing."""
+ return ""
+
+
+class HeaderAuthServerMiddlewareFactory(ServerMiddlewareFactory):
+ """Validates incoming username and password."""
+
+ def start_call(self, info, headers):
+ auth_header = headers.get('authorization')
+ values = auth_header[0].split(' ')
+ token = ''
+
+ if values[0] == 'Basic':
+ decoded = base64.b64decode(values[1])
+ pair = decoded.decode("utf-8").split(':')
+ if not (pair[0] == 'test' and pair[1] == 'password'):
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+ token = 'token1234'
+ elif values[0] == 'Bearer':
+ token = values[1]
+ if not token == 'token1234':
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+ else:
+ raise flight.FlightUnauthenticatedError('Invalid credentials')
+
+ return HeaderAuthServerMiddleware(token)
+
+
+class HeaderAuthServerMiddleware(ServerMiddleware):
+ """A ServerMiddleware that transports incoming username and passowrd."""
+
+ def __init__(self, token):
+ self.token = token
+
+ def sending_headers(self):
+ return {'authorization': 'Bearer ' + self.token}
Review comment:
We had been using Authorization (title case) in other languages.
----------------------------------------------------------------
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:
users@infra.apache.org