fix setting OIDCPKCEMethod none; closes #1256; thanks @eoliphan

Signed-off-by: Hans Zandbelt <[email protected]>

ChangeLog

@@ -1,3 +1,6 @@
+08/29/2024
+- fix setting OIDCPKCEMethod none; closes #1256; thanks @eoliphan
+
 08/28/2024
 - re-introduce OIDCSessionMaxDuration 0; see #1252
 - bump to 2.4.16.2dev

src/cfg/provider.c

@@ -254,7 +254,7 @@ const char *oidc_cfg_provider_pkce_set(apr_pool_t *pool, oidc_provider_t *provid
 		provider->pkce = &oidc_pkce_s256;
 		return NULL;
 	} else if (_oidc_strcmp(arg, OIDC_PKCE_METHOD_NONE) == 0) {
-		provider->pkce = NULL;
+		provider->pkce = &oidc_pkce_none;
 		return NULL;
 	}
 	return oidc_cfg_parse_is_valid_option(pool, arg, options);

src/handle/request.c

@@ -186,7 +186,7 @@ int oidc_request_authenticate_user(request_rec *r, oidc_cfg_t *c, oidc_provider_
 
 	if ((oidc_util_spaced_string_contains(r->pool, oidc_cfg_provider_response_type_get(provider),
 					      OIDC_PROTO_CODE) == TRUE) &&
-	    (oidc_cfg_provider_pkce_get(provider) != NULL)) {
+	    (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none)) {
 
 		/* generate the code verifier value that correlates authorization requests and code exchange requests */
 		if (oidc_cfg_provider_pkce_get(provider)->state(r, &pkce_state) == FALSE)

src/metadata.c

@@ -1334,9 +1334,7 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg_t *cfg, json_t *j_c
 	/* get the PKCE method to use */
 	/* NB: pass "none" rather than NULL to allow fallback when the default in base is explicitly set to "none" */
 	oidc_util_json_object_get_string(r->pool, j_conf, OIDC_METADATA_PKCE_METHOD, &value,
-					 oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg))
-					     ? oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg))->method
-					     : OIDC_PKCE_METHOD_NONE);
+					 oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg))->method);
 	OIDC_METADATA_PROVIDER_SET(pkce, value, rv)
 
 	/* see if we've got a custom DPoP mode */

src/proto/pkce.c

@@ -103,3 +103,8 @@ oidc_proto_pkce_t oidc_pkce_plain = {OIDC_PKCE_METHOD_PLAIN, oidc_proto_pkce_sta
  */
 oidc_proto_pkce_t oidc_pkce_s256 = {OIDC_PKCE_METHOD_S256, oidc_proto_pkce_state_s256, oidc_proto_pkce_verifier_s256,
 				    oidc_proto_pkce_challenge_s256};
+
+/*
+ * PKCE none
+ */
+oidc_proto_pkce_t oidc_pkce_none = {OIDC_PKCE_METHOD_NONE, NULL, NULL, NULL};

src/proto/proto.h

@@ -178,6 +178,7 @@ apr_byte_t oidc_proto_jwt_sign_and_serialize(request_rec *r, oidc_jwk_t *jwk, oi
 
 extern oidc_proto_pkce_t oidc_pkce_plain;
 extern oidc_proto_pkce_t oidc_pkce_s256;
+extern oidc_proto_pkce_t oidc_pkce_none;
 
 const char *oidc_proto_state_get_pkce_state(oidc_proto_state_t *proto_state);
 void oidc_proto_state_set_pkce_state(oidc_proto_state_t *proto_state, const char *pkce_state);

src/proto/request.c

@@ -650,7 +650,7 @@ int oidc_proto_request_auth(request_rec *r, struct oidc_provider_t *provider, co
 		apr_table_setn(params, OIDC_PROTO_NONCE, nonce);
 
 	/* add PKCE code challenge if set */
-	if ((code_challenge != NULL) && (oidc_cfg_provider_pkce_get(provider) != NULL)) {
+	if ((code_challenge != NULL) && (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none)) {
 		apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE, code_challenge);
 		apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE_METHOD, oidc_cfg_provider_pkce_get(provider)->method);
 	}

src/proto/response.c

@@ -327,7 +327,7 @@ static apr_byte_t oidc_proto_resolve_code_and_validate_response(request_rec *r,
 	char *refresh_token = NULL;
 	char *code_verifier = NULL;
 
-	if (oidc_cfg_provider_pkce_get(provider) != NULL)
+	if (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none)
 		oidc_cfg_provider_pkce_get(provider)->verifier(r, oidc_proto_state_get_pkce_state(proto_state),
 							       &code_verifier);