Rework the logout handler to

 1) Generally act in reverse order of login regarding gdp project logout, 2FA
    session clean up and local+remote IDP logout
 2) Disable the OpenID 2.0 workaround with interleaved remote IDP logout when
    using OpenID Connect

This should make logout more consistent and more importantly address the issue
with missing local 2FA session termination during OpenID Connect logout we
currently see in issue 222.

Author: jonasbardino

mig/shared/functionality/logout.py

@@ -4,7 +4,7 @@
 # --- BEGIN_HEADER ---
 #
 # logout - force-expire local login session(s)
-# Copyright (C) 2003-2023  The MiG Project lead by Brian Vinter
+# Copyright (C) 2003-2025  The MiG Project lead by Brian Vinter
 #
 # This file is part of MiG.
 #
@@ -43,6 +43,7 @@
     require_twofactor_setup, build_logout_url, extract_client_openid
 from mig.shared.init import initialize_main_variables, find_entry
 from mig.shared.useradm import expire_oid_sessions, find_oid_sessions
+from mig.shared.url import urlencode
 
 
 def signature():
@@ -115,13 +116,25 @@ def main(client_id, user_arguments_dict, environ=None):
                                     if i in ['setup', 'logout']]
         title_entry['user_menu'] = []
 
-    # OpenID requires logout on provider and in local mod-auth-openid database.
+    # NOTE: actual logout is done in reverse order of login. Namely, start with
+    # any gdp project logout, then 2FA session removal and finally complete any
+    # IDP logout.
+
+    # NOTE: OpenID 2.0 requires logout on provider and in local mod-auth-openid
+    # database.
     # IMPORTANT: some browsers like Firefox may inadvertently renew the local
     # OpenID session while loading the resources for this page (in parallel).
     # User clicks logout at OpenID provider, which sends her back here to
     # finish the local logout.
+    # NOTE: OpenID Connect on the other hand cannot return after logout and
+    # must complete gdp and 2FA session clean up before calling remote logout.
 
     if do_logout:
+        if configuration.site_enable_gdp:
+            logger.debug("close any open GDP project for %s" % client_id)
+            project_logout(configuration, 'https', environ['REMOTE_ADDR'],
+                           client_id)
+
         if configuration.site_enable_twofactor:
             real_user = client_id
             addr_filter = environ['REMOTE_ADDR']
@@ -131,6 +144,7 @@ def main(client_id, user_arguments_dict, environ=None):
                 real_user = get_client_id_from_project_client_id(configuration,
                                                                  client_id)
                 addr_filter = None
+            logger.debug("expire twofactor session for %s" % client_id)
             if real_user and not expire_twofactor_session(configuration,
                                                           real_user, environ,
                                                           allow_missing=True,
@@ -144,7 +158,28 @@ def main(client_id, user_arguments_dict, environ=None):
 the %s Admins if it happens repeatedly.
 </p>""" % configuration.short_title
                      })
+
+        if auth_type == AUTH_OPENID_V2:
+            if auth_flavor == AUTH_MIG_OID:
+                reentry_page = configuration.migserver_https_mig_oid_url
+            elif auth_flavor == AUTH_EXT_OID:
+                reentry_page = configuration.migserver_https_ext_oid_url
+        elif auth_type == AUTH_OPENID_CONNECT:
+            if auth_flavor == AUTH_MIG_OIDC:
+                reentry_page = configuration.migserver_https_mig_oidc_url
+            elif auth_flavor == AUTH_EXT_OIDC:
+                reentry_page = configuration.migserver_https_ext_oidc_url
+        elif auth_type == AUTH_CERTIFICATE:
+            if auth_flavor == AUTH_MIG_CERT:
+                reentry_page = configuration.migserver_https_mig_cert_url
+            elif auth_flavor == AUTH_EXT_CERT:
+                reentry_page = configuration.migserver_https_ext_cert_url
+        else:
+            reentry_page = "https://%s" % configuration.server_fqdn
+
         if auth_type == AUTH_OPENID_V2:
+            # TODO: truncate open_id_session_id instead as suggested in the FAQ?
+            #       https://findingscience.com/mod_auth_openid/faq.html
             (oid_db, identity) = extract_client_openid(configuration, environ,
                                                        lookup_dn=False)
             logger.info("expiring active sessions for %s in %s" % (identity,
@@ -155,6 +190,18 @@ def main(client_id, user_arguments_dict, environ=None):
                 configuration, oid_db, identity)
         elif auth_type == AUTH_OPENID_CONNECT:
             # NOTE: mod auth oidc handles local logout automatically
+            logger.debug("pass through local %s auth module for %s session end"
+                         % (auth_type, identity))
+            # NOTE: OpenID Connect module handles remote logout on vanity url
+            terminate_session_url = '/dynamic/redirect_uri'
+            # NOTE: upstream currently requires fixed logout callback url
+            # TODO: change return_page to reentry_page when upstream allows it
+            return_page = build_logout_url(configuration, environ)
+            terminate_query = urlencode({'logout': return_page})
+            reentry_page = terminate_session_url + '?%s' % terminate_query
+            logger.debug("send %s to %s for logout" % (client_id, reentry_page))
+            success, found, remaining = True, True, []
+        elif auth_type == AUTH_CERTIFICATE:
             logger.info("no manual cleanup needed for %s session of %s" %
                         (auth_type, identity))
             success, found, remaining = True, True, []
@@ -170,34 +217,13 @@ def main(client_id, user_arguments_dict, environ=None):
             return (output_objects, status)
 
         if success and found and not remaining:
-            if configuration.site_enable_gdp:
-                reentry_page = "https://%s" % configuration.server_fqdn
-                if auth_type == AUTH_OPENID_V2:
-                    if auth_flavor == AUTH_MIG_OID:
-                        reentry_page = configuration.migserver_https_mig_oid_url
-                    elif auth_flavor == AUTH_EXT_OID:
-                        reentry_page = configuration.migserver_https_ext_oid_url
-                elif auth_type == AUTH_OPENID_CONNECT:
-                    if auth_flavor == AUTH_MIG_OIDC:
-                        reentry_page = configuration.migserver_https_mig_oidc_url
-                    elif auth_flavor == AUTH_EXT_OIDC:
-                        reentry_page = configuration.migserver_https_ext_oidc_url
-                project_logout(
-                    configuration, 'https', environ['REMOTE_ADDR'], client_id)
-                html = """
-                <a id='gdp_logout' href='%s'></a>
+            # TODO: switch to 'window.location = reentry_page' ?
+            html = """
+                <a id='finish_logout' href='%s'></a>
                 <script type='text/javascript'>
-                    document.getElementById('gdp_logout').click();
+                    document.getElementById('finish_logout').click();
                 </script>""" % reentry_page
-                output_objects.append(
-                    {'object_type': 'html_form', 'text': html})
-            else:
-                output_objects.append(
-                    {'object_type': 'text', 'text':
-                     """You are now logged out of %s locally - you may want to
-close your web browser to finish""" % configuration.short_title})
-                title_entry['skipmenu'] = True
-
+            output_objects.append({'object_type': 'html_form', 'text': html})
         else:
             logger.error("remaining active sessions for %s: %s" % (identity,
                                                                    remaining))
@@ -218,6 +244,7 @@ def main(client_id, user_arguments_dict, environ=None):
             {'object_type': 'link',
              'destination': 'javascript:history.back();',
              'class': 'genericbutton greenBtn', 'text': "No, go back"})
+
     # sub-container end
     output_objects.append({'object_type': 'html_form', 'text': '''
             </div>

mig/shared/httpsclient.py

@@ -233,34 +233,19 @@ def build_logout_url(configuration, environ):
     (auth_type, auth_flavor) = detect_client_auth(configuration, environ)
     local_logout = '%(SCRIPT_URI)s?logout=true' % environ
     if auth_type == AUTH_OPENID_V2:
+        _logger.debug("logout chaining needed for %s auth" % auth_type)
         # NOTE: OpenID 2.0 logout requires local session database scrubbing.
         #       Logout at provider and return to local logout page for that.
         login = extract_plain_login(configuration, environ)
         logout_base = os.path.dirname(os.path.dirname(login))
         logout_query = urlencode({'return_to': local_logout})
         logout_url = logout_base + '/logout?%s' % logout_query
     elif auth_type == AUTH_OPENID_CONNECT:
-        # NOTE: OpenID Connect module handles chained logout on vanity url.
-        #       Use it to first log out at provider if supported and then
-        #       return for local logout with 2fa session clean up.
-        # TODO: test RP-initiated logout on provider with end_session endpoint!
-        logout_base = '/dynamic/redirect_uri'
-        logout_query = urlencode({'logout': local_logout})
-        logout_url = logout_base + '?%s' % logout_query
-
-        # NOTE: the MicroFocus provider we know does not offer end_session
-        #       so the user currently needs to close browser upon logout.
-        # TODO: can we implement an alterntive explicit logout?
-        # Something like this might be a qualified guess based on
-        # https://www.microfocus.com/documentation/access-manager/developer-documentation-5.0/oauth-application-developer-guide/logout-endpoint.html
-        # if auth_flavor == AUTH_MIG_OIDC:
-        #     logout_base = configuration.migserver_https_mig_oidc_url
-        # elif auth_flavor == AUTH_EXT_OIDC:
-        #     logout_base = configuration.migserver_https_ext_oidc_url
-        # logout_query = urlencode({'post_logout_redirect_uri': local_logout})
-        # logout_url = os.path.join(logout_base, 'nidp/oauth/v1/nam',
-        #                           'end_session?%s' % logout_query)
-
+        _logger.debug("no logout chaining needed for %s auth" % auth_type)
+        logout_url = local_logout
+    elif auth_type == AUTH_CERTIFICATE:
+        _logger.debug("no logout chaining needed for %s auth" % auth_type)
+        logout_url = local_logout
     else:
         _logger.warning("unknown logout chaining for %s" % auth_type)
         logout_url = local_logout