From 2f7780ab04eb210f9056def21cf16ac5836d7f8c Mon Sep 17 00:00:00 2001 From: Brian DeRocher Date: Tue, 29 Aug 2023 14:53:35 -0400 Subject: [PATCH] Add OIDC authentication using Flask-pyoidc. --- setup.py | 1 + src/moin/app.py | 8 ++ src/moin/apps/frontend/views.py | 133 +++++++++++++++++++++- src/moin/auth/__init__.py | 10 ++ src/moin/auth/oidc.py | 17 +++ src/moin/config/default.py | 2 + src/moin/config/wikiconfig.py | 10 ++ src/moin/constants/keys.py | 3 + src/moin/storage/middleware/indexing.py | 2 + src/moin/storage/middleware/validation.py | 2 + src/moin/templates/register_sso.html | 19 ++++ src/moin/templates/utils.html | 14 +++ src/moin/themes/__init__.py | 18 +++ src/moin/user.py | 1 + 14 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 src/moin/auth/oidc.py create mode 100644 src/moin/templates/register_sso.html diff --git a/setup.py b/setup.py index 52967d3a0..acc918521 100644 --- a/setup.py +++ b/setup.py @@ -107,6 +107,7 @@ # optional features and their list of requirements extras_require={ # 'featurename': ["req1", "req2", ], + 'Flask-pyoidc>=3.14.2', # OIDC SSO support 'pillow': ["pillow"], # successor to PIL; used by image get for scaling/rotating/etc.; # requires special libs/header to be installed before it can be compiled successfully 'ldap': ["python-ldap"], # used by ldap auth; requires special libs/header diff --git a/src/moin/app.py b/src/moin/app.py index 5988d02d7..d2df36f5a 100644 --- a/src/moin/app.py +++ b/src/moin/app.py @@ -153,6 +153,9 @@ class ItemNameConverter(PathConverter): app.register_blueprint(serve, url_prefix='/+serve') clock.stop('create_app register') clock.start('create_app flask-cache') + if app.cfg['sso']: + from moin.auth.oidc import oidc + oidc.init_app(app) # 'SimpleCache' caching uses a dict and is not thread safe according to the docs. cache = Cache(config={'CACHE_TYPE': 'SimpleCache'}) cache.init_app(app) @@ -237,6 +240,8 @@ def setup_user(): session.clear() userobj = auth.setup_from_session() + # BCD: This is the wrong place to handle login and logout. + # then handle login/logout forms form = request.values.to_dict() if 'login_submit' in form: @@ -248,6 +253,9 @@ def setup_user(): else: userobj = auth.handle_request(userobj) + # BCD: Getting the session from the user object (after handling a login even) + # is backwards. + # if we still have no user obj, create a dummy: if not userobj: userobj = user.User(name=ANON, auth_method='invalid') diff --git a/src/moin/apps/frontend/views.py b/src/moin/apps/frontend/views.py index 7f78ff5e9..d1d09f7c2 100644 --- a/src/moin/apps/frontend/views.py +++ b/src/moin/apps/frontend/views.py @@ -33,7 +33,7 @@ from werkzeug.utils import secure_filename -from flask import request, url_for, flash, Response, make_response, redirect, abort, jsonify +from flask import request, url_for, flash, Response, make_response, redirect, abort, jsonify, session from flask import current_app as app from flask import g as flaskg from flask_babel import format_datetime @@ -46,7 +46,7 @@ import pytz from babel import Locale - +from passlib.pwd import genword from whoosh import sorting from whoosh.query import Term, Prefix, And, Or, Not, DateRange, Every from whoosh.query.qcore import QueryError, TermNotFound @@ -55,6 +55,7 @@ from moin.i18n import _, L_ from moin.themes import render_template, contenttype_to_class, get_editor_info from moin.apps.frontend import frontend +from moin.auth.oidc import oidc from moin.forms import (OptionalText, RequiredText, URL, YourEmail, RequiredPassword, Checkbox, InlineCheckbox, Select, Names, Tags, Natural, Hidden, MultiSelect, Enum, Subscriptions, Quicklinks, RadioChoice, @@ -69,6 +70,7 @@ from moin.constants.itemtypes import ITEMTYPE_DEFAULT, ITEMTYPE_TICKET from moin.constants.contenttypes import * # noqa from moin.constants.rights import SUPERUSER +from moin.user import search_users, User from moin.utils import crypto, rev_navigation, close_file, show_time from moin.utils.crypto import make_uuid, hash_hexdigest from moin.utils.interwiki import url_for_item, split_fqname, CompositeName @@ -136,9 +138,11 @@ def robots(): Disallow: /+wanteds/ Disallow: /+orphans/ Disallow: /+register +Disallow: /+register_oidc_idp Disallow: /+recoverpass Disallow: /+usersettings Disallow: /+login +Disallow: /+login_oidc_idp Disallow: /+logout Disallow: /+bookmark Disallow: /+diff/ @@ -1870,6 +1874,17 @@ class RegistrationForm(Form): validators = [ValidRegistration()] +class RegistrationSsoForm(Form): + """a simple user registration form""" + name = 'register_sso' + + username = RequiredText.using(label=L_('Username')).with_properties( + placeholder=L_("The login username you want to use"), autofocus=True + ) + email = YourEmail + submit_label = L_('Register') + + def _using_moin_auth(): """Check if MoinAuth is being used for authentication. @@ -1931,6 +1946,87 @@ def register(): ) +@frontend.route('/+register_oidc_idp', methods=['GET', 'POST']) +@oidc.oidc_auth('idp') +def register_oidc_idp(): + """ + Flow is like this: + 1. GET /+register_sso + 2. Decorator will redirect user to IDP *before* this function is called. + 3. After OIDC authenticates the user, it will redirect the user back to this function. + 4. If there's no form submission, print the form. Let the user enter username and email address. + 5. Second time around the decorator will redirect again, and IDP will redirect back quickly. + 6. Validate the form submission + 7. Create the user. + + In the future, when this project wants to support multiple IDPs, it can parameterize + the endpoint, e.g. /+register_oidc/idp1, /+register_oidc/idp2, and a generic registration + page could have links/buttons for each IDP. See flask_pyoidc documentation. + """ + return _register_oidc_using_provider('idp') + + +def _register_oidc_using_provider(provider): + if app.cfg.registration_only_by_superuser and not getattr(flaskg.user.may, SUPERUSER)(): + # deny registration to bots + abort(404) + + title_name = _('Register') + oidc_name = session['userinfo']["given_name"] + + if request.method in ['GET']: + return render_template( + 'register_sso.html', + title_name=title_name, + form=RegistrationSsoForm.from_defaults(), + name=oidc_name, + ) + + # At this point, it must be a POST because that's all we allow. + + # Validate the form + form = RegistrationSsoForm.from_flat(request.form) + if not form.validate(): + return render_template( + 'register_sso.html', + title_name=title_name, + form=RegistrationSsoForm, + name=oidc_name, + ) + + # Create the user. + user_kwargs = { + 'username': form['username'].value, + 'oidc': provider, + 'oidc_uid': session['userinfo']['sub'], + 'password': genword(length=32), # BCD: Password should not be required. + 'email': form['email'].value, + 'trusted': True, + } + # BCD: These should be default values of the user. Consider using factory that + # uses application configuration to generate blank users. + if app.cfg.user_email_verification: + user_kwargs['is_disabled'] = True + user_kwargs['verify_email'] = True + msg = user.create_user(**user_kwargs) + if msg: + flash(msg, "error") + else: + # TODO: Remove code duplication here. + if app.cfg.user_email_verification: + u = user.User(auth_username=user_kwargs['username']) + is_ok, msg = u.mail_email_verification() + if is_ok: + flash(_('Account verification required, please see the email we sent to your address.'), "info") + else: + flash(_('An error occurred while sending the verification email: "%(message)s" ' + 'Please contact an administrator to activate your account.', + message=msg), "error") + else: + flash(_('Account created, please log in now.'), "info") + return redirect(url_for('.show_root')) + + @frontend.route('/+verifyemail', methods=['GET']) def verifyemail(): u = token = None @@ -2149,6 +2245,39 @@ def login(): ) +@frontend.route('/+login_oidc_idp', methods=['GET', 'POST']) +@oidc.oidc_auth('idp') +def login_oidc_idp(): + _login_oidc_using_provider('idp') + return redirect(url_for('.show_root')) + + +def _login_oidc_using_provider(provider): + info = session['userinfo'] + users = search_users(**{ + OIDC: provider, + OIDC_UID: info['sub'], + DISABLED: False, + }) + if not users: + logging.warning('When logging in, no users were found.') + flash('User does not exist', 'error') + return + # TODO: What if user has multiple moin accounts for this oidc sub. + user = User(uid=users[0].meta['itemid']) + start_session(user, 'oidc', (), True) + + +def start_session(user: User, auth_method, auth_attribs, trusted): + """ Regardless of authentication mechanism, this function starts a session. """ + logging.info(user.itemid + ' has started a sesstion') + session['user.itemid'] = user.itemid + session['user.trusted'] = trusted # The user meta document loaded does not have this info. + session['user.auth_method'] = auth_method + session['user.auth_attribs'] = auth_attribs + session['user.session_token'] = user.get_session_token() + + @frontend.route('/+logout') def logout(): flash(_("You are logged out."), "info") diff --git a/src/moin/auth/__init__.py b/src/moin/auth/__init__.py index a9d3633a9..611069fd1 100644 --- a/src/moin/auth/__init__.py +++ b/src/moin/auth/__init__.py @@ -276,6 +276,15 @@ def login_hint(self): return Markup(msg) +class OidcAuth(BaseAuth): + """ This authentication is different from others and doesn't fit the design pattern. """ + def __init__(self, **kw): + super(OidcAuth, self).__init__(**kw) + + name = 'oidc' + logout_possible = True + + class GivenAuth(BaseAuth): """ reuse a given authentication, e.g. http basic auth (or any other auth) done by the web server, that sets REMOTE_USER environment variable. @@ -456,6 +465,7 @@ def setup_from_session(): session_token = session['user.session_token'] logging.debug("got from session: {0!r} {1!r} {2!r} {3!r}".format(itemid, trusted, auth_method, auth_attribs)) logging.debug("current auth methods: {0!r}".format(app.cfg.auth_methods)) + # BCD: Why do we care about auth method? All that matters is the session_token. if auth_method and auth_method in app.cfg.auth_methods: userobj = user.User(itemid, auth_method=auth_method, diff --git a/src/moin/auth/oidc.py b/src/moin/auth/oidc.py new file mode 100644 index 000000000..07431f607 --- /dev/null +++ b/src/moin/auth/oidc.py @@ -0,0 +1,17 @@ +from flask_pyoidc import OIDCAuthentication +from flask_pyoidc.provider_configuration import ClientMetadata, ProviderConfiguration + +from wikiconfig import OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, OIDC_LOGOUT_URI + + +oidc = OIDCAuthentication({ + 'idp': ProviderConfiguration( + issuer=OIDC_ISSUER, + # static client registration + client_metadata=ClientMetadata( + client_id=OIDC_CLIENT_ID, + client_secret=OIDC_CLIENT_SECRET, + post_logout_redirect_uris=[OIDC_LOGOUT_URI], + ), + ), +}) diff --git a/src/moin/config/default.py b/src/moin/config/default.py index 5b3eb3897..65cf42504 100644 --- a/src/moin/config/default.py +++ b/src/moin/config/default.py @@ -485,6 +485,8 @@ def __init__(self, exprstr): LOCALE: None, # None -> do browser language detection, otherwise just use this locale TIMEZONE: None, # None -> use cfg.timezone_default EMAIL_UNVALIDATED: None, + OIDC: None, + OIDC_UID: None, }, 'Default attributes of the user object'), )), # ========================================================================== diff --git a/src/moin/config/wikiconfig.py b/src/moin/config/wikiconfig.py index 470508ca5..070e5ce4d 100644 --- a/src/moin/config/wikiconfig.py +++ b/src/moin/config/wikiconfig.py @@ -237,6 +237,7 @@ class Config(DefaultConfig): # root_mapping = {'user': 'UserHome'} # default root, use this value by default for all namespaces default_root = 'Home' + sso = False # flask settings require all caps @@ -256,3 +257,12 @@ class Config(DefaultConfig): # config for flask-cache: # CACHE_TYPE = 'filesystem' # CACHE_DIR = '/path/to/flask-cache-dir' + +# Using + (plus) in the OIDC_REDIRECT_URI works on the python side, i.e. it is encoded +# correctly into %2B, but sadly Keycloak converts it to ' '. See +# https://en.wikipedia.org/wiki/Percent-encoding#The_application/x-www-form-urlencoded_type +# OIDC_REDIRECT_URI = 'http://localhost:5000/oidc_redirect_uri' +# OIDC_ISSUER="https://localhost:8080/auth/realms/myfolks" +# OIDC_CLIENT_ID='example.com/wiki' # best practice is to use domain name and path here +# OIDC_CLIENT_SECRET='get this from the idp administrator' +# OIDC_LOGOUT_URI='todo' diff --git a/src/moin/constants/keys.py b/src/moin/constants/keys.py index 4c4d67d31..e611771a4 100644 --- a/src/moin/constants/keys.py +++ b/src/moin/constants/keys.py @@ -116,6 +116,8 @@ EMAIL_UNVALIDATED = "email_unvalidated" NAMERE = "namere" NAMEPREFIX = "nameprefix" +OIDC = "oidc" +OIDC_UID = "oidc_uid" # in which backend is some revision stored? BACKENDNAME = "backendname" @@ -126,6 +128,7 @@ ISO_8601, MAILTO_AUTHOR, SHOW_COMMENTS, RESULTS_PER_PAGE, EDIT_ON_DOUBLECLICK, SCROLL_PAGE_AFTER_EDIT, EDIT_ROWS, THEME_NAME, LOCALE, TIMEZONE, SUBSCRIPTIONS, QUICKLINKS, CSS_URL, + OIDC, OIDC_UID, ] # keys for blog homepages diff --git a/src/moin/storage/middleware/indexing.py b/src/moin/storage/middleware/indexing.py index 97deb9d9b..424f36d2b 100644 --- a/src/moin/storage/middleware/indexing.py +++ b/src/moin/storage/middleware/indexing.py @@ -420,6 +420,8 @@ def __init__(self, index_storage, backend, wiki_name=None, acl_rights_contents=[ LOCALE: ID(stored=True), SUBSCRIPTION_IDS: ID(), SUBSCRIPTION_PATTERNS: ID(), + OIDC: ID(stored=True), + OIDC_UID: ID(stored=True), } latest_revs_fields.update(**userprofile_fields) diff --git a/src/moin/storage/middleware/validation.py b/src/moin/storage/middleware/validation.py index c08d92d82..6af6494a6 100644 --- a/src/moin/storage/middleware/validation.py +++ b/src/moin/storage/middleware/validation.py @@ -421,6 +421,8 @@ def subscription_validator(element, state): .of(String.named('subscription').validated_by(subscription_validator)).using(optional=True), List.named(keys.EMAIL_SUBSCRIBED_EVENTS).of(String.named('email_subscribed_event')).using(optional=True), # TODO: DuckDict.named('bookmarks').using(optional=True), + String.named(keys.OIDC).using(optional=True), + String.named(keys.OIDC_UID).using(optional=True), *common_meta ) diff --git a/src/moin/templates/register_sso.html b/src/moin/templates/register_sso.html new file mode 100644 index 000000000..91fdcc4f5 --- /dev/null +++ b/src/moin/templates/register_sso.html @@ -0,0 +1,19 @@ +{% extends theme("layout.html") %} +{% import "forms.html" as forms %} + +{% block content %} +

{{ _("Create Account using SSO") }}

+

+ {{ name }}, You have already been identified by the Identity Provider. Please continue registration here. +

+
+ {{ gen.form.open(form, method="post", action=url_for('frontend.register_oidc_idp')) }} + {{ forms.render_errors(form) }} +
+ {{ forms.render(form['username']) }} + {{ forms.render(form['email']) }} +
+ {{ forms.render_submit(form) }} + {{ gen.form.close() }} +
+{% endblock %} diff --git a/src/moin/templates/utils.html b/src/moin/templates/utils.html index 11e76c35a..ddace430b 100644 --- a/src/moin/templates/utils.html +++ b/src/moin/templates/utils.html @@ -476,6 +476,20 @@ {{ _('Login') }} {%- endif %} + {%- set login_idp_url = theme_supp.login_oidc_idp_url() %} + {% if login_idp_url %} +
  • + {# TODO: translate #} + +
    • + {% endif %} + {%- set register_idp_url = theme_supp.register_oidc_idp_url() %} + {% if register_idp_url %} +
  • + {# TODO: translate #} + +
    • + {% endif %} {%- endif %} {% endmacro %} diff --git a/src/moin/themes/__init__.py b/src/moin/themes/__init__.py index 459446b32..ccdbdd7d5 100644 --- a/src/moin/themes/__init__.py +++ b/src/moin/themes/__init__.py @@ -527,6 +527,24 @@ def login_url(self): url = url or url_for('frontend.login') return url + def login_oidc_idp_url(self): + """ + Return URL usable for user login with SSO. + + :rtype: unicode (or None, if no login url is supported) + :returns: url for user login + """ + return url_for('frontend.login_oidc_idp') if self.cfg.sso else None + + def register_oidc_idp_url(self): + """ + Return URL usable for user registration with SSO. + + :rtype: unicode (or None, if no login url is supported) + :returns: url for user registration + """ + return url_for('frontend.register_oidc_idp') if self.cfg.sso else None + def get_fqnames(self, fqname): """ Return the list of other fqnames associated with the item. diff --git a/src/moin/user.py b/src/moin/user.py index 711fd04fe..f26db227f 100644 --- a/src/moin/user.py +++ b/src/moin/user.py @@ -509,6 +509,7 @@ def set_password(self, password, is_encrypted=False): self.profile[ENC_PASSWORD] = password # Invalidate all other browser sessions except this one. try: + # BCD: A model class should not be touching the session. session['user.session_token'] = self.generate_session_token(False) except RuntimeError: # CLI call has no valid session context pass