Skip to content

Commit

Permalink
restore backwards compatibility: allow parallel refresh token requests
Browse files Browse the repository at this point in the history
assuming static refresh tokens by default; add envvar option
OIDC_PARALLEL_REFRESH_NOT_ALLOWED to prevent that i.e. in case of
rolling refresh tokens; return HTTP 500 on token refresh errors instead
of HTTP 401; bump to 2.4.14.5rc1

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
zandbelt committed Oct 30, 2023
1 parent fd30c4c commit 12394bf
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 19 deletions.
8 changes: 5 additions & 3 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
10/25/2023
10/30/2023
- do not apply logout_on_error and authenticate_on_error when a parallel refresh token request is detected
see https://github.com/OpenIDC/mod_auth_openidc/discussions/1132; thanks @esunke
- add backwards compatibility option to allow parallel refresh token requests using envvar OIDC_PARALLEL_REFRESH_ALLOWED
- bump to 2.4.14.5rc0
- restore backwards compatibility wrt. allowing parallel refresh token requests by default, and add an
option to prevent that (i.e. in case of rolling refresh tokens) using envvar OIDC_PARALLEL_REFRESH_NOT_ALLOWED
- return HTTP 500 on token refresh errors instead of HTTP 401
- bump to 2.4.14.5rc1

10/12/2023
- release 2.4.14.4
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
AC_INIT([mod_auth_openidc],[2.4.14.5rc0],[[email protected]])
AC_INIT([mod_auth_openidc],[2.4.14.5rc1],[[email protected]])

AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())

Expand Down
32 changes: 17 additions & 15 deletions src/mod_auth_openidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ static void oidc_store_userinfo_claims(request_rec *r, oidc_cfg *c,
#define OIDC_REFRESH_ERROR_GENERAL 2
#define OIDC_REFRESH_ERROR_PARALLEL_REFRESH 3

#define OIDC_PARALLEL_REFRESH_ALLOWED_ENVVAR "OIDC_PARALLEL_REFRESH_ALLOWED"
#define OIDC_PARALLEL_REFRESH_NOT_ALLOWED_ENVVAR "OIDC_PARALLEL_REFRESH_NOT_ALLOWED"

/*
* execute refresh token grant to refresh the existing access token
Expand Down Expand Up @@ -1091,14 +1091,17 @@ static apr_byte_t oidc_refresh_token_grant(request_rec *r, oidc_cfg *c,
goto end;
}

// check existing refresh is going on
// check if an existing refresh is going on or if it was just exchanged for a new one in another server
oidc_cache_get_refresh_token(r, refresh_token, &value);
if (value != NULL) {
oidc_warn(r,
"refresh token routine called but existing parallel refresh is in progress");
oidc_debug(r,
"refresh token routine called again within %d seconds for the same refresh token: %s",
c->http_timeout_long, refresh_token);
*error_code = OIDC_REFRESH_ERROR_PARALLEL_REFRESH;
if (apr_table_get(r->subprocess_env,
OIDC_PARALLEL_REFRESH_ALLOWED_ENVVAR) == NULL) {
*error_code = OIDC_REFRESH_ERROR_PARALLEL_REFRESH;
OIDC_PARALLEL_REFRESH_NOT_ALLOWED_ENVVAR) != NULL) {
oidc_warn(r,
"aborting refresh token grant for a refresh token that was already used before");
goto end;
}
}
Expand All @@ -1115,7 +1118,8 @@ static apr_byte_t oidc_refresh_token_grant(request_rec *r, oidc_cfg *c,
oidc_error(r,
"access_token could not be refreshed with refresh_token: %s",
refresh_token);
*error_code = OIDC_REFRESH_ERROR_GENERAL;
if (*error_code != OIDC_REFRESH_ERROR_PARALLEL_REFRESH)
*error_code = OIDC_REFRESH_ERROR_GENERAL;
goto end;
}

Expand Down Expand Up @@ -1753,43 +1757,41 @@ static int oidc_handle_existing_session(request_rec *r, oidc_cfg *cfg,
oidc_cfg_dir_refresh_access_token_before_expiry(r), needs_save,
&error_code);
if (rv == FALSE) {
*needs_save = FALSE;
oidc_debug(r, "dir_action_on_error_refresh: %d",
oidc_cfg_dir_action_on_error_refresh(r));
if (error_code != OIDC_REFRESH_ERROR_PARALLEL_REFRESH) {
if (oidc_cfg_dir_action_on_error_refresh(r) == OIDC_ON_ERROR_LOGOUT) {
*needs_save = FALSE;
return oidc_handle_logout_request(r, cfg, session,
oidc_get_absolute_url(r, cfg, cfg->default_slo_url));
}
if (oidc_cfg_dir_action_on_error_refresh(
r) == OIDC_ON_ERROR_AUTHENTICATE) {
*needs_save = FALSE;
oidc_session_kill(r, session);
return oidc_handle_unauthenticated_user(r, cfg);
}
}
*needs_save = FALSE;
return HTTP_UNAUTHORIZED;
return HTTP_INTERNAL_SERVER_ERROR;
}

/* if needed, refresh claims from the user info endpoint */
rv = oidc_refresh_claims_from_userinfo_endpoint(r, cfg, session, needs_save,
&error_code);
if (rv == FALSE) {
*needs_save = FALSE;
oidc_debug(r, "action_on_userinfo_error: %d",
cfg->action_on_userinfo_error);
if (error_code != OIDC_REFRESH_ERROR_PARALLEL_REFRESH) {
if (cfg->action_on_userinfo_error == OIDC_ON_ERROR_LOGOUT) {
*needs_save = FALSE;
return oidc_handle_logout_request(r, cfg, session,
oidc_get_absolute_url(r, cfg, cfg->default_slo_url));
}
if (cfg->action_on_userinfo_error == OIDC_ON_ERROR_AUTHENTICATE) {
*needs_save = FALSE;
oidc_session_kill(r, session);
return oidc_handle_unauthenticated_user(r, cfg);
}
}
*needs_save = FALSE;
return HTTP_UNAUTHORIZED;
return HTTP_INTERNAL_SERVER_ERROR;
}

/* set the user authentication HTTP header if set and required */
Expand Down

0 comments on commit 12394bf

Please sign in to comment.