Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions web/pgadmin/utils/driver/psycopg3/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import secrets
import datetime
import asyncio
import copy
from collections import deque
from pathlib import Path
import psycopg
from flask import g, current_app
from flask_babel import gettext
Expand Down Expand Up @@ -281,7 +281,6 @@ def connect(self, **kwargs):
password, encpass, is_update_password = \
self._check_user_password(kwargs)

passfile = kwargs['passfile'] if 'passfile' in kwargs else None
tunnel_password = kwargs['tunnel_password'] if 'tunnel_password' in \
kwargs else ''

Expand Down Expand Up @@ -313,6 +312,15 @@ def connect(self, **kwargs):
if is_error:
return False, errmsg

# Retrieve password from passfile, if one has not been set yet.
passfile = Path(kwargs['passfile']) if 'passfile' in kwargs else None
if not password and passfile:
try:
password = passfile.read_text(encoding='utf-8').strip()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passfile is read by psycopg driver. We just need to pass the args in connection.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! It turns out that passfile is indeed passed to psycopg. What was confusing is that psycopg won't emit any warnings if the format is invalid. In my case it was a docker secret containing only the password, which of course failed (silently).

So I guess that the initially proposed changes are not needed.

But the logic built around passfile in connect() was still funny: A potential passfile kwarg is used for a check, but then the connection is created by whatever passfile is picked up internally by ServerManager, ignoring the kwarg.

I tried to clean up the logic, so that the value used by ServerManager is also used for the checks in connect(), and warn if a kwarg is supplied and is different than that. (This probably won't happen with the current code AFAICT, but updates may change that.)

Let me know what you think and, if the changes look ok, whether I should update the description of this PR or close it and create a new one.

except (OSError, UnicodeDecodeError) as e:
error = _("Failed to read PassFile.\nError: {0}").format(str(e))
return False, error

# If no password credential is found then connect request might
# come from Query tool, ViewData grid, debugger etc tools.
# we will check for pgpass file availability from connection manager
Expand Down