BUG #15025: PSQL CLI - inconsistency when both -d and -U supplies a username

On Tue, Jan 23, 2018 at 01:00:13PM +0000, PG Bug reporting form wrote:

The following bug has been logged on the website:

Bug reference: 15025
Logged by: Akos Vandra
Email address: akos@elegran.com
PostgreSQL version: 9.5.4
Operating system: Debian
Description:

Repro:

case 1: psql -U other_user -d "postgresql://some_user@host/db"

case 2: psql -d "postgresql://some_user@host/db" -U other_user

Expectation:

Use whatever is given later:

case 1: log in as user
case 2: log in as other_user

Actual:

case 1: logs in as user
case 2:
- the password prompt asks for the pw of user
- psql uses the password given to log in with other_user
- if the pw is correct for user, or incorrect it displays that the
password is incorrect for other_user
- if the password is correct for other_user, it connects to the db as
other_user

$ psql "postgresql://user@host/db" -U other_user
Password for user other_user:
psql: FATAL: password authentication failed for user "user"
FATAL: password authentication failed for user "user"

I was able to make a clearer example. First create two users:

CREATE USER user1 PASSWORD 'abc1';
CREATE USER user2 PASSWORD 'abc2';

then:

psql -d "postgresql://user2@momjian.us/test" -U user1
--> Password for user user1:

but it wants the user2 password. Same with:

psql -U user1 -d "postgresql://user2@momjian.us/test"
--> Password for user user1:

It doesn't matter whether -U is first or last, it always prompts for the
-U user, but connects as the -d user.

Because the URI is parsed by libpq, I don't think we can do any better
than just suppress the user name in the password prompt when a URI is
used. This is done in the attached patch, e.g.:
e.g.:

$psql -U user1 -d "postgresql://user2@momjian.us/test"
Password:

Can someone tell me if I need to update the prompt in
psql/command.c::prompt_for_password()? I can't figure out how to
trigger that prompt.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

psql -d "postgresql://user2@momjian.us/test" -U user1
--> Password for user user1:
but it wants the user2 password.

It doesn't matter whether -U is first or last, it always prompts for the
-U user, but connects as the -d user.

Bleah.

Because the URI is parsed by libpq, I don't think we can do any better
than just suppress the user name in the password prompt when a URI is
used.

That doesn't seem very user-friendly at all. I think that the most
correct behavior in this case would be to throw an error because of the
conflicting command line parameters. Now admittedly, psql doesn't throw
an error for

psql -U alice -U bob

so maybe just silently making a choice is OK, but we need to be clear
as to which choice we made. Isn't it possible to get the URI parse
results back out of libpq?

regards, tom lane

On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

psql -d "postgresql://user2@momjian.us/test" -U user1
--> Password for user user1:
but it wants the user2 password.

It doesn't matter whether -U is first or last, it always prompts for the
-U user, but connects as the -d user.

Bleah.

Because the URI is parsed by libpq, I don't think we can do any better
than just suppress the user name in the password prompt when a URI is
used.

That doesn't seem very user-friendly at all. I think that the most
correct behavior in this case would be to throw an error because of the
conflicting command line parameters. Now admittedly, psql doesn't throw
an error for

psql -U alice -U bob

so maybe just silently making a choice is OK, but we need to be clear
as to which choice we made. Isn't it possible to get the URI parse
results back out of libpq?

Well, there is PQuser(), but you need to pass a connection struct to
that, and before you connect you don't have one. libpq's
conninfo_uri_parse_options() is a static function so that can't be used.

I don't see any libpq documentation saying that URIs override
command-line specifications.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote:

Isn't it possible to get the URI parse
results back out of libpq?

Well, there is PQuser(), but you need to pass a connection struct to
that, and before you connect you don't have one.

Yeah, but we normally don't prompt for password till after a failed
connection attempt, at which point we can get the info. So I propose
something like the attached.

There's room for debate about what we ought to do when -W (--password) is
specified, but I think that that's not really that exciting because the
only real use-cases for it are noninteractive applications that aren't
going to care what the prompt is. So in the startup.c case I have it
just offering the neutral "Password: " prompt always. In the \c case,
I left it using the same initial username as it was before, because the
odds that that's right seem considerably higher with \c. You can still
fool it by giving a URI dbname to \c, so maybe there's an argument for
lobotomizing the initial prompt in \c too, but I didn't do that here.

regards, tom lane

On Sun, Jan 28, 2018 at 03:30:54PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote:

Isn't it possible to get the URI parse
results back out of libpq?

Well, there is PQuser(), but you need to pass a connection struct to
that, and before you connect you don't have one.

Yeah, but we normally don't prompt for password till after a failed
connection attempt, at which point we can get the info. So I propose
something like the attached.

There's room for debate about what we ought to do when -W (--password) is
specified, but I think that that's not really that exciting because the
only real use-cases for it are noninteractive applications that aren't
going to care what the prompt is. So in the startup.c case I have it
just offering the neutral "Password: " prompt always. In the \c case,
I left it using the same initial username as it was before, because the
odds that that's right seem considerably higher with \c. You can still
fool it by giving a URI dbname to \c, so maybe there's an argument for
lobotomizing the initial prompt in \c too, but I didn't do that here.

Oh, I wasn't aware a failed login left us with a 'conn'.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

Oh, I wasn't aware a failed login left us with a 'conn'.

Sure, it's what's carrying the error message. But it's also got all
the actual connection parameters filled in.

After further experimentation it seems like the has_connection_string
logic in do_connect() causes the \c case to behave pretty much as you'd
want:

$ psql -U user1 -d "postgresql://user2@localhost/postgres" -W
Password:

postgres=> \c -
Password for user user2:
You are now connected to database "postgres" as user "user2".
postgres=> \c postgresql://user1@localhost/postgres
Password:
You are now connected to database "postgres" as user "user1".

So I now think the comment I added to do_connect() is unduly pessimistic,
and it's fine to keep using "prompt_for_password(user)" for a forced
password prompt there. There may still be use-cases where it gets it
wrong, but they're too narrow to be worth giving up the helpful prompt
altogether.

regards, tom lane

I wrote:

So I now think the comment I added to do_connect() is unduly pessimistic,
and it's fine to keep using "prompt_for_password(user)" for a forced
password prompt there. There may still be use-cases where it gets it
wrong, but they're too narrow to be worth giving up the helpful prompt
altogether.

After further thought about that I changed my mind again: it seems better
to be able to say "we never issue a misleading password prompt" than that
"it's right 99% of the time" --- and it looks like in some cases with
nondefault reuse_previous_specification, we'd still get it wrong. So
I made it shorten the prompt if the dbname is a connstring or URI.
Committed with that change.

regards, tom lane