accept strings as well as integers in the "expires_in" claim

from the token endpoint; 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 Signed-off-by: Hans Zandbelt <[email protected]>
OpenIDC · Mar 1, 2024 · fff609e · fff609e
1 parent b19bb92
commit fff609e
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 40 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,9 @@
+03/01/2024
+- accept strings as well as integers in the "expires_in" claim from the token endpoint
+- 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
+
 02/29/2024
 - hash the cache key if it is larger than 512 bytes so large cache key entries (i.e. for JWT tokens)
   are no longer a problem in unencrypted SHM cache configs, i.e. the default shared memory cache setup;

diff --git a/src/handle/info.c b/src/handle/info.c
@@ -143,7 +143,7 @@ int oidc_info_request(request_rec *r, oidc_cfg *c, oidc_session_t *session, apr_
 
 	/* include the access token expiry timestamp in the session info */
 	if (apr_hash_get(c->info_hook_data, OIDC_HOOK_INFO_ACCES_TOKEN_EXP, APR_HASH_KEY_STRING)) {
-		const char *access_token_expires = oidc_session_get_access_token_expires(r, session);
+		const char *access_token_expires = oidc_session_get_access_token_expires_str(r, session);
 		if (access_token_expires != NULL)
 			json_object_set_new(json, OIDC_HOOK_INFO_ACCES_TOKEN_EXP, json_string(access_token_expires));
 	}

diff --git a/src/handle/refresh.c b/src/handle/refresh.c
@@ -384,7 +384,6 @@ int oidc_refresh_token_request(request_rec *r, oidc_cfg *c, oidc_session_t *sess
 apr_byte_t oidc_refresh_access_token_before_expiry(request_rec *r, oidc_cfg *cfg, oidc_session_t *session,
 						   int ttl_minimum, apr_byte_t *needs_save) {
 
-	const char *s_access_token_expires = NULL;
 	apr_time_t t_expires = -1;
 	oidc_provider_t *provider = NULL;
 
@@ -393,8 +392,8 @@ apr_byte_t oidc_refresh_access_token_before_expiry(request_rec *r, oidc_cfg *cfg
 	if (ttl_minimum < 0)
 		return TRUE;
 
-	s_access_token_expires = oidc_session_get_access_token_expires(r, session);
-	if (s_access_token_expires == NULL) {
+	t_expires = oidc_session_get_access_token_expires(r, session);
+	if (t_expires == -1) {
 		oidc_debug(r, "no access token expires_in stored in the session (i.e. returned from in the "
 			      "authorization response), so cannot refresh the access token based on TTL requirement");
 		return FALSE;
@@ -406,11 +405,6 @@ apr_byte_t oidc_refresh_access_token_before_expiry(request_rec *r, oidc_cfg *cfg
 		return FALSE;
 	}
 
-	if (sscanf(s_access_token_expires, "%" APR_TIME_T_FMT, &t_expires) != 1) {
-		oidc_error(r, "could not parse s_access_token_expires %s", s_access_token_expires);
-		return FALSE;
-	}
-
 	t_expires = apr_time_from_sec(t_expires - ttl_minimum);
 
 	oidc_debug(r, "refresh needed in: %" APR_TIME_T_FMT " seconds", apr_time_sec(t_expires - apr_time_now()));

diff --git a/src/handle/response.c b/src/handle/response.c
@@ -467,16 +467,6 @@ static apr_byte_t oidc_response_flows(request_rec *r, oidc_cfg *c, oidc_proto_st
 	return rc;
 }
 
-/*
- * parse the expiry for the access token
- */
-static int oidc_response_parse_expires_in(request_rec *r, const char *expires_in) {
-	int number = _oidc_str_to_int(expires_in);
-	if (number <= 0)
-		oidc_warn(r, "could not parse \"expires_in\" value (%s) into a positive integer", expires_in);
-	return number;
-}
-
 /*
  * set the unique user identifier that will be propagated in the Apache r->user and REMOTE_USER variables
  */
@@ -609,7 +599,9 @@ static int oidc_response_process(request_rec *r, oidc_cfg *c, oidc_session_t *se
 		return oidc_response_authorization_error(r, c, proto_state, "No id_token was provided.", NULL);
 	}
 
-	int expires_in = oidc_response_parse_expires_in(r, apr_table_get(params, OIDC_PROTO_EXPIRES_IN));
+	int expires_in = apr_table_get(params, OIDC_PROTO_EXPIRES_IN)
+			     ? _oidc_str_to_int(apr_table_get(params, OIDC_PROTO_EXPIRES_IN))
+			     : -1;
 	char *userinfo_jwt = NULL;
 
 	/*

diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
@@ -776,7 +776,7 @@ apr_byte_t oidc_session_pass_tokens(request_rec *r, oidc_cfg *cfg, oidc_session_
 	}
 
 	/* set the expiry timestamp in the app headers/variables */
-	const char *access_token_expires = oidc_session_get_access_token_expires(r, session);
+	const char *access_token_expires = oidc_session_get_access_token_expires_str(r, session);
 	if ((oidc_cfg_dir_pass_access_token(r) != 0) && access_token_expires != NULL) {
 		/* pass it to the app in a header or environment variable */
 		oidc_util_set_app_info(r, OIDC_APP_INFO_ACCESS_TOKEN_EXP, access_token_expires,
@@ -834,7 +834,7 @@ static apr_byte_t oidc_userinfo_create_signed_jwt(request_rec *r, oidc_cfg *cfg,
 	oidc_jwt_t *jwt = NULL;
 	oidc_jwk_t *jwk = NULL;
 	oidc_jose_error_t err;
-	const char *access_token_expires = NULL;
+	apr_time_t access_token_expires = -1;
 	char *jti = NULL;
 	char *key = NULL;
 	json_t *json = NULL;
@@ -902,10 +902,11 @@ 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 ? _oidc_str_to_int(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 != -1
+				     ? access_token_expires
+				     : apr_time_sec(apr_time_now()) + OIDC_USERINFO_SIGNED_JWT_EXPIRE_DEFAULT));
 	}
 
 	if (oidc_jwt_sign(r->pool, jwt, jwk, FALSE, &err) == FALSE) {

diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
@@ -930,7 +930,8 @@ const char *oidc_session_get_idtoken(request_rec *r, oidc_session_t *z);
 void oidc_session_set_access_token(request_rec *r, oidc_session_t *z, const char *access_token);
 const char *oidc_session_get_access_token(request_rec *r, oidc_session_t *z);
 void oidc_session_set_access_token_expires(request_rec *r, oidc_session_t *z, const int expires_in);
-const char *oidc_session_get_access_token_expires(request_rec *r, oidc_session_t *z);
+apr_time_t oidc_session_get_access_token_expires(request_rec *r, oidc_session_t *z);
+const char *oidc_session_get_access_token_expires_str(request_rec *r, oidc_session_t *z);
 void oidc_session_set_refresh_token(request_rec *r, oidc_session_t *z, const char *refresh_token);
 const char *oidc_session_get_refresh_token(request_rec *r, oidc_session_t *z);
 void oidc_session_set_session_expires(request_rec *r, oidc_session_t *z, const apr_time_t expires);

diff --git a/src/proto.c b/src/proto.c
@@ -1331,6 +1331,19 @@ apr_byte_t oidc_proto_token_endpoint_auth(request_rec *r, oidc_cfg *cfg, const c
 	return FALSE;
 }
 
+/*
+ * parse the expiry for the access token
+ */
+static int oidc_proto_parse_expires_in(request_rec *r, const char *expires_in, const int default_value) {
+	int number = _oidc_str_to_int(expires_in);
+	if (number <= 0) {
+		oidc_warn(r, "could not parse \"expires_in\" value (%s) into a positive integer", expires_in);
+		number = default_value;
+	}
+	oidc_debug(r, "return: %d", number);
+	return number;
+}
+
 /*
  * send a code/refresh request to the token endpoint and return the parsed contents
  */
@@ -1341,6 +1354,7 @@ static apr_byte_t oidc_proto_token_endpoint_request(request_rec *r, oidc_cfg *cf
 	char *response = NULL;
 	char *basic_auth = NULL;
 	char *bearer_auth = NULL;
+	json_t *j_result = NULL, *j_expires_in = NULL;
 
 	/* add the token endpoint authentication credentials */
 	if (oidc_proto_token_endpoint_auth(r, cfg, provider->token_endpoint_auth, provider->client_id,
@@ -1363,18 +1377,17 @@ static apr_byte_t oidc_proto_token_endpoint_request(request_rec *r, oidc_cfg *cf
 	}
 
 	/* check for errors, the response itself will have been logged already */
-	json_t *result = NULL;
-	if (oidc_util_decode_json_and_check_error(r, response, &result) == FALSE)
+	if (oidc_util_decode_json_and_check_error(r, response, &j_result) == FALSE)
 		return FALSE;
 
 	/* get the id_token from the parsed response */
-	oidc_json_object_get_string(r->pool, result, OIDC_PROTO_ID_TOKEN, id_token, NULL);
+	oidc_json_object_get_string(r->pool, j_result, OIDC_PROTO_ID_TOKEN, id_token, NULL);
 
 	/* get the access_token from the parsed response */
-	oidc_json_object_get_string(r->pool, result, OIDC_PROTO_ACCESS_TOKEN, access_token, NULL);
+	oidc_json_object_get_string(r->pool, j_result, OIDC_PROTO_ACCESS_TOKEN, access_token, NULL);
 
 	/* get the token type from the parsed response */
-	oidc_json_object_get_string(r->pool, result, OIDC_PROTO_TOKEN_TYPE, token_type, NULL);
+	oidc_json_object_get_string(r->pool, j_result, OIDC_PROTO_TOKEN_TYPE, token_type, NULL);
 
 	/* check the new token type */
 	if (token_type != NULL) {
@@ -1384,13 +1397,21 @@ static apr_byte_t oidc_proto_token_endpoint_request(request_rec *r, oidc_cfg *cf
 		}
 	}
 
-	/* get the expires_in value */
-	oidc_json_object_get_int(result, OIDC_PROTO_EXPIRES_IN, expires_in, -1);
+	/* get the access token expires_in value */
+	*expires_in = -1;
+	j_expires_in = json_object_get(j_result, OIDC_PROTO_EXPIRES_IN);
+	if (j_expires_in != NULL) {
+		/* cater for string values (old Azure AD) */
+		if (json_is_string(j_expires_in))
+			*expires_in = oidc_proto_parse_expires_in(r, json_string_value(j_expires_in), -1);
+		else if (json_is_integer(j_expires_in))
+			*expires_in = json_integer_value(j_expires_in);
+	}
 
 	/* get the refresh_token from the parsed response */
-	oidc_json_object_get_string(r->pool, result, OIDC_PROTO_REFRESH_TOKEN, refresh_token, NULL);
+	oidc_json_object_get_string(r->pool, j_result, OIDC_PROTO_REFRESH_TOKEN, refresh_token, NULL);
 
-	json_decref(result);
+	json_decref(j_result);
 
 	return TRUE;
 }

diff --git a/src/session.c b/src/session.c
@@ -594,13 +594,18 @@ const char *oidc_session_get_access_token(request_rec *r, oidc_session_t *z) {
  */
 void oidc_session_set_access_token_expires(request_rec *r, oidc_session_t *z, const int expires_in) {
 	if (expires_in != -1) {
-		oidc_session_set(r, z, OIDC_SESSION_KEY_ACCESSTOKEN_EXPIRES,
-				 apr_psprintf(r->pool, "%" APR_TIME_T_FMT, apr_time_sec(apr_time_now()) + expires_in));
+		oidc_debug(r, "storing access token expires_in in the session: %d", expires_in);
+		oidc_session_set_timestamp(r, z, OIDC_SESSION_KEY_ACCESSTOKEN_EXPIRES,
+					   apr_time_now() + apr_time_from_sec(expires_in));
 	}
 }
 
-const char *oidc_session_get_access_token_expires(request_rec *r, oidc_session_t *z) {
-	return oidc_session_get_key2string(r, z, OIDC_SESSION_KEY_ACCESSTOKEN_EXPIRES);
+apr_time_t oidc_session_get_access_token_expires(request_rec *r, oidc_session_t *z) {
+	return apr_time_sec(oidc_session_get_key2timestamp(r, z, OIDC_SESSION_KEY_ACCESSTOKEN_EXPIRES));
+}
+
+const char *oidc_session_get_access_token_expires_str(request_rec *r, oidc_session_t *z) {
+	return apr_psprintf(r->pool, "%" APR_TIME_T_FMT, oidc_session_get_access_token_expires(r, z));
 }
 
 /*