Prev: CREATE DATABASE cannot be executed from a function or multi-command string
Next: pgadmin debug on windows
From: Tom Lane on 14 Oct 2007 17:20
I wrote:
> Stephen Frost <sfrost(a)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 ----

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

From: Andrew Dunstan on 14 Oct 2007 17:56


Tom Lane wrote:
>
> Should we just do this, or is it worth working harder?
>
>
>

Not worth more, IMNSHO.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

From: Stephen Frost on 14 Oct 2007 18:16
* Tom Lane (tgl(a)sss.pgh.pa.us) wrote:
> > Stephen Frost <sfrost(a)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:
[...]
> 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.

Thanks,

Stephen
From: Michael Paesold on 15 Oct 2007 02:18
Tom Lane wrote:
> 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?

IMHO this is exactly what we want. It does only offer more information when
you already got authentication right and therefore doesn't open an
information leak.

Not sure about the warning when creating a role with a password but
nologin. Could be useful.

Best Regards
Michael Paesold

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

From: Magnus Hagander on 17 Oct 2007 10:46
On Sun, Oct 14, 2007 at 06:16:04PM -0400, Stephen Frost wrote:
> * Tom Lane (tgl(a)sss.pgh.pa.us) wrote:
> > > Stephen Frost <sfrost(a)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:
> [...]
> > 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

First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: CREATE DATABASE cannot be executed from a function or multi-command string
Next: pgadmin debug on windows