moksha/middleware/csrf.py | 121 +++++++++++++++++++++++++---------- moksha/middleware/middleware.py | 3 moksha/tests/functional/test_csrf.py | 3 3 files changed, 93 insertions(+), 34 deletions(-)
New commits: commit 57fe8183a1e80807cba35face1b999eb3f94fc2b Author: Luke Macken lmacken@redhat.com Date: Tue Mar 17 23:07:56 2009 -0400
Fix a csrf test case
diff --git a/moksha/tests/functional/test_csrf.py b/moksha/tests/functional/test_csrf.py index 5dbb030..b98ea2d 100644 --- a/moksha/tests/functional/test_csrf.py +++ b/moksha/tests/functional/test_csrf.py @@ -59,7 +59,8 @@ class TestCSRFProtection(TestController):
# Make sure we can't get back to the page without the token resp = self.app.get('/moksha_admin/', status=302) - assert 'The resource was found at /post_logout' in resp, resp + assert 'The resource was found at /post_logout' in resp or \ + 'The resource was found at /login' in resp, resp
# Make sure that we can't get back after we got rejected once resp = self.app.post('/moksha_admin/', {'_csrf_token': token}, status=302)
commit 94c5f846ce949a54d00a7c6737eb459385dee9a1 Author: Luke Macken lmacken@redhat.com Date: Tue Mar 17 23:03:19 2009 -0400
CSRF Fixes
- Handle when environ['repoze.who.application'] exists, which will cause repoze.who to redirect to it, potentially bypassing our CSRF WSGI middleware. - Look for a auth_session_id and fallback to the session cookie - Put the token in environ['CSRF_TOKEN'] - Strip out existing csrf_tokens when re-writing redirect locations - Create a CSRFMetadataProvider.extract_csrf_token method - Create a CSRFMetadataProvider.clean_environ method
diff --git a/moksha/middleware/csrf.py b/moksha/middleware/csrf.py index 1551d01..019f7da 100644 --- a/moksha/middleware/csrf.py +++ b/moksha/middleware/csrf.py @@ -124,36 +124,20 @@ class CSRFProtectionMiddleware(object): request = Request(environ) log.debug("CSRFProtectionMiddleware(%s)" % request.path)
- csrf_token = None - - if self.csrf_token_id in request.GET: - log.debug("%s in GET" % self.csrf_token_id) - csrf_token = request.GET[self.csrf_token_id] - del(request.GET[self.csrf_token_id]) - request.query_string = '&'.join(['%s=%s' % (k, v) for k, v in - request.GET.items()]) - - if self.csrf_token_id in request.POST: - log.debug("%s in POST" % self.csrf_token_id) - csrf_token = request.POST[self.csrf_token_id] - del(request.POST[self.csrf_token_id]) - token = environ.get('repoze.who.identity', {}).get(self.csrf_token_id) + csrf_token = environ.get(self.token_env)
- if token and token == csrf_token: + if token and csrf_token and token == csrf_token: log.debug("User supplied CSRF token matches environ!") else: - if not environ.get('CSRF_AUTH_STATE'): + if not environ.get(self.auth_state): log.debug("Clearing identity") - for key in self.clear_env: - if key in environ: - log.debug("Deleting %s from environ" % key) - del(environ[key]) - if token: + CSRFMetadataProvider.clean_environ(environ, self.clear_env) + if csrf_token: log.warning("Invalid CSRF token. User supplied (%s) " "does not match what's in our environ (%s)" % (csrf_token, token)) - if not environ.get('CSRF_AUTH_STATE'): + if not environ.get(self.auth_state): log.debug("Logging the user out") request.path_info = '/logout_handler' response = request.get_response(self.application) @@ -162,14 +146,19 @@ class CSRFProtectionMiddleware(object):
response = request.get_response(self.application)
- if environ.get('CSRF_AUTH_STATE'): + if environ.get(self.auth_state): log.debug("CSRF_AUTH_STATE; rewriting headers") token = environ.get('repoze.who.identity', {}).get(self.csrf_token_id) - location = list(urlparse(response.location)) - location[4] += self.csrf_token_id + '=' + token - response.location = urlunparse(location) + path = [] + for part in urlparse(response.location): + if part.startswith(self.csrf_token_id): + path.append('') + else: + path.append(part) + path[4] += self.csrf_token_id + '=' + token + response.location = urlunparse(path) log.debug("response.location = %s" % response.location) - environ['CSRF_AUTH_STATE'] = None + environ[self.auth_state] = None
return response(environ, start_response)
@@ -210,20 +199,77 @@ class CSRFMetadataProvider(object): :session_cookie: The name of the session cookie :login_handler: The path to the login handler, used to determine if the user logged in during this request + :token_env: The name of the token variable in the environ + :auth_session_id: The environ key containing an optional session id + :auth_state: The environ key that indicates when we are logging in """ self.csrf_token_id = csrf_token_id self.session_cookie = session_cookie + self.clear_env = clear_env self.login_handler = login_handler + self.token_env = token_env + self.auth_session_id = auth_session_id + self.auth_state = auth_state
def add_metadata(self, environ, identity): request = Request(environ) log.debug("CSRFMetadataProvider.add_metadata(%s)" % request.path) - session_id = request.cookies.get(self.session_cookie) - log.debug('session cookie= %r' % session_id) + + session_id = environ.get(self.auth_session_id) + if not session_id: + session_id = request.cookies.get(self.session_cookie) + log.debug('session_id = %r' % session_id) + if session_id and session_id != 'Set-Cookie:': - identity.update({self.csrf_token_id: sha1(session_id).hexdigest()}) + token = sha1(session_id).hexdigest() + identity.update({self.csrf_token_id: token}) log.debug("Identity updated with CSRF token") if request.path == self.login_handler: - environ['CSRF_AUTH_STATE'] = True + log.debug('Setting CSRF_AUTH_STATE') + environ[self.auth_state] = True + environ[self.token_env] = token + else: + environ[self.token_env] = self.extract_csrf_token(request) + + app = environ.get('repoze.who.application') + if app: + if isinstance(app, HTTPFound) and environ.get(self.auth_state): + log.debug('Got HTTPFound(302) from repoze.who.application') + path = [] + for part in urlparse(app.location()): + if part.startswith(self.csrf_token_id): + path.append('') + else: + path.append(part) + path[4] += self.csrf_token_id + '=' + str(token) + replace_header(app.headers, 'location', urlunparse(path)) + log.debug('Altered headers: %s' % str(app.headers)) else: log.warning("Invalid session cookie %r, not setting CSRF token!" %session_id) + + def extract_csrf_token(self, request): + """ Extract and remove the CSRF token from a given :class:`webob.Request` """ + csrf_token = None + + if self.csrf_token_id in request.GET: + log.debug("%s in GET" % self.csrf_token_id) + csrf_token = request.GET[self.csrf_token_id] + del(request.GET[self.csrf_token_id]) + request.query_string = '&'.join(['%s=%s' % (k, v) for k, v in + request.GET.items()]) + + if self.csrf_token_id in request.POST: + log.debug("%s in POST" % self.csrf_token_id) + csrf_token = request.POST[self.csrf_token_id] + del(request.POST[self.csrf_token_id]) + + return csrf_token + + @classmethod + def clean_environ(cls, environ, keys): + """ Delete the ``keys`` from the supplied ``environ`` """ + log.debug('clean_environ(%s)' % keys) + for key in keys.split(): + if key in environ: + log.debug("Deleting %s from environ" % key) + del(environ[key])
commit bb4d601fba04dc158267e748322e6f75d0c87929 Author: Luke Macken lmacken@redhat.com Date: Tue Mar 17 23:02:45 2009 -0400
Pass a couple more variables to our CSRF objects
diff --git a/moksha/middleware/csrf.py b/moksha/middleware/csrf.py index 641f6f1..1551d01 100644 --- a/moksha/middleware/csrf.py +++ b/moksha/middleware/csrf.py @@ -96,17 +96,22 @@ class CSRFProtectionMiddleware(object): """
def __init__(self, application, csrf_token_id='_csrf_token', - clear_env='repoze.who.identity repoze.what.credentials'): + clear_env='repoze.who.identity repoze.what.credentials', + token_env='CSRF_TOKEN', auth_state='CSRF_AUTH_STATE'): """ Initialize the CSRF Protection WSGI Middleware.
:csrf_token_id: The name of the CSRF token variable :clear_env: Variables to clear out of the `environ` on invalid token + :token_env: The name of the token variable in the environ + :auth_state: The environ key that will be set when we are logging in """ log.info('Creating CSRFProtectionMiddleware') self.application = application self.csrf_token_id = csrf_token_id - self.clear_env = clear_env.split() + self.clear_env = clear_env + self.token_env = token_env + self.auth_state = auth_state
def __call__(self, environ, start_response): """ @@ -194,7 +199,10 @@ class CSRFMetadataProvider(object): implements(IMetadataProvider)
def __init__(self, csrf_token_id='_csrf_token', session_cookie='authtkt', - login_handler='/post_login'): + clear_env='repoze.who.identity repoze.what.credentials', + login_handler='/post_login', token_env='CSRF_TOKEN', + auth_session_id='CSRF_AUTH_SESSION_ID', + auth_state='CSRF_AUTH_STATE'): """ Create the CSRF Metadata Provider Plugin.
diff --git a/moksha/middleware/middleware.py b/moksha/middleware/middleware.py index 9085f8f..a90f340 100644 --- a/moksha/middleware/middleware.py +++ b/moksha/middleware/middleware.py @@ -365,6 +365,9 @@ def make_moksha_middleware(app): csrf_token_id=config.get('moksha.csrf.token_id', '_csrf_token'), clear_env=config.get('moksha.csrf.clear_env', 'repoze.who.identity repoze.what.credentials'), + token_env=config.get('moksha.csrf.token_env', 'CSRF_TOKEN'), + auth_state=config.get('moksha.csrf.auth_state', + 'CSRF_AUTH_STATE'), )
return app
commit 4a8355ca5f4eabe036a5633a12fb21d77b265ce1 Author: Luke Macken lmacken@redhat.com Date: Tue Mar 17 18:43:32 2009 -0400
Reset the CSRF_AUTH_STATE
diff --git a/moksha/middleware/csrf.py b/moksha/middleware/csrf.py index 2ea7b0f..641f6f1 100644 --- a/moksha/middleware/csrf.py +++ b/moksha/middleware/csrf.py @@ -164,6 +164,7 @@ class CSRFProtectionMiddleware(object): location[4] += self.csrf_token_id + '=' + token response.location = urlunparse(location) log.debug("response.location = %s" % response.location) + environ['CSRF_AUTH_STATE'] = None
return response(environ, start_response)
moksha-commits@lists.fedorahosted.org