fix userinfo refresh interval parsing; closes #1200; thanks @HolgerHees

avoid refreshing userinfo on each request until access token expiry store interval as JSON integer in session Signed-off-by: Hans Zandbelt <[email protected]>
OpenIDC · Mar 14, 2024 · bc447a8 · bc447a8
1 parent 28bda50
commit bc447a8
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 19 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,8 @@
+03/14/2024
+- fix userinfo refresh interval parsing; closes #1200; thanks @HolgerHees
+  avoid refreshing userinfo on each request until access token expiry
+- store interval as JSON integer in session
+
 03/13/2024
 - fix compilation without libhiredis; closes #1195 ; thanks @HolgerHees
   conditionally define oidc_set_redis_connect_timeout

diff --git a/src/config.c b/src/config.c
@@ -1450,6 +1450,9 @@ static void oidc_cfg_provider_init(oidc_provider_t *provider) {
 	provider->userinfo_encrypted_response_enc = NULL;
 	provider->userinfo_token_method = OIDC_USER_INFO_TOKEN_METHOD_HEADER;
 	provider->auth_request_method = OIDC_DEFAULT_AUTH_REQUEST_METHOD;
+
+	provider->userinfo_refresh_interval = OIDC_DEFAULT_USERINFO_REFRESH_INTERVAL;
+	provider->request_object = NULL;
 }
 
 oidc_provider_t *oidc_cfg_provider_create(apr_pool_t *pool) {
@@ -1696,9 +1699,6 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
 	c->post_preserve_template = NULL;
 	c->post_restore_template = NULL;
 
-	c->provider.userinfo_refresh_interval = OIDC_DEFAULT_USERINFO_REFRESH_INTERVAL;
-	c->provider.request_object = NULL;
-
 	c->provider_metadata_refresh_interval = OIDC_DEFAULT_PROVIDER_METADATA_REFRESH_INTERVAL;
 
 	c->info_hook_data = NULL;

diff --git a/src/handle/userinfo.c b/src/handle/userinfo.c
@@ -74,7 +74,7 @@ void oidc_userinfo_store_claims(request_rec *r, oidc_cfg *c, oidc_session_t *ses
 	}
 
 	/* store the last refresh time if we've configured a userinfo refresh interval */
-	if (provider->userinfo_refresh_interval > 0)
+	if (provider->userinfo_refresh_interval > -1)
 		oidc_session_reset_userinfo_last_refresh(r, session);
 }
 
@@ -180,10 +180,10 @@ apr_byte_t oidc_userinfo_refresh_claims(request_rec *r, oidc_cfg *cfg, oidc_sess
 	const char *access_token = NULL;
 	char *userinfo_jwt = NULL;
 
-	/* see if we can do anything here, i.e. a refresh interval is configured */
-	apr_time_t interval = oidc_session_get_userinfo_refresh_interval(r, session);
+	/* see int we can do anything here, i.e. a refresh interval is configured */
+	int interval = oidc_session_get_userinfo_refresh_interval(r, session);
 
-	oidc_debug(r, "interval=%" APR_TIME_T_FMT, apr_time_sec(interval));
+	oidc_debug(r, "interval=%d", interval);
 
 	if (interval > -1) {
 

diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
@@ -937,7 +937,7 @@ void oidc_session_set_cookie_domain(request_rec *r, oidc_session_t *z, const cha
 const char *oidc_session_get_cookie_domain(request_rec *r, oidc_session_t *z);
 void oidc_session_reset_userinfo_last_refresh(request_rec *r, oidc_session_t *z);
 void oidc_session_set_userinfo_refresh_interval(request_rec *r, oidc_session_t *z, const int interval);
-apr_time_t oidc_session_get_userinfo_refresh_interval(request_rec *r, oidc_session_t *z);
+int oidc_session_get_userinfo_refresh_interval(request_rec *r, oidc_session_t *z);
 apr_time_t oidc_session_get_userinfo_last_refresh(request_rec *r, oidc_session_t *z);
 void oidc_session_set_access_token_last_refresh(request_rec *r, oidc_session_t *z, apr_time_t ts);
 apr_time_t oidc_session_get_access_token_last_refresh(request_rec *r, oidc_session_t *z);

diff --git a/src/session.c b/src/session.c
@@ -252,6 +252,12 @@ static apr_byte_t oidc_session_save_cookie(request_rec *r, oidc_session_t *z, ap
 	return TRUE;
 }
 
+static inline int oidc_session_get_int(request_rec *r, oidc_session_t *z, const char *key, int def_val) {
+	int v;
+	oidc_json_object_get_int(z->state, key, &v, def_val);
+	return v;
+}
+
 static inline apr_time_t oidc_session_get_key2timestamp(request_rec *r, oidc_session_t *z, const char *key) {
 	int value = -1;
 	oidc_json_object_get_int(z->state, key, &value, -1);
@@ -317,12 +323,15 @@ apr_byte_t oidc_session_load(request_rec *r, oidc_session_t **zz) {
 	return rc;
 }
 
+static void oidc_session_set_int(request_rec *r, oidc_session_t *z, const char *key, int v) {
+	if (z->state == NULL)
+		z->state = json_object();
+	json_object_set_new(z->state, key, json_integer(v));
+}
+
 static void oidc_session_set_timestamp(request_rec *r, oidc_session_t *z, const char *key, const apr_time_t timestamp) {
-	if (timestamp > -1) {
-		if (z->state == NULL)
-			z->state = json_object();
-		json_object_set_new(z->state, key, json_integer(apr_time_sec(timestamp)));
-	}
+	if (timestamp > -1)
+		oidc_session_set_int(r, z, key, apr_time_sec(timestamp));
 }
 
 /*
@@ -645,14 +654,11 @@ const char *oidc_session_get_cookie_domain(request_rec *r, oidc_session_t *z) {
  */
 
 void oidc_session_set_userinfo_refresh_interval(request_rec *r, oidc_session_t *z, const int interval) {
-	// avoid apr_time_from_sec calculation on -1
-	if (interval > -1)
-		oidc_session_set_timestamp(r, z, OIDC_SESSION_KEY_USERINFO_REFRESH_INTERVAL,
-					   apr_time_from_sec(interval));
+	oidc_session_set_int(r, z, OIDC_SESSION_KEY_USERINFO_REFRESH_INTERVAL, interval);
 }
 
-apr_time_t oidc_session_get_userinfo_refresh_interval(request_rec *r, oidc_session_t *z) {
-	return oidc_session_get_key2timestamp(r, z, OIDC_SESSION_KEY_USERINFO_REFRESH_INTERVAL);
+int oidc_session_get_userinfo_refresh_interval(request_rec *r, oidc_session_t *z) {
+	return oidc_session_get_int(r, z, OIDC_SESSION_KEY_USERINFO_REFRESH_INTERVAL, -1);
 }
 
 void oidc_session_reset_userinfo_last_refresh(request_rec *r, oidc_session_t *z) {