logout: avoid trying to revoke tokens during error handling

on post-access-token-refresh or post-userinfo-refresh-errors logouts

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

ChangeLog

@@ -3,6 +3,7 @@
 - fix default HTTP short retry interval setting
 - accept 0 in OIDCUserInfoRefreshInterval
 - userinfo refresh: don't try to refresh the access token and retry when a connectivity error has occurred
+- logout: don't try to revoke tokens on post-access-token-refresh or post-userinfo-refresh-errors logouts
 
 03/01/2024
 - accept strings as well as integers in the "expires_in" claim from the token endpoint

src/handle/handle.h

@@ -75,7 +75,8 @@ int oidc_jwks_request(request_rec *r, oidc_cfg *c);
 
 // logout.c
 int oidc_logout(request_rec *r, oidc_cfg *c, oidc_session_t *session);
-int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, const char *url);
+int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, const char *url,
+			apr_byte_t revoke_tokens);
 
 // refresh.c
 apr_byte_t oidc_refresh_token_grant(request_rec *r, oidc_cfg *c, oidc_session_t *session, oidc_provider_t *provider,

src/handle/logout.c

@@ -111,7 +111,8 @@ static void oidc_logout_revoke_tokens(request_rec *r, oidc_cfg *c, oidc_session_
 	oidc_debug(r, "leave");
 }
 
-static apr_byte_t oidc_logout_cleanup_by_sid(request_rec *r, char *sid, oidc_cfg *cfg, oidc_provider_t *provider) {
+static apr_byte_t oidc_logout_cleanup_by_sid(request_rec *r, char *sid, oidc_cfg *cfg, oidc_provider_t *provider,
+					     apr_byte_t revoke_tokens) {
 
 	char *uuid = NULL;
 	oidc_session_t session;
@@ -139,7 +140,7 @@ static apr_byte_t oidc_logout_cleanup_by_sid(request_rec *r, char *sid, oidc_cfg
 
 	// revoke tokens if we can get a handle on those
 	if (cfg->session_type != OIDC_SESSION_TYPE_CLIENT_COOKIE) {
-		if (oidc_session_load_cache_by_uuid(r, cfg, uuid, &session) != FALSE)
+		if ((oidc_session_load_cache_by_uuid(r, cfg, uuid, &session) != FALSE) && (revoke_tokens == TRUE))
 			if (oidc_session_extract(r, &session) != FALSE)
 				oidc_logout_revoke_tokens(r, cfg, &session);
 	}
@@ -170,7 +171,8 @@ static apr_byte_t oidc_logout_is_back_channel(const char *logout_param_value) {
 /*
  * handle a local logout
  */
-int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, const char *url) {
+int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, const char *url,
+			apr_byte_t revoke_tokens) {
 
 	int no_session_provided = 1;
 
@@ -179,7 +181,8 @@ int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, co
 	/* if there's no remote_user then there's no (stored) session to kill */
 	if (session->remote_user != NULL) {
 		no_session_provided = 0;
-		oidc_logout_revoke_tokens(r, c, session);
+		if (revoke_tokens)
+			oidc_logout_revoke_tokens(r, c, session);
 	}
 
 	/*
@@ -219,7 +222,7 @@ int oidc_logout_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, co
 						provider = NULL;
 				}
 				if (provider) {
-					oidc_logout_cleanup_by_sid(r, sid, c, provider);
+					oidc_logout_cleanup_by_sid(r, sid, c, provider, revoke_tokens);
 				} else {
 					oidc_info(r, "No provider for front channel logout found");
 				}
@@ -398,7 +401,8 @@ static int oidc_logout_backchannel(request_rec *r, oidc_cfg *cfg) {
 		goto out;
 	}
 
-	oidc_logout_cleanup_by_sid(r, sid, cfg, provider);
+	// a backchannel logout comes from the provider, so no need to revoke the tokens
+	oidc_logout_cleanup_by_sid(r, sid, cfg, provider, FALSE);
 
 	rc = OK;
 
@@ -439,7 +443,7 @@ int oidc_logout(request_rec *r, oidc_cfg *c, oidc_session_t *session) {
 	oidc_debug(r, "enter (url=%s)", url);
 
 	if (oidc_logout_is_front_channel(url)) {
-		return oidc_logout_request(r, c, session, url);
+		return oidc_logout_request(r, c, session, url, TRUE);
 	} else if (oidc_logout_is_back_channel(url)) {
 		return oidc_logout_backchannel(r, c);
 	}
@@ -497,5 +501,5 @@ int oidc_logout(request_rec *r, oidc_cfg *c, oidc_session_t *session) {
 		url = s_logout_request;
 	}
 
-	return oidc_logout_request(r, c, session, url);
+	return oidc_logout_request(r, c, session, url, TRUE);
 }

src/handle/session_management.c

@@ -158,7 +158,7 @@ int oidc_session_management(request_rec *r, oidc_cfg *c, oidc_session_t *session
 		oidc_debug(
 		    r,
 		    "[session=logout] calling oidc_handle_logout_request because of session mgmt local logout call.");
-		return oidc_logout_request(r, c, session, oidc_get_absolute_url(r, c, c->default_slo_url));
+		return oidc_logout_request(r, c, session, oidc_get_absolute_url(r, c, c->default_slo_url), TRUE);
 	}
 
 	if (oidc_get_provider_from_session(r, c, session, &provider) == FALSE) {

src/mod_auth_openidc.c

@@ -1064,8 +1064,8 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg, oidc_sess
 		oidc_debug(r, "dir_action_on_error_refresh: %d", oidc_cfg_dir_action_on_error_refresh(r));
 		OIDC_METRICS_COUNTER_INC(r, cfg, OM_SESSION_ERROR_REFRESH_ACCESS_TOKEN);
 		if (oidc_cfg_dir_action_on_error_refresh(r) == OIDC_ON_ERROR_LOGOUT) {
-			return oidc_logout_request(r, cfg, session,
-						   oidc_get_absolute_url(r, cfg, cfg->default_slo_url));
+			return oidc_logout_request(r, cfg, session, oidc_get_absolute_url(r, cfg, cfg->default_slo_url),
+						   FALSE);
 		}
 		if (oidc_cfg_dir_action_on_error_refresh(r) == OIDC_ON_ERROR_AUTHENTICATE) {
 			oidc_session_kill(r, session);
@@ -1081,8 +1081,8 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg, oidc_sess
 		oidc_debug(r, "action_on_userinfo_error: %d", cfg->action_on_userinfo_error);
 		OIDC_METRICS_COUNTER_INC(r, cfg, OM_SESSION_ERROR_REFRESH_USERINFO);
 		if (cfg->action_on_userinfo_error == OIDC_ON_ERROR_LOGOUT) {
-			return oidc_logout_request(r, cfg, session,
-						   oidc_get_absolute_url(r, cfg, cfg->default_slo_url));
+			return oidc_logout_request(r, cfg, session, oidc_get_absolute_url(r, cfg, cfg->default_slo_url),
+						   FALSE);
 		}
 		if (cfg->action_on_userinfo_error == OIDC_ON_ERROR_AUTHENTICATE) {
 			oidc_session_kill(r, session);