Re: rolcanlogin vs. the flat password file

I wrote:
> Stephen Frost <sfrost@snowman.net> writes:
>> I wonder if the OP was unhappy because he created a role w/ a pw and
>> then couldn't figure out why the user couldn't log in?

> Hm, maybe.  In that case just not filtering the entry out of the flat
> file would be good enough.

I've confirmed the confusing behavior in CVS HEAD.  With password auth
selected in pg_hba.conf:

postgres=# create user foo nologin;
CREATE ROLE
postgres=# \c - foo    
Password for user "foo": 
FATAL:  password authentication failed for user "foo"
Previous connection kept
postgres=# alter user foo password 'foo';
ALTER ROLE
postgres=# \c - foo
Password for user "foo": << correct password entered here
FATAL:  password authentication failed for user "foo"
Previous connection kept

With the attached patch to not drop nologin roles from the flat password
file, it acts more sanely:

postgres=# create user foo nologin;
CREATE ROLE
postgres=# \c - foo
Password for user "foo": 
FATAL:  password authentication failed for user "foo"
Previous connection kept
postgres=# alter user foo password 'foo';
ALTER ROLE
postgres=# \c - foo
Password for user "foo": << correct password entered here
FATAL:  role "foo" is not permitted to log in
Previous connection kept

Should we just do this, or is it worth working harder?

			regards, tom lane


*** src/backend/utils/init/flatfiles.c.orig	Wed Aug  1 18:45:08 2007
--- src/backend/utils/init/flatfiles.c	Sun Oct 14 17:14:27 2007
***************
*** 298,304 ****
   *
   * The format for the flat auth file is
   *		"rolename" "password" "validuntil" "memberof" "memberof" ...
-  * Only roles that are marked rolcanlogin are entered into the auth file.
   * Each role's line lists all the roles (groups) of which it is directly
   * or indirectly a member, except for itself.
   *
--- 298,303 ----
***************
*** 312,318 ****
  typedef struct
  {
  	Oid			roleid;
- 	bool		rolcanlogin;
  	char	   *rolname;
  	char	   *rolpassword;
  	char	   *rolvaliduntil;
--- 311,316 ----
***************
*** 407,414 ****
  						tempname)));
  
  	/*
! 	 * Read pg_authid and fill temporary data structures.  Note we must read
! 	 * all roles, even those without rolcanlogin.
  	 */
  	totalblocks = RelationGetNumberOfBlocks(rel_authid);
  	totalblocks = totalblocks ? totalblocks : 1;
--- 405,411 ----
  						tempname)));
  
  	/*
! 	 * Read pg_authid and fill temporary data structures.
  	 */
  	totalblocks = RelationGetNumberOfBlocks(rel_authid);
  	totalblocks = totalblocks ? totalblocks : 1;
***************
*** 433,439 ****
  		}
  
  		auth_info[curr_role].roleid = HeapTupleGetOid(tuple);
- 		auth_info[curr_role].rolcanlogin = aform->rolcanlogin;
  		auth_info[curr_role].rolname = pstrdup(NameStr(aform->rolname));
  		auth_info[curr_role].member_of = NIL;
  
--- 430,435 ----
***************
*** 565,574 ****
  			List	   *roles_names_list = NIL;
  			ListCell   *mem;
  
- 			/* We can skip this for non-login roles */
- 			if (!auth_info[curr_role].rolcanlogin)
- 				continue;
- 
  			/*
  			 * This search algorithm is the same as in is_member_of_role; we
  			 * are just working with a different input data structure.
--- 561,566 ----
***************
*** 642,650 ****
  	for (curr_role = 0; curr_role < total_roles; curr_role++)
  	{
  		auth_entry *arole = &auth_info[curr_role];
- 
- 		if (arole->rolcanlogin)
- 		{
  			ListCell   *mem;
  
  			fputs_quote(arole->rolname, fp);
--- 634,639 ----
***************
*** 660,666 ****
  			}
  
  			fputs("\n", fp);
- 		}
  	}
  
  	if (FreeFile(fp))
--- 649,654 ----