From 46f8d5a80c870993cddd20c04277bd7cad4abb3a Mon Sep 17 00:00:00 2001 From: Slavek Kabrda Date: Dec 05 2017 13:03:27 +0000 Subject: [PATCH 1/3] Enable OpenID Connect authentication --- diff --git a/pagure/__init__.py b/pagure/__init__.py index 22cc39a..227b30d 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -22,9 +22,11 @@ import logging # noqa: E402 import logging.config # noqa: E402 import os # noqa: E402 import re # noqa: E402 +import time # noqa: E402 import urlparse # noqa: E402 import flask # noqa: E402 +import munch # noqa: E402 import pygit2 # noqa: E402 import werkzeug # noqa: E402 from functools import wraps # noqa: E402 @@ -100,90 +102,110 @@ import pagure.login_forms # noqa: E402 import pagure.mail_logging # noqa: E402 import pagure.proxy # noqa: E402 +def set_user(): + if flask.g.fas_user.username is None: + flask.flash( + 'It looks like your Identity Provider did not provide an ' + 'username we could retrieve, username being needed we cannot ' + 'go further.', 'error') + logout() + return + + flask.session['_new_user'] = False + if not pagure.lib.search_user( + SESSION, username=flask.g.fas_user.username): + flask.session['_new_user'] = True + + try: + pagure.lib.set_up_user( + session=SESSION, + username=flask.g.fas_user.username, + fullname=flask.g.fas_user.fullname, + default_email=flask.g.fas_user.email, + ssh_key=flask.g.fas_user.get('ssh_key'), + keydir=APP.config.get('GITOLITE_KEYDIR', None), + ) + + # If groups are managed outside pagure, set up the user at login + if not APP.config.get('ENABLE_GROUP_MNGT', False): + user = pagure.lib.search_user( + SESSION, username=flask.g.fas_user.username) + old_groups = set(user.groups) + fas_groups = set(flask.g.fas_user.groups) + # Add the new groups + for group in fas_groups - old_groups: + groupobj = None + if group: + groupobj = pagure.lib.search_groups( + SESSION, group_name=group) + if groupobj: + try: + pagure.lib.add_user_to_group( + session=SESSION, + username=flask.g.fas_user.username, + group=groupobj, + user=flask.g.fas_user.username, + is_admin=is_admin(), + from_external=True, + ) + except pagure.exceptions.PagureException as err: + APP.logger.error(err) + # Remove the old groups + for group in old_groups - fas_groups: + if group: + try: + pagure.lib.delete_user_of_group( + session=SESSION, + username=flask.g.fas_user.username, + groupname=group, + user=flask.g.fas_user.username, + is_admin=is_admin(), + force=True, + from_external=True, + ) + except pagure.exceptions.PagureException as err: + APP.logger.error(err) + + SESSION.commit() + except SQLAlchemyError as err: + SESSION.rollback() + APP.logger.exception(err) + flask.flash( + 'Could not set up you as a user properly, please contact ' + 'an admin', 'error') + # Ensure the user is logged out if we cannot set them up + # correctly + logout() + # Only import flask_fas_openid if it is needed if APP.config.get('PAGURE_AUTH', None) in ['fas', 'openid']: from flask_fas_openid import FAS FAS = FAS(APP) @FAS.postlogin - def set_user(return_url): + def set_user_fas(return_url): ''' After login method. ''' - if flask.g.fas_user.username is None: - flask.flash( - 'It looks like your OpenID provider did not provide an ' - 'username we could retrieve, username being needed we cannot ' - 'go further.', 'error') - logout() - return flask.redirect(return_url) + set_user() + return flask.redirect(return_url) - flask.session['_new_user'] = False - if not pagure.lib.search_user( - SESSION, username=flask.g.fas_user.username): - flask.session['_new_user'] = True - try: - pagure.lib.set_up_user( - session=SESSION, - username=flask.g.fas_user.username, - fullname=flask.g.fas_user.fullname, - default_email=flask.g.fas_user.email, - ssh_key=flask.g.fas_user.get('ssh_key'), - keydir=APP.config.get('GITOLITE_KEYDIR', None), +if APP.config.get('PAGURE_AUTH', None) == 'oidc': + from flask_oidc import OpenIDConnect + oidc = OpenIDConnect(APP) + + @APP.before_request + def fas_user_from_oidc(): + if oidc.user_loggedin and 'oidc_logintime' in flask.session: + email = flask.g.oidc_id_token['email'] + flask.g.fas_user = munch.Munch( + username=email.split('@')[0], + fullname='', + email=email, + ssh_key=None, + groups=[], + login_time=flask.session['oidc_logintime'], ) - # If groups are managed outside pagure, set up the user at login - if not APP.config.get('ENABLE_GROUP_MNGT', False): - user = pagure.lib.search_user( - SESSION, username=flask.g.fas_user.username) - groups = set(user.groups) - fas_groups = set(flask.g.fas_user.groups) - # Add the new groups - for group in fas_groups - groups: - groupobj = None - if group: - groupobj = pagure.lib.search_groups( - SESSION, group_name=group) - if groupobj: - try: - pagure.lib.add_user_to_group( - session=SESSION, - username=flask.g.fas_user.username, - group=groupobj, - user=flask.g.fas_user.username, - is_admin=is_admin(), - from_external=True, - ) - except pagure.exceptions.PagureException as err: - APP.logger.error(err) - # Remove the old groups - for group in groups - fas_groups: - if group: - try: - pagure.lib.delete_user_of_group( - session=SESSION, - username=flask.g.fas_user.username, - groupname=group, - user=flask.g.fas_user.username, - is_admin=is_admin(), - force=True, - from_external=True, - ) - except pagure.exceptions.PagureException as err: - APP.logger.error(err) - - SESSION.commit() - except SQLAlchemyError as err: - SESSION.rollback() - APP.logger.exception(err) - flask.flash( - 'Could not set up you as a user properly, please contact ' - 'an admin', 'error') - # Ensure the user is logged out if we cannot set them up - # correctly - logout() - return flask.redirect(return_url) - - SESSION = pagure.lib.create_session(APP.config['DB_URL']) REDIS = None if APP.config['EVENTSOURCE_SOURCE'] \ @@ -225,6 +247,8 @@ def logout(): if auth in ['fas', 'openid']: if hasattr(flask.g, 'fas_user') and flask.g.fas_user is not None: FAS.logout() + elif auth == 'oidc': + oidc.logout() elif auth == 'local': import pagure.ui.login as login login.logout() @@ -574,6 +598,7 @@ def unauthorized(error): # pragma: no cover @APP.route('/login/', methods=('GET', 'POST')) +@oidc.require_login def auth_login(): # pragma: no cover """ Method to log into the application using FAS OpenID. """ return_point = flask.url_for('index') @@ -581,6 +606,17 @@ def auth_login(): # pragma: no cover if is_safe_url(flask.request.args['next']): return_point = flask.request.args['next'] + auth = APP.config.get('PAGURE_AUTH', None) + if not authenticated() and auth == 'oidc' and oidc.user_loggedin: + # If oidc is used and user hits this endpoint, it will redirect + # to IdP with destination=/login?next= + # After confirming user identity, the IdP will redirect user here + # again, but this time `@oidc.require_login` will admit user inside + # this function and this clause will make sure the Pagure + # authentication machinery picks the user up + flask.session['oidc_logintime'] = time.time() + fas_user_from_oidc() + set_user() if authenticated(): return flask.redirect(return_point) @@ -590,7 +626,7 @@ def auth_login(): # pragma: no cover else: # pragma: no cover admins = set([admins]) - if APP.config.get('PAGURE_AUTH', None) in ['fas', 'openid']: + if auth in ['fas', 'openid']: groups = set() if not APP.config.get('ENABLE_GROUP_MNGT', False): groups = [ @@ -602,7 +638,7 @@ def auth_login(): # pragma: no cover ext_committer = set(APP.config.get('EXTERNAL_COMMITTER', {})) groups = set(groups).union(ext_committer) return FAS.login(return_url=return_point, groups=groups) - elif APP.config.get('PAGURE_AUTH', None) == 'local': + elif auth == 'local': form = pagure.login_forms.LoginForm() return flask.render_template( 'login/login.html', diff --git a/pagure/default_config.py b/pagure/default_config.py index f9365df..b476fb4 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -205,11 +205,17 @@ FROM_EMAIL = 'pagure@pagure.org' DOMAIN_EMAIL_NOTIFICATIONS = 'pagure.org' SALT_EMAIL = '' -# Specify which authentication method to use, defaults to `fas` can be or -# `local` +# Specify which authentication method to use, defaults to `fas`, other +# possibilities are `local` or `oidc` # Default: ``fas``. PAGURE_AUTH = 'fas' +# If PAGURE_AUTH is set to 'oidc', the following variables must be set: +# The path to JSON file with client secrets (provided by your IdP) +# OIDC_CLIENT_SECRETS = 'client_secrets.json' +# Set this to True in production +# OIDC_ID_TOKEN_COOKIE_SECURE = False + # When this is set to True, the session cookie will only be returned to the # server via ssl (https). If you connect to the server via plain http, the # cookie will not be sent. This prevents sniffing of the cookie contents. diff --git a/requirements.txt b/requirements.txt index 6da7995..64b93bb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,6 +32,9 @@ wtforms # Needed only for local authentication and/or Pagure CI cryptography +# Required only for the `oidc` authentication backend +flask-oidc + # Required only for the `fas` and `openid` authentication backends python-fedora From 79a36e9c5e51c376cac460eb716ee1dbf513b0ee Mon Sep 17 00:00:00 2001 From: Slavek Kabrda Date: Dec 05 2017 13:03:27 +0000 Subject: [PATCH 2/3] Improve oidc auth - introduce better config, don't use require_login decorator --- diff --git a/pagure/__init__.py b/pagure/__init__.py index 227b30d..4e49b2c 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -102,6 +102,7 @@ import pagure.login_forms # noqa: E402 import pagure.mail_logging # noqa: E402 import pagure.proxy # noqa: E402 + def set_user(): if flask.g.fas_user.username is None: flask.flash( @@ -177,6 +178,7 @@ def set_user(): # correctly logout() + # Only import flask_fas_openid if it is needed if APP.config.get('PAGURE_AUTH', None) in ['fas', 'openid']: from flask_fas_openid import FAS @@ -196,13 +198,29 @@ if APP.config.get('PAGURE_AUTH', None) == 'oidc': @APP.before_request def fas_user_from_oidc(): if oidc.user_loggedin and 'oidc_logintime' in flask.session: - email = flask.g.oidc_id_token['email'] + email_key, fulln_key, usern_key, ssh_key, groups_key = [ + APP.config['OIDC_PAGURE_EMAIL'], + APP.config['OIDC_PAGURE_FULLNAME'], + APP.config['OIDC_PAGURE_USERNAME'], + APP.config['OIDC_PAGURE_SSH_KEY'], + APP.config['OIDC_PAGURE_GROUPS'], + ] + info = oidc.user_getinfo( + [email_key, fulln_key, usern_key, ssh_key, groups_key] + ) + username = info.get(usern_key) + if not username: + fb = APP.config['OIDC_PAGURE_USERNAME_FALLBACK'] + if fb == 'email': + username = info[email_key].split('@')[0] + elif fb == 'sub': + username = flask.g.oidc_id_token['sub'] flask.g.fas_user = munch.Munch( - username=email.split('@')[0], - fullname='', - email=email, - ssh_key=None, - groups=[], + username=username, + fullname=info.get(fulln_key, ''), + email=info[email_key], + ssh_key=info.get(ssh_key), + groups=info.get(groups_key, []), login_time=flask.session['oidc_logintime'], ) @@ -598,7 +616,6 @@ def unauthorized(error): # pragma: no cover @APP.route('/login/', methods=('GET', 'POST')) -@oidc.require_login def auth_login(): # pragma: no cover """ Method to log into the application using FAS OpenID. """ return_point = flask.url_for('index') @@ -607,16 +624,19 @@ def auth_login(): # pragma: no cover return_point = flask.request.args['next'] auth = APP.config.get('PAGURE_AUTH', None) - if not authenticated() and auth == 'oidc' and oidc.user_loggedin: + if not authenticated() and auth == 'oidc': # If oidc is used and user hits this endpoint, it will redirect # to IdP with destination=/login?next= # After confirming user identity, the IdP will redirect user here - # again, but this time `@oidc.require_login` will admit user inside - # this function and this clause will make sure the Pagure - # authentication machinery picks the user up - flask.session['oidc_logintime'] = time.time() - fas_user_from_oidc() - set_user() + # again, but this time oidc.user_loggedin will be True and thus + # execution will go through the else clause, making the Pagure + # authentication machinery pick the user up + if not oidc.user_loggedin: + return oidc.redirect_to_auth_server(flask.request.url) + else: + flask.session['oidc_logintime'] = time.time() + fas_user_from_oidc() + set_user() if authenticated(): return flask.redirect(return_point) diff --git a/pagure/default_config.py b/pagure/default_config.py index b476fb4..7e07f0f 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -213,8 +213,25 @@ PAGURE_AUTH = 'fas' # If PAGURE_AUTH is set to 'oidc', the following variables must be set: # The path to JSON file with client secrets (provided by your IdP) # OIDC_CLIENT_SECRETS = 'client_secrets.json' -# Set this to True in production +# When this is set to True, the cookie with OpenID Connect Token will only +# be returned to the server via ssl (https). If you connect to the server +# via plain http, the cookie will not be sent. This prevents sniffing +# of the cookie contents. This may be set to False when testing your +# application but should always be set to True in production. # OIDC_ID_TOKEN_COOKIE_SECURE = False +# OIDC_SCOPES = ['openid', 'email', 'profile'] +# These specify names of expected keys provided as userinfo by IdP. +# They may vary across different IdPs +# OIDC_PAGURE_EMAIL = 'email' +# OIDC_PAGURE_FULLNAME = 'name' +# OIDC_PAGURE_USERNAME = 'preferred_username' +# OIDC_PAGURE_SSH_KEY = 'ssh_key' +# OIDC_PAGURE_GROUPS = 'groups' +# This specifies fallback for getting username assuming OIDC_PAGURE_USERNAME +# is empty - can be `email` (to use the part before `@`) or `sub` +# (IdP-specific user id, can be a nickname, email or a numeric ID +# depending on IdP). +# OIDC_PAGURE_USERNAME_FALLBACK = 'email' # When this is set to True, the session cookie will only be returned to the # server via ssl (https). If you connect to the server via plain http, the From df7bf84aad5fcb1971ef222bea880a284d7b1a40 Mon Sep 17 00:00:00 2001 From: Slavek Kabrda Date: Dec 05 2017 13:03:27 +0000 Subject: [PATCH 3/3] Add documentation for OpenID Connect authentication settings --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 1c3ec2e..8abc172 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -578,13 +578,79 @@ PAGURE_AUTH ~~~~~~~~~~~~ This configuration key specifies which authentication method to use. -Pagure currently supports two authentication methods: one relying on the -Fedora Account System `FAS `_, -and the other using only the local database. -It can therefore be either ``fas`` or ``local``. +Pagure currently supports three authentication methods: the first one +is relying on the Fedora Account System +`FAS `_, the second is using +OpenID Connect (any provider) and the third is using only the local database. +It can therefore be either ``fas``, ``oidc`` or ``local``. + +If ``oidc`` is used, the configuration options starting with ``OIDC_`` +(see below) must be provided. Defaults to: ``fas``. +OIDC Settings +~~~~~~~~~~~~~ + +OIDC_CLIENT_SECRETS +^^^^^^^^^^^^^^^^^^^ + +Provide a path to client secrets file on local filesystem. This file can be +obtained from your OpenID Connect identity provider. Note that some providers +don't fill in ``userinfo_uri``. If that is the case, you need to add it to +the secrets file manually. + +OIDC_ID_TOKEN_COOKIE_SECURE +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +When this is set to True, the cookie with OpenID Connect Token will only be +returned to the server via ssl (https). If you connect to the server via plain +http, the cookie will not be sent. This prevents sniffing of the cookie contents. +This may be set to False when testing your application but should always +be set to True in production. + +Defaults to: ``True`` for production with https, can be set to ``False`` for +convenient development. + +OIDC_SCOPES +^^^^^^^^^^^ + +List of `OpenID Connect scopes http://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims` +to request from identity provider. + +OIDC_PAGURE_EMAIL +^^^^^^^^^^^^^^^^^ + +Name of key of user's email in userinfo JSON returned by identity provider. + +OIDC_PAGURE_FULLNAME +^^^^^^^^^^^^^^^^^^^^ + +Name of key of user's full name in userinfo JSON returned by identity provider. + +OIDC_PAGURE_USERNAME +^^^^^^^^^^^^^^^^^^^^ + +Name of key of user's preferred username in userinfo JSON returned by identity +provider. + +OIDC_PAGURE_SSH_KEY +^^^^^^^^^^^^^^^^^^^ + +Name of key of user's ssh key in userinfo JSON returned by identity provider. + +OIDC_PAGURE_GROUPS +^^^^^^^^^^^^^^^^^^ + +Name of key of user's groups in userinfo JSON returned by identity provider. + +OIDC_PAGURE_USERNAME_FALLBACK +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This specifies fallback for getting username assuming ``OIDC_PAGURE_USERNAME`` +is empty - can be ``email`` (to use the part before ``@``) or ``sub`` +(IdP-specific user id, can be a nickname, email or a numeric ID +depending on identity provider). IP_ALLOWED_INTERNAL ~~~~~~~~~~~~~~~~~~~