improve handling session duration expiry

when combined with OIDCUnAuthAction or Discovery
also clear r->user in oidc_session_clear for such cases; see #778
bump to 2.4.11rc8

Signed-off-by: Hans Zandbelt <[email protected]>

ChangeLog

@@ -1,3 +1,8 @@
+01/26/2022
+- improve handling session duration expiry when combined with OIDCUnAuthAction or Discovery
+  also clear r->user in oidc_session_clear for such cases; see #778
+- bump to 2.4.11rc8
+
 01/24/2022
 - fix race condition in file cache backend reading truncated files under load; see #777; thanks @dbakker
 - bump to 2.4.11rc7

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([mod_auth_openidc],[2.4.11rc7],[[email protected]])
+AC_INIT([mod_auth_openidc],[2.4.11rc8],[[email protected]])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 

src/mod_auth_openidc.c

@@ -906,8 +906,8 @@ static int oidc_handle_unauthenticated_user(request_rec *r, oidc_cfg *c) {
 /*
  * check if maximum session duration was exceeded
  */
-static int oidc_check_max_session_duration(request_rec *r, oidc_cfg *cfg,
-		oidc_session_t *session) {
+static apr_byte_t oidc_check_max_session_duration(request_rec *r, oidc_cfg *cfg,
+		oidc_session_t *session, int *rc) {
 
 	/* get the session expiry from the session data */
 	apr_time_t session_expires = oidc_session_get_session_expires(r, session);
@@ -917,13 +917,16 @@ static int oidc_check_max_session_duration(request_rec *r, oidc_cfg *cfg,
 		oidc_warn(r, "maximum session duration exceeded for user: %s",
 				session->remote_user);
 		oidc_session_kill(r, session);
-		return oidc_handle_unauthenticated_user(r, cfg);
+		*rc = oidc_handle_unauthenticated_user(r, cfg);
+		return FALSE;
 	}
 
 	/* log message about max session duration */
 	oidc_log_session_expires(r, "session max lifetime", session_expires);
 
-	return OK;
+	*rc = OK;
+
+	return TRUE;
 }
 
 /*
@@ -1385,6 +1388,9 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg,
 		oidc_session_t *session, apr_byte_t *needs_save) {
 
 	apr_byte_t rv = FALSE;
+	int rc = OK;
+	const char *s_claims = NULL;
+	const char *s_id_token = NULL;
 
 	oidc_debug(r, "enter");
 
@@ -1399,13 +1405,23 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg,
 	apr_byte_t pass_base64url = oidc_cfg_dir_pass_info_base64url(r);
 
 	/* verify current cookie domain against issued cookie domain */
-	if (oidc_check_cookie_domain(r, cfg, session) == FALSE)
+	if (oidc_check_cookie_domain(r, cfg, session) == FALSE) {
+		*needs_save = FALSE;
 		return HTTP_UNAUTHORIZED;
+	}
+
+	/*
+	 * we're going to pass the information that we have to the application,
+	 * but first we need to scrub the headers that we're going to use for security reasons
+	 * NB: need it before oidc_check_max_session_duration since OIDCUnAuthAction pass may be set
+	 */
+	oidc_scrub_headers(r);
 
 	/* check if the maximum session duration was exceeded */
-	int rc = oidc_check_max_session_duration(r, cfg, session);
-	if (rc != OK)
+	if (oidc_check_max_session_duration(r, cfg, session, &rc) == FALSE) {
+		*needs_save = FALSE;
 		return rc;
+	}
 
 	/* if needed, refresh the access token */
 	rv = oidc_refresh_access_token_before_expiry(r, cfg, session,
@@ -1423,19 +1439,10 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg,
 	if (oidc_refresh_claims_from_userinfo_endpoint(r, cfg, session) == TRUE)
 		*needs_save = TRUE;
 
-	/*
-	 * we're going to pass the information that we have to the application,
-	 * but first we need to scrub the headers that we're going to use for security reasons
-	 */
-	oidc_scrub_headers(r);
-
 	/* set the user authentication HTTP header if set and required */
 	if ((r->user != NULL) && (authn_header != NULL))
 		oidc_util_hdr_in_set(r, authn_header, r->user);
 
-	const char *s_claims = NULL;
-	const char *s_id_token = NULL;
-
 	/* copy id_token and claims from session to request state and obtain their values */
 	oidc_copy_tokens_to_request_state(r, session, &s_id_token, &s_claims);
 

src/session.c

@@ -99,6 +99,7 @@ static void oidc_session_uuid_new(request_rec *r, oidc_session_t *z) {
  * clear contents of a session
  */
 static void oidc_session_clear(request_rec *r, oidc_session_t *z) {
+	r->user = NULL;
 	z->uuid[0] = '\0';
 	z->remote_user = NULL;
 	// NB: don't clear sid