From 46f8d5a80c870993cddd20c04277bd7cad4abb3a Mon Sep 17 00:00:00 2001
From: Slavek Kabrda <bkabrda@redhat.com>
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=<pagure>/login?next=<location>
+        # 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 = '<secret key to be changed>'
 
-# 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 <bkabrda@redhat.com>
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=<pagure>/login?next=<location>
         # 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 <bkabrda@redhat.com>
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 <https://admin.fedoraproject.org/accounts>`_,
-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 <https://admin.fedoraproject.org/accounts>`_, 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
 ~~~~~~~~~~~~~~~~~~~

