From 2597bcef3fc330457a6818362ac4b99e7dbec8cc Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 11 Aug 2014 15:47:44 -0400 Subject: [PATCH 1/6] Add support for login with Google. Note that this CL is not complete --- config.py | 12 +- endpoints/callbacks.py | 128 ++++++++++++++---- initdb.py | 2 + static/directives/external-login-button.html | 17 +++ static/directives/signin-form.html | 6 +- static/directives/signup-form.html | 6 +- static/js/app.js | 100 +++++++++----- static/js/controllers.js | 5 +- static/partials/user-admin.html | 30 +++- .../{githuberror.html => ologinerror.html} | 8 +- 10 files changed, 231 insertions(+), 83 deletions(-) create mode 100644 static/directives/external-login-button.html rename templates/{githuberror.html => ologinerror.html} (66%) diff --git a/config.py b/config.py index a903fa29a..708449714 100644 --- a/config.py +++ b/config.py @@ -19,7 +19,7 @@ def build_requests_session(): CLIENT_WHITELIST = ['SERVER_HOSTNAME', 'PREFERRED_URL_SCHEME', 'GITHUB_CLIENT_ID', 'GITHUB_LOGIN_CLIENT_ID', 'MIXPANEL_KEY', 'STRIPE_PUBLISHABLE_KEY', 'ENTERPRISE_LOGO_URL', 'SENTRY_PUBLIC_DSN', 'AUTHENTICATION_TYPE', - 'REGISTRY_TITLE', 'REGISTRY_TITLE_SHORT'] + 'REGISTRY_TITLE', 'REGISTRY_TITLE_SHORT', 'GOOGLE_LOGIN_CLIENT_ID'] def getFrontendVisibleConfig(config_dict): @@ -115,6 +115,13 @@ class DefaultConfig(object): GITHUB_LOGIN_CLIENT_ID = '' GITHUB_LOGIN_CLIENT_SECRET = '' + # Google Config. + GOOGLE_TOKEN_URL = 'https://accounts.google.com/o/oauth2/token' + GOOGLE_USER_URL = 'https://www.googleapis.com/oauth2/v1/userinfo' + + GOOGLE_LOGIN_CLIENT_ID = '' + GOOGLE_LOGIN_CLIENT_SECRET = '' + # Requests based HTTP client with a large request pool HTTPCLIENT = build_requests_session() @@ -144,6 +151,9 @@ class DefaultConfig(object): # Feature Flag: Whether GitHub login is supported. FEATURE_GITHUB_LOGIN = False + # Feature Flag: Whether Google login is supported. + FEATURE_GOOGLE_LOGIN = False + # Feature flag, whether to enable olark chat FEATURE_OLARK_CHAT = False diff --git a/endpoints/callbacks.py b/endpoints/callbacks.py index 015f3c3a7..ba53cbc5c 100644 --- a/endpoints/callbacks.py +++ b/endpoints/callbacks.py @@ -7,6 +7,7 @@ from endpoints.common import render_page_template, common_login, route_show_if from app import app, analytics from data import model from util.names import parse_repository_name +from util.validation import generate_valid_usernames from util.http import abort from auth.permissions import AdministerRepositoryPermission from auth.auth import require_session_login @@ -21,19 +22,31 @@ client = app.config['HTTPCLIENT'] callback = Blueprint('callback', __name__) -def exchange_github_code_for_token(code, for_login=True): +def exchange_code_for_token(code, service_name='GITHUB', for_login=True, form_encode=False): code = request.args.get('code') + id_config = service_name + '_LOGIN_CLIENT_ID' if for_login else service_name + '_CLIENT_ID' + secret_config = service_name + '_LOGIN_CLIENT_SECRET' if for_login else service_name + '_CLIENT_SECRET' + payload = { - 'client_id': app.config['GITHUB_LOGIN_CLIENT_ID' if for_login else 'GITHUB_CLIENT_ID'], - 'client_secret': app.config['GITHUB_LOGIN_CLIENT_SECRET' if for_login else 'GITHUB_CLIENT_SECRET'], + 'client_id': app.config[id_config], + 'client_secret': app.config[secret_config], 'code': code, + 'grant_type': 'authorization_code', + 'redirect_uri': '%s://%s/oauth2/%s/callback' % (app.config['PREFERRED_URL_SCHEME'], + app.config['SERVER_HOSTNAME'], + service_name.lower()) } + headers = { 'Accept': 'application/json' } - get_access_token = client.post(app.config['GITHUB_TOKEN_URL'], - params=payload, headers=headers) + if form_encode: + get_access_token = client.post(app.config[service_name + '_TOKEN_URL'], + data=payload, headers=headers) + else: + get_access_token = client.post(app.config[service_name + '_TOKEN_URL'], + params=payload, headers=headers) json_data = get_access_token.json() if not json_data: @@ -52,17 +65,76 @@ def get_github_user(token): return get_user.json() +def get_google_user(token): + token_param = { + 'access_token': token, + 'alt': 'json', + } + + get_user = client.get(app.config['GOOGLE_USER_URL'], params=token_param) + return get_user.json() + +def conduct_oauth_login(service_name, user_id, username, email): + to_login = model.verify_federated_login(service_name.lower(), user_id) + if not to_login: + # try to create the user + try: + valid = next(generate_valid_usernames(username)) + to_login = model.create_federated_user(valid, email, service_name.lower(), + user_id, set_password_notification=True) + + # 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.DataModelException, ex: + return render_page_template('ologinerror.html', service_name=service_name, + error_message=ex.message) + + if common_login(to_login): + return redirect(url_for('web.index')) + + return render_page_template('ologinerror.html', service_name=service_name, + error_message='Unknown error') + + +@callback.route('/google/callback', methods=['GET']) +@route_show_if(features.GOOGLE_LOGIN) +def google_oauth_callback(): + error = request.args.get('error', None) + if error: + return render_page_template('ologinerror.html', service_name='Google', error_message=error) + + token = exchange_code_for_token(request.args.get('code'), service_name='GOOGLE', form_encode=True) + user_data = get_google_user(token) + if not user_data or not user_data.get('id', None) or not user_data.get('email', None): + return render_page_template('ologinerror.html', service_name = 'Google', + error_message='Could not load user data') + + username = user_data['email'] + at = username.find('@') + if at > 0: + username = username[0:at] + + return conduct_oauth_login('Google', user_data['id'], username, user_data['email']) + + @callback.route('/github/callback', methods=['GET']) @route_show_if(features.GITHUB_LOGIN) def github_oauth_callback(): error = request.args.get('error', None) if error: - return render_page_template('githuberror.html', error_message=error) + return render_page_template('ologinerror.html', service_name = 'GitHub', error_message=error) - token = exchange_github_code_for_token(request.args.get('code')) + token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB') user_data = get_github_user(token) if not user_data: - return render_page_template('githuberror.html', error_message='Could not load user data') + return render_page_template('ologinerror.html', service_name = 'GitHub', + error_message='Could not load user data') username = user_data['login'] github_id = user_data['id'] @@ -84,38 +156,34 @@ def github_oauth_callback(): if user_email['primary']: break - to_login = model.verify_federated_login('github', github_id) - if not to_login: - # try to create the user - try: - to_login = model.create_federated_user(username, found_email, 'github', - github_id, set_password_notification=True) + return conduct_oauth_login('github', github_id, username, found_email) - # Success, tell analytics - analytics.track(to_login.username, 'register', {'service': 'github'}) - state = request.args.get('state', None) - if state: - logger.debug('Aliasing with state: %s' % state) - analytics.alias(to_login.username, state) +@callback.route('/google/callback/attach', methods=['GET']) +@route_show_if(features.GOOGLE_LOGIN) +@require_session_login +def google_oauth_attach(): + token = exchange_code_for_token(request.args.get('code'), service_name='GOOGLE') + user_data = get_google_user(token) + if not user_data or not user_data.get('id', None): + return render_page_template('ologinerror.html', service_name = 'Google', + error_message='Could not load user data') - except model.DataModelException, ex: - return render_page_template('githuberror.html', error_message=ex.message) - - if common_login(to_login): - return redirect(url_for('web.index')) - - return render_page_template('githuberror.html') + google_id = user_data['id'] + user_obj = current_user.db_user() + model.attach_federated_login(user_obj, 'google', google_id) + return redirect(url_for('web.user')) @callback.route('/github/callback/attach', methods=['GET']) @route_show_if(features.GITHUB_LOGIN) @require_session_login def github_oauth_attach(): - token = exchange_github_code_for_token(request.args.get('code')) + token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB') user_data = get_github_user(token) if not user_data: - return render_page_template('githuberror.html', error_message='Could not load user data') + return render_page_template('ologinerror.html', service_name = 'GitHub', + error_message='Could not load user data') github_id = user_data['id'] user_obj = current_user.db_user() @@ -130,7 +198,7 @@ def github_oauth_attach(): def attach_github_build_trigger(namespace, repository): permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - token = exchange_github_code_for_token(request.args.get('code'), for_login=False) + token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB', for_login=False) repo = model.get_repository(namespace, repository) if not repo: msg = 'Invalid repository: %s/%s' % (namespace, repository) diff --git a/initdb.py b/initdb.py index 7e48ae3af..5d10a2039 100644 --- a/initdb.py +++ b/initdb.py @@ -179,6 +179,8 @@ def initialize_database(): TeamRole.create(name='member') Visibility.create(name='public') Visibility.create(name='private') + + LoginService.create(name='google') LoginService.create(name='github') LoginService.create(name='quayrobot') LoginService.create(name='ldap') diff --git a/static/directives/external-login-button.html b/static/directives/external-login-button.html new file mode 100644 index 000000000..cc1d39bbd --- /dev/null +++ b/static/directives/external-login-button.html @@ -0,0 +1,17 @@ + + + + + Sign In with GitHub + Attach to GitHub Account + + + + + + + Sign In with Google + Attach to Google Account + + + diff --git a/static/directives/signin-form.html b/static/directives/signin-form.html index f56b8f8db..ec57619a8 100644 --- a/static/directives/signin-form.html +++ b/static/directives/signin-form.html @@ -11,10 +11,8 @@ OR - - Sign In with GitHub - +
+
Invalid username or password.
diff --git a/static/directives/signup-form.html b/static/directives/signup-form.html index fb0ccc6fa..249bff31c 100644 --- a/static/directives/signup-form.html +++ b/static/directives/signup-form.html @@ -18,10 +18,8 @@ OR - - Sign In with GitHub - +
+

No credit card required.

diff --git a/static/js/app.js b/static/js/app.js index 8daec75d4..978012bb6 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -1278,10 +1278,41 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading var keyService = {} keyService['stripePublishableKey'] = Config['STRIPE_PUBLISHABLE_KEY']; + keyService['githubClientId'] = Config['GITHUB_CLIENT_ID']; keyService['githubLoginClientId'] = Config['GITHUB_LOGIN_CLIENT_ID']; keyService['githubRedirectUri'] = Config.getUrl('/oauth2/github/callback'); + keyService['googleLoginClientId'] = Config['GOOGLE_LOGIN_CLIENT_ID']; + keyService['googleRedirectUri'] = Config.getUrl('/oauth2/google/callback'); + + keyService['googleLoginUrl'] = 'https://accounts.google.com/o/oauth2/auth?response_type=code&'; + keyService['githubLoginUrl'] = 'https://github.com/login/oauth/authorize?'; + + keyService['googleLoginScope'] = 'openid email'; + keyService['githubLoginScope'] = 'user:email'; + + keyService.getExternalLoginUrl = function(service, action) { + var state_clause = ''; + if (Config.MIXPANEL_KEY && window.mixpanel) { + if (mixpanel.get_distinct_id !== undefined) { + state_clause = "&state=" + encodeURIComponent(mixpanel.get_distinct_id()); + } + } + + var client_id = keyService[service + 'LoginClientId']; + var scope = keyService[service + 'LoginScope']; + var redirect_uri = keyService[service + 'RedirectUri']; + if (action == 'attach') { + redirect_uri += '/attach'; + } + + var url = keyService[service + 'LoginUrl'] + 'client_id=' + client_id + '&scope=' + scope + + '&redirect_uri=' + redirect_uri + state_clause; + + return url; + }; + return keyService; }]); @@ -2150,6 +2181,41 @@ quayApp.directive('userSetup', function () { }); +quayApp.directive('externalLoginButton', function () { + var directiveDefinitionObject = { + priority: 0, + templateUrl: '/static/directives/external-login-button.html', + replace: false, + transclude: true, + restrict: 'C', + scope: { + 'signInStarted': '&signInStarted', + 'redirectUrl': '=redirectUrl', + 'provider': '@provider', + 'action': '@action' + }, + controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, CookieService, Features, Config) { + $scope.startSignin = function(service) { + $scope.signInStarted({'service': service}); + + var url = KeyService.getExternalLoginUrl(service, $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); + + // Needed to ensure that UI work done by the started callback is finished before the location + // changes. + $timeout(function() { + document.location = url; + }, 250); + }; + } + }; + return directiveDefinitionObject; +}); + + quayApp.directive('signinForm', function () { var directiveDefinitionObject = { priority: 0, @@ -2163,29 +2229,6 @@ quayApp.directive('signinForm', function () { 'signedIn': '&signedIn' }, controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, CookieService, Features, Config) { - $scope.showGithub = function() { - if (!Features.GITHUB_LOGIN) { return; } - - $scope.markStarted(); - - var mixpanelDistinctIdClause = ''; - if (Config.MIXPANEL_KEY && mixpanel.get_distinct_id !== undefined) { - $scope.mixpanelDistinctIdClause = "&state=" + encodeURIComponent(mixpanel.get_distinct_id()); - } - - // Save the redirect URL in a cookie so that we can redirect back after GitHub 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. - $timeout(function() { - var url = 'https://github.com/login/oauth/authorize?client_id=' + encodeURIComponent(KeyService.githubLoginClientId) + - '&scope=user:email' + mixpanelDistinctIdClause; - document.location = url; - }, 250); - }; - $scope.markStarted = function() { if ($scope.signInStarted != null) { $scope.signInStarted(); @@ -2235,18 +2278,9 @@ quayApp.directive('signupForm', function () { scope: { }, - controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, Config, UIService) { + controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, Config, UIService) { $('.form-signup').popover(); - if (Config.MIXPANEL_KEY) { - angulartics.waitForVendorApi(mixpanel, 500, function(loadedMixpanel) { - var mixpanelId = loadedMixpanel.get_distinct_id(); - $scope.github_state_clause = '&state=' + mixpanelId; - }); - } - - $scope.githubClientId = KeyService.githubLoginClientId; - $scope.awaitingConfirmation = false; $scope.registering = false; diff --git a/static/js/controllers.js b/static/js/controllers.js index d77ffb298..7690f79f6 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -1681,6 +1681,10 @@ function UserAdminCtrl($scope, $timeout, $location, ApiService, PlanService, Use $scope.githubLogin = resp.login; }); } + + if ($scope.cuser.logins[i].service == 'google') { + $scope.hasGoogleLogin = true; + } } } }); @@ -1697,7 +1701,6 @@ function UserAdminCtrl($scope, $timeout, $location, ApiService, PlanService, Use $scope.convertStep = 0; $scope.org = {}; $scope.githubRedirectUri = KeyService.githubRedirectUri; - $scope.githubClientId = KeyService.githubLoginClientId; $scope.authorizedApps = null; $scope.logsShown = 0; diff --git a/static/partials/user-admin.html b/static/partials/user-admin.html index 783c5f87a..fc0acb89e 100644 --- a/static/partials/user-admin.html +++ b/static/partials/user-admin.html @@ -33,7 +33,7 @@
  • Account E-mail
  • Robot Accounts
  • Change Password
    • -
  • GitHub Login
    • +
  • External Logins
  • Authorized Applications
  • Usage Logs @@ -162,12 +162,14 @@ - -
    + +
    -
    + + +
    GitHub Login:
    @@ -175,12 +177,28 @@ {{githubLogin}}
    -
    - Connect with GitHub +
    +
    + + +
    +
    +
    Google Login:
    +
    +
    + Account tied to your Google account. +
    +
    + +
    +
    +
    +
    +
    diff --git a/templates/githuberror.html b/templates/ologinerror.html similarity index 66% rename from templates/githuberror.html rename to templates/ologinerror.html index acb803f57..cd921eec2 100644 --- a/templates/githuberror.html +++ b/templates/ologinerror.html @@ -1,14 +1,14 @@ {% extends "base.html" %} {% block title %} - Error Logging in with GitHub · Quay.io + Error Logging in with {{ service_name }} · Quay.io {% endblock %} {% block body_content %}
    -

    There was an error logging in with GitHub.

    +

    There was an error logging in with {{ service_name }}.

    {% if error_message %}
    {{ error_message }}
    @@ -16,11 +16,11 @@
    Please register using the registration form to continue. - You will be able to connect your github account to your Quay.io account + You will be able to connect your account to your Quay.io account in the user settings.
    -{% endblock %} \ No newline at end of file +{% endblock %} From 389c88a7c4b2b08b879b772a0deca139e004ef3e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 11 Aug 2014 18:25:01 -0400 Subject: [PATCH 2/6] Update federated login to store metadata and have the UI pull the information from the metadata --- data/database.py | 1 + data/model/legacy.py | 13 ++++-- endpoints/api/user.py | 8 ++++ endpoints/callbacks.py | 81 ++++++++++++++++++++++++++------- static/js/controllers.js | 9 ++-- static/partials/user-admin.html | 17 +++++-- 6 files changed, 99 insertions(+), 30 deletions(-) diff --git a/data/database.py b/data/database.py index 76a0af9df..6099cf5d9 100644 --- a/data/database.py +++ b/data/database.py @@ -116,6 +116,7 @@ class FederatedLogin(BaseModel): user = ForeignKeyField(User, index=True) service = ForeignKeyField(LoginService, index=True) service_ident = CharField() + metadata_json = TextField(default='{}') class Meta: database = db diff --git a/data/model/legacy.py b/data/model/legacy.py index b5afdfeb8..bfa310046 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -346,7 +346,8 @@ def set_team_org_permission(team, team_role_name, set_by_username): return team -def create_federated_user(username, email, service_name, service_id, set_password_notification): +def create_federated_user(username, email, service_name, service_id, + set_password_notification, metadata={}): if not is_create_user_allowed(): raise TooManyUsersException() @@ -356,7 +357,8 @@ def create_federated_user(username, email, service_name, service_id, set_passwor service = LoginService.get(LoginService.name == service_name) FederatedLogin.create(user=new_user, service=service, - service_ident=service_id) + service_ident=service_id, + metadata_json=json.dumps(metadata)) if set_password_notification: create_notification('password_required', new_user) @@ -364,9 +366,10 @@ def create_federated_user(username, email, service_name, service_id, set_passwor return new_user -def attach_federated_login(user, service_name, service_id): +def attach_federated_login(user, service_name, service_id, metadata={}): service = LoginService.get(LoginService.name == service_name) - FederatedLogin.create(user=user, service=service, service_ident=service_id) + FederatedLogin.create(user=user, service=service, service_ident=service_id, + metadata_json=json.dumps(metadata)) return user @@ -385,7 +388,7 @@ def verify_federated_login(service_name, service_id): def list_federated_logins(user): selected = FederatedLogin.select(FederatedLogin.service_ident, - LoginService.name) + LoginService.name, FederatedLogin.metadata_json) joined = selected.join(LoginService) return joined.where(LoginService.name != 'quayrobot', FederatedLogin.user == user) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 3d79a806d..23bc3137a 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -39,9 +39,16 @@ def user_view(user): organizations = model.get_user_organizations(user.username) def login_view(login): + print login.metadata_json + try: + metadata = json.loads(login.metadata_json) + except: + metadata = None + return { 'service': login.service.name, 'service_identifier': login.service_ident, + 'metadata': metadata } logins = model.list_federated_logins(user) @@ -88,6 +95,7 @@ class User(ApiResource): """ Operations related to users. """ schemas = { 'NewUser': { + 'id': 'NewUser', 'type': 'object', 'description': 'Fields which must be specified for a new user.', diff --git a/endpoints/callbacks.py b/endpoints/callbacks.py index ba53cbc5c..49fa1e8a6 100644 --- a/endpoints/callbacks.py +++ b/endpoints/callbacks.py @@ -11,6 +11,7 @@ from util.validation import generate_valid_usernames from util.http import abort from auth.permissions import AdministerRepositoryPermission from auth.auth import require_session_login +from peewee import IntegrityError import features @@ -22,7 +23,8 @@ client = app.config['HTTPCLIENT'] callback = Blueprint('callback', __name__) -def exchange_code_for_token(code, service_name='GITHUB', for_login=True, form_encode=False): +def exchange_code_for_token(code, service_name='GITHUB', for_login=True, form_encode=False, + redirect_suffix=''): code = request.args.get('code') id_config = service_name + '_LOGIN_CLIENT_ID' if for_login else service_name + '_CLIENT_ID' secret_config = service_name + '_LOGIN_CLIENT_SECRET' if for_login else service_name + '_CLIENT_SECRET' @@ -32,9 +34,10 @@ def exchange_code_for_token(code, service_name='GITHUB', for_login=True, form_en 'client_secret': app.config[secret_config], 'code': code, 'grant_type': 'authorization_code', - 'redirect_uri': '%s://%s/oauth2/%s/callback' % (app.config['PREFERRED_URL_SCHEME'], - app.config['SERVER_HOSTNAME'], - service_name.lower()) + 'redirect_uri': '%s://%s/oauth2/%s/callback%s' % (app.config['PREFERRED_URL_SCHEME'], + app.config['SERVER_HOSTNAME'], + service_name.lower(), + redirect_suffix) } headers = { @@ -74,14 +77,15 @@ def get_google_user(token): get_user = client.get(app.config['GOOGLE_USER_URL'], params=token_param) return get_user.json() -def conduct_oauth_login(service_name, user_id, username, email): +def conduct_oauth_login(service_name, user_id, username, email, metadata={}): to_login = model.verify_federated_login(service_name.lower(), user_id) if not to_login: # try to create the user try: valid = next(generate_valid_usernames(username)) to_login = model.create_federated_user(valid, email, service_name.lower(), - user_id, set_password_notification=True) + user_id, set_password_notification=True, + metadata=metadata) # Success, tell analytics analytics.track(to_login.username, 'register', {'service': service_name.lower()}) @@ -102,6 +106,15 @@ def conduct_oauth_login(service_name, user_id, username, email): error_message='Unknown error') +def get_google_username(user_data): + username = user_data['email'] + at = username.find('@') + if at > 0: + username = username[0:at] + + return username + + @callback.route('/google/callback', methods=['GET']) @route_show_if(features.GOOGLE_LOGIN) def google_oauth_callback(): @@ -115,12 +128,13 @@ def google_oauth_callback(): return render_page_template('ologinerror.html', service_name = 'Google', error_message='Could not load user data') - username = user_data['email'] - at = username.find('@') - if at > 0: - username = username[0:at] + username = get_google_username(user_data) + metadata = { + 'service_username': username + } - return conduct_oauth_login('Google', user_data['id'], username, user_data['email']) + return conduct_oauth_login('Google', user_data['id'], username, user_data['email'], + metadata=metadata) @callback.route('/github/callback', methods=['GET']) @@ -156,14 +170,20 @@ def github_oauth_callback(): if user_email['primary']: break - return conduct_oauth_login('github', github_id, username, found_email) + metadata = { + 'service_username': username + } + + return conduct_oauth_login('github', github_id, username, found_email, metadata=metadata) @callback.route('/google/callback/attach', methods=['GET']) @route_show_if(features.GOOGLE_LOGIN) @require_session_login def google_oauth_attach(): - token = exchange_code_for_token(request.args.get('code'), service_name='GOOGLE') + token = exchange_code_for_token(request.args.get('code'), service_name='GOOGLE', + redirect_suffix='/attach', form_encode=True) + user_data = get_google_user(token) if not user_data or not user_data.get('id', None): return render_page_template('ologinerror.html', service_name = 'Google', @@ -171,7 +191,21 @@ def google_oauth_attach(): google_id = user_data['id'] user_obj = current_user.db_user() - model.attach_federated_login(user_obj, 'google', google_id) + + username = get_google_username(user_data) + metadata = { + 'service_username': username + } + + try: + model.attach_federated_login(user_obj, 'google', google_id, metadata=metadata) + except IntegrityError: + err = 'Google account %s is already attached to a %s account' % ( + username, app.config['REGISTRY_TITLE_SHORT']) + + return render_page_template('ologinerror.html', service_name = 'Google', + error_message=err) + return redirect(url_for('web.user')) @@ -187,7 +221,21 @@ def github_oauth_attach(): github_id = user_data['id'] user_obj = current_user.db_user() - model.attach_federated_login(user_obj, 'github', github_id) + + username = user_data['login'] + metadata = { + 'service_username': username + } + + try: + model.attach_federated_login(user_obj, 'github', github_id, metadata=metadata) + except IntegrityError: + err = 'Github account %s is already attached to a %s account' % ( + username, app.config['REGISTRY_TITLE_SHORT']) + + return render_page_template('ologinerror.html', service_name = 'Github', + error_message=err) + return redirect(url_for('web.user')) @@ -198,7 +246,8 @@ def github_oauth_attach(): def attach_github_build_trigger(namespace, repository): permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB', for_login=False) + token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB', + for_login=False) repo = model.get_repository(namespace, repository) if not repo: msg = 'Invalid repository: %s/%s' % (namespace, repository) diff --git a/static/js/controllers.js b/static/js/controllers.js index 7690f79f6..5278e4efe 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -1673,17 +1673,16 @@ function UserAdminCtrl($scope, $timeout, $location, ApiService, PlanService, Use UserService.updateUserIn($scope, function(user) { $scope.cuser = jQuery.extend({}, user); - if (Features.GITHUB_LOGIN && $scope.cuser.logins) { + if ($scope.cuser.logins) { for (var i = 0; i < $scope.cuser.logins.length; i++) { if ($scope.cuser.logins[i].service == 'github') { - var githubId = $scope.cuser.logins[i].service_identifier; - $http.get('https://api.github.com/user/' + githubId).success(function(resp) { - $scope.githubLogin = resp.login; - }); + $scope.hasGithubLogin = true; + $scope.githubLogin = $scope.cuser.logins[i].metadata['service_username']; } if ($scope.cuser.logins[i].service == 'google') { $scope.hasGoogleLogin = true; + $scope.googleLogin = $scope.cuser.logins[i].metadata['service_username']; } } } diff --git a/static/partials/user-admin.html b/static/partials/user-admin.html index fc0acb89e..ca76342dd 100644 --- a/static/partials/user-admin.html +++ b/static/partials/user-admin.html @@ -173,11 +173,15 @@
    GitHub Login:
    -
    +
    {{githubLogin}}
    -
    +
    + + Account attached to Github Account +
    +
    @@ -189,8 +193,13 @@
    Google Login:
    -
    - Account tied to your Google account. +
    + + {{ googleLogin }} +
    +
    + + Account attached to Google Account
    From 11176215e10a069045892420a2c41c6492fe11af Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 11 Aug 2014 18:35:26 -0400 Subject: [PATCH 3/6] Commit new DB changes and make sure the metadata is always present in some form --- endpoints/api/user.py | 2 +- test/data/test.db | Bin 614400 -> 614400 bytes 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 23bc3137a..d0d089dcd 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -43,7 +43,7 @@ def user_view(user): try: metadata = json.loads(login.metadata_json) except: - metadata = None + metadata = {} return { 'service': login.service.name, diff --git a/test/data/test.db b/test/data/test.db index 4d04283311e6feb845dff3fde9a4d370858119be..b64829db209fca64a5f122a3c96d74f8f7d66845 100644 GIT binary patch delta 6425 zcmds*dwdjCmdCrgs#9HgbucId2x&qhB&5>yeq<6zr;|?7-RVb?&Z7lMRdrPYA!*2y zfEdad{mjhnXI5y}yY4WGpd-=XBWN=0h~m1aJ32Zus581cj2{P=QQ|oJQFdKX_J)Xp zLC4SV-{y}_cb$8__uSt-=hXe&okt6I9xc3eF;cs0i3h2Dq~q*nEa%-fQj;B9_F%5U z=eO*k$G_aQA*YmwbC*94qX; z)1OQZ-j9ZVASWw__FzkozPLL1hoO^L`TJyg^52J#Vfem>Hzap&oz6;Id;QC1kNr!E zc>q$L9vR2hl=bdS_HP@*O1r`Oq;vZotU^IHC68`Di7kBUog0&5Hypzk+*#o=dr?qw z6;Y9#*l`eFgSZcwJywvL4Ik{kaU5Tz{`W$&7y%jciSk3cNBefBV~*n%$48EL9d9^} zADZWU+EIaHsO9QSabJDcoQQ2%>hp2A@%|+1Ig4A7aIn4=y!Q#>tfayLMyhY~ zxC2cQxq*uLLSkKWb)A>-Mrvqxv(M{ou4xa|1gfH~?ez_UJLahs!!em|YWDF`wB7^! zXOR{zB6A5rZEIH>BEhJ;w0a{~9jwu}4b&LJK_92a^hm6?YinFE+$zo0ceS>9MjF5? zXOSYlv77Zbck3+!Ra>>VNDsGjgYrnJU#>QWs{BLEMwgGHShr_$f8Fp_mpI%LYV4;* z0Q)IY!h2+Ggi1751vbZ+E#7*cXWMWi8>(vQ>Z~22wrtv@M+Q6f`r*!iNV&uPyJ8`%JG&NW|*rNvG+v44^YPFwDZ0(n;{On+3tI=L6dMS{74iPG=WTu60iP24U zF_=@WO>QpO=8pQsP(9_1`Poo3)+kaQt~Suv5DZgI6e~8x+8MsN$sLx&04IVUoI{rI zO-)rkPpq`lU*8|@-XyU;$=%#qI>Pq1^%i_!)PK6H zdCt@qERI7C!;zEu0_;nVXO8+Y+=3ux#=La&N=4hg!;gLrOGCeh6?{W zbF|h5ttw-ojZT}Qv#aM8~}SE{MiJF_t25;-rU=CZ$9{gby^-*GHL(=i-F zpqy#w?0=gz1^y;b>rX2YYVudsPcm0FOOn66p)S@G^$GqcTUG0;WhJ?l^E7)Km~b2I zuN4|4p^X7gp0E~zx)avWN~yM`p`8yia?mG)-BlsFIxL1LpWN0U1gqN`o1;=)buAO9 zoq{eOxFWKy$3>NsNlGzvl8ZA6DNCA4a)u6@C&d_)LHFsK6J<)gXG8CR-ajyq=+=7~ zisnd4B5B4+(Pa!(M)Rct&j>6vnT5XLDEKzmTb$^m6HCcTdD!9@$$S_l&pnyo&DB_X z0mpDGEoeHNL5!1}u1TaSN)$=QIYpv%!-&fo$XJ8%qcrVw4|Y0d!f`T88OxTjw3DRZ zUw(-*e;~0wt_Z=&5sSuoqnn)O=2a)?Cz0Tz&!P9TV82j)rd| zs&xe#CBCx8%Pw(pC$`lwO2KWGB|{{6Ru@Sq4NM+Jlwd9~l1Qr*BPjxEby@8eBuc=p zn@UTxtV#kL!6^#KNg72eicFDlgM(Ww@VX?kR@8-@cR(yisuquP0;@`7JT5~AR%1vx zZp2BMk_8^;SWc7~Vd}Ggz(RWpL`9?Gl%SIgEyyIt=y6gKMT3+CN)=T_RpOcmwv^ba zmm3BJCz1?O5aCvG46TwX&kCe2NdhN{hHmiScO^EVU?xnsqdeSR8ZHoyBo&ocNI~ME zt1halqyUDrRTLNk1+Ah&Qi1>j)l{97SXw3pT~p$+pz)HZf+T6HDiCGSkaWo8DVm0p z^BgHFyh>^!C+U*L>+sMAJ|=Ce3OJgkG?_6-M$_PeWC_M0$~?(xA_Em;SxuKfU8&7o zz|xAqiVVy}L4qSCUM3ZW5lAYoC~$SMs2B=3SZZ5R04s=5R1p@h0gIl4@spBFF=Sja z;+mjAnHU93m)hzIC`B?@9abGLD-x6*f>lFeNtUMcIIYtRN2{RSWm~nJRfIUlL5pT^ zB&Wc(po(#dlr&aVIF^CCDuVl5HoichXibqc4eAqzlPfb}sv^t*O5_X}sw{}{Ny>K5 zx$C^X`Y2x)Xb;7_;WkdL^3+7A7B<>K$w5(StFLQqiO2%$W@FJvh;3}*DLG1e+i7o; z+)&pfR<+sZRs&zkvy?>fQ|DYMPdO^DbcV^wlz(#uq&#(s*(&t z#Wh)xc~zV`l1K?%KmX#og`r5i#0bFoV#;s7Y7u!x=0Vy^b1ecdNc7a1mr{ndU*aT{ z@(j<*6xiv+C@rISd#O!N2xss!-r%TM?F27D!;8gLAp4B&;A z9|OG?0jU9Bb9pmF6oE$@@N2%-yc$|4@j_3&II2a!7sRifF$;6d=R+G0;wvw2J{80x zGZuc1BaHWIpoZ|0%bWLwaPhL1sp$}Y|MjC6@A8FM)0d)dWNtoQFsFU9IrW>(#3km@ zFJ287==MwObIka~=D9MbTw~q9MzuA1rTW#)EO+Ji2{ZK_P z7KzX`5gkL3om|ZtT~(5`BG@aeZh&Q8($OTB4`g$Ja}(g1b7#P_i!; zn(VOuhokVT2x~S<5_5)bwy$0G|ML*JT=P-fKUNRlHWc3v}FVzdz;ur6p!=0r<6@C7acMa>f%W&;r!n;K&mRHddm%CXkq=LS z%AeZD@RCpdRW);$f=4nSvgAI9OxyL{3Dd|0kxYpE<$Vx&RsAa;ncgK}d@UUF*a65a zx^~Arux1Q0R~~DDz3G0)oELZ?2AmH-=KNDTaWi)zKn~fAeO77&y!A2kkdwwzl9j$a`>xdFHlkkoA!LG>38b9nXN*UfGs}#o@^G zW~@s;lAe|3Omz{4eFy#~PNh`ZLf9?nPpqF>!$=1lR=LZaUE+#DCyRwpIe4;^IGq$Z z8J@KTgC=E}hi4m_(>Pv+r(Qt^TOPJg<10@Ld|>uS;L`;#o_&wl$MI!%txh$IMKHY( zu6ox5+&X)!?*Z`HgnbW|@j$l^+$}pEbSQ%sR@!Gmk=T<|_x^2I0rxItth2 zl>_(W%nttQ+V=rHWgo*<9grO0_fz&gxL|$hv}w4&qC7a`6F-BRlqS#An%>gn>ArGs z?_r2?>I-ea^)C?RI=EyzaQ+;kYzyDK9%MZZWnNk_-C*VxgTJ6~>a&kSnYrUPbel#I z*o8slPmavUe8>HVOz+C%3&Z8$<|iPM9dGFajwc~tngaSN71FCGCM2mv!{%Do&!tON|}=*H)O8zsiD zMZ@jI;C+xmd)nzkOMLtg}v>!XAw;+q`0cc9Bq&s z1<#%w4^P+%JfxDk&S(myQ39(grm=pu&Ze@6d$C2|e|r*i%p;Crmcv&)4~njWfiQ>b zdcmS>7|5DCpFR#!=M!VN@c2#7n7Ogp0y{IGScex=yZ>k!?Xv|2u7<4n2a2mf$OC7UU5pG3OndyC$8)+Id*+De)-ipow?T+xrGUJwqJB`B$&`uGyOarx(K{Yh;Oo zIR8(*3(THg@a7Wu#KRHeIJflK2D7*a{4y7&Cc_gOF{kb0F0hw}`%(H(+XtXhfYYRh zK8=7P5r(?#iR1|QFOite%3fFWBeSRL%Uk)GyM3@+CdM)6vhLI1xC}SfWuWuG&N2wg ze*W1Tf%7{Mw)l+D1c-77<5sOG1-~hWuqAD;JZSc)U&3at_yM|_IEZ?VnBdTA$jW|Z z(J9ce2C|Sp-ZU{;NsQ*;C68`zGkdn3zsm>UK@Gwozz9TX|&j^93I_6*E6nElt!i1hcB Zz%1^cZ7@42q7wXOJyDF6-1R-{{{h72SJ(gm delta 6334 zcmds*dwf%6n#XfaPA;dkO@S&>C=F1ALQiro@3}#Nq)pQ#ZEj7{rUiwQe+z-9cxRy3XwAdWC&U2S;qjb(a6O zfAo^)eZKGW{@&+(-ly-@BlET%nYVog=Dl%d4dy+t`twah*{lO4xz(LJh!VpyhMTgd zh5A{=4t9+c70j1!*V_sw84`L0H(Or>@7!$#x&P=MB&J-sq$HQ? zx!2e@H;{An?jQ=X>z3r+>ODmW=_gm_p1$fRQF8ljQ*$?M9x0mdcSw4h^{*6tFQPov zH%Js_rs=t^{(VI8o#E$l_ANVzs-W}D+>tG(h~hM7hhCKEW`luig|{)$NadZ(9&l4 zIcXqu@{_y=CdKmi7tjTd*!pa1upYx3hU3_BY)NjiJu=*R1mi5^lDU>?6&2WZ4BY>( z*e~GKzrtv^^`yB7I!>6T55IQ|JHom+DH&?1t!tGVYEvV(%oDS zWTBe&&@BN$6hJbXh`T*>YkjN7@AK7(HSv1*-e;KI6^qmd8@)_(4PTRVvrWEeq+Sj% zk`RoAg+M?O6OEETDf$`$kpPoys^t<~qa;V_8pTkfKgK*7Z*IB&f^Fto$-#WBh@3({jJ%USRHAV}we^Ccb^!r$UtpDd(z!n5?O|(L zWtOkw>KojxYaEiA?&)%F%*0~Jeznu%4u|7FX!6E3_oSVT&c2oy{PC!(ytbyVwuf~@ zvRbw`l1eo6MN(pTV>ILJ?`1U6)8q+tc{Z+zM>i&fPy^E+YTr!vzz@H`m;u%=ib4|; z@V0uBJRJ~de@)!$X^OOjYnwnQ7>L(32c+6SlnXWSv1X}0=@ugHL{ek|b&|x%u`7s4 zhBDnWpEOvlRxPV`t?lVjv*oN(&amZjYoNB>r#RO+*0#0B+Wg(^tlt@SWYX2~mWZZq zSW8b00Xace2f#Y6GsMRF7?4unp)=Tg(8hAHx;05bxh~pK!};LUvzXwLM6R0gNBxbgH&pFO#KK&Rjs+XSHA%6V_O$X+ z5Jesevm)Q>s|CCqa(e?oCR|$=Y6yZxpAX(}7MsZ{iJrvz_5ja!b8Q_$%jR&B@7kcM z&0#OSrZ3sv5o`(9U*&Cxgi@QL8`4cZeoz0J@bK$rF^Ao1qY9VXb`@SfTx7>V;lRbF zc783P7hwPA)V7?@?PnbLTGz#HILtb6{-QM=e#eQI+4GJYY(utAVfZBD*iRE~Mt_qyezyywhhD%xD_jtiXdferNq+#{GA%q; zTjK{bsF9i@Y$z%;#l>i>iD?p|@QIW7Jm@=#hZjU+3E3O3kF(KcZ)?yW^|plSnt`A8 z#yzOSv9-bahIl0AZ5&1}A2_3eAkj*O6)2wJG8E8Qj#AVVpg1W-i;|RPc#+Mjn>rRN znYMN9-D+2NcgF^`ouwH-(IUmlcA8nt(u--}06Y*RFjQoG!&d%nu-oj$Av<9%GQVlC z-Cg+Rx#WQlE+g{G(=rg#EYDGbAfzcE3MncDQZkiMfSgG&ESr%t@U3M8Kfp@%K+i_| zXgGG3UCePP7CS|wuQ`?WIo%zrGD^2PXR&=wu%o@we!*z&g3;G}xgyEVNMj}k12`ZE zNz7_UV5HM+RoM(+BuSx!3@=h3qb^fWV4_Cxn#5~7r)mnr=WUtO)zNlt(q3g_k2+^! zG$%^@@Y&_Wb50h?IBrtB)aWKBjA_iVgLv2${5Cq_^Nk5|T;LPLJ+zhBY#We4ie*!b zLNPp-qJS)mlp@ePC9!mx6=_Cg0BgjZxYd9qNK`HJQVcDj?nt3DhCzFk(EydwXh5;Z zK?W=mgiGN@C(;l(UqXlXB%#Vohj`EBq z^C?;4sf?ON*_la8R7zweiqX=%MvIKh2omh9GctTfkG)_3J58nuq@zUjxtr3bBdBqBZVkNQfLY&yhh0=G>TyuAtR|7nUh%f31wPT zuA!*_t%#JWo(ogpDTP)Tij&a?tc)U&Xy|j8+~qQdmZOS*QdlGo04hx>+-T}?GK=Kn z0Ve~v-(i|r&H})3Da2G1TBZOBn?gfU6eCKsB*-EFX%UV%Oulk~0U8ntMVCSo0|n6~ z6$zkZG8`+YEDcxzwm40T=F*}fr&7qGr3DH|j7CX70aS*gH98}pR6u>>Jx&u}o@QtU zsjE>Lc2p#l2b7F_DV7n?RFVJ`Mj4__cb#34jJj(=;Ygf`OOfV4q&46XVqssMB-g|w z5Q{`&LQ<~vxtn4_qrWch3pTd$VSja$=e<<*QJs=SKiRM6UK7X6l1Sqg1(3ZP77(%SIh4s-a*;;)`t6pDn@l2DQHXC^Q3P!=Y6x8yudv0<<)SWZoR-h&TP5BJWgiw1?F)wo4#(|Fm@T9 zpPabBK2B`oH=8fK)y8=bJ!pCAfu)gVxjtImPzQX8dT%`JT{POa@)U`$?@F)j$Yxy` zwYOVIZB!lUjy9gIX>O?owY-Pn>m;USXtnjXws~Ji;8TqhId14C>&n^xKc8ygP5*3N zPt0BM=$~)dX>BM+g^NiEDo@c0J=)}P=n_LEiBeK&QOl@2tH}!7w9|T;z;7$R0j|E& zdarp(Ymr`B4qvpARj^@~^$JT-^G|-EYja^INiKyWyR3UGQ&zlwSohA!4PIRZU)gQl zMu^*gI{;VRWgQ_Vd#>t%kv)jGr2XA1q3dqzK1;>k@BNEjItv~sK*-8_5YqP2W=Ypd zVYCn-pS=emOKv`NK=;msgDcUPNA@D-6zZu#xP0Fz^NRhiLHb_AT+p)32JQD*_Yn*? z;{<&6KI;x*TJej2f^CD=Q-EI!DU0^PvilJOFZ=!{u;>Bn zX;5_dH%}L!m#L|SPJ_)>=qgoSbiG8(e+Bdb+5L z${x@w#JsULzxl@86I~qq(Lw8aWAXP7!Y>Y@W!N6xeJi~EA+(I*${jOxFBn}$6+HSd zl7ej6wFrhELCDL0_q!*cYY0h^_vf9*AU%wvuu*$IhHnmAcTArA^y2|tb)J6|EH&l^ z`Z;*`kadrF%V+QCUPtb9whHd~DMHS9uJ|M9d=w$gzg&9)+8;wm@TqYK7X1wE*rhAx z@6$^c!ao{O2%rB9?bwWwclYaB1-y|!+TMN`?O4%2y`R&)3v$1@stR8JI6_t|xwRSE zoPLM8hQ!!aX^qMT2pyil{7)-WZl~^8rRLQlG}&{ zKd(6hS7Rg!>yCkWFp86>iRtls2uhYHW zb72gb5JF!5o!5K~|K z$IEr^;<5DYwIRmRU4aj8DI#O@dGrsB!k`9CR8Yr@GDo1KR7OHAm8Jqo(J~Sv>e{NY z<277F?jfp1RuORZWb!D1-%#a-6;n_k3(Ftd21}-*K)Cl#JPq@T$$gd?zifL+FKrz= zS05FV=%^mLzFXH?#?BSI1hI%SGtAI+DPk3bx3G|$hFHX`>m2aYX^2(k`tL^g?q!H& z%KRX#m&V}>_gSmptJ4unzWVLEbS(xq4O*ANt4a{7bkCvNpnV2nP5;ep+u$E&AQsiO z6@#zOM69A%@iF*JDPk@BqaB_15PWMGv4&ta~Wb4efN$PLw0iI zWCFZuZh@;A@@?Z!Zm>f?OCGfl_B~JN?H%y+43un*9J${zSuZ`Vx3$AJXQEGP06A#k zr|tYeU)TnpEk!BH^5lAg%l}{j+{vSTSwtTB6uJcR6aiW;bHWM{gK{4G^hm*4sNUU*~$iXrdi?Ki@G zE0IV`#cOu!?L9Dm8j|GT6-cC6|0(tBZQbyT(NA_?iJ0`u*S6>jyWq%WNTjVa>4Gn< MA{P?No{u#D4?#ajMgRZ+ From 1c2de35f28ed10ba4339e1d393f1f3d3959cd52b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 4 Sep 2014 17:54:51 -0400 Subject: [PATCH 4/6] Code review fixes --- endpoints/api/user.py | 1 - endpoints/callbacks.py | 43 +++++++++++++++++--------------------- templates/ologinerror.html | 2 +- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index d0d089dcd..fb09d012a 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -39,7 +39,6 @@ def user_view(user): organizations = model.get_user_organizations(user.username) def login_view(login): - print login.metadata_json try: metadata = json.loads(login.metadata_json) except: diff --git a/endpoints/callbacks.py b/endpoints/callbacks.py index 49fa1e8a6..1cbd46192 100644 --- a/endpoints/callbacks.py +++ b/endpoints/callbacks.py @@ -4,7 +4,7 @@ from flask import request, redirect, url_for, Blueprint from flask.ext.login import current_user from endpoints.common import render_page_template, common_login, route_show_if -from app import app, analytics +from app import app, analytics, get_app_url from data import model from util.names import parse_repository_name from util.validation import generate_valid_usernames @@ -22,6 +22,11 @@ client = app.config['HTTPCLIENT'] callback = Blueprint('callback', __name__) +def render_ologin_error(service_name, + error_message='Could not load user data. The token may have expired.'): + return render_page_template('ologinerror.html', service_name=service_name, + error_message=error_message, + service_url=get_app_url()) def exchange_code_for_token(code, service_name='GITHUB', for_login=True, form_encode=False, redirect_suffix=''): @@ -96,15 +101,12 @@ def conduct_oauth_login(service_name, user_id, username, email, metadata={}): analytics.alias(to_login.username, state) except model.DataModelException, ex: - return render_page_template('ologinerror.html', service_name=service_name, - error_message=ex.message) + return render_ologin_error(service_name, ex.message) if common_login(to_login): return redirect(url_for('web.index')) - return render_page_template('ologinerror.html', service_name=service_name, - error_message='Unknown error') - + return render_ologin_error(service_name) def get_google_username(user_data): username = user_data['email'] @@ -120,17 +122,16 @@ def get_google_username(user_data): def google_oauth_callback(): error = request.args.get('error', None) if error: - return render_page_template('ologinerror.html', service_name='Google', error_message=error) + return render_ologin_error('Google', error) token = exchange_code_for_token(request.args.get('code'), service_name='GOOGLE', form_encode=True) user_data = get_google_user(token) if not user_data or not user_data.get('id', None) or not user_data.get('email', None): - return render_page_template('ologinerror.html', service_name = 'Google', - error_message='Could not load user data') - + return render_ologin_error('Google') + username = get_google_username(user_data) metadata = { - 'service_username': username + 'service_username': user_data['email'] } return conduct_oauth_login('Google', user_data['id'], username, user_data['email'], @@ -142,13 +143,12 @@ def google_oauth_callback(): def github_oauth_callback(): error = request.args.get('error', None) if error: - return render_page_template('ologinerror.html', service_name = 'GitHub', error_message=error) + return render_ologin_error('GitHub', error) token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB') user_data = get_github_user(token) if not user_data: - return render_page_template('ologinerror.html', service_name = 'GitHub', - error_message='Could not load user data') + return render_ologin_error('GitHub') username = user_data['login'] github_id = user_data['id'] @@ -186,15 +186,14 @@ def google_oauth_attach(): user_data = get_google_user(token) if not user_data or not user_data.get('id', None): - return render_page_template('ologinerror.html', service_name = 'Google', - error_message='Could not load user data') + return render_ologin_error('Google') google_id = user_data['id'] user_obj = current_user.db_user() username = get_google_username(user_data) metadata = { - 'service_username': username + 'service_username': user_data['email'] } try: @@ -202,9 +201,7 @@ def google_oauth_attach(): except IntegrityError: err = 'Google account %s is already attached to a %s account' % ( username, app.config['REGISTRY_TITLE_SHORT']) - - return render_page_template('ologinerror.html', service_name = 'Google', - error_message=err) + return render_ologin_error('Google', err) return redirect(url_for('web.user')) @@ -216,8 +213,7 @@ def github_oauth_attach(): token = exchange_code_for_token(request.args.get('code'), service_name='GITHUB') user_data = get_github_user(token) if not user_data: - return render_page_template('ologinerror.html', service_name = 'GitHub', - error_message='Could not load user data') + return render_ologin_error('GitHub') github_id = user_data['id'] user_obj = current_user.db_user() @@ -233,8 +229,7 @@ def github_oauth_attach(): err = 'Github account %s is already attached to a %s account' % ( username, app.config['REGISTRY_TITLE_SHORT']) - return render_page_template('ologinerror.html', service_name = 'Github', - error_message=err) + return render_ologin_error('GitHub', err) return redirect(url_for('web.user')) diff --git a/templates/ologinerror.html b/templates/ologinerror.html index cd921eec2..304b7f554 100644 --- a/templates/ologinerror.html +++ b/templates/ologinerror.html @@ -15,7 +15,7 @@ {% endif %}
    - Please register using the registration form to continue. + Please register using the registration form to continue. You will be able to connect your account to your Quay.io account in the user settings.
    From b9a4d2835f89da831abf3ce35ce79808a2cd79b4 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 4 Sep 2014 18:18:19 -0400 Subject: [PATCH 5/6] Add migration for the new DB field --- ...a_add_metadata_field_to_external_logins.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py diff --git a/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py b/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py new file mode 100644 index 000000000..c642dcbee --- /dev/null +++ b/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py @@ -0,0 +1,26 @@ +"""add metadata field to external logins + +Revision ID: 1594a74a74ca +Revises: f42b0ea7a4d +Create Date: 2014-09-04 18:17:35.205698 + +""" + +# revision identifiers, used by Alembic. +revision = '1594a74a74ca' +down_revision = 'f42b0ea7a4d' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('federatedlogin', sa.Column('metadata_json', sa.Text(), nullable=False)) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('federatedlogin', 'metadata_json') + ### end Alembic commands ### From 6fa5a365b3f664346d150fd18a7fd0255194034b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 4 Sep 2014 18:45:23 -0400 Subject: [PATCH 6/6] Add loginservice for Google --- ...4ca_add_metadata_field_to_external_logins.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py b/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py index c642dcbee..a59116c7f 100644 --- a/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py +++ b/data/migrations/versions/1594a74a74ca_add_metadata_field_to_external_logins.py @@ -12,6 +12,9 @@ down_revision = 'f42b0ea7a4d' from alembic import op import sqlalchemy as sa +from sqlalchemy.dialects import mysql +from data.model.sqlalchemybridge import gen_sqlalchemy_metadata +from data.database import all_models def upgrade(): @@ -19,8 +22,22 @@ def upgrade(): op.add_column('federatedlogin', sa.Column('metadata_json', sa.Text(), nullable=False)) ### end Alembic commands ### + schema = gen_sqlalchemy_metadata(all_models) + + op.bulk_insert(schema.tables['loginservice'], + [ + {'id':4, 'name':'google'}, + ]) def downgrade(): ### commands auto generated by Alembic - please adjust! ### op.drop_column('federatedlogin', 'metadata_json') ### end Alembic commands ### + + schema = gen_sqlalchemy_metadata(all_models) + loginservice = schema.table['loginservice'] + + op.execute( + (loginservice.delete() + .where(loginservice.c.name == op.inline_literal('google'))) + )