Describe the bug
validate_json_data() in web/pgadmin/utils/__init__.py checks only that the Username key is present in a server definition, never that it holds anything useful, so {"Username": ""} and {"Username": null} both import cleanly and leave behind a server that cannot sensibly connect.
An empty username is worse than it first appears, because it does not fail cleanly. libpq treats an empty string for user as "unset" and substitutes the OS account that owns the connecting process, so the connection is attempted as whoever pgAdmin runs as rather than being rejected for having no username:
>>> psycopg.connect(host='127.0.0.1', port=5432, dbname='postgres', user='', password='...')
FATAL: password authentication failed for user "dpage"
That name appears nowhere in the imported configuration, which makes the resulting authentication failure quite hard to trace back to the servers.json that caused it. {"Username": null} is the other half of the same gap: Server.username is declared nullable=False in the model, so on a schema created from the model the import fails with an IntegrityError surfaced as a raw "Error creating server '%s': %s" string, whilst on a database migrated over the years, where the column carries no NOT NULL, it imports silently.
The server dialog does not allow either of these; web/pgadmin/browser/server_groups/servers/static/js/server.ui.js:635 rejects an empty username with "Username must be specified." Import is simply more permissive than the UI it is meant to mirror.
To Reproduce
- Save the following as
servers.json:
{
"Servers": {
"1": {
"Group": "Servers",
"Name": "Empty username",
"Host": "127.0.0.1",
"Port": 5432,
"Username": "",
"MaintenanceDB": "postgres"
}
}
}
- Import it, either through Tools > Import/Export Servers or with
setup.py --load-servers.
- The import reports success.
- Try to connect to the new server, and note that the failure names an OS account rather than complaining about the missing username.
Expected behavior
Import should reject an empty or null Username with the same clarity as any other missing attribute, i.e. an error naming the server and the attribute, rather than accepting it and deferring the consequences to connection time.
The Service escape hatch must stay as it is: a server defined with Service legitimately needs no Username, since the connection service file supplies it.
Additional context
This came up in review of #9971 (fixing #9226), where @hiteshjambhale spotted that a {"Username": ""} definition slipped through the then-new SharedUsername fallback and was stored as the server's username. That specific path was closed when #9971 was merged, and validation now tests the values rather than the keys for shared servers, so {"Shared": true, "SharedUsername": ""} and its null form are rejected. The equivalent hole on the non-shared path was explicitly left alone as being pre-existing and outside that PR's scope, which is what this issue tracks.
The fix is small: in the non-shared branch, test the value rather than the key presence, in the way the shared branch now does. It is worth deciding at the same time whether whitespace-only usernames deserve the same treatment.
Describe the bug
validate_json_data()inweb/pgadmin/utils/__init__.pychecks only that theUsernamekey is present in a server definition, never that it holds anything useful, so{"Username": ""}and{"Username": null}both import cleanly and leave behind a server that cannot sensibly connect.An empty username is worse than it first appears, because it does not fail cleanly. libpq treats an empty string for
useras "unset" and substitutes the OS account that owns the connecting process, so the connection is attempted as whoever pgAdmin runs as rather than being rejected for having no username:That name appears nowhere in the imported configuration, which makes the resulting authentication failure quite hard to trace back to the
servers.jsonthat caused it.{"Username": null}is the other half of the same gap:Server.usernameis declarednullable=Falsein the model, so on a schema created from the model the import fails with anIntegrityErrorsurfaced as a raw"Error creating server '%s': %s"string, whilst on a database migrated over the years, where the column carries noNOT NULL, it imports silently.The server dialog does not allow either of these;
web/pgadmin/browser/server_groups/servers/static/js/server.ui.js:635rejects an empty username with "Username must be specified." Import is simply more permissive than the UI it is meant to mirror.To Reproduce
servers.json:{ "Servers": { "1": { "Group": "Servers", "Name": "Empty username", "Host": "127.0.0.1", "Port": 5432, "Username": "", "MaintenanceDB": "postgres" } } }setup.py --load-servers.Expected behavior
Import should reject an empty or null
Usernamewith the same clarity as any other missing attribute, i.e. an error naming the server and the attribute, rather than accepting it and deferring the consequences to connection time.The
Serviceescape hatch must stay as it is: a server defined withServicelegitimately needs noUsername, since the connection service file supplies it.Additional context
This came up in review of #9971 (fixing #9226), where @hiteshjambhale spotted that a
{"Username": ""}definition slipped through the then-newSharedUsernamefallback and was stored as the server's username. That specific path was closed when #9971 was merged, and validation now tests the values rather than the keys for shared servers, so{"Shared": true, "SharedUsername": ""}and its null form are rejected. The equivalent hole on the non-shared path was explicitly left alone as being pre-existing and outside that PR's scope, which is what this issue tracks.The fix is small: in the non-shared branch, test the value rather than the key presence, in the way the shared branch now does. It is worth deciding at the same time whether whitespace-only usernames deserve the same treatment.