fix setting the "exp" claim in userinfo signed JWTs

- exp would be now + 0 - fix caching when ttl set to 0 or "" so the "exp" claim should be used Signed-off-by: Hans Zandbelt <hans.zandbelt@openidc.com>
OpenIDC · Mar 1, 2024 · 2a1c81c · 2a1c81c
1 parent 8a3d71e
commit 2a1c81c
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 30 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -3,6 +3,7 @@
 - fix for "expires_in" string values returned from the token endpoint that would be interpreted as 0
   this fixes using OIDCRefreshAccessTokenBeforeExpiry or OIDCUserInfoRefreshInterval with (older)
   Azure AD configs that would result in a token refresh on every request since 2.4.15 or a 401 in 2.4.14.4
+- fix setting the "exp" claim in userinfo signed JWTs (exp would be now+0) and caching when ttl set to 0 or ""
 
 02/29/2024
 - hash the cache key if it is larger than 512 bytes so large cache key entries (i.e. for JWT tokens)

diff --git a/auth_openidc.conf b/auth_openidc.conf
@@ -807,7 +807,7 @@
 #    - the "expires_in" hint from the access_token is used in the "exp" claim; defaults to 60 seconds if not returned by the OP.
 #    - caching of the signed JWT - use with care only - can be configured using:
 #        SetEnvIfExpr true OIDC_USERINFO_SIGNED_JWT_CACHE_TTL=<seconds>
-#      or for the duration of the - possibly processed - "exp" claim when set to ""
+#      or for the duration of the - possibly processed - "exp" claim when set to "0"
 # Can be configured on a per Directory/Location basis. When not defined the default "claims" is used.. 
 #OIDCPassUserInfoAs [claims|json[:<name>]|jwt[:<name>]|signed_jwt[:<name>]]+
 

diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
@@ -812,20 +812,13 @@ apr_byte_t oidc_session_pass_tokens(request_rec *r, oidc_cfg *cfg, oidc_session_
 	return TRUE;
 }
 
-#define OIDC_USERINFO_SIGNED_JWT_EXPIRE_DEFAULT 0
+#define OIDC_USERINFO_SIGNED_JWT_EXP_DEFAULT 60
+#define OIDC_USERINFO_SIGNED_JWT_CACHE_TTL_DEFAULT -1
 #define OIDC_USERINFO_SIGNED_JWT_CACHE_TTL_ENVVAR "OIDC_USERINFO_SIGNED_JWT_CACHE_TTL"
 
 static int oidc_userinfo_signed_jwt_cache_ttl(request_rec *r) {
 	const char *s_ttl = apr_table_get(r->subprocess_env, OIDC_USERINFO_SIGNED_JWT_CACHE_TTL_ENVVAR);
-	return (s_ttl ? _oidc_str_to_int(s_ttl) : OIDC_USERINFO_SIGNED_JWT_EXPIRE_DEFAULT);
-}
-
-#define OIDC_JQ_FILTER_EXPIRE_DEFAULT 600
-#define OIDC_JQ_FILTER_CACHE_TTL_ENVVAR "OIDC_JQ_FILTER_CACHE_TTL"
-
-int oidc_jq_filter_cache_ttl(request_rec *r) {
-	const char *s_ttl = apr_table_get(r->subprocess_env, OIDC_JQ_FILTER_CACHE_TTL_ENVVAR);
-	return (s_ttl ? _oidc_str_to_int(s_ttl) : OIDC_JQ_FILTER_EXPIRE_DEFAULT);
+	return (s_ttl ? _oidc_str_to_int(s_ttl) : OIDC_USERINFO_SIGNED_JWT_CACHE_TTL_DEFAULT);
 }
 
 static apr_byte_t oidc_userinfo_create_signed_jwt(request_rec *r, oidc_cfg *cfg, oidc_session_t *session,
@@ -883,7 +876,7 @@ static apr_byte_t oidc_userinfo_create_signed_jwt(request_rec *r, oidc_cfg *cfg,
 	}
 
 	ttl = oidc_userinfo_signed_jwt_cache_ttl(r);
-	if (ttl != 0)
+	if (ttl > -1)
 		oidc_cache_get_signed_jwt(r, key, cser);
 
 	if (*cser != NULL) {
@@ -902,11 +895,10 @@ static apr_byte_t oidc_userinfo_create_signed_jwt(request_rec *r, oidc_cfg *cfg,
 	}
 	if (json_object_get(jwt->payload.value.json, OIDC_CLAIM_EXP) == NULL) {
 		access_token_expires = oidc_session_get_access_token_expires(r, session);
-		json_object_set_new(
-		    jwt->payload.value.json, OIDC_CLAIM_EXP,
-		    json_integer(access_token_expires > 0
-				     ? access_token_expires
-				     : apr_time_sec(apr_time_now()) + OIDC_USERINFO_SIGNED_JWT_EXPIRE_DEFAULT));
+		json_object_set_new(jwt->payload.value.json, OIDC_CLAIM_EXP,
+				    json_integer(access_token_expires > 0 ? access_token_expires
+									  : apr_time_sec(apr_time_now()) +
+										OIDC_USERINFO_SIGNED_JWT_EXP_DEFAULT));
 	}
 
 	if (oidc_jwt_sign(r->pool, jwt, jwk, FALSE, &err) == FALSE) {
@@ -920,19 +912,24 @@ static apr_byte_t oidc_userinfo_create_signed_jwt(request_rec *r, oidc_cfg *cfg,
 		goto end;
 	}
 
-	if (ttl != 0) {
-		if (apr_table_get(r->subprocess_env, OIDC_USERINFO_SIGNED_JWT_CACHE_TTL_ENVVAR) == NULL) {
-			oidc_json_object_get_int(jwt->payload.value.json, OIDC_CLAIM_EXP, &exp, 0);
-			if (exp != 0)
-				expiry = apr_time_from_sec(exp);
-		}
-		if (expiry == 0)
-			expiry = apr_time_now() + apr_time_from_sec(ttl);
-		oidc_debug(r, "caching signed JWT with ~ttl(%ld)", apr_time_sec(expiry - apr_time_now()));
-		oidc_cache_set_signed_jwt(r, key, *cser, expiry);
+	rv = TRUE;
+
+	if (ttl < 0)
+		goto end;
+
+	if (ttl == 0) {
+		// need to get the cache ttl from the exp claim
+		oidc_json_object_get_int(jwt->payload.value.json, OIDC_CLAIM_EXP, &exp, 0);
+		// actually the exp claim always exists by now
+		expiry = (exp > 0) ? apr_time_from_sec(exp)
+				   : apr_time_now() + apr_time_from_sec(OIDC_USERINFO_SIGNED_JWT_EXP_DEFAULT);
+	} else {
+		// ttl > 0
+		expiry = apr_time_now() + apr_time_from_sec(ttl);
 	}
 
-	rv = TRUE;
+	oidc_debug(r, "caching signed JWT with ~ttl(%ld)", apr_time_sec(expiry - apr_time_now()));
+	oidc_cache_set_signed_jwt(r, key, *cser, expiry);
 
 end:
 

diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
@@ -818,7 +818,6 @@ int oidc_cfg_delete_oldest_state_cookies(oidc_cfg *cfg);
 oidc_provider_t *oidc_cfg_provider_create(apr_pool_t *pool);
 oidc_provider_t *oidc_cfg_provider_copy(apr_pool_t *pool, const oidc_provider_t *src);
 void oidc_config_check_x_forwarded(request_rec *r, const apr_byte_t x_forwarded_headers);
-int oidc_jq_filter_cache_ttl(request_rec *r);
 
 // oidc_util.c
 apr_byte_t oidc_util_random_bytes(unsigned char *buf, apr_size_t length);

diff --git a/src/util.c b/src/util.c
@@ -1986,6 +1986,14 @@ static const char *oidc_util_jq_exec(request_rec *r, jq_state *jq, struct jv_par
 	return rv;
 }
 
+#define OIDC_JQ_FILTER_EXPIRE_DEFAULT 600
+#define OIDC_JQ_FILTER_CACHE_TTL_ENVVAR "OIDC_JQ_FILTER_CACHE_TTL"
+
+static int oidc_jq_filter_cache_ttl(request_rec *r) {
+	const char *s_ttl = apr_table_get(r->subprocess_env, OIDC_JQ_FILTER_CACHE_TTL_ENVVAR);
+	return (s_ttl ? _oidc_str_to_int(s_ttl) : OIDC_JQ_FILTER_EXPIRE_DEFAULT);
+}
+
 #endif
 
 const char *oidc_util_jq_filter(request_rec *r, const char *input, const char *filter) {
@@ -2011,7 +2019,7 @@ const char *oidc_util_jq_filter(request_rec *r, const char *input, const char *f
 	oidc_debug(r, "processing filter: %s", filter);
 
 	ttl = oidc_jq_filter_cache_ttl(r);
-	if (ttl != 0) {
+	if (ttl > 0) {
 		if (oidc_util_hash_string_and_base64url_encode(
 			r, OIDC_JOSE_ALG_SHA256, apr_pstrcat(r->pool, input, filter, NULL), &key) == FALSE) {
 			oidc_error(r, "oidc_util_hash_string_and_base64url_encode returned an error");