On Tue, Apr 15, 2025 at 01:16:12PM -0700, Jacob Champion wrote:
> On Tue, Apr 15, 2025 at 12:14 PM Noah Misch <noah@leadboat.com> wrote:
> > I suspect this should use .dispchar="*" to encourage UIs to display
> > oauth_client_secret like a password field. Thoughts?
>
> Hmm, from a UI perspective I agree. (The builtin flow targets "public
> clients", where secrets are discouraged and/or understood to be
> not-really-secret, but there's no reason to assume that all flows used
> by the application are public.)
>
> From a proxy perspective, this would mess with FDW handling. By making
> the dispchar '*', oauth_client_secret will be made into a user mapping
> option rather than a server option. (Neither is very useful to
> postgres_fdw anyway, because the builtin flow needs an end user to
> interact with the provider.) But I'm not sure if we'll need to handle
> compatibility in the future if we implement a proxy-friendly flow.
If we think oauth_client_secret should get dispchar="*" UI treatment yet be a
postgres_fdw server option, postgres_fdw code can make it so. postgres_fdw
already has explicit code to reclassify the "user" option.
> Is
> it okay to move options back and forth during a major version bump? I
> assume it would present a problem for pg_upgrade?
It would break dump/reload, so it's best not to move options between
optcontext=ForeignServerRelationId and optcontext=UserMappingRelationId, even
at major versions. As above, we can change dispchar separately from
optcontext. The documentation of dispchar likely should tell people to update
the postgres_fdw documentation when adding a dispchar="*' option.
It sounds like we might end up wanting to allow oauth_client_secret in both of
ForeignServerRelationId and UserMappingRelationId, each catering to different
oauth implementations. Is that right? (It would be fine to allow one today
and later change to allow both.)
