diff --git a/ChangeLog b/ChangeLog index 4ead91e8..3b7c153f 100644 --- 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 index feeffd77..a982c7df 100644 --- 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 index 68211b1b..f721ddd6 100644 --- 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 index d1e6b0d4..ab4b6aef 100644 --- 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 index 3383fdc8..4e557e85 100644 --- 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) {