From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>, Markus Wanner <markus(at)bluegap(dot)ch>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Change authentication error message (patch) |
Date: | 2014-01-24 15:10:00 |
Message-ID: | 11985.1390576200@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Thu, Jan 23, 2014 at 10:39:34PM -0500, Tom Lane wrote:
>> I'm not convinced that this improves anything. The problem might not in
>> fact be either of the things you mention, in which case the new message
>> is outright misleading. Also, what of the policy stated in the header
>> comment for the function you're hacking, ie we intentionally don't reveal
>> the precise cause of the failure to the client?
> Well, the only solution then would be to add some weasel words like
> "perhaps expired password", but that seems so rare that I doubt it would
> apply very often and seems like an odd suggestion. We could go with:
> password authentication failed for user \"%s\": perhaps invalid or expired password
> We did have two threads on this issue in the past 12 months so I figured
> we should try to do something.
I agree with doing *something*, but this particular thing seems to violate
our very long-standing policy on how to deal with authentication failures,
as well as being too vague to be really useful.
What would be well within that policy is to log additional information
into the postmaster log. I see that md5_crypt_verify knows perfectly
well whether the problem is no password set, wrong password, or expired
password. I don't see anything wrong with having it emit a log entry
--- maybe not in the second case for fear of log-spam complaints, but
certainly the third case and maybe the first one. Or possibly cleaner,
have it return additional status so that auth_failed() can include the
info in the main ereport using errdetail_log().
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Raiskup | 2014-01-24 16:19:18 | Re: pg_upgrade: make the locale comparison more tolerating |
Previous Message | Magnus Hagander | 2014-01-24 14:46:35 | Re: proposal: hide application_name from other users |