diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index d9c3d0fb0..6a9369d8d 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -32,7 +32,7 @@ logger = logging.getLogger(__name__) api_bp = Blueprint('api', __name__) api = Api() api.init_app(api_bp) -api.decorators = [csrf_protect, +api.decorators = [csrf_protect(), crossdomain(origin='*', headers=['Authorization', 'Content-Type']), process_oauth, time_decorator(api_bp.name, metric_queue)] diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 5457132e8..114ac7277 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -26,6 +26,7 @@ from endpoints.api import (ApiResource, nickname, resource, validate_json_reques from endpoints.exception import NotFound, InvalidToken from endpoints.api.subscribe import subscribe from endpoints.common import common_login +from endpoints.csrf import generate_csrf_token, OAUTH_CSRF_TOKEN_NAME from endpoints.decorators import anon_allowed from util.useremails import (send_confirmation_email, send_recovery_email, send_change_email, send_password_changed, send_org_recovery_email) @@ -673,6 +674,15 @@ class Signout(ApiResource): return {'success': True} +@resource('/v1/externaltoken') +@internal_only +class GenerateExternalToken(ApiResource): + """ Resource for generating a token for external login. """ + @nickname('generateExternalLoginToken') + def post(self): + """ Generates a CSRF token explicitly for OIDC/OAuth-associated login. """ + return {'token': generate_csrf_token(OAUTH_CSRF_TOKEN_NAME)} + @resource('/v1/detachexternal/') @show_if(features.DIRECT_LOGIN) diff --git a/endpoints/csrf.py b/endpoints/csrf.py index 39a0d636b..28fee6c74 100644 --- a/endpoints/csrf.py +++ b/endpoints/csrf.py @@ -12,31 +12,46 @@ from util.http import abort logger = logging.getLogger(__name__) +OAUTH_CSRF_TOKEN_NAME = '_oauth_csrf_token' +_QUAY_CSRF_TOKEN_NAME = '_csrf_token' -def generate_csrf_token(): - if '_csrf_token' not in session: - session['_csrf_token'] = base64.b64encode(os.urandom(48)) +def generate_csrf_token(session_token_name=_QUAY_CSRF_TOKEN_NAME): + """ If not present in the session, generates a new CSRF token with the given name + and places it into the session. Returns the generated token. + """ + if session_token_name not in session: + session[session_token_name] = base64.b64encode(os.urandom(48)) - return session['_csrf_token'] + return session[session_token_name] -def verify_csrf(): - token = session.get('_csrf_token', None) - found_token = request.values.get('_csrf_token', None) +def verify_csrf(session_token_name=_QUAY_CSRF_TOKEN_NAME, + request_token_name=_QUAY_CSRF_TOKEN_NAME): + """ Verifies that the CSRF token with the given name is found in the session and + that the matching token is found in the request args or values. + """ + token = session.get(session_token_name, None) + found_token = request.values.get(request_token_name, None) if not token or token != found_token: - msg = 'CSRF Failure. Session token was %s and request token was %s' - logger.error(msg, token, found_token) + msg = 'CSRF Failure. Session token (%s) was %s and request token (%s) was %s' + logger.error(msg, session_token_name, token, request_token_name, found_token) abort(403, message='CSRF token was invalid or missing.') -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": - verify_csrf() - return func(*args, **kwargs) - return wrapper +def csrf_protect(session_token_name=_QUAY_CSRF_TOKEN_NAME, + request_token_name=_QUAY_CSRF_TOKEN_NAME, + all_methods=False): + def inner(func): + @wraps(func) + def wrapper(*args, **kwargs): + oauth_token = get_validated_oauth_token() + if oauth_token is None: + if all_methods or (request.method != "GET" and request.method != "HEAD"): + verify_csrf(session_token_name, request_token_name) + + return func(*args, **kwargs) + return wrapper + return inner app.jinja_env.globals['csrf_token'] = generate_csrf_token diff --git a/endpoints/oauthlogin.py b/endpoints/oauthlogin.py index a750f5519..865991d8c 100644 --- a/endpoints/oauthlogin.py +++ b/endpoints/oauthlogin.py @@ -12,6 +12,7 @@ from auth.process import require_session_login from data import model from endpoints.common import common_login, route_show_if from endpoints.web import index +from endpoints.csrf import csrf_protect, OAUTH_CSRF_TOKEN_NAME from util.security.jwtutil import decode, InvalidTokenError from util.validation import generate_valid_usernames @@ -19,6 +20,7 @@ logger = logging.getLogger(__name__) client = app.config['HTTPCLIENT'] oauthlogin = Blueprint('oauthlogin', __name__) +oauthlogin_csrf_protect = csrf_protect(OAUTH_CSRF_TOKEN_NAME, 'state', all_methods=True) def render_ologin_error(service_name, error_message=None, register_redirect=False): user_creation = bool(features.USER_CREATION and features.DIRECT_LOGIN) @@ -32,6 +34,7 @@ def render_ologin_error(service_name, error_message=None, register_redirect=Fals } return index('', error_info=error_info) + def get_user(service, token): token_param = { 'access_token': token, @@ -44,7 +47,7 @@ def get_user(service, token): return got_user.json() -def conduct_oauth_login(service, user_id, username, email, metadata={}): +def conduct_oauth_login(service, user_id, username, email, metadata=None): service_name = service.service_name() to_login = model.user.verify_federated_login(service_name.lower(), user_id) if not to_login: @@ -66,17 +69,12 @@ def conduct_oauth_login(service, user_id, username, email, metadata={}): prompts = model.user.get_default_user_prompts(features) to_login = model.user.create_federated_user(new_username, email, service_name.lower(), user_id, set_password_notification=True, - metadata=metadata, + metadata=metadata or {}, prompts=prompts) # Success, tell analytics analytics.track(to_login.username, 'register', {'service': service_name.lower()}) - state = request.args.get('state', None) - if state: - logger.debug('Aliasing with state: %s', state) - analytics.alias(to_login.username, state) - except model.InvalidEmailAddressException: message = "The e-mail address %s is already associated " % (email, ) message = message + "with an existing %s account." % (app.config['REGISTRY_TITLE_SHORT'], ) @@ -96,6 +94,7 @@ def conduct_oauth_login(service, user_id, username, email, metadata={}): return render_ologin_error(service_name) + def get_email_username(user_data): username = user_data['email'] at = username.find('@') @@ -107,6 +106,7 @@ def get_email_username(user_data): @oauthlogin.route('/google/callback', methods=['GET']) @route_show_if(features.GOOGLE_LOGIN) +@oauthlogin_csrf_protect def google_oauth_callback(): error = request.args.get('error', None) if error: @@ -136,6 +136,7 @@ def google_oauth_callback(): @oauthlogin.route('/github/callback', methods=['GET']) @route_show_if(features.GITHUB_LOGIN) +@oauthlogin_csrf_protect def github_oauth_callback(): error = request.args.get('error', None) if error: @@ -199,6 +200,7 @@ def github_oauth_callback(): @oauthlogin.route('/google/callback/attach', methods=['GET']) @route_show_if(features.GOOGLE_LOGIN) @require_session_login +@oauthlogin_csrf_protect def google_oauth_attach(): code = request.args.get('code') token = google_login.exchange_code_for_token(app.config, client, code, @@ -236,6 +238,7 @@ def google_oauth_attach(): @oauthlogin.route('/github/callback/attach', methods=['GET']) @route_show_if(features.GITHUB_LOGIN) @require_session_login +@oauthlogin_csrf_protect def github_oauth_attach(): code = request.args.get('code') token = github_login.exchange_code_for_token(app.config, client, code) @@ -276,6 +279,7 @@ def decode_user_jwt(token, oidc_provider): @oauthlogin.route('/dex/callback', methods=['GET', 'POST']) @route_show_if(features.DEX_LOGIN) +@oauthlogin_csrf_protect def dex_oauth_callback(): error = request.values.get('error', None) if error: @@ -318,6 +322,7 @@ def dex_oauth_callback(): @oauthlogin.route('/dex/callback/attach', methods=['GET', 'POST']) @route_show_if(features.DEX_LOGIN) @require_session_login +@oauthlogin_csrf_protect def dex_oauth_attach(): code = request.args.get('code') token = dex_login.exchange_code_for_token(app.config, client, code, redirect_suffix='/attach', diff --git a/endpoints/web.py b/endpoints/web.py index 03fec46f6..f3a6f7ce7 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -494,7 +494,7 @@ def oauth_local_handler(): @web.route('/oauth/denyapp', methods=['POST']) -@csrf_protect +@csrf_protect() def deny_application(): if not current_user.is_authenticated: abort(401) diff --git a/static/js/app.js b/static/js/app.js index bdffb9451..ba49e6181 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -215,7 +215,8 @@ quayApp.config(['$routeProvider', '$locationProvider', 'pages', function($routeP // 404/403 .route('/:catchall', 'error-view') .route('/:catch/:all', 'error-view') - .route('/:catch/:all/:things', 'error-view'); + .route('/:catch/:all/:things', 'error-view') + .route('/:catch/:all/:things/:here', 'error-view'); }]); // Configure compile provider to add additional URL prefixes to the sanitization list. We use diff --git a/static/js/directives/ui/external-login-button.js b/static/js/directives/ui/external-login-button.js index 509b2b489..f8ec07e53 100644 --- a/static/js/directives/ui/external-login-button.js +++ b/static/js/directives/ui/external-login-button.js @@ -21,19 +21,21 @@ angular.module('quay').directive('externalLoginButton', function () { $scope.startSignin = function() { $scope.signInStarted({'service': $scope.provider}); + ApiService.generateExternalLoginToken().then(function(data) { + var url = ExternalLoginService.getLoginUrl($scope.provider, $scope.action || 'login'); + url = url + '&state=' + encodeURIComponent(data['token']); - var url = ExternalLoginService.getLoginUrl($scope.provider, $scope.action || 'login'); + // Save the redirect URL in a cookie so that we can redirect back after the service returns to us. + var redirectURL = $scope.redirectUrl || window.location.toString(); + CookieService.putPermanent('quay.redirectAfterLoad', redirectURL); - // Save the redirect URL in a cookie so that we can redirect back after the service returns to us. - var redirectURL = $scope.redirectUrl || window.location.toString(); - CookieService.putPermanent('quay.redirectAfterLoad', redirectURL); - - // Needed to ensure that UI work done by the started callback is finished before the location - // changes. - $scope.signingIn = true; - $timeout(function() { - document.location = url; - }, 250); + // Needed to ensure that UI work done by the started callback is finished before the location + // changes. + $scope.signingIn = true; + $timeout(function() { + document.location = url; + }, 250); + }, ApiService.errorDisplay('Could not perform sign in')); }; } }; diff --git a/static/js/services/external-login-service.js b/static/js/services/external-login-service.js index 91197e7b4..7d6061df6 100644 --- a/static/js/services/external-login-service.js +++ b/static/js/services/external-login-service.js @@ -9,14 +9,6 @@ angular.module('quay').factory('ExternalLoginService', ['KeyService', 'Features' var serviceInfo = externalLoginService.getProvider(service); if (!serviceInfo) { return ''; } - var stateClause = ''; - - if (Config.MIXPANEL_KEY && window.mixpanel) { - if (mixpanel.get_distinct_id !== undefined) { - stateClause = "&state=" + encodeURIComponent(mixpanel.get_distinct_id()); - } - } - var loginUrl = KeyService.getConfiguration(serviceInfo.key, 'AUTHORIZE_ENDPOINT'); var clientId = KeyService.getConfiguration(serviceInfo.key, 'CLIENT_ID'); @@ -28,8 +20,7 @@ angular.module('quay').factory('ExternalLoginService', ['KeyService', 'Features' } var url = loginUrl + 'client_id=' + clientId + '&scope=' + scope + '&redirect_uri=' + - redirectUri + stateClause; - + redirectUri; return url; };