request: factor out cookie domain check

Signed-off-by: Hans Zandbelt <[email protected]>
OpenIDC · Mar 3, 2024 · 41bbf40 · 41bbf40
1 parent 505f501
commit 41bbf40
Showing 1 changed file with 55 additions and 44 deletions.
diff --git a/src/handle/request.c b/src/handle/request.c
@@ -43,6 +43,58 @@
 #include "handle/handle.h"
 #include "metrics.h"
 
+static int oidc_request_check_cookie_domain(request_rec *r, oidc_cfg *c, oidc_proto_state_t *proto_state,
+					    const char *original_url) {
+	/*
+	 * printout errors if Cookie settings are not going to work
+	 * TODO: separate this code out into its own function
+	 */
+	apr_uri_t o_uri;
+	_oidc_memset(&o_uri, 0, sizeof(apr_uri_t));
+	apr_uri_t r_uri;
+	_oidc_memset(&r_uri, 0, sizeof(apr_uri_t));
+	apr_uri_parse(r->pool, original_url, &o_uri);
+	apr_uri_parse(r->pool, oidc_get_redirect_uri(r, c), &r_uri);
+	if ((_oidc_strcmp(o_uri.scheme, r_uri.scheme) != 0) && (_oidc_strcmp(r_uri.scheme, "https") == 0)) {
+		oidc_error(r,
+			   "the URL scheme (%s) of the configured " OIDCRedirectURI
+			   " does not match the URL scheme of the URL being accessed (%s): the \"state\" and "
+			   "\"session\" cookies will not be shared between the two!",
+			   r_uri.scheme, o_uri.scheme);
+		oidc_proto_state_destroy(proto_state);
+		return HTTP_INTERNAL_SERVER_ERROR;
+	}
+
+	if (c->cookie_domain == NULL) {
+		if (_oidc_strcmp(o_uri.hostname, r_uri.hostname) != 0) {
+			char *p = _oidc_strstr(o_uri.hostname, r_uri.hostname);
+			if ((p == NULL) || (_oidc_strcmp(r_uri.hostname, p) != 0)) {
+				oidc_error(r,
+					   "the URL hostname (%s) of the configured " OIDCRedirectURI
+					   " does not match the URL hostname of the URL being accessed (%s): the "
+					   "\"state\" and \"session\" cookies will not be shared between the two!",
+					   r_uri.hostname, o_uri.hostname);
+				oidc_proto_state_destroy(proto_state);
+				OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
+				return HTTP_INTERNAL_SERVER_ERROR;
+			}
+		}
+	} else {
+		if (!oidc_util_cookie_domain_valid(r_uri.hostname, c->cookie_domain)) {
+			oidc_error(r,
+				   "the domain (%s) configured in " OIDCCookieDomain
+				   " does not match the URL hostname (%s) of the URL being accessed (%s): setting "
+				   "\"state\" and \"session\" cookies will not work!!",
+				   c->cookie_domain, o_uri.hostname, original_url);
+			oidc_proto_state_destroy(proto_state);
+			OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
+			return HTTP_INTERNAL_SERVER_ERROR;
+		}
+	}
+
+	return OK;
+}
+
 /*
  * set the state that is maintained between an authorization request and an authorization response
  * in a cookie in the browser that is cryptographically bound to that state
@@ -195,51 +247,10 @@ int oidc_request_authenticate_user(request_rec *r, oidc_cfg *c, oidc_provider_t
 		return rc;
 	}
 
-	/*
-	 * printout errors if Cookie settings are not going to work
-	 * TODO: separate this code out into its own function
-	 */
-	apr_uri_t o_uri;
-	_oidc_memset(&o_uri, 0, sizeof(apr_uri_t));
-	apr_uri_t r_uri;
-	_oidc_memset(&r_uri, 0, sizeof(apr_uri_t));
-	apr_uri_parse(r->pool, original_url, &o_uri);
-	apr_uri_parse(r->pool, oidc_get_redirect_uri(r, c), &r_uri);
-	if ((_oidc_strcmp(o_uri.scheme, r_uri.scheme) != 0) && (_oidc_strcmp(r_uri.scheme, "https") == 0)) {
-		oidc_error(r,
-			   "the URL scheme (%s) of the configured " OIDCRedirectURI
-			   " does not match the URL scheme of the URL being accessed (%s): the \"state\" and "
-			   "\"session\" cookies will not be shared between the two!",
-			   r_uri.scheme, o_uri.scheme);
+	rc = oidc_request_check_cookie_domain(r, c, proto_state, original_url);
+	if (rc != OK) {
 		oidc_proto_state_destroy(proto_state);
-		return HTTP_INTERNAL_SERVER_ERROR;
-	}
-
-	if (c->cookie_domain == NULL) {
-		if (_oidc_strcmp(o_uri.hostname, r_uri.hostname) != 0) {
-			char *p = _oidc_strstr(o_uri.hostname, r_uri.hostname);
-			if ((p == NULL) || (_oidc_strcmp(r_uri.hostname, p) != 0)) {
-				oidc_error(r,
-					   "the URL hostname (%s) of the configured " OIDCRedirectURI
-					   " does not match the URL hostname of the URL being accessed (%s): the "
-					   "\"state\" and \"session\" cookies will not be shared between the two!",
-					   r_uri.hostname, o_uri.hostname);
-				oidc_proto_state_destroy(proto_state);
-				OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
-				return HTTP_INTERNAL_SERVER_ERROR;
-			}
-		}
-	} else {
-		if (!oidc_util_cookie_domain_valid(r_uri.hostname, c->cookie_domain)) {
-			oidc_error(r,
-				   "the domain (%s) configured in " OIDCCookieDomain
-				   " does not match the URL hostname (%s) of the URL being accessed (%s): setting "
-				   "\"state\" and \"session\" cookies will not work!!",
-				   c->cookie_domain, o_uri.hostname, original_url);
-			oidc_proto_state_destroy(proto_state);
-			OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
-			return HTTP_INTERNAL_SERVER_ERROR;
-		}
+		return rc;
 	}
 
 	/* send off to the OpenID Connect Provider */