Skip to content

Commit 0d51b38

Browse files
committed
make OIDCUnautzAction 302|auth work with RequireAny
handle 302/auth in the fixup handler so step up authentication works with multiple/complex Require expressions e.g. RequireAny and Require not; bump to 2.4.14.1rc2 Signed-off-by: Hans Zandbelt <hans.zandbelt@openidc.com>
1 parent 462f97a commit 0d51b38

File tree

5 files changed

+67
-29
lines changed

5 files changed

+67
-29
lines changed

ChangeLog

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
05/24/2023
22
- fix RequireAny behaviour on 401/403/302: revert 9d6192b2ab0716d8f7d2a29754a80b6ab1e804eb for non-stepup authentication cases
3-
- bump to 2.4.14.1rc1
3+
- make OIDCUnautzAction 302|auth (i.e. step up authentication) work with multiple/complex Require expressions e.g. RequireAny
4+
- bump to 2.4.14.1rc2
45

56
05/17/2023
67
- fix refreshing claims from the userinfo endpoint when no id_token claims are stored in the session

configure.ac

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
AC_INIT([mod_auth_openidc],[2.4.14.1rc1],[hans.zandbelt@openidc.com])
1+
AC_INIT([mod_auth_openidc],[2.4.14.1rc2],[hans.zandbelt@openidc.com])
22

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

src/config.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3042,6 +3042,7 @@ void oidc_register_hooks(apr_pool_t *pool) {
30423042
ap_hook_check_user_id(oidc_check_user_id, NULL, NULL, APR_HOOK_MIDDLE);
30433043
ap_hook_auth_checker(oidc_auth_checker, NULL, authzSucc, APR_HOOK_MIDDLE);
30443044
#endif
3045+
ap_hook_fixups(oidc_fixup_handler, NULL, NULL, APR_HOOK_MIDDLE);
30453046
}
30463047

30473048
/*

src/mod_auth_openidc.c

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4388,21 +4388,16 @@ static authz_status oidc_handle_unauthorized_user24(request_rec *r) {
43884388
switch (oidc_dir_cfg_unautz_action(r)) {
43894389
case OIDC_UNAUTZ_RETURN403:
43904390
case OIDC_UNAUTZ_RETURN401:
4391-
oidc_util_html_send_error(r, c->error_template, "Authorization Error",
4392-
oidc_dir_cfg_unauthz_arg(r),
4393-
HTTP_UNAUTHORIZED);
4394-
if (c->error_template)
4395-
r->header_only = 1;
4391+
oidc_request_state_set(r, OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_MSG,
4392+
oidc_dir_cfg_unauthz_arg(r) ? oidc_dir_cfg_unauthz_arg(r) : "");
4393+
oidc_debug(r,
4394+
"defer authorization error handling to the fixup handler");
43964395
return AUTHZ_DENIED;
43974396
case OIDC_UNAUTZ_RETURN302:
4398-
html_head = apr_psprintf(r->pool,
4399-
"<meta http-equiv=\"refresh\" content=\"0; url=%s\">",
4400-
oidc_dir_cfg_unauthz_arg(r));
4401-
oidc_util_html_send(r, "Authorization Error Redirect", html_head, NULL,
4402-
NULL,
4403-
HTTP_UNAUTHORIZED);
4404-
if (c->error_template)
4405-
r->header_only = 1;
4397+
oidc_request_state_set(r, OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT,
4398+
oidc_dir_cfg_unauthz_arg(r) ? oidc_dir_cfg_unauthz_arg(r) : "");
4399+
oidc_debug(r,
4400+
"defer authorization error redirect to the fixup handler");
44064401
return AUTHZ_DENIED;
44074402
case OIDC_UNAUTZ_AUTHENTICATE:
44084403
/*
@@ -4429,8 +4424,8 @@ static authz_status oidc_handle_unauthorized_user24(request_rec *r) {
44294424
if (location != NULL) {
44304425
oidc_request_state_set(r, OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT,
44314426
location);
4432-
oidc_debug(r, "defer step up authentication to the content handler");
4433-
return AUTHZ_GRANTED;
4427+
oidc_debug(r, "defer step up authentication to the fixup handler");
4428+
return AUTHZ_DENIED;
44344429
}
44354430

44364431
return AUTHZ_DENIED;
@@ -4612,6 +4607,7 @@ apr_byte_t oidc_enabled(request_rec *r) {
46124607

46134608
return FALSE;
46144609
}
4610+
46154611
/*
46164612
* handle content generating requests
46174613
*/
@@ -4663,18 +4659,56 @@ int oidc_content_handler(request_rec *r) {
46634659

46644660
/* sending POST preserve */
46654661
rc = OK;
4662+
}
46664663

4667-
} else if (oidc_request_state_get(r,
4668-
OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT) != NULL) {
4664+
return rc;
4665+
}
4666+
4667+
int oidc_fixup_handler(request_rec *r) {
4668+
oidc_cfg *c = ap_get_module_config(r->server->module_config,
4669+
&auth_openidc_module);
4670+
int rc = DECLINED;
4671+
const char *location = NULL;
4672+
const char *msg = NULL;
46694673

4670-
/* there was an authorization error, redirect for stepup authentication */
4671-
oidc_util_hdr_out_location_set(r, oidc_request_state_get(r,
4672-
OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT));
4674+
oidc_debug(r, "enter: status=%d (%s)", r->status,
4675+
r->status_line ? r->status_line : "");
46734676

4674-
rc = HTTP_MOVED_TEMPORARILY;
4677+
/* this handler is (only) used to deal with the overall authorization result of RequireAll/RequireAny etc. */
4678+
if ((r->status != 401) && (r->status != 403))
4679+
goto end;
46754680

4681+
msg = oidc_request_state_get(r,
4682+
OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_MSG);
4683+
if (msg) {
4684+
oidc_util_html_send_error(r, c->error_template, "Authorization Error",
4685+
msg, HTTP_UNAUTHORIZED);
4686+
r->status =
4687+
(oidc_dir_cfg_unautz_action(r) == OIDC_UNAUTZ_RETURN403) ?
4688+
HTTP_FORBIDDEN : HTTP_UNAUTHORIZED;
4689+
;
4690+
if (c->error_template) {
4691+
rc = r->status;
4692+
} else {
4693+
/* need OK as we are processing internal 401 and a document that is already written */
4694+
rc = OK;
4695+
}
4696+
goto end;
46764697
}
46774698

4699+
location = oidc_request_state_get(r,
4700+
OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT);
4701+
if (location != NULL) {
4702+
/* there was an authorization error, redirect for an external error page or step up authentication */
4703+
apr_table_set(r->headers_out, OIDC_HTTP_HDR_LOCATION, location);
4704+
r->status = HTTP_MOVED_TEMPORARILY;
4705+
/* need OK as we are processing internal 401 and a document that is already written */
4706+
rc = OK;
4707+
goto end;
4708+
}
4709+
4710+
end:
4711+
46784712
return rc;
46794713
}
46804714

src/mod_auth_openidc.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,13 @@ APLOG_USE_MODULE(auth_openidc);
112112
#endif
113113

114114
/* keys for storing info in the request state */
115-
#define OIDC_REQUEST_STATE_KEY_IDTOKEN "i"
116-
#define OIDC_REQUEST_STATE_KEY_CLAIMS "c"
117-
#define OIDC_REQUEST_STATE_KEY_DISCOVERY "d"
118-
#define OIDC_REQUEST_STATE_KEY_AUTHN "a"
119-
#define OIDC_REQUEST_STATE_KEY_SAVE "s"
120-
#define OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT "ar"
115+
#define OIDC_REQUEST_STATE_KEY_IDTOKEN "i"
116+
#define OIDC_REQUEST_STATE_KEY_CLAIMS "c"
117+
#define OIDC_REQUEST_STATE_KEY_DISCOVERY "d"
118+
#define OIDC_REQUEST_STATE_KEY_AUTHN "a"
119+
#define OIDC_REQUEST_STATE_KEY_SAVE "s"
120+
#define OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_MSG "am"
121+
#define OIDC_REQUEST_STATE_KEY_AUTHZ_ERR_REDIRECT "ar"
121122

122123
/* parameter name of the callback URL in the discovery response */
123124
#define OIDC_DISC_CB_PARAM "oidc_callback"
@@ -507,6 +508,7 @@ apr_byte_t oidc_post_preserve_javascript(request_rec *r, const char *location, c
507508
void oidc_scrub_headers(request_rec *r);
508509
void oidc_strip_cookies(request_rec *r);
509510
int oidc_content_handler(request_rec *r);
511+
int oidc_fixup_handler(request_rec *r);
510512
apr_byte_t oidc_get_remote_user(request_rec *r, const char *claim_name, const char *replace, const char *reg_exp,
511513
json_t *json, char **request_user);
512514

0 commit comments

Comments
 (0)