Skip to content

Commit

Permalink
Change HTTP authentication base to use async and multiple auth methods
Browse files Browse the repository at this point in the history
n.b., The naming of variables, etc. is a bit confused in places...this
should probably be fixed

Fixes #4
Partially addresses #9
  • Loading branch information
Xophmeister committed Sep 8, 2017
1 parent 5119e8a commit 98d5fc9
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 46 deletions.
4 changes: 2 additions & 2 deletions irobot/authentication/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ def valid(self, invalidation_time:Optional[timedelta]) -> bool:

class BaseAuthHandler(metaclass=ABCMeta):
@abstractmethod
def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]:
async def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]:
"""
Validate the authorisation header
Asynchronously validate the authorisation header
@param auth_header Contents of the "Authorization" header (string)
@return Authenticated user (AuthenticatedUser); None on validation failure
Expand Down
102 changes: 62 additions & 40 deletions irobot/authentication/_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@
import logging
from abc import abstractmethod
from threading import Lock, Timer
from typing import Any, Dict, Optional, Tuple
from typing import Dict, Optional

from requests import Response, Request, Session
from aiohttp import ClientSession, ClientResponse

from irobot.authentication._base import AuthenticatedUser, BaseAuthHandler
from irobot.authentication.parser import HTTPAuthMethod, ParseError, auth_parser
from irobot.config import Configuration
from irobot.logging import LogWriter

Expand All @@ -36,31 +37,38 @@ class HTTPAuthHandler(LogWriter, BaseAuthHandler):
## Implement these #################################################

@abstractmethod
def parse_auth_header(self, auth_header:str) -> Tuple[str, ...]:
def match_auth_challenge(self, auth_challenge:HTTPAuthMethod) -> bool:
"""
Parse the authentication header
Test the given authentication challenge matches the requirements
of the handler class
@param auth_header Contents of the "Authorization" header (string)
@return Parsed contents (tuple of strings)
@params auth_challenge Authentication method challenge (HTTPAuthMethod)
@return Match (bool)
"""

@abstractmethod
def auth_request(self, *args:Tuple[str]) -> Request:
def get_auth_challenge_response(self, auth_challenge:HTTPAuthMethod) -> Dict:
"""
Create an authentication request
Get the parameters for the authentication challenge response.
That is, a dictionary with the following keys:
* url URL to make authentication response to (string)
* auth_response Authentication challenge response (string)
* method HTTP method to use (optional; string)
* headers Additional request headers to pass (optional; dictionary)
@params *args Request arguments (strings)
@return Authentication request (requests.Request)
@params auth_challenge Authentication method challenge (HTTPAuthMethod)
@return Authentication request parameters (dictionary)
"""

@abstractmethod
def get_user(self, req:Request, res:Response) -> str:
def get_authenticated_user(self, auth_challenge:HTTPAuthMethod, response:ClientResponse) -> AuthenticatedUser:
"""
Get the user from the authentication request and response
Get the user from the authentication challenge and response
@param req Authentication request (requests.Request)
@param res Response from authentication URL (requests.Response)
@return Username (string)
@params auth_challenge Authentication method challenge (HTTPAuthMethod)
@param response Response from authentication request (ClientResponse)
@return Authenticated user (AuthenticatedUser)
"""

####################################################################
Expand All @@ -69,15 +77,18 @@ def __init__(self, config:Configuration, logger:Optional[logging.Logger] = None)
"""
Constructor
@param config Arvados authentication configuration
@param config Authentication configuration
@param logger Logger
"""
super().__init__(logger=logger)
self._config = config

# Get the first word of the WWW-Authenticate string
self._auth_method, *_ = self.www_authenticate.split()

# Initialise the cache, if required
if self._config.cache:
self.log(logging.DEBUG, f"Creating {self.www_authenticate} authentication cache")
self.log(logging.DEBUG, f"Creating {self._auth_method} authentication cache")
self._cache:Dict[str, AuthenticatedUser] = {}
self._cache_lock = Lock()
self._schedule_cleanup()
Expand All @@ -97,46 +108,58 @@ def __del__(self) -> None:
def _cleanup(self) -> None:
""" Clean up expired entries from the cache """
with self._cache_lock:
self.log(logging.DEBUG, f"Cleaning {self.www_authenticate} authentication cache")
self.log(logging.DEBUG, f"Cleaning {self._auth_method} authentication cache")
for key, user in list(self._cache.items()):
if not user.valid(self._config.cache):
del self._cache[key]

self._schedule_cleanup()

def _validate_request(self, req:Request) -> Optional[Response]:
async def _validate_request(self, url:str, auth_response:str, *, method:str = "GET", headers:Optional[Dict] = None) -> Optional[ClientResponse]:
"""
Make an authentication request to check for validity
Asynchronously make an authentication request to check validity
@param req Authentication request (requests.Request)
@return Authentication response (requests.Response; None on failure)
@param url URL to make the request to (string)
@param auth_response Authentication challenge response (string)
@param method HTTP method to use (string; default "GET")
@param headers Additional headers to send (optional dictionary)
@return Authentication response (ClientResponse; None on failure)
"""
session = Session()
res = session.send(req.prepare())

if 200 <= res.status_code < 300:
self.log(logging.DEBUG, f"{self.www_authenticate} authenticated")
return res

if res.status_code in [401, 403]:
self.log(logging.WARNING, f"{self.www_authenticate} couldn't authenticate")
else:
res.raise_for_status()
async with ClientSession() as session:
req_headers = {
"Authorization": auth_response,
**(headers or {})
}

async with session.request(method, url, headers=req_headers) as response:
if 200 <= response.status < 300:
self.log(logging.DEBUG, f"{self._auth_method} authenticated")
return response

if response.status in [401, 403]:
self.log(logging.WARNING, f"{self._auth_method} couldn't authenticate")
else:
response.raise_for_status()

return None

def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]:
async def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]:
"""
Validate the authorisation header
@param auth_header Contents of the "Authorization" header (string)
@return Authenticated user (AuthenticatedUser)
"""
try:
parsed = self.parse_auth_header(auth_header)
_auth_methods = auth_parser(auth_header)
auth_challenge, *_ = filter(self.match_auth_challenge, _auth_methods)

except ParseError:
self.log(logging.WARNING, f"{self._auth_method} authentication handler couldn't parse authentication header")
return None

except ValueError:
self.log(logging.WARNING, f"{self.www_authenticate} authentication handler couldn't parse authentication header")
self.log(logging.ERROR, f"No HTTP {self._auth_method} authentication challenge found")
return None

# Check the cache
Expand All @@ -151,10 +174,9 @@ def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]:
# Clean up expired users
del self._cache[auth_header]

req = self.auth_request(*parsed)
res = self._validate_request(req)
if res:
user = AuthenticatedUser(self.get_user(req, res))
response = await self._validate_request(**self.get_auth_challenge_response(auth_challenge))
if response:
user = self.get_authenticated_user(auth_challenge, response)

# Put validated user in the cache
if self._config.cache:
Expand Down
4 changes: 0 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
# Deprecated: Move to using aiohttp
requests==2.13.0

# aiohttp
aiohttp==2.2.5
aiodns==1.1.1
cchardet==2.1.1
Expand Down

0 comments on commit 98d5fc9

Please sign in to comment.