On Fri, 22 Sept 2023 at 15:02, vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 22 Sept 2023 at 01:45, Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Thu, Sep 21, 2023 at 3:48 PM vignesh C <vignesh21@gmail.com> wrote:
> > > Attached patch has the changes for the same.
> >
> > Almost everything about this patch seems incorrect to me.
> >
> > It seems to rip out all of the must_use_password = passwordrequired &&
> > !superuser logic, which is not at all what was being discussed here,
> > and which I think is not desirable.
>
> I thought of moving the passwordrequired && !superuser logic inside
> libpq connect but it is not simplifying the code. Reverted those
> changes and kept only the changes to check if the password is present
> in the connection string in case of must_use_password.
>
> > And it does stuff like this:
> >
> > @@ -275,10 +288,18 @@ libpqrcv_check_conninfo(const char *conninfo,
> > bool must_use_password)
> > }
> >
> > if (!uses_password)
> > + {
> > + if (conn)
> > + {
> > + libpqsrv_disconnect(conn->streamConn);
> > + pfree(conn);
> > + }
> > +
> > ereport(ERROR,
> > (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
> > errmsg("password is required"),
> > errdetail("Non-superusers must provide a password in the connection
> > string.")));
> > + }
> > }
> >
> > PQconninfoFree(opts);
> >
> > There are zero comments explaining what this is supposed to
> > accomplish, but I don't think it's any of the things discussed on this
> > thread.
>
> We can have this check before the connection, so this can be removed.
>
> > + CacheRegisterSyscacheCallback(AUTHOID,
> > + subscription_change_cb,
> > + (Datum) 0);
> >
> > I think if we want to do this it should be a separate patch from
> > adding the additional error checks. And I think it should be
> > accompanied by a comment update.
>
> I will post these changes in a separate email.
Posted this changes to hackers at [1]
[1] - https://www.postgresql.org/message-id/CALDaNm2Dxmhq08nr4P6G%2B24QvdBo_GAVyZ_Q1TcGYK%2B8NHs9xw%40mail.gmail.com
Regards,
Vignesh