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 23:44:09 UTC

[GitHub] [arrow] tifflhl commented on a change in pull request #8959: ARROW-11004: [FlightRPC][Python] Header-based auth in clients

tifflhl commented on a change in pull request #8959:
URL: https://github.com/apache/arrow/pull/8959#discussion_r546987299



##########
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:
       Addressed in new commit.

##########
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:
       Addressed in new commit.

##########
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:
       Returning None gives the error `Flight RPC failed with Python exception "TypeError: expected bytes, NoneType found"`. Based on the method's documentation [here](https://github.com/apache/arrow/blob/a2e7d3a87fb8fa1cc98a54029c0262df468838fa/python/pyarrow/_flight.pyx#L2046), returning an empty string is allowed, I added a comment to clarify the behaviour.

##########
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:
       Addressed in new commit.

##########
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:
       It would seem the C++ grpc impl does not allow capitals in key, when using capital A, the header was rejected due to https://github.com/grpc/grpc/blob/b1df40104c1720bde3b22ded451a037f11b5dc54/src/core/lib/surface/validate_metadata.cc#L78

##########
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:
       Addressed in new commit.




----------------------------------------------------------------
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