On Sun, Oct 14, 2007 at 06:16:04PM -0400, Stephen Frost wrote:
> * Tom Lane ([EMAIL PROTECTED]) wrote:
> > > Stephen Frost <[EMAIL PROTECTED]> 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:
> [...]
> > Should we just do this, or is it worth working harder?
>
> I certainly like this. Honestly, I'd also like the warning when doing a
> 'create role'/'alter role' that sets/changes the pw on an account that
> doesn't have 'rolcanlogin'. Much better to have me notice that I goof'd
> the command and fix it before telling the user 'go ahead and log in'
> than to have the user complain that it's not working. :)
>
> Just my 2c.
I think that's a good idea. Attached is a patch that implements this (I
think - haven't messed around in that area of the code before). Thoughts?
//Magnus
Index: src/backend/commands/user.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/user.c,v
retrieving revision 1.177
diff -c -r1.177 user.c
*** src/backend/commands/user.c 3 Sep 2007 18:46:30 -0000 1.177
--- src/backend/commands/user.c 17 Oct 2007 14:45:33 -0000
***************
*** 250,255 ****
--- 250,260 ----
if (dvalidUntil)
validUntil = strVal(dvalidUntil->arg);
+ /* Warn about combination that's likely incorrect */
+ if (password && !canlogin)
+ ereport(WARNING,
+ (errmsg("created user with password that cannot
log in")));
+
/* Check some permissions first */
if (issuper)
{
***************
*** 649,654 ****
--- 654,664 ----
DirectFunctionCall1(textin,
CStringGetDatum(encrypted_password));
}
new_record_repl[Anum_pg_authid_rolpassword - 1] = 'r';
+ /* Warn about combination that's likely incorrect */
+ if (canlogin == 0 ||
+ ((((Form_pg_authid) GETSTRUCT(tuple))->rolcanlogin == 0) &&
canlogin != 1))
+ ereport(WARNING,
+ (errmsg("set password for a user that
cannot log in")));
}
/* unset password */
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
