From f060fd6ae0f27fde6d4c57b9f05da954f6e7976f Mon Sep 17 00:00:00 2001 From: jakedt Date: Tue, 25 Mar 2014 14:32:26 -0400 Subject: [PATCH] Fix and unify CSRF support across web and API endpoints. --- endpoints/api/__init__.py | 2 ++ endpoints/common.py | 15 ++------------ endpoints/csrf.py | 43 +++++++++++++++++++++++++++++++++++++++ endpoints/web.py | 17 ++++------------ templates/oauthorize.html | 4 ++-- 5 files changed, 53 insertions(+), 28 deletions(-) create mode 100644 endpoints/csrf.py diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 43627663d..f45ac1127 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -18,6 +18,7 @@ from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermissi from auth import scopes from auth.auth_context import get_authenticated_user, get_validated_oauth_token from auth.auth import process_oauth +from endpoints.csrf import csrf_protect logger = logging.getLogger(__name__) @@ -25,6 +26,7 @@ api_bp = Blueprint('api', __name__) api = Api() api.init_app(api_bp) api.decorators = [process_oauth, + csrf_protect, crossdomain(origin='*', headers=['Authorization', 'Content-Type'])] diff --git a/endpoints/common.py b/endpoints/common.py index 264d73c16..4832aaaf0 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -1,11 +1,9 @@ import logging -import os -import base64 import urlparse import json -from flask import session, make_response, render_template, request -from flask.ext.login import login_user, UserMixin, current_user +from flask import make_response, render_template, request +from flask.ext.login import login_user, UserMixin from flask.ext.principal import identity_changed from data import model @@ -85,15 +83,6 @@ def handle_dme(ex): return make_response(json.dumps({'message': ex.message}), 400) -def generate_csrf_token(): - if '_csrf_token' not in session: - session['_csrf_token'] = base64.b64encode(os.urandom(48)) - - return session['_csrf_token'] - -app.jinja_env.globals['csrf_token'] = generate_csrf_token - - def render_page_template(name, **kwargs): resp = make_response(render_template(name, route_data=json.dumps(get_route_data()), **kwargs)) resp.headers['X-FRAME-OPTIONS'] = 'DENY' diff --git a/endpoints/csrf.py b/endpoints/csrf.py new file mode 100644 index 000000000..3ac25bf2a --- /dev/null +++ b/endpoints/csrf.py @@ -0,0 +1,43 @@ +import logging + +from flask import session, request +from functools import wraps + +from app import app +from auth.auth_context import get_validated_oauth_token +from util.http import abort + + +logger = logging.getLogger(__name__) + + +def generate_csrf_token(): + if '_csrf_token' not in session: + session['_csrf_token'] = base64.b64encode(os.urandom(48)) + + return session['_csrf_token'] + + +def csrf_protect(func): + @wraps(func) + def wrapper(*args, **kwargs): + oauth_token = get_validated_oauth_token() + if oauth_token is None and request.method != "GET" and request.method != "HEAD": + token = session.get('_csrf_token', None) + found_token = request.values.get('_csrf_token', None) + + # TODO: add if not token here, once we are sure all sessions have a token. + if token != found_token: + msg = 'CSRF Failure. Session token was %s and request token was %s' + logger.error(msg, token, found_token) + abort(403, message='CSRF token was invalid or missing.') + + if not token: + logger.warning('No CSRF token in session.') + else: + logger.debug('Found and validated CSRF token.') + return func(*args, **kwargs) + return wrapper + + +app.jinja_env.globals['csrf_token'] = generate_csrf_token \ No newline at end of file diff --git a/endpoints/web.py b/endpoints/web.py index a99e132b8..664a0968d 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -14,7 +14,8 @@ from auth.permissions import AdministerOrganizationPermission from util.invoice import renderInvoiceToPdf from util.seo import render_snapshot from util.cache import no_cache -from endpoints.common import common_login, render_page_template, generate_csrf_token +from endpoints.common import common_login, render_page_template +from endpoints.csrf import csrf_protect, generate_csrf_token from util.names import parse_repository_name from util.gravatar import compute_hash from auth import scopes @@ -248,6 +249,7 @@ class FlaskAuthorizationProvider(DatabaseAuthorizationProvider): @web.route('/oauth/authorizeapp', methods=['POST']) +@csrf_protect def authorize_application(): if not current_user.is_authenticated(): abort(401) @@ -257,18 +259,13 @@ def authorize_application(): client_id = request.form.get('client_id', None) redirect_uri = request.form.get('redirect_uri', None) scope = request.form.get('scope', None) - csrf = request.form.get('csrf', None) - - # Verify the csrf token. - if csrf != generate_csrf_token(): - abort(404) - return # Add the access token. return provider.get_token_response('token', client_id, redirect_uri, scope=scope) @web.route('/oauth/denyapp', methods=['POST']) +@csrf_protect def deny_application(): if not current_user.is_authenticated(): abort(401) @@ -278,12 +275,6 @@ def deny_application(): client_id = request.form.get('client_id', None) redirect_uri = request.form.get('redirect_uri', None) scope = request.form.get('scope', None) - csrf = request.form.get('csrf', None) - - # Verify the csrf token. - if csrf != generate_csrf_token(): - abort(404) - return # Add the access token. return provider.get_auth_denied_response('token', client_id, redirect_uri, scope=scope) diff --git a/templates/oauthorize.html b/templates/oauthorize.html index d5bdfc027..858692ec0 100644 --- a/templates/oauthorize.html +++ b/templates/oauthorize.html @@ -54,13 +54,13 @@ - +
- +