Skip to content

Commit

Permalink
remove support for OIDCHTMLErrorTemplate
Browse files Browse the repository at this point in the history
deprecated since 2.4.14

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
zandbelt committed Aug 28, 2024
1 parent 0cfe620 commit 9b60dc5
Show file tree
Hide file tree
Showing 12 changed files with 20 additions and 98 deletions.
1 change: 1 addition & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- bump to 2.4.16.2dev
- add some resilience when both Forwarded and X-Forwarded-* are configured
- fix disabled OIDCStateCookiePrefix command; closes #1254; thanks @damisanet
- remove support for OIDCHTMLErrorTemplate, deprecated since 2.4.14

08/26/2024
- fix parsing OIDCXForwardedHeaders; closes #1250; thanks @maltesmann
Expand Down
21 changes: 0 additions & 21 deletions src/cfg/cfg.c
Original file line number Diff line number Diff line change
Expand Up @@ -495,25 +495,6 @@ const char *oidc_cmd_post_preserve_templates_set(cmd_parms *cmd, void *m, const
OIDC_CFG_MEMBER_FUNC_GET(post_preserve_template, const char *)
OIDC_CFG_MEMBER_FUNC_GET(post_restore_template, const char *)

const char *oidc_cmd_html_error_template_set(cmd_parms *cmd, void *ptr, const char *arg) {
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(cmd->server->module_config, &auth_openidc_module);
oidc_swarn(
cmd->server, OIDCHTMLErrorTemplate
" is deprecated; please use the standard Apache features to deal with the " OIDC_ERROR_ENVVAR
" and " OIDC_ERROR_DESC_ENVVAR
" environment variables set by this module, see: https://httpd.apache.org/docs/2.4/custom-error.html");
const char *rv = NULL;
if (_oidc_strcmp(arg, OIDC_HTML_ERROR_TEMPLATE_DEPRECATED) == 0)
cfg->error_template = OIDC_HTML_ERROR_TEMPLATE_DEPRECATED;
else
rv = oidc_cfg_parse_filename(cmd->pool, arg, &cfg->error_template);
return OIDC_CONFIG_DIR_RV(cmd, rv);
}

const char *oidc_cfg_html_error_template_get(oidc_cfg_t *cfg) {
return cfg->error_template;
}

const char *oidc_cmd_ca_bundle_path_set(cmd_parms *cmd, void *ptr, const char *arg) {
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(cmd->server->module_config, &auth_openidc_module);
const char *rv = oidc_cfg_parse_filename(cmd->pool, arg, &cfg->ca_bundle_path);
Expand Down Expand Up @@ -665,7 +646,6 @@ void *oidc_cfg_server_create(apr_pool_t *pool, server_rec *svr) {
c->crypto_passphrase.secret1 = NULL;
c->crypto_passphrase.secret2 = NULL;

c->error_template = NULL;
c->post_preserve_template = NULL;
c->post_restore_template = NULL;

Expand Down Expand Up @@ -802,7 +782,6 @@ void *oidc_cfg_server_merge(apr_pool_t *pool, void *BASE, void *ADD) {
c->crypto_passphrase.secret2 = base->crypto_passphrase.secret2;
}

c->error_template = add->error_template != NULL ? add->error_template : base->error_template;
c->post_preserve_template =
add->post_preserve_template != NULL ? add->post_preserve_template : base->post_preserve_template;
c->post_restore_template =
Expand Down
1 change: 0 additions & 1 deletion src/cfg/cfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
#define OIDCMetadataDir "OIDCMetadataDir"
#define OIDCSessionCacheFallbackToCookie "OIDCSessionCacheFallbackToCookie"
#define OIDCSessionCookieChunkSize "OIDCSessionCookieChunkSize"
#define OIDCHTMLErrorTemplate "OIDCHTMLErrorTemplate"
#define OIDCPreservePostTemplates "OIDCPreservePostTemplates"
#define OIDCProviderMetadataRefreshInterval "OIDCProviderMetadataRefreshInterval"
#define OIDCBlackListedClaims "OIDCBlackListedClaims"
Expand Down
2 changes: 0 additions & 2 deletions src/cfg/cfg_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ struct oidc_cfg_t {
/* (optional) default URL to go to after logout */
char *default_slo_url;

/* HTML to display error messages+description */
char *error_template;
/* Javascript template to preserve POST data */
char *post_preserve_template;
/* Javascript template to restore POST data */
Expand Down
5 changes: 0 additions & 5 deletions src/cfg/cmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,6 @@ const command_rec oidc_cfg_cmds[] = {
OIDCSessionCookieChunkSize,
session_cookie_chunk_size,
"Chunk size for client-cookie session storage type in bytes. Defaults to 4k. Set 0 to suppress chunking."),
OIDC_CFG_CMD(
AP_INIT_TAKE1,
OIDCHTMLErrorTemplate,
html_error_template,
"Name of a HTML error template: needs to contain two \"%s\" characters, one for the error message, one for the description."),
OIDC_CFG_CMD(
AP_INIT_TAKE2,
OIDCPreservePostTemplates,
Expand Down
5 changes: 1 addition & 4 deletions src/handle/authz.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,7 @@ static authz_status oidc_authz_24_unauthorized_user(request_rec *r) {
case OIDC_UNAUTZ_RETURN403:
case OIDC_UNAUTZ_RETURN401:
OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHZ_ACTION_401);
oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Authorization Error",
oidc_cfg_dir_unauthz_arg_get(r), HTTP_UNAUTHORIZED);
if (oidc_cfg_html_error_template_get(c))
r->header_only = 1;
oidc_util_html_send_error(r, "Authorization Error", oidc_cfg_dir_unauthz_arg_get(r), HTTP_UNAUTHORIZED);
return AUTHZ_DENIED;
case OIDC_UNAUTZ_RETURN302:
OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHZ_ACTION_302);
Expand Down
17 changes: 8 additions & 9 deletions src/handle/discovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ int oidc_discovery_request(request_rec *r, oidc_cfg_t *cfg) {
/* get a list of all providers configured in the metadata directory */
apr_array_header_t *arr = NULL;
if (oidc_metadata_list(r, cfg, &arr) == FALSE)
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(cfg), "Configuration Error",
return oidc_util_html_send_error(r, "Configuration Error",
"No configured providers found, contact your administrator",
HTTP_UNAUTHORIZED);

Expand Down Expand Up @@ -324,7 +324,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {

if (target_link_uri == NULL) {
if (oidc_cfg_default_sso_url_get(c) == NULL) {
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Invalid Request",
return oidc_util_html_send_error(r, "Invalid Request",
"SSO to this module without specifying a \"target_link_uri\" "
"parameter is not possible because " OIDCDefaultURL
" is not set.",
Expand All @@ -335,24 +335,23 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {

/* do open redirect prevention, step 1 */
if (oidc_discovery_target_link_uri_match(r, c, target_link_uri) == FALSE) {
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Invalid Request",
return oidc_util_html_send_error(r, "Invalid Request",
"\"target_link_uri\" parameter does not match configuration settings, "
"aborting to prevent an open redirect.",
HTTP_UNAUTHORIZED);
}

/* do input validation on the target_link_uri parameter value, step 2 */
if (oidc_validate_redirect_url(r, c, target_link_uri, TRUE, &error_str, &error_description) == FALSE) {
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), error_str, error_description,
HTTP_UNAUTHORIZED);
return oidc_util_html_send_error(r, error_str, error_description, HTTP_UNAUTHORIZED);
}

/* see if this is a static setup */
if (oidc_cfg_metadata_dir_get(c) == NULL) {
if ((oidc_provider_static_config(r, c, &provider) == TRUE) && (issuer != NULL)) {
if (_oidc_strcmp(oidc_cfg_provider_issuer_get(provider), issuer) != 0) {
return oidc_util_html_send_error(
r, oidc_cfg_html_error_template_get(c), "Invalid Request",
r, "Invalid Request",
apr_psprintf(
r->pool,
"The \"iss\" value must match the configured providers' one (%s != %s).",
Expand All @@ -378,7 +377,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
if (oidc_proto_discovery_url_based(r, c, user, &issuer) == FALSE) {

/* something did not work out, show a user facing error */
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Invalid Request",
return oidc_util_html_send_error(r, "Invalid Request",
"Could not resolve the provided user identifier to an OpenID "
"Connect provider; check your syntax.",
HTTP_NOT_FOUND);
Expand All @@ -398,7 +397,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
if (oidc_proto_discovery_account_based(r, c, issuer, &issuer) == FALSE) {

/* something did not work out, show a user facing error */
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Invalid Request",
return oidc_util_html_send_error(r, "Invalid Request",
"Could not resolve the provided account name to an OpenID "
"Connect provider; check your syntax.",
HTTP_NOT_FOUND);
Expand Down Expand Up @@ -439,7 +438,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
}

/* something went wrong */
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), "Invalid Request",
return oidc_util_html_send_error(r, "Invalid Request",
"Could not find valid provider metadata for the selected OpenID Connect "
"provider; contact the administrator",
HTTP_NOT_FOUND);
Expand Down
3 changes: 1 addition & 2 deletions src/handle/logout.c
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,7 @@ int oidc_logout(request_rec *r, oidc_cfg_t *c, oidc_session_t *session) {

/* do input validation on the logout parameter value */
if (oidc_validate_redirect_url(r, c, url, TRUE, &error_str, &error_description) == FALSE) {
return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c), error_str,
error_description, HTTP_BAD_REQUEST);
return oidc_util_html_send_error(r, error_str, error_description, HTTP_BAD_REQUEST);
}
}

Expand Down
24 changes: 5 additions & 19 deletions src/handle/response.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ static int oidc_response_authorization_error(request_rec *r, oidc_cfg_t *c, oidc
if ((prompt != NULL) && (_oidc_strcmp(prompt, OIDC_PROTO_PROMPT_NONE) == 0)) {
return oidc_response_redirect_parent_window_to_logout(r, c);
}
return oidc_util_html_send_error(
r, oidc_cfg_html_error_template_get(c), apr_psprintf(r->pool, "OpenID Connect Provider error: %s", error),
error_description, oidc_cfg_html_error_template_get(c) ? OK : HTTP_BAD_REQUEST);
return oidc_util_html_send_error(r, apr_psprintf(r->pool, "OpenID Connect Provider error: %s", error),
error_description, HTTP_BAD_REQUEST);
}

/* handle the browser back on an authorization response */
Expand Down Expand Up @@ -380,7 +379,7 @@ static apr_byte_t oidc_response_proto_state_restore(request_rec *r, oidc_cfg_t *
if ((oidc_cfg_default_sso_url_get(c) == NULL) ||
(apr_table_get(r->subprocess_env, "OIDC_NO_DEFAULT_URL_ON_STATE_TIMEOUT") != NULL)) {
oidc_util_html_send_error(
r, oidc_cfg_html_error_template_get(c), "Invalid Authentication Response",
r, "Invalid Authentication Response",
apr_psprintf(r->pool,
"This is due to a timeout; please restart your authentication session by "
"re-entering the URL/bookmark you originally wanted to access: %s",
Expand Down Expand Up @@ -564,18 +563,6 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
oidc_error(r,
"invalid authorization response state and no default SSO URL is set, sending an error...");

if (oidc_cfg_html_error_template_get(c)) {
// retain backwards compatibility
int rc = HTTP_BAD_REQUEST;
if ((r->user) && (_oidc_strncmp(r->user, "", 1) == 0)) {
r->header_only = 1;
r->user = NULL;
rc = OK;
}
OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_STATE_MISMATCH);
return rc;
}

// if error text was already produced (e.g. state timeout) then just return with a 400
if (apr_table_get(r->subprocess_env, OIDC_ERROR_ENVVAR) != NULL) {
OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_STATE_EXPIRED);
Expand All @@ -584,8 +571,7 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *

OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_STATE_MISMATCH);

return oidc_util_html_send_error(r, oidc_cfg_html_error_template_get(c),
"Invalid Authorization Response",
return oidc_util_html_send_error(r, "Invalid Authorization Response",
"Could not match the authorization response to an earlier request via "
"the state parameter and corresponding state cookie",
HTTP_BAD_REQUEST);
Expand Down Expand Up @@ -721,7 +707,7 @@ int oidc_response_authorization_post(request_rec *r, oidc_cfg_t *c, oidc_session
((apr_table_elts(params)->nelts == 1) && apr_table_get(params, OIDC_PROTO_RESPONSE_MODE) &&
(_oidc_strcmp(apr_table_get(params, OIDC_PROTO_RESPONSE_MODE), OIDC_PROTO_RESPONSE_MODE_FRAGMENT) == 0))) {
return oidc_util_html_send_error(
r, oidc_cfg_html_error_template_get(c), "Invalid Request",
r, "Invalid Request",
"You've hit an OpenID Connect Redirect URI with no parameters, this is an invalid request; you "
"should not open this URL in your browser directly, or have the server administrator use a "
"different " OIDCRedirectURI " setting.",
Expand Down
3 changes: 1 addition & 2 deletions src/mod_auth_openidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1504,8 +1504,7 @@ int oidc_handle_redirect_uri_request(request_rec *r, oidc_cfg_t *c, oidc_session

/* something went wrong */
return oidc_util_html_send_error(
r, oidc_cfg_html_error_template_get(c), "Invalid Request",
apr_psprintf(r->pool, "The OpenID Connect callback URL received an invalid request"),
r, "Invalid Request", apr_psprintf(r->pool, "The OpenID Connect callback URL received an invalid request"),
HTTP_INTERNAL_SERVER_ERROR);
}

Expand Down
33 changes: 2 additions & 31 deletions src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1102,8 +1102,6 @@ int oidc_util_html_send(request_rec *r, const char *title, const char *html_head
return oidc_util_http_send(r, html, _oidc_strlen(html), OIDC_HTTP_CONTENT_TYPE_TEXT_HTML, status_code);
}

static char *html_error_template_contents = NULL;

/*
* escape characters in an HTML/Javascript template
*/
Expand Down Expand Up @@ -1145,42 +1143,15 @@ apr_byte_t oidc_util_html_send_in_template(request_rec *r, const char *filename,
/*
* send a user-facing error to the browser
*/
int oidc_util_html_send_error(request_rec *r, const char *html_template, const char *error, const char *description,
int status_code) {

char *html = "";
int rc = status_code;

if (html_template != NULL) {

if (_oidc_strcmp(html_template, OIDC_HTML_ERROR_TEMPLATE_DEPRECATED) != 0) {

rc = oidc_util_html_send_in_template(r, html_template, &html_error_template_contents, error,
OIDC_POST_PRESERVE_ESCAPE_HTML, description,
OIDC_POST_PRESERVE_ESCAPE_HTML, status_code);

} else {

if (error != NULL) {
html = apr_psprintf(r->pool, "%s<p>Error: <pre>%s</pre></p>", html,
oidc_util_html_escape(r->pool, error));
}
if (description != NULL) {
html = apr_psprintf(r->pool, "%s<p>Description: <pre>%s</pre></p>", html,
oidc_util_html_escape(r->pool, description));
}

rc = oidc_util_html_send(r, "Error", NULL, NULL, html, status_code);
}
}
int oidc_util_html_send_error(request_rec *r, const char *error, const char *description, int status_code) {

oidc_debug(r, "setting " OIDC_ERROR_ENVVAR " environment variable to: %s", error);
apr_table_set(r->subprocess_env, OIDC_ERROR_ENVVAR, error ? error : "");

oidc_debug(r, "setting " OIDC_ERROR_DESC_ENVVAR " environment variable to: %s", description);
apr_table_set(r->subprocess_env, OIDC_ERROR_DESC_ENVVAR, description ? description : "");

return rc;
return status_code;
}

/* the maximum size of data that we accept in a single POST value: 1MB */
Expand Down
3 changes: 1 addition & 2 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ int oidc_util_html_send(request_rec *r, const char *title, const char *html_head
apr_byte_t oidc_util_file_read(request_rec *r, const char *path, apr_pool_t *pool, char **result);
apr_byte_t oidc_util_file_write(request_rec *r, const char *path, const char *data);
apr_byte_t oidc_util_issuer_match(const char *a, const char *b);
int oidc_util_html_send_error(request_rec *r, const char *html_template, const char *error, const char *description,
int status_code);
int oidc_util_html_send_error(request_rec *r, const char *error, const char *description, int status_code);
apr_byte_t oidc_util_json_array_has_value(request_rec *r, json_t *haystack, const char *needle);
void oidc_util_set_app_info(request_rec *r, const char *s_key, const char *s_value, const char *claim_prefix,
oidc_appinfo_pass_in_t pass_in, oidc_appinfo_encoding_t encoding);
Expand Down

0 comments on commit 9b60dc5

Please sign in to comment.