Skip to content

Commit

Permalink
fix session updates on userinfo requests
Browse files Browse the repository at this point in the history
See #1077
The authentication handler does not save session updates on requests to
the redirect URI. Rather than storing the session there, add special
handling for the info request to avoid double session saving on requests
that go through the content handler as well (which  would probably break
client-cookie based sessions anyhow).
This bug was introduced in v2.4.11 with
d9fff15.
Bump version to 2.4.14.3rc2.

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
zandbelt committed Jul 14, 2023
1 parent ce3e831 commit 1dcd911
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 2 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
07/14/2023
- fix session updates on userinfo requests; see https://github.com/OpenIDC/mod_auth_openidc/discussions/1077
this bug was introduced in v2.4.11 with d9fff154ee6ee8a7e4e969dd6a68cbaf18354598
- bump to 2.4.14.3rc2

07/12/2023
- add a sanity alg/enc check on self-encrypted AES GCM JWTs
- add `OIDCPassAccessToken Off` option to disable (the default of) passing the access token and its expiry
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.3rc1],[[email protected]])
AC_INIT([mod_auth_openidc],[2.4.14.3rc2],[[email protected]])

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

Expand Down
8 changes: 7 additions & 1 deletion src/mod_auth_openidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -4139,6 +4139,8 @@ int oidc_handle_redirect_uri_request(request_rec *r, oidc_cfg *c,
// need to establish user/claims for authorization purposes
rc = oidc_handle_existing_session(r, c, session, &needs_save);

apr_pool_userdata_set(oidc_session_copy(r, session), OIDC_USERDATA_SESSION, NULL, r->pool);

if (needs_save)
oidc_request_state_set(r, OIDC_REQUEST_STATE_KEY_SAVE, "");

Expand Down Expand Up @@ -4649,7 +4651,11 @@ int oidc_content_handler(request_rec *r) {
if (oidc_util_request_has_parameter(r,
OIDC_REDIRECT_URI_REQUEST_INFO)) {

oidc_session_load(r, &session);
apr_pool_userdata_get((void**) &session, OIDC_USERDATA_SESSION, r->pool);
if (session == NULL) {
oidc_error(r, "session could not be found in userdata pool");
return HTTP_INTERNAL_SERVER_ERROR;
}

needs_save = (oidc_request_state_get(r, OIDC_REQUEST_STATE_KEY_SAVE)
!= NULL);
Expand Down
2 changes: 2 additions & 0 deletions src/mod_auth_openidc.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ APLOG_USE_MODULE(auth_openidc);

/* the (global) key for the mod_auth_openidc related state that is stored in the request userdata context */
#define OIDC_USERDATA_KEY "mod_auth_openidc_state"
#define OIDC_USERDATA_SESSION "mod_auth_openidc_session"
#define OIDC_USERDATA_POST_PARAMS_KEY "oidc_userdata_post_params"

/* input filter hook name */
Expand Down Expand Up @@ -961,6 +962,7 @@ apr_byte_t oidc_session_free(request_rec *r, oidc_session_t *z);
apr_byte_t oidc_session_extract(request_rec *r, oidc_session_t *z);
apr_byte_t oidc_session_load_cache_by_uuid(request_rec *r, oidc_cfg *c, const char *uuid, oidc_session_t *z);
void oidc_session_id_new(request_rec *r, oidc_session_t *z);
oidc_session_t *oidc_session_copy(request_rec *r, oidc_session_t *z);

void oidc_session_set_userinfo_jwt(request_rec *r, oidc_session_t *z, const char *userinfo_jwt);
const char * oidc_session_get_userinfo_jwt(request_rec *r, oidc_session_t *z);
Expand Down
15 changes: 15 additions & 0 deletions src/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ static void oidc_session_clear(request_rec *r, oidc_session_t *z) {
}
}

oidc_session_t *oidc_session_copy(request_rec *r, oidc_session_t *z) {
oidc_session_t *zz = apr_pcalloc(r->pool, sizeof(oidc_session_t));
oidc_session_clear(r, zz);
zz->expiry = z->expiry;
if (z->remote_user)
zz->remote_user = apr_pstrdup(r->pool, z->remote_user);
if (z->sid)
zz->sid = apr_pstrdup(r->pool, z->sid);
if (z->state)
zz->state = json_deep_copy(z->state);
if (z->uuid)
zz->uuid = apr_pstrdup(r->pool, z->uuid);
return zz;
}

apr_byte_t oidc_session_load_cache_by_uuid(request_rec *r, oidc_cfg *c,
const char *uuid, oidc_session_t *z) {
const char *stored_uuid = NULL;
Expand Down

0 comments on commit 1dcd911

Please sign in to comment.