v37comments.diff.txt

text/plain

Filename: v37comments.diff.txt
Type: text/plain
Part: 0
Message: Re: [PoC] Federated Authn/z with OAUTHBEARER
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index eea5032de8..2bb84b7bc8 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -424,7 +424,7 @@ parse_kvpairs_for_auth(char **input)
 		value = sep + 1;
 		validate_kvpair(key, value);
 
-		if (!strcmp(key, AUTH_KEY))
+		if (strcmp(key, AUTH_KEY) == 0)
 		{
 			if (auth)
 				ereport(ERROR,
@@ -619,6 +619,15 @@ validate(Port *port, const char *auth)
 	if (!(token = validate_token_format(auth)))
 		return false;
 
+	/*
+	 * Ensure that we have a validation library loaded, this should always be
+	 * the case and an error here is indicative of a bug.
+	 */
+	if (!ValidatorCallbacks || !ValidatorCallbacks->validate_cb)
+		ereport(FATAL,
+				errcode(ERRCODE_INTERNAL_ERROR),
+				errmsg("validation of OAuth token requested without a validator loaded"));
+
 	/* Call the validation function from the validator module */
 	ret = ValidatorCallbacks->validate_cb(validator_module_state,
 										  token, port->user_name);
@@ -708,6 +717,7 @@ load_validator_library(const char *libname)
 					   "OAuth validator", libname, "_PG_oauth_validator_module_init"));
 
 	ValidatorCallbacks = (*validator_init) ();
+	Assert(ValidatorCallbacks);
 
 	/* Allocate memory for validator library private state data */
 	validator_module_state = (ValidatorModuleState *) palloc0(sizeof(ValidatorModuleState));
@@ -738,7 +748,6 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
 	char	   *file_name = hbaline->sourcefile;
 	char	   *rawstring;
 	List	   *elemlist = NIL;
-	ListCell   *l;
 
 	*err_msg = NULL;
 
@@ -787,10 +796,8 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
 		goto done;
 	}
 
-	foreach(l, elemlist)
+	foreach_ptr(char, allowed, elemlist)
 	{
-		char	   *allowed = lfirst(l);
-
 		if (strcmp(allowed, hbaline->oauth_validator) == 0)
 			goto done;
 	}
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index 0d4185194d..5400e6df7a 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -841,6 +841,11 @@ oauth_exchange(void *opaq, bool final,
 			 * Respond with the required dummy message (RFC 7628, sec. 3.2.3).
 			 */
 			*output = strdup(kvsep);
+			if (unlikely(!*output))
+			{
+				libpq_append_conn_error(conn, "out of memory");
+				return SASL_FAILED;
+			}
 			*outputlen = strlen(*output);	/* == 1 */
 
 			state->state = FE_OAUTH_SERVER_ERROR;
diff --git a/src/test/modules/oauth_validator/fail_validator.c b/src/test/modules/oauth_validator/fail_validator.c
index b0fcc07c2a..c438ed4d17 100644
--- a/src/test/modules/oauth_validator/fail_validator.c
+++ b/src/test/modules/oauth_validator/fail_validator.c
@@ -16,9 +16,6 @@
 
 #include "fmgr.h"
 #include "libpq/oauth.h"
-#include "miscadmin.h"
-#include "utils/guc.h"
-#include "utils/memutils.h"
 
 PG_MODULE_MAGIC;
 
diff --git a/src/test/python/client/test_oauth.py b/src/test/python/client/test_oauth.py
index 60e57dba86..ce8e0f12c9 100644
--- a/src/test/python/client/test_oauth.py
+++ b/src/test/python/client/test_oauth.py
@@ -108,7 +108,7 @@ def get_auth_value(initial):
 def fail_oauth_handshake(conn, sasl_resp, *, errmsg="doesn't matter"):
     """
     Sends a failure response via the OAUTHBEARER mechanism, consumes the
-    client's dummy reponse, and issues a FATAL error to end the exchange.
+    client's dummy response, and issues a FATAL error to end the exchange.
 
     sasl_resp is a dictionary which will be serialized as the OAUTHBEARER JSON
     response. If provided, errmsg is used in the FATAL ErrorResponse.
diff --git a/src/test/python/server/oauthtest.c b/src/test/python/server/oauthtest.c
index ee39c2a14e..415748b9a6 100644
--- a/src/test/python/server/oauthtest.c
+++ b/src/test/python/server/oauthtest.c
@@ -108,7 +108,7 @@ test_validate(ValidatorModuleState *state, const char *token, const char *role)
 	}
 	else
 	{
-		if (*expected_bearer && !strcmp(token, expected_bearer))
+		if (*expected_bearer && strcmp(token, expected_bearer) == 0)
 			res->authorized = true;
 		if (set_authn_id)
 			res->authn_id = authn_id;