From 389c88a7c4b2b08b879b772a0deca139e004ef3e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 11 Aug 2014 18:25:01 -0400 Subject: [PATCH] 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:
-
+ -
+
+ + Account attached to Github Account +
+
@@ -189,8 +193,13 @@
Google Login:
-
- Account tied to your Google account. +
+ + {{ googleLogin }} +
+
+ + Account attached to Google Account