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
workingwith 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 ----