From 9fa7eab5f66fb47f6e817574c13a0767246dbb03 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 09:55:41 +0000 Subject: [PATCH 1/26] Replace openid by username in the model With the move from openid to openid-connect, there are no more openid url that we can rely on. Apparently, there is also nothing else than the username we can use that would uniquely identify the user while being stable over multiple hosts. --- diff --git a/hubs/models.py b/hubs/models.py index 681048b..ad3bc16 100755 --- a/hubs/models.py +++ b/hubs/models.py @@ -45,13 +45,13 @@ from hubs.utils import username2avatar class HubsBase(object): - def notify(self, openid, changed): + def notify(self, username, changed): obj = type(self).__name__.lower() topic = obj + ".update" fedmsg.publish( topic=topic, msg=dict( - openid=openid, + username=username, changed=changed, ) ) @@ -103,7 +103,7 @@ class Association(BASE): sa.ForeignKey('hubs.name'), primary_key=True) user_id = sa.Column(sa.Text, - sa.ForeignKey('users.openid'), + sa.ForeignKey('users.username'), primary_key=True) role = sa.Column(sa.Enum(*roles), primary_key=True) @@ -350,14 +350,13 @@ class Widget(BASE): class User(BASE): __tablename__ = 'users' - openid = sa.Column(sa.Text, primary_key=True) + username = sa.Column(sa.Text, primary_key=True) fullname = sa.Column(sa.Text) created_on = sa.Column(sa.DateTime, default=datetime.datetime.utcnow) def __json__(self, session): return { 'username': self.username, - 'openid': self.openid, 'avatar': username2avatar(self.username), 'fullname': self.fullname, 'created_on': self.created_on, @@ -396,32 +395,24 @@ class User(BASE): and assoc.hub.name != self.username ])), key=operator.attrgetter('name')) - @property - def username(self): - return self.openid.split('.')[0] - @classmethod def by_username(cls, session, username): - return cls.by_openid(session, "%s.id.fedoraproject.org" % username) - - @classmethod - def by_openid(cls, session, openid): - return session.query(cls).filter_by(openid=openid).first() + return session.query(cls).filter_by(username=username).first() - get = by_openid + get = by_username @classmethod def all(cls, session): return session.query(cls).all() @classmethod - def get_or_create(cls, session, openid, fullname): - if not openid: - raise ValueError("Must provide openid, not %r" % openid) + def get_or_create(cls, session, username, fullname): + if not username: + raise ValueError("Must provide an username, not %r" % username) - self = cls.by_openid(session, openid) + self = cls.by_username(session, username) if not self: - self = cls(openid=openid, fullname=fullname) + self = cls(username=username, fullname=fullname) session.add(self) if not Hub.by_name(session, self.username): Hub.create_user_hub(session, self.username, self.fullname) From 3060feaa92bd94f4735420cefab1d286e15485bf Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:02:40 +0000 Subject: [PATCH 2/26] Start porting hubs to OpenID-Connect from OpenID --- diff --git a/hubs/app.py b/hubs/app.py index 1f30e15..81c77d6 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -12,7 +12,7 @@ import munch import pygments.formatters import six -from flask.ext.openid import OpenID +from flask.ext.oidc import OpenIDConnect import fmn.lib import hubs.models @@ -447,15 +447,12 @@ def widget_source(name): # Set up OpenID in stateless mode -oid = OpenID(app, - safe_roots=[], - store_factory=lambda: None, - url_root_as_trust_root=True) +FAS = OpenIDConnect(app, credentials_store=flask.session ) @app.route('/login/', methods=('GET', 'POST')) @app.route('/login', methods=('GET', 'POST')) -@oid.loginhandler +@FAS.require_login def login(): default = flask.url_for('index') next_url = flask.request.args.get('next', default) @@ -530,21 +527,29 @@ def login_required(function): @app.before_request def check_auth(): + flask.session.permanent = True flask.g.fedmsg_config = fedmsg_config - flask.g.auth = munch.Munch(logged_in=False) - if 'openid' in flask.session: - openid = flask.session.get('openid') - if isinstance(openid, six.binary_type): - openid = openid.decode('utf-8') - openid = openid.strip('/').split('/')[-1] - flask.g.auth.logged_in = True - flask.g.auth.openid = openid - flask.g.auth.user = hubs.models.User.by_openid(session, openid) - flask.g.auth.openid_url = flask.session.get('openid') - flask.g.auth.fullname = flask.session.get('fullname', None) - flask.g.auth.nickname = flask.session.get('nickname', None) - flask.g.auth.email = flask.session.get('email', None) - flask.g.auth.avatar = username2avatar(flask.g.auth.nickname) + + if FAS.user_loggedin: + if not hasattr(flask.session, 'fas_user') or not flask.session.fas_user: + flask.session.auth = munch.Munch( + fullname=FAS.user_getfield('name'), + nickname=FAS.user_getfield('nickname'), + email=FAS.user_getfield('email'), + timezone=FAS.user_getfield('zoneinfo'), + cla_done=\ + 'http://admin.fedoraproject.org/accounts/cla/done' \ + in FAS.user_getfield('cla'), + groups=FAS.user_getfield('groups'), + logged_in=True, + ) + flask.session.auth.avatar=username2avatar( + flask.session.auth.nickname) + flask.session.auth.user = hubs.models.User.by_username( + session, flask.session.auth.nickname) + flask.g.auth = flask.session.auth + else: + flask.g.auth = munch.Munch(logged_in=False) def get_hub(session, name): diff --git a/hubs/default_config.py b/hubs/default_config.py index a21f138..b0d3eb7 100755 --- a/hubs/default_config.py +++ b/hubs/default_config.py @@ -1,3 +1,5 @@ +import os + SECRET_KEY = 'changemeforreal' PROMOTED_GROUPS = [ @@ -6,3 +8,11 @@ PROMOTED_GROUPS = [ ] HUB_OF_THE_MONTH = 'commops' + + +OIDC_CLIENT_SECRETS = os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..', 'client_secrets.json') +OIDC_ID_TOKEN_COOKIE_SECURE = False +OIDC_REQUIRE_VERIFIED_EMAIL = False +OIDC_OPENID_REALM = 'http://localhost:5000/oidc_callback' +OIDC_SCOPES = ['openid', 'email', 'profile', 'fedora'] From 80cb43833a8e9dd4bf88e70afb4337d70451f6c5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:04:07 +0000 Subject: [PATCH 3/26] Add a method to check if a given URL is safe to return to --- diff --git a/hubs/app.py b/hubs/app.py index 81c77d6..98c86a1 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -3,6 +3,7 @@ import functools import json import logging import os +import urlparse import uuid import flask @@ -75,6 +76,17 @@ def authenticated(): and flask.g.auth.logged_in +def is_safe_url(target): + """ Checks that the target url is safe and sending to the current + website not some other malicious one. + """ + ref_url = urlparse.urlparse(flask.request.host_url) + test_url = urlparse.urlparse( + urlparse.urljoin(flask.request.host_url, target)) + return test_url.scheme in ('http', 'https') and \ + ref_url.netloc == test_url.netloc + + @app.template_filter('commas') def commas(numeric): return "{:,.2f}".format(numeric) From 82b6b815f3d27d7b9da9abbd97cb05ccd13d048b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:04:51 +0000 Subject: [PATCH 4/26] Adjust the login and logout pages for OIDC --- diff --git a/hubs/app.py b/hubs/app.py index 98c86a1..74c751a 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -101,7 +101,7 @@ def shutdown_session(exception=None): @app.route('/') def index(): if not authenticated(): - return flask.redirect(flask.url_for('login_fedora')) + return flask.redirect(flask.url_for('login')) return flask.redirect(flask.url_for('hub', name=flask.g.auth.nickname)) @@ -468,56 +468,33 @@ FAS = OpenIDConnect(app, credentials_store=flask.session ) def login(): default = flask.url_for('index') next_url = flask.request.args.get('next', default) - if authenticated(): - return flask.redirect(next_url) - - openid_server = flask.request.form.get('openid', None) - if openid_server: - return oid.try_login( - openid_server, ask_for=['email', 'fullname', 'nickname'], - ask_for_optional=[]) - - return flask.render_template( - 'login.html', next=oid.get_next_url(), error=oid.fetch_error()) + if is_safe_url(next_url): + return_point = next_url + else: + return_point = default + hubs.models.User.get_or_create( + session, username=flask.g.auth.user, fullname=flask.g.auth.fullname) -@app.route('/login/fedora/') -@app.route('/login/fedora') -@oid.loginhandler -def login_fedora(): - # default = flask.url_for('profile_redirect') - # next_url = flask.request.args.get('next', default) - return oid.try_login( - 'https://id.fedoraproject.org', - ask_for=['email', 'fullname', 'nickname'], - ask_for_optional=[]) + return flask.redirect(return_point) @app.route('/logout/') @app.route('/logout') def logout(): - if 'openid' in flask.app.session: - flask.app.session.pop('openid') - return flask.redirect(flask.url_for('index')) + next_url = flask.url_for('index') + if 'next' in flask.request.values: # pragma: no cover + if is_safe_url(flask.request.args['next']): + next_url = flask.request.values['next'] + if next_url == flask.url_for('auth_login'): # pragma: no cover + next_url = flask.url_for('index') -@oid.after_login -def after_openid_login(resp): - default = flask.url_for('index') - if not resp.identity_url: - return flask.redirect(default) - - openid_url = resp.identity_url - flask.app.session['openid'] = openid_url - flask.app.session['fullname'] = resp.fullname - flask.app.session['nickname'] = resp.nickname or resp.fullname - flask.app.session['email'] = resp.email - - openid = openid_url.strip('/').split('/')[-1] - hubs.models.User.get_or_create( - session, openid=openid, fullname=resp.fullname) + if authenticated(): + FAS.logout() + flask.session.auth = None + flask.flash(gettext('You have been logged out')) - next_url = flask.request.args.get('next', default) return flask.redirect(next_url) diff --git a/hubs/templates/master.html b/hubs/templates/master.html index 899847c..6fce42f 100644 --- a/hubs/templates/master.html +++ b/hubs/templates/master.html @@ -37,7 +37,7 @@ IRC Chats {% else %} - Not logged in. Click to login. + Not logged in. Click to login. {% endif %}
From 2a0657f6a444f2a102493da71d833af5b4681d21 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:17:23 +0000 Subject: [PATCH 5/26] Fix typos in the app controller --- diff --git a/hubs/app.py b/hubs/app.py index 74c751a..9f731b3 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -474,7 +474,7 @@ def login(): return_point = default hubs.models.User.get_or_create( - session, username=flask.g.auth.user, fullname=flask.g.auth.fullname) + session, username=flask.g.auth.nickname, fullname=flask.g.auth.fullname) return flask.redirect(return_point) @@ -487,13 +487,13 @@ def logout(): if is_safe_url(flask.request.args['next']): next_url = flask.request.values['next'] - if next_url == flask.url_for('auth_login'): # pragma: no cover + if next_url == flask.url_for('login'): # pragma: no cover next_url = flask.url_for('index') if authenticated(): FAS.logout() flask.session.auth = None - flask.flash(gettext('You have been logged out')) + flask.flash('You have been logged out') return flask.redirect(next_url) From 1c90812ea4c4157b034f34f14d6433a3b0de8d79 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:18:57 +0000 Subject: [PATCH 6/26] Retrieve the user by its username since there are no more openid url to use --- diff --git a/hubs/app.py b/hubs/app.py index 9f731b3..bddaa29 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -458,7 +458,7 @@ def widget_source(name): return flask.redirect(SOURCE_URL + fname) -# Set up OpenID in stateless mode +# Set up OpenIDConnect FAS = OpenIDConnect(app, credentials_store=flask.session ) @@ -575,7 +575,7 @@ def get_widget(session, hub, idx): @login_required def hub_subscribe(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) hub.subscribe(session, user) session.commit() return flask.redirect(flask.url_for('hub', name=hub.name)) @@ -585,7 +585,7 @@ def hub_subscribe(hub): @login_required def hub_unsubscribe(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) try: hub.unsubscribe(session, user) except KeyError: @@ -598,7 +598,7 @@ def hub_unsubscribe(hub): @login_required def hub_star(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) hub.subscribe(session, user, role='stargazer') session.commit() return flask.redirect(flask.url_for('hub', name=hub.name)) @@ -608,7 +608,7 @@ def hub_star(hub): @login_required def hub_unstar(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) try: hub.unsubscribe(session, user, role='stargazer') except KeyError: @@ -621,7 +621,7 @@ def hub_unstar(hub): @login_required def hub_join(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) hub.subscribe(session, user, role='member') session.commit() return flask.redirect(flask.url_for('hub', name=hub.name)) @@ -631,7 +631,7 @@ def hub_join(hub): @login_required def hub_leave(hub): hub = get_hub(session, hub) - user = hubs.models.User.by_openid(session, flask.g.auth.openid) + user = hubs.models.User.by_username(session, flask.g.auth.nickname) try: hub.unsubscribe(session, user, role='member') except KeyError: From baf268ad647f9cfdf304a6c6b648d1038d38ba44 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 10:27:57 +0000 Subject: [PATCH 7/26] Adjust the username validator since we no longer use username --- diff --git a/hubs/validators.py b/hubs/validators.py index f6da8dc..c76c9f2 100755 --- a/hubs/validators.py +++ b/hubs/validators.py @@ -21,8 +21,7 @@ def link(session, value): def username(session, value): - openid = 'http://%s.id.fedoraproject.org/' % value - return not hubs.models.User.by_openid(session, openid) is None + return not hubs.models.User.by_username(session, value) is None def github_organization(session, value): From 4c24033c3ca53104a7284dd1caec4991e256dbcc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:05:32 +0000 Subject: [PATCH 8/26] Fix calls to login_fedora --- diff --git a/hubs/app.py b/hubs/app.py index bddaa29..136c844 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -109,7 +109,7 @@ def index(): @app.route('/groups') def groups(): if not authenticated(): - return flask.redirect(flask.url_for('login_fedora')) + return flask.redirect(flask.url_for('login')) # Get the list of promoted and non-promoted group hubs from the DB promoted_names = app.config.get('PROMOTED_GROUPS') @@ -507,7 +507,7 @@ def login_required(function): if not authenticated(): flask.flash('Login required', 'errors') return flask.redirect(flask.url_for( - 'login_fedora', next=flask.request.url)) + 'login', next=flask.request.url)) return function(*args, **kwargs) From d907adba437fdc96a16dc7b10380d94581a9ab96 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:06:59 +0000 Subject: [PATCH 9/26] Adjust the populate script to not use .id.fp.o This for the change to openid-connect where we no longer use the openid url --- diff --git a/populate-from-fas.py b/populate-from-fas.py index 8c3531b..cab611e 100755 --- a/populate-from-fas.py +++ b/populate-from-fas.py @@ -38,19 +38,19 @@ for letter in reversed(sorted(list(set(string.letters.lower())))): session = hubs.models.init( fedmsg_config['hubs.sqlalchemy.uri'], True, True) print "Querying FAS for the %r users.. hang on." % letter - request = fasclient.send_request('/user/list', - req_params={'search': '%s*' % letter}, - auth=True, - timeout=500) + request = fasclient.send_request( + '/user/list', + req_params={'search': '%s*' % letter, 'status': 'active'}, + auth=True, + timeout=500) users = request['people'] for user in users: username = user['username'] fullname = user['human_name'] - openid = '%s.id.fedoraproject.org' % username print "Creating account for %r" % openid hubs_user = hubs.models.User.get_or_create( - session, openid=openid, fullname=fullname) + session, username=username, fullname=fullname) session.commit() diff --git a/populate.py b/populate.py index 9762426..8487aa9 100755 --- a/populate.py +++ b/populate.py @@ -15,10 +15,9 @@ users = ['mrichard', 'duffy', 'ryanlerch', 'gnokii', 'nask0', 'pravins', 'keekri', 'linuxmodder', 'bee2502', 'jflory7'] for username in users: fullname = 'Full Name Goes Here' - openid = '%s.id.fedoraproject.org' % username - print("Creating account for %r" % openid) + print("Creating account for %r" % username) hubs.models.User.get_or_create( - session, openid=openid, fullname=fullname) + session, username=username, fullname=fullname) session.commit() @@ -59,9 +58,9 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'pravins.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph.id.fedoraproject.org'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_openid(session, 'pravins'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') session.commit() @@ -102,11 +101,11 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'jflory7.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'bee2502.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'keekri.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'linuxmodder.id.fedoraproject.org'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'jflory7'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'bee2502'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'keekri'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'linuxmodder'), 'member') session.commit() @@ -147,12 +146,12 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'croberts.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mattdm.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph.id.fedoraproject.org'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_openid(session, 'croberts'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'mattdm'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') session.commit() @@ -193,13 +192,13 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'duffy.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'gnokii.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph.id.fedoraproject.org'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_openid(session, 'duffy'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'gnokii'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') session.commit() @@ -239,10 +238,10 @@ widget = hubs.models.Widget(plugin='dummy', index=3, left=True) hub.widgets.append(widget) -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'abompard.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'lmacken.id.fedoraproject.org'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0.id.fedoraproject.org'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause.id.fedoraproject.org'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'abompard'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'lmacken'), 'owner') +hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0'), 'member') +hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'subscriber') session.commit() From e7f108b958bd1a01e495aa7c8a592a29a0c42b53 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:11:57 +0000 Subject: [PATCH 10/26] Fix tests for /login/fedora which we dropped --- diff --git a/hubs/tests/test_api/test_hub.py b/hubs/tests/test_api/test_hub.py index 785aef3..2fd513f 100644 --- a/hubs/tests/test_api/test_hub.py +++ b/hubs/tests/test_api/test_hub.py @@ -18,7 +18,7 @@ class TestHubSubscribe(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/subscribe'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_subscribe_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') @@ -39,7 +39,7 @@ class TestHubUnsubscribe(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/unsubscribe'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_unsubscribe_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') @@ -65,7 +65,7 @@ class TestHubStar(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/star'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_star_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') @@ -86,7 +86,7 @@ class TestHubUnstar(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/unstar'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_unstar_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') @@ -113,7 +113,7 @@ class TestHubJoin(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/join'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_join_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') @@ -134,7 +134,7 @@ class TestHubLeave(hubs.tests.APPTest): resp = self.app.post('/api/hub/{}/leave'.format(hub.name), follow_redirects=False) self.assertEqual(resp.status_code, 302) - self.assertEqual(urlparse(resp.location).path, '/login/fedora') + self.assertEqual(urlparse(resp.location).path, '/login') def test_star_when_logged_in(self): hub = hubs.models.Hub.by_name(self.session, 'infra') diff --git a/hubs/tests/test_fedora_hubs_flask_api.py b/hubs/tests/test_fedora_hubs_flask_api.py index d9d2325..7500199 100644 --- a/hubs/tests/test_fedora_hubs_flask_api.py +++ b/hubs/tests/test_fedora_hubs_flask_api.py @@ -21,7 +21,7 @@ class HubsAPITest(hubs.tests.APPTest): def test_index_logged_out(self): result = self.app.get('/', follow_redirects=False) self.assertEqual(result.status_code, 302) - self.assertEqual(urlparse(result.location).path, "/login/fedora") + self.assertEqual(urlparse(result.location).path, "/login") def test_index_logged_in(self): user = tests.FakeAuthorization('ralph') @@ -31,7 +31,7 @@ class HubsAPITest(hubs.tests.APPTest): # assert the status code of the response self.assertEqual(result.status_code, 200) self.assertFalse('Not logged in. Click to login' in result.data) + '/login">login' in result.data) def test_hub_logged_out(self): with app.test_request_context('/ralph'): @@ -42,7 +42,7 @@ class HubsAPITest(hubs.tests.APPTest): # assert the status code of the response self.assertEqual(result.status_code, 200) str_expected = 'Not logged in. Click to ' \ - 'login' + 'login' self.assertTrue(str_expected in result.data) def test_groups_logged_out(self): @@ -50,7 +50,7 @@ class HubsAPITest(hubs.tests.APPTest): # assert the status code of the response self.assertEqual(result.status_code, 302) # this will redirect to fedora.login - self.assertEqual(urlparse(result.location).path, "/login/fedora") + self.assertEqual(urlparse(result.location).path, "/login") def test_groups_logged_in(self): user = tests.FakeAuthorization('ralph') @@ -66,7 +66,7 @@ class HubsAPITest(hubs.tests.APPTest): result = self.app.get('/ralph', follow_redirects=True) self.assertEqual(result.status_code, 200) self.assertFalse('Not logged in. Click to login' in result.data) + '/login">login' in result.data) def test_hub_json(self): result = self.app.get('/ralph/json', follow_redirects=True) @@ -134,7 +134,7 @@ class HubsAPITest(hubs.tests.APPTest): follow_redirects=True) self.assertEqual(result.status_code, 200) self.assertFalse('Not logged in. Click to login' in result.data) + '/login">login' in result.data) self.assertTrue('Full Name: ' 'fullname: ralph' in result.data) @@ -204,7 +204,9 @@ class HubsAPITest(hubs.tests.APPTest): with tests.auth_set(app, user): result = self.app.get('/login', follow_redirects=False) self.assertEqual(result.status_code, 302) - self.assertEqual(urlparse(result.location).path, "/") + self.assertEqual( + urlparse(result.location).path, + "/openidc/Authorization") def test_hub_add_widget_get_no_args(self): result = self.app.get('/ralph/add', follow_redirects=False) From d3f277a029da435d865a2350fab7d044b712ef47 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:13:53 +0000 Subject: [PATCH 11/26] Adjust the tests to drop the openid url in favor of the user's nickname --- diff --git a/hubs/tests/__init__.py b/hubs/tests/__init__.py index 5c10118..f9faa42 100644 --- a/hubs/tests/__init__.py +++ b/hubs/tests/__init__.py @@ -42,10 +42,9 @@ class APPTest(unittest.TestCase): def populate(self): for user in ['devyani7', 'dhrish', 'shalini', 'ralph', 'decause']: - openid = '%s.id.fedoraproject.org' % user fullname = user.title() hubs.models.User.get_or_create( - hubs.app.session, openid=openid, fullname=fullname) + hubs.app.session, username=user, fullname=fullname) hubs.app.session.flush() @@ -122,7 +121,6 @@ class FakeUser(object): supposed to be. """ self.username = username - self.openid = username + '.id.fedoraproject.org' self.booksmarks = [] def __getitem__(self, key): @@ -140,7 +138,6 @@ class FakeAuthorization(object): self.logged_in = True self.fullname = 'fullname: ' + username self.email = 'email: ' + username - self.openid = username + '.id.fedoraproject.org' self.user = FakeUser(username) self.avatar = 'avatar_src_url' self.nickname = username diff --git a/hubs/tests/test_api/test_fedmsg.py b/hubs/tests/test_api/test_fedmsg.py index d6e67ef..332d4d1 100644 --- a/hubs/tests/test_api/test_fedmsg.py +++ b/hubs/tests/test_api/test_fedmsg.py @@ -54,7 +54,6 @@ class TestFeed(hubs.tests.APPTest): 'plugin': 'feed' } with self.app.session_transaction() as sess: - sess['openid'] = 'atelic@fedoraproject.org' sess['nickname'] = 'atelic' with hubs.tests.auth_set(app, self.user): @@ -71,7 +70,6 @@ class TestFeed(hubs.tests.APPTest): 'plugin': 'feed' } with self.app.session_transaction() as sess: - sess['openid'] = 'atelic@fedoraproject.org' sess['nickname'] = 'atelic' with hubs.tests.auth_set(app, self.user): diff --git a/hubs/tests/test_models.py b/hubs/tests/test_models.py index c836a51..47a6eca 100644 --- a/hubs/tests/test_models.py +++ b/hubs/tests/test_models.py @@ -12,8 +12,7 @@ class ModelTest(hubs.tests.APPTest): # verify user exists username = 'ralph' - openid = '%s.id.fedoraproject.org' % username - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) self.assertIsNotNone(user) # check if association exists @@ -23,7 +22,7 @@ class ModelTest(hubs.tests.APPTest): # delete the user self.session.delete(user) - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) self.assertIsNone(user) # checking to see if the hub is still intact @@ -45,8 +44,7 @@ class ModelTest(hubs.tests.APPTest): # check if association exists username = 'ralph' - openid = '%s.id.fedoraproject.org' % username - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) assoc = hubs.models.Association.get(self.session, hub, user, 'owner') self.assertIsNotNone(assoc) @@ -70,18 +68,17 @@ class ModelTest(hubs.tests.APPTest): self.assertIsNone(hub) # check if user is still intact - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) self.assertIsNotNone(user) - self.assertIn('ralph', user.openid) + self.assertEqual('ralph', user.username) def test_delete_user_then_hubs(self): self.session = hubs.models.init(fedmsg_config['hubs.sqlalchemy.uri']) username = 'ralph' - openid = '%s.id.fedoraproject.org' % username - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) self.assertIsNotNone(user) self.session.delete(user) - user = hubs.models.User.get(self.session, openid) + user = hubs.models.User.get(self.session, username) self.assertIsNone(user) # checking to see if the hub is still intact diff --git a/hubs/tests/test_widgets/__init__.py b/hubs/tests/test_widgets/__init__.py index 38d77af..b0b7d64 100644 --- a/hubs/tests/test_widgets/__init__.py +++ b/hubs/tests/test_widgets/__init__.py @@ -36,10 +36,9 @@ class WidgetTest(unittest.TestCase): def populate(self): for user in ['devyani7', 'dhrish', 'shalini', 'ralph', 'decause']: - openid = '%s.id.fedoraproject.org' % user fullname = user.title() hubs.models.User.get_or_create( - hubs.app.session, openid=openid, fullname=fullname) + hubs.app.session, username=user, fullname=fullname) hubs.app.session.flush() From 820f95b928e4c36604f6e249f0b437892cddafd6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:14:24 +0000 Subject: [PATCH 12/26] Set a g.oidc_id_token to None by default to cover 'bug' in flask-oidc --- diff --git a/hubs/tests/__init__.py b/hubs/tests/__init__.py index f9faa42..153374a 100644 --- a/hubs/tests/__init__.py +++ b/hubs/tests/__init__.py @@ -105,6 +105,7 @@ def auth_set(APP, auth): def handler(sender, **kwargs): g.auth = auth + g.oidc_id_token = None if not auth: g.auth = munch.Munch(logged_in=False) diff --git a/hubs/tests/test_fedora_hubs_flask_api.py b/hubs/tests/test_fedora_hubs_flask_api.py index 7500199..254b40e 100644 --- a/hubs/tests/test_fedora_hubs_flask_api.py +++ b/hubs/tests/test_fedora_hubs_flask_api.py @@ -35,6 +35,8 @@ class HubsAPITest(hubs.tests.APPTest): def test_hub_logged_out(self): with app.test_request_context('/ralph'): + import flask + flask.g.oidc_id_token = None # need to manually call the @app.before_request # since unittest don't call it hubs.app.check_auth() From bdf32b9cf9fcacffaddf2e21c7873c8eee670dd6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:14:49 +0000 Subject: [PATCH 13/26] Use the test db instead of relying on the one from the populate script --- diff --git a/hubs/tests/test_models.py b/hubs/tests/test_models.py index 47a6eca..e787c68 100644 --- a/hubs/tests/test_models.py +++ b/hubs/tests/test_models.py @@ -8,8 +8,6 @@ fedmsg_config = fedmsg.config.load_config() class ModelTest(hubs.tests.APPTest): def test_delete_user(self): - self.session = hubs.models.init(fedmsg_config['hubs.sqlalchemy.uri']) - # verify user exists username = 'ralph' user = hubs.models.User.get(self.session, username) @@ -32,11 +30,9 @@ class ModelTest(hubs.tests.APPTest): # check if widgets still are intact widgets = hubs.models.Widget.by_hub_id_all(self.session, hub.name) - self.assertEqual(8, len(widgets)) + self.assertEqual(11, len(widgets)) def test_delete_hubs(self): - self.session = hubs.models.init(fedmsg_config['hubs.sqlalchemy.uri']) - # verify the hub exists hub_name = 'ralph' hub = hubs.models.Hub.get(self.session, hub_name) @@ -50,7 +46,7 @@ class ModelTest(hubs.tests.APPTest): # check if widgets exist widgets = hubs.models.Widget.by_hub_id_all(self.session, hub.name) - self.assertEqual(8, len(widgets)) + self.assertEqual(11, len(widgets)) # delete the hub self.session.delete(hub) @@ -73,7 +69,6 @@ class ModelTest(hubs.tests.APPTest): self.assertEqual('ralph', user.username) def test_delete_user_then_hubs(self): - self.session = hubs.models.init(fedmsg_config['hubs.sqlalchemy.uri']) username = 'ralph' user = hubs.models.User.get(self.session, username) self.assertIsNotNone(user) From 2e2f31687642c1d2cdfb86e82aad984e29140b7b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:24:39 +0000 Subject: [PATCH 14/26] Replace the dependency on flask-openid by the one of flask-oidc --- diff --git a/requirements.txt b/requirements.txt index c3666e6..883cb24 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ dogpile.cache fedmsg fedmsg_meta_fedora_infrastructure flask -flask-openid +flask-oidc fmn.lib fmn.rules gunicorn From 335c2c90f58bc8f1014a172cf27594f387992a3e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:24:57 +0000 Subject: [PATCH 15/26] Document how to registed the oidc server in the README --- diff --git a/README.rst b/README.rst index 24d78eb..b8c0aa7 100644 --- a/README.rst +++ b/README.rst @@ -47,6 +47,10 @@ OK -- with that done, now install the dependencies from PyPI:: $ pip install -r requirements.txt +Configure the project to authentify against iddev.fedorainfraclouid.org:: + + oidc-register --debug https://iddev.fedorainfracloud.org/ http://localhost:5000 + With that, try running the app with:: $ python populate.py # To create the db From 77cdfe3fead47efab5993d4756bf77f8cdf37322 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:25:13 +0000 Subject: [PATCH 16/26] Change the host to localhost since 127.0.0.1 won't work with oidc --- diff --git a/runserver.py b/runserver.py index 54e1c5f..e85a2b4 100755 --- a/runserver.py +++ b/runserver.py @@ -10,9 +10,9 @@ parser.add_argument( '--port', '-p', default=5000, help='Port for Hubs to run on.') parser.add_argument( - '--host', default="127.0.0.1", + '--host', default="localhost", help='Hostname to listen on. When set to 0.0.0.0 the server is available \ - externally. Defaults to 127.0.0.1 making the it only visible on \ + externally. Defaults to localhost making the it only visible on \ localhost') args = parser.parse_args() From f82873bf01fbc22c54f5015477b72935428ec4cc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:27:17 +0000 Subject: [PATCH 17/26] Adjust default value regarding HTTPS for cookie --- diff --git a/hubs/default_config.py b/hubs/default_config.py index b0d3eb7..0ce12f8 100755 --- a/hubs/default_config.py +++ b/hubs/default_config.py @@ -12,7 +12,10 @@ HUB_OF_THE_MONTH = 'commops' OIDC_CLIENT_SECRETS = os.path.join(os.path.dirname( os.path.abspath(__file__)), '..', 'client_secrets.json') -OIDC_ID_TOKEN_COOKIE_SECURE = False +# This settings means that the application needs to be run behind http*s* for +# the cookie to be saved. For development you will likely need to make it +# `False` +OIDC_ID_TOKEN_COOKIE_SECURE = True OIDC_REQUIRE_VERIFIED_EMAIL = False OIDC_OPENID_REALM = 'http://localhost:5000/oidc_callback' OIDC_SCOPES = ['openid', 'email', 'profile', 'fedora'] From 05d5cd3283ad535b613042911ec6a3ff51cdc181 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:28:17 +0000 Subject: [PATCH 18/26] Replace calls to by_openid to calls to by_username thanks @puiterwijk --- diff --git a/populate.py b/populate.py index 8487aa9..a9cbe39 100755 --- a/populate.py +++ b/populate.py @@ -58,9 +58,9 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'pravins'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_username(session, 'pravins'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'ralph'), 'subscriber') session.commit() @@ -101,11 +101,11 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'jflory7'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'bee2502'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'keekri'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'linuxmodder'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'decause'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'jflory7'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'bee2502'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'keekri'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'linuxmodder'), 'member') session.commit() @@ -146,12 +146,12 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'croberts'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mattdm'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_username(session, 'croberts'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'ryanlerch'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'mrichard'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'mattdm'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'ralph'), 'subscriber') session.commit() @@ -192,13 +192,13 @@ hub.widgets.append(widget) # Set up some memberships -hub.subscribe(session, hubs.models.User.by_openid(session, 'duffy'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ryanlerch'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'gnokii'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'mrichard'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_username(session, 'duffy'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'ryanlerch'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'gnokii'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'mrichard'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'nask0'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'decause'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'ralph'), 'subscriber') session.commit() @@ -238,10 +238,10 @@ widget = hubs.models.Widget(plugin='dummy', index=3, left=True) hub.widgets.append(widget) -hub.subscribe(session, hubs.models.User.by_openid(session, 'ralph'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'abompard'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'lmacken'), 'owner') -hub.subscribe(session, hubs.models.User.by_openid(session, 'nask0'), 'member') -hub.subscribe(session, hubs.models.User.by_openid(session, 'decause'), 'subscriber') +hub.subscribe(session, hubs.models.User.by_username(session, 'ralph'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'abompard'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'lmacken'), 'owner') +hub.subscribe(session, hubs.models.User.by_username(session, 'nask0'), 'member') +hub.subscribe(session, hubs.models.User.by_username(session, 'decause'), 'subscriber') session.commit() From fc0f4d96a3c9d885a05d38e2d07c49545ab341ab Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 30 2016 11:37:28 +0000 Subject: [PATCH 19/26] Rename FAS to OIDC and fix the test to flask.session.auth --- diff --git a/hubs/app.py b/hubs/app.py index 136c844..6cc095e 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -459,12 +459,12 @@ def widget_source(name): # Set up OpenIDConnect -FAS = OpenIDConnect(app, credentials_store=flask.session ) +OIDC = OpenIDConnect(app, credentials_store=flask.session ) @app.route('/login/', methods=('GET', 'POST')) @app.route('/login', methods=('GET', 'POST')) -@FAS.require_login +@OIDC.require_login def login(): default = flask.url_for('index') next_url = flask.request.args.get('next', default) @@ -491,7 +491,7 @@ def logout(): next_url = flask.url_for('index') if authenticated(): - FAS.logout() + OIDC.logout() flask.session.auth = None flask.flash('You have been logged out') @@ -519,17 +519,17 @@ def check_auth(): flask.session.permanent = True flask.g.fedmsg_config = fedmsg_config - if FAS.user_loggedin: - if not hasattr(flask.session, 'fas_user') or not flask.session.fas_user: + if OIDC.user_loggedin: + if not hasattr(flask.session, 'auth') or not flask.session.auth: flask.session.auth = munch.Munch( - fullname=FAS.user_getfield('name'), - nickname=FAS.user_getfield('nickname'), - email=FAS.user_getfield('email'), - timezone=FAS.user_getfield('zoneinfo'), + fullname=OIDC.user_getfield('name'), + nickname=OIDC.user_getfield('nickname'), + email=OIDC.user_getfield('email'), + timezone=OIDC.user_getfield('zoneinfo'), cla_done=\ 'http://admin.fedoraproject.org/accounts/cla/done' \ - in FAS.user_getfield('cla'), - groups=FAS.user_getfield('groups'), + in OIDC.user_getfield('cla'), + groups=OIDC.user_getfield('groups'), logged_in=True, ) flask.session.auth.avatar=username2avatar( From 18d8043cacfcec9b911a51a47e47b68fb8da7699 Mon Sep 17 00:00:00 2001 From: Eric Barbour Date: Jul 05 2016 08:24:04 +0000 Subject: [PATCH 20/26] Fix issue with Feed elements not rendering Don't bind Feed component to DOM twice --- diff --git a/hubs/static/client/app/index.jsx b/hubs/static/client/app/index.jsx index d169745..29f5129 100644 --- a/hubs/static/client/app/index.jsx +++ b/hubs/static/client/app/index.jsx @@ -3,8 +3,3 @@ import { render } from 'react-dom'; import Feed from './components/Feed.jsx' - -render( - , - document.getElementById('feed') -); From 06c532f1037f1c40ac2d7ace336969012affda31 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 05 2016 08:24:04 +0000 Subject: [PATCH 21/26] Make the username validator's code a little easier to read --- diff --git a/hubs/validators.py b/hubs/validators.py index c76c9f2..bdc3c96 100755 --- a/hubs/validators.py +++ b/hubs/validators.py @@ -21,7 +21,7 @@ def link(session, value): def username(session, value): - return not hubs.models.User.by_username(session, value) is None + return hubs.models.User.by_username(session, value) is not None def github_organization(session, value): From 9392d8929770921204d9e386be7b578877e80496 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 05 2016 08:24:04 +0000 Subject: [PATCH 22/26] Fix typo found by @puiterwijk --- diff --git a/README.rst b/README.rst index b8c0aa7..d4ad78e 100644 --- a/README.rst +++ b/README.rst @@ -47,7 +47,7 @@ OK -- with that done, now install the dependencies from PyPI:: $ pip install -r requirements.txt -Configure the project to authentify against iddev.fedorainfraclouid.org:: +Configure the project to authenticate against iddev.fedorainfraclouid.org:: oidc-register --debug https://iddev.fedorainfracloud.org/ http://localhost:5000 From d0684f0dcacf08b7365ff32aeeabd6a5df2b119b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 05 2016 08:24:04 +0000 Subject: [PATCH 23/26] Fall back to sub if there is no nickname provided by the auth system --- diff --git a/hubs/app.py b/hubs/app.py index 6cc095e..c975952 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -523,7 +523,8 @@ def check_auth(): if not hasattr(flask.session, 'auth') or not flask.session.auth: flask.session.auth = munch.Munch( fullname=OIDC.user_getfield('name'), - nickname=OIDC.user_getfield('nickname'), + nickname=OIDC.user_getfield('nickname') \ + or OIDC.user_getfield('sub'), email=OIDC.user_getfield('email'), timezone=OIDC.user_getfield('zoneinfo'), cla_done=\ From 9cf87981f5f79a4d54685fc2264bb97fe35ae4e8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 06 2016 13:41:01 +0000 Subject: [PATCH 24/26] Make the app in debug mode earlier since it's not an argument of run() --- diff --git a/runserver.py b/runserver.py index e85a2b4..05e2594 100755 --- a/runserver.py +++ b/runserver.py @@ -19,4 +19,5 @@ args = parser.parse_args() from hubs.app import app +app.debug = True app.run(debug=True, host=args.host, port=int(args.port)) From 0e906fba20c786cd43ed69eb98b6fd23f4d73724 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 06 2016 13:49:12 +0000 Subject: [PATCH 25/26] Add a runtest.sh script to run the tests properly with the right config --- diff --git a/hubs/tests/client_secrets.json b/hubs/tests/client_secrets.json new file mode 100644 index 0000000..0ce3ce2 --- /dev/null +++ b/hubs/tests/client_secrets.json @@ -0,0 +1,12 @@ +{ + "web": { + "redirect_uris": ["http://localhost:5002/oidc_callback"], + "token_uri": "https://iddev.fedorainfracloud.org/openidc/Token", + "auth_uri": "https://iddev.fedorainfracloud.org/openidc/Authorization", + "client_id": "client_id", + "client_secret": "client_secret", + "userinfo_uri": "https://iddev.fedorainfracloud.org/openidc/UserInfo", + "token_introspection_uri": "https://iddev.fedorainfracloud.org/openidc/TokenInfo", + "issuer": "https://iddev.fedorainfracloud.org/openidc/" + } +} diff --git a/hubs/tests/hubs_test.cfg b/hubs/tests/hubs_test.cfg new file mode 100644 index 0000000..66ccd47 --- /dev/null +++ b/hubs/tests/hubs_test.cfg @@ -0,0 +1,11 @@ +### Secret key for the Flask application +SECRET_KEY='' + +### url to the database server: +import os +DB_URL = 'sqlite:///%s/test.db' % (os.path.dirname(os.path.abspath(__file__))) +#DB_URL='sqlite:////tmp/fedocal_dev.sqlite' + +import os +OIDC_CLIENT_SECRETS = os.path.join(os.path.dirname( + os.path.abspath(__file__)), 'client_secrets.json') diff --git a/nosetests b/nosetests new file mode 100755 index 0000000..2f5271b --- /dev/null +++ b/nosetests @@ -0,0 +1,9 @@ +#!/usr/bin/env python +# EASY-INSTALL-ENTRY-SCRIPT: 'nose==0.10.4','console_scripts','nosetests' +__requires__ = ['nose>=0.10.4', 'SQLAlchemy >= 0.7', 'jinja2 >= 2.4'] +import sys +from pkg_resources import load_entry_point + +sys.exit( + load_entry_point('nose>=0.10.4', 'console_scripts', 'nosetests')() +) diff --git a/runtest.sh b/runtest.sh new file mode 100755 index 0000000..ad0de9c --- /dev/null +++ b/runtest.sh @@ -0,0 +1,3 @@ +#!/bin/bash +HUBS_CONFIG=`pwd`/hubs/tests/hubs_test.cfg PYTHONPATH=. ./nosetests \ + --with-coverage --cover-erase --cover-package=hubs $* From aec647a51bf9995242e1f85d92530641acfbf665 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 06 2016 13:56:49 +0000 Subject: [PATCH 26/26] Adjust the instructions on how to run the tests --- diff --git a/README.rst b/README.rst index d4ad78e..5379dad 100644 --- a/README.rst +++ b/README.rst @@ -81,7 +81,11 @@ didn't inadvertently break something else. You can run it with:: $ pip install -r test-requirements.txt - $ PYTHONPATH=. nosetests + $ ./runtest.sh + +See the options available with:: + + $ ./runtest.sh --help Some credentials... -------------------