Rename some variables related to ident files in hba.{c,h}

michaelpq · michaelpq · commit 8607630d74cd · 2023-01-12T14:23:20.000+09:00
The code that handles authentication for user maps was pretty confusing with its choice of variable names. It involves two types of users: a system user and a Postgres user (well, role), and these were not named consistently throughout the code that processes the user maps loaded from pg_ident.conf at authentication. This commit changes the following things to improve the situation: - Rename "pg_role" to "pg_user" and "token" to "system_user" in IndetLine. These choices are more consistent with the pg_ident.conf example in the docs, as well. "token" has been introduced recently in fc579e1, and it is way worse than the choice before that, "ident_user". - Switch the order of the fields in IdentLine to map with the order of the items in the ident files, as of map name, system user and PG user. - In check_ident_usermap(), rename "regexp_pgrole" to "expanded_pg_user" when processing a regexp for the system user entry in a user map. This variable does not store a regular expression at all: it would be either a string or a substitution to \1 if the Postgres role is specified as such. Author: Jelte Fennema Discussion: https://postgr.es/m/CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
@@ -2792,21 +2792,21 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
 	token = linitial(tokens);
 
 	/* Copy the ident user token */
-	parsedline->token = copy_auth_token(token);
+	parsedline->system_user = copy_auth_token(token);
 
 	/* Get the PG rolename token */
 	field = lnext(tok_line->fields, field);
 	IDENT_FIELD_ABSENT(field);
 	tokens = lfirst(field);
 	IDENT_MULTI_VALUE(tokens);
 	token = linitial(tokens);
-	parsedline->pg_role = pstrdup(token->string);
+	parsedline->pg_user = pstrdup(token->string);
 
 	/*
 	 * Now that the field validation is done, compile a regex from the user
 	 * token, if necessary.
 	 */
-	if (regcomp_auth_token(parsedline->token, file_name, line_num,
+	if (regcomp_auth_token(parsedline->system_user, file_name, line_num,
 						   err_msg, elevel))
 	{
 		/* err_msg includes the error to report */
@@ -2819,12 +2819,12 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
 /*
  *	Process one line from the parsed ident config lines.
  *
- *	Compare input parsed ident line to the needed map, pg_role and ident_user.
+ *	Compare input parsed ident line to the needed map, pg_user and system_user.
  *	*found_p and *error_p are set according to our results.
  */
 static void
 check_ident_usermap(IdentLine *identLine, const char *usermap_name,
-					const char *pg_role, const char *ident_user,
+					const char *pg_user, const char *system_user,
 					bool case_insensitive, bool *found_p, bool *error_p)
 {
 	*found_p = false;
@@ -2835,7 +2835,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		return;
 
 	/* Match? */
-	if (token_has_regexp(identLine->token))
+	if (token_has_regexp(identLine->system_user))
 	{
 		/*
 		 * Process the system username as a regular expression that returns
@@ -2845,27 +2845,27 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		int			r;
 		regmatch_t	matches[2];
 		char	   *ofs;
-		char	   *regexp_pgrole;
+		char	   *expanded_pg_user;
 
-		r = regexec_auth_token(ident_user, identLine->token, 2, matches);
+		r = regexec_auth_token(system_user, identLine->system_user, 2, matches);
 		if (r)
 		{
 			char		errstr[100];
 
 			if (r != REG_NOMATCH)
 			{
 				/* REG_NOMATCH is not an error, everything else is */
-				pg_regerror(r, identLine->token->regex, errstr, sizeof(errstr));
+				pg_regerror(r, identLine->system_user->regex, errstr, sizeof(errstr));
 				ereport(LOG,
 						(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
 						 errmsg("regular expression match for \"%s\" failed: %s",
-								identLine->token->string + 1, errstr)));
+								identLine->system_user->string + 1, errstr)));
 				*error_p = true;
 			}
 			return;
 		}
 
-		if ((ofs = strstr(identLine->pg_role, "\\1")) != NULL)
+		if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
 		{
 			int			offset;
 
@@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 				ereport(LOG,
 						(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
 						 errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"",
-								identLine->token->string + 1, identLine->pg_role)));
+								identLine->system_user->string + 1, identLine->pg_user)));
 				*error_p = true;
 				return;
 			}
@@ -2884,18 +2884,18 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 			 * length: original length minus length of \1 plus length of match
 			 * plus null terminator
 			 */
-			regexp_pgrole = palloc0(strlen(identLine->pg_role) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
-			offset = ofs - identLine->pg_role;
-			memcpy(regexp_pgrole, identLine->pg_role, offset);
-			memcpy(regexp_pgrole + offset,
-				   ident_user + matches[1].rm_so,
+			expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
+			offset = ofs - identLine->pg_user;
+			memcpy(expanded_pg_user, identLine->pg_user, offset);
+			memcpy(expanded_pg_user + offset,
+				   system_user + matches[1].rm_so,
 				   matches[1].rm_eo - matches[1].rm_so);
-			strcat(regexp_pgrole, ofs + 2);
+			strcat(expanded_pg_user, ofs + 2);
 		}
 		else
 		{
 			/* no substitution, so copy the match */
-			regexp_pgrole = pstrdup(identLine->pg_role);
+			expanded_pg_user = pstrdup(identLine->pg_user);
 		}
 
 		/*
@@ -2904,15 +2904,15 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		 */
 		if (case_insensitive)
 		{
-			if (pg_strcasecmp(regexp_pgrole, pg_role) == 0)
+			if (pg_strcasecmp(expanded_pg_user, pg_user) == 0)
 				*found_p = true;
 		}
 		else
 		{
-			if (strcmp(regexp_pgrole, pg_role) == 0)
+			if (strcmp(expanded_pg_user, pg_user) == 0)
 				*found_p = true;
 		}
-		pfree(regexp_pgrole);
+		pfree(expanded_pg_user);
 
 		return;
 	}
@@ -2921,14 +2921,14 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		/* Not regular expression, so make complete match */
 		if (case_insensitive)
 		{
-			if (pg_strcasecmp(identLine->pg_role, pg_role) == 0 &&
-				pg_strcasecmp(identLine->token->string, ident_user) == 0)
+			if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
+				pg_strcasecmp(identLine->system_user->string, system_user) == 0)
 				*found_p = true;
 		}
 		else
 		{
-			if (strcmp(identLine->pg_role, pg_role) == 0 &&
-				strcmp(identLine->token->string, ident_user) == 0)
+			if (strcmp(identLine->pg_user, pg_user) == 0 &&
+				strcmp(identLine->system_user->string, system_user) == 0)
 				*found_p = true;
 		}
 	}
@@ -2938,20 +2938,20 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 /*
  *	Scan the (pre-parsed) ident usermap file line by line, looking for a match
  *
- *	See if the user with ident username "auth_user" is allowed to act
- *	as Postgres user "pg_role" according to usermap "usermap_name".
+ *	See if the system user with ident username "system_user" is allowed to act as
+ *	Postgres user "pg_user" according to usermap "usermap_name".
  *
  *	Special case: Usermap NULL, equivalent to what was previously called
  *	"sameuser" or "samerole", means don't look in the usermap file.
- *	That's an implied map wherein "pg_role" must be identical to
- *	"auth_user" in order to be authorized.
+ *	That's an implied map wherein "pg_user" must be identical to
+ *	"system_user" in order to be authorized.
  *
  *	Iff authorized, return STATUS_OK, otherwise return STATUS_ERROR.
  */
 int
 check_usermap(const char *usermap_name,
-			  const char *pg_role,
-			  const char *auth_user,
+			  const char *pg_user,
+			  const char *system_user,
 			  bool case_insensitive)
 {
 	bool		found_entry = false,
@@ -2961,17 +2961,17 @@ check_usermap(const char *usermap_name,
 	{
 		if (case_insensitive)
 		{
-			if (pg_strcasecmp(pg_role, auth_user) == 0)
+			if (pg_strcasecmp(pg_user, system_user) == 0)
 				return STATUS_OK;
 		}
 		else
 		{
-			if (strcmp(pg_role, auth_user) == 0)
+			if (strcmp(pg_user, system_user) == 0)
 				return STATUS_OK;
 		}
 		ereport(LOG,
 				(errmsg("provided user name (%s) and authenticated user name (%s) do not match",
-						pg_role, auth_user)));
+						pg_user, system_user)));
 		return STATUS_ERROR;
 	}
 	else
@@ -2981,7 +2981,7 @@ check_usermap(const char *usermap_name,
 		foreach(line_cell, parsed_ident_lines)
 		{
 			check_ident_usermap(lfirst(line_cell), usermap_name,
-								pg_role, auth_user, case_insensitive,
+								pg_user, system_user, case_insensitive,
 								&found_entry, &error);
 			if (found_entry || error)
 				break;
@@ -2991,7 +2991,7 @@ check_usermap(const char *usermap_name,
 	{
 		ereport(LOG,
 				(errmsg("no match in usermap \"%s\" for user \"%s\" authenticated as \"%s\"",
-						usermap_name, pg_role, auth_user)));
+						usermap_name, pg_user, system_user)));
 	}
 	return found_entry ? STATUS_OK : STATUS_ERROR;
 }
@@ -3073,7 +3073,7 @@ load_ident(void)
 		foreach(parsed_line_cell, new_parsed_lines)
 		{
 			newline = (IdentLine *) lfirst(parsed_line_cell);
-			free_auth_token(newline->token);
+			free_auth_token(newline->system_user);
 		}
 		MemoryContextDelete(ident_context);
 		return false;
@@ -3085,7 +3085,7 @@ load_ident(void)
 		foreach(parsed_line_cell, parsed_ident_lines)
 		{
 			newline = (IdentLine *) lfirst(parsed_line_cell);
-			free_auth_token(newline->token);
+			free_auth_token(newline->system_user);
 		}
 	}
 	if (parsed_ident_context != NULL)
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
@@ -492,8 +492,8 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
 	if (ident != NULL)
 	{
 		values[index++] = CStringGetTextDatum(ident->usermap);
-		values[index++] = CStringGetTextDatum(ident->token->string);
-		values[index++] = CStringGetTextDatum(ident->pg_role);
+		values[index++] = CStringGetTextDatum(ident->system_user->string);
+		values[index++] = CStringGetTextDatum(ident->pg_user);
 	}
 	else
 	{
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
@@ -142,8 +142,8 @@ typedef struct IdentLine
 	int			linenumber;
 
 	char	   *usermap;
-	char	   *pg_role;
-	AuthToken  *token;
+	AuthToken  *system_user;
+	char	   *pg_user;
 } IdentLine;
 
 /*
@@ -172,7 +172,7 @@ extern bool load_ident(void);
 extern const char *hba_authname(UserAuth auth_method);
 extern void hba_getauthmethod(hbaPort *port);
 extern int	check_usermap(const char *usermap_name,
-						  const char *pg_role, const char *auth_user,
+						  const char *pg_user, const char *system_user,
 						  bool case_insensitive);
 extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel);
 extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel);

Original file line number	Diff line number	Diff line change
`@@ -492,8 +492,8 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,`
`492`	`492`	`if (ident != NULL)`
`493`	`493`	`{`
`494`	`494`	`values[index++] = CStringGetTextDatum(ident->usermap);`
`495`		`- values[index++] = CStringGetTextDatum(ident->token->string);`
`496`		`- values[index++] = CStringGetTextDatum(ident->pg_role);`
	`495`	`+ values[index++] = CStringGetTextDatum(ident->system_user->string);`
	`496`	`+ values[index++] = CStringGetTextDatum(ident->pg_user);`
`497`	`497`	`}`
`498`	`498`	`else`
`499`	`499`	`{`