From 98d5fc985bf3a754f2177b2722a6417f7b7e1a81 Mon Sep 17 00:00:00 2001 From: Christopher Harrison Date: Fri, 8 Sep 2017 11:10:10 +0100 Subject: [PATCH] Change HTTP authentication base to use async and multiple auth methods n.b., The naming of variables, etc. is a bit confused in places...this should probably be fixed Fixes #4 Partially addresses #9 --- irobot/authentication/_base.py | 4 +- irobot/authentication/_http.py | 102 ++++++++++++++++++++------------- requirements.txt | 4 -- 3 files changed, 64 insertions(+), 46 deletions(-) diff --git a/irobot/authentication/_base.py b/irobot/authentication/_base.py index 43b772a..ecf2a7c 100644 --- a/irobot/authentication/_base.py +++ b/irobot/authentication/_base.py @@ -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 diff --git a/irobot/authentication/_http.py b/irobot/authentication/_http.py index cf5e515..c177796 100644 --- a/irobot/authentication/_http.py +++ b/irobot/authentication/_http.py @@ -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 @@ -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) """ #################################################################### @@ -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() @@ -97,35 +108,42 @@ 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 @@ -133,10 +151,15 @@ def authenticate(self, auth_header:str) -> Optional[AuthenticatedUser]: @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 @@ -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: diff --git a/requirements.txt b/requirements.txt index 6911129..8378084 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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