Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
44 changes: 44 additions & 0 deletions docs/en_US/user_management.rst
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,50 @@ username/email address.
/path/to/python /path/to/setup.py get-users --username user1@gmail.com


Load Users
**********

To bulk import users from a JSON file, invoke ``setup.py`` with ``load-users`` command line option,
followed by the path to the JSON file.

.. code-block:: bash

/path/to/python /path/to/setup.py load-users /path/to/users.json

**JSON File Format**

The input JSON file must contain a ``users`` array with user objects:

.. code-block:: json

{
"users": [
{
"username": "admin@example.com",
"email": "admin@example.com",
"password": "securepassword",
"role": "Administrator",
"active": true,
"auth_source": "internal"
},
{
"username": "ldap_user",
"email": "ldap_user@example.com",
"role": "User",
"active": true,
"auth_source": "ldap"
}
]
}

The command handles errors gracefully:

* Users that already exist are skipped
* Invalid roles are reported and skipped
* Missing passwords for internal auth are reported and skipped
* Passwords shorter than 6 characters are reported and skipped


Output
******

Expand Down
151 changes: 151 additions & 0 deletions web/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,157 @@ def get_role(role: str):

class ManageUsers:

@app.command()
@update_sqlite_path
def load_users(input_file: str,
sqlite_path: Optional[str] = None,
json: Optional[bool] = False):
Comment on lines +166 to +170
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

json parameter is declared but never used — output format is unaffected.

The json parameter (line 170) is accepted but never referenced in the method body, so passing --json has no effect. Either implement JSON output (consistent with other commands like get-users) or remove the parameter. The sqlite_path parameter is consumed by the @update_sqlite_path decorator, so that one is fine.

🧰 Tools
🪛 Ruff (0.15.1)

[warning] 169-169: Unused method argument: sqlite_path

(ARG002)


[warning] 170-170: Unused method argument: json

(ARG002)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/setup.py` around lines 166 - 170, The load_users command currently
accepts a json parameter but never uses it; update the load_users function to
honor the json flag (or remove the parameter) — specifically, in the load_users
function implement conditional output: after loading users, if json is True
serialize the resulting users list to JSON (matching the format used by the
get-users command) and print to stdout (or write to the same destination
get-users uses for JSON), otherwise keep the existing human-readable output;
keep the `@update_sqlite_path` decorator as-is and use the existing user-loading
logic in load_users to produce the object you will JSON-serialize.

"""Load users from a JSON file.

Expected JSON format:
{
"users": [
{
"username": "user@example.com",
"email": "user@example.com",
"password": "password123",
"role": "User",
"active": true,
"auth_source": "internal"
},
{
"username": "ldap_user",
"email": "ldap@example.com",
"role": "Administrator",
"active": true,
"auth_source": "ldap"
}
]
}
"""
from urllib.parse import unquote

print('----------')
print('Loading users from:', input_file)
print('SQLite pgAdmin config:', config.SQLITE_PATH)
print('----------')

# Parse the input file path
try:
file_path = unquote(input_file)
except Exception as e:
print(str(e))
return _handle_error(str(e), True)

# Read and parse JSON file
try:
with open(file_path) as f:
data = jsonlib.load(f)
except jsonlib.decoder.JSONDecodeError as e:
return _handle_error(
gettext("Error parsing input file %s: %s" % (file_path, e)),
True)
except Exception as e:
return _handle_error(
gettext("Error reading input file %s: [%d] %s" %
(file_path, e.errno, e.strerror)), True)
Comment on lines +202 to +219
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Double error output on unquote failure (lines 204-206).

When the unquote call fails, str(e) is printed on line 205, then _handle_error(str(e), True) (line 206) prints the same message again before calling sys.exit(1). The JSON-parsing error paths (lines 212-219) correctly delegate to _handle_error only. Remove the extra print on line 205.

Proposed fix
         try:
             file_path = unquote(input_file)
         except Exception as e:
-            print(str(e))
             return _handle_error(str(e), True)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
file_path = unquote(input_file)
except Exception as e:
print(str(e))
return _handle_error(str(e), True)
# Read and parse JSON file
try:
with open(file_path) as f:
data = jsonlib.load(f)
except jsonlib.decoder.JSONDecodeError as e:
return _handle_error(
gettext("Error parsing input file %s: %s" % (file_path, e)),
True)
except Exception as e:
return _handle_error(
gettext("Error reading input file %s: [%d] %s" %
(file_path, e.errno, e.strerror)), True)
try:
file_path = unquote(input_file)
except Exception as e:
return _handle_error(str(e), True)
# Read and parse JSON file
try:
with open(file_path) as f:
data = jsonlib.load(f)
except jsonlib.decoder.JSONDecodeError as e:
return _handle_error(
gettext("Error parsing input file %s: %s" % (file_path, e)),
True)
except Exception as e:
return _handle_error(
gettext("Error reading input file %s: [%d] %s" %
(file_path, e.errno, e.strerror)), True)
🧰 Tools
🪛 Ruff (0.15.1)

[warning] 204-204: Do not catch blind exception: Exception

(BLE001)


[warning] 216-216: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/setup.py` around lines 202 - 219, Remove the redundant print of the
exception in the unquote error path: when unquote(input_file) raises, stop
calling print(str(e)) and instead directly return _handle_error(str(e), True)
(same behavior as the JSON error branches). Update the try/except around unquote
to catch Exception as e, assign file_path = unquote(input_file) in the try, and
in the except simply return _handle_error(str(e), True) (references: unquote,
input_file, file_path, _handle_error).


# Validate JSON structure
if 'users' not in data:
return _handle_error(
gettext("Invalid JSON format: 'users' key not found"), True)

users_data = data['users']
if not isinstance(users_data, list):
return _handle_error(
gettext("Invalid JSON format: 'users' must be a list"), True)

created_count = 0
skipped_count = 0
error_count = 0

app = create_app(config.APP_NAME + '-cli')
with (app.test_request_context()):
for user_entry in users_data:
try:
# Validate required fields
if 'username' not in user_entry and\
'email' not in user_entry:
print(f"Skipping user: missing 'username' or 'email'")
error_count += 1
continue
Comment on lines +240 to +244
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

f-string on line 242 has no placeholders — include the entry context for debuggability.

Static analysis (Ruff F541) flags this. More importantly, when a user entry lacks both username and email, the current message doesn't identify which entry in the array caused the issue, making debugging hard for large imports.

Proposed fix
-                    if 'username' not in user_entry and\
-                            'email' not in user_entry:
-                        print(f"Skipping user: missing 'username' or 'email'")
+                    if 'username' not in user_entry and\
+                            'email' not in user_entry:
+                        print("Skipping user entry: missing 'username' "
+                              "or 'email'")
🧰 Tools
🪛 Ruff (0.15.1)

[error] 242-242: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/setup.py` around lines 240 - 244, The print statement in the user import
block (where user_entry is validated) uses an f-string with no placeholders and
doesn't identify which entry failed; update the message to include context
(e.g., the user_entry contents or its index) so failures are debuggable and to
satisfy Ruff F541. Locate the validation around the user_entry variable in
web/setup.py (the block that checks 'username' and 'email') and change the print
to include either the current index from an enumerate or the user_entry dict (or
both) in the output, and keep incrementing error_count and continue as before.


# Determine auth_source (default to internal)
auth_source = user_entry.get('auth_source', INTERNAL)

# Build user data dict
user_data = {
'username': user_entry.get('username',
user_entry.get('email')),
'email': user_entry.get('email'),
'role': user_entry.get('role', 'User'),
'active': user_entry.get('active', True),
'auth_source': auth_source
}

# For internal auth, password is required
if auth_source == INTERNAL:
if 'password' not in user_entry:
print(f"Skipping user '{user_data['username']}': "
f"password required for internal auth")
error_count += 1
continue
user_data['newPassword'] = user_entry['password']
user_data['confirmPassword'] = user_entry['password']

# Check if user already exists
uid = ManageUsers.get_user(
username=user_data['username'],
auth_source=auth_source)
if uid:
print(f"Skipping user '{user_data['username']}': "
f"already exists")
skipped_count += 1
continue

# Get role ID
rid = ManageRoles.get_role(user_data['role'])
if rid is None:
print(f"Skipping user '{user_data['username']}': "
f"role '{user_data['role']}' does not exist")
error_count += 1
continue
Comment on lines +270 to +285
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.py" | xargs grep -l "def get_user" | head -5

Repository: pgadmin-org/pgadmin4

Length of output: 452


🏁 Script executed:

find . -type f -name "*.py" | xargs grep -l "def get_role" | head -5

Repository: pgadmin-org/pgadmin4

Length of output: 143


🏁 Script executed:

cat -n web/setup.py | sed -n '200,315p'

Repository: pgadmin-org/pgadmin4

Length of output: 5715


🏁 Script executed:

cat -n web/setup.py | grep -A 20 "def get_user"

Repository: pgadmin-org/pgadmin4

Length of output: 3337


🏁 Script executed:

cat -n web/setup.py | grep -A 20 "def get_role"

Repository: pgadmin-org/pgadmin4

Length of output: 763


🏁 Script executed:

# Check the json parameter usage
cat -n web/setup.py | sed -n '166,200p'

Repository: pgadmin-org/pgadmin4

Length of output: 1395


🏁 Script executed:

cat -n web/setup.py | sed -n '166,320p' | grep -E "(json|_handle_error)"

Repository: pgadmin-org/pgadmin4

Length of output: 444


🏁 Script executed:

# Find _handle_error definition
cat -n web/setup.py | grep -A 10 "def _handle_error"

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Search for _handle_error definition more broadly
rg "def _handle_error" web/setup.py -A 5

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Check if json parameter is actually used anywhere in load_users
rg "json" web/setup.py | grep -i "load_users\|def load_users" -A 30

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Search for _handle_error in the entire file
grep -n "_handle_error" web/setup.py | head -10

Repository: pgadmin-org/pgadmin4

Length of output: 490


🏁 Script executed:

# Look for the definition of _handle_error
grep -B 2 -A 8 "_handle_error" web/setup.py | head -40

Repository: pgadmin-org/pgadmin4

Length of output: 1620


🏁 Script executed:

# Check how json parameter is used in the entire ManageUsers class
sed -n '166,320p' web/setup.py | cat -n

Repository: pgadmin-org/pgadmin4

Length of output: 7212


🏁 Script executed:

# Find the INTERNAL, LDAP etc. constants
rg "INTERNAL\s*=" web/pgadmin/utils/constants.py

Repository: pgadmin-org/pgadmin4

Length of output: 85


🏁 Script executed:

# Also check what _handle_error does
rg "def _handle_error" web/pgadmin -A 8

Repository: pgadmin-org/pgadmin4

Length of output: 551


Creating a new Flask app per user inside the loop is very expensive.

ManageUsers.get_user() (line 270) and ManageRoles.get_role() (line 280) each internally call create_app() and open their own test_request_context. Since load_users already has an app context at line 236, these calls create 2×N redundant Flask application instances for N users. This will make bulk import extremely slow.

Query the User and Role models directly within the existing app context instead.

Additionally:

  • Line 242: f-string without placeholders (f"Skipping user: missing...") should be a regular string
  • Line 291: hardcoded 6 should use config.PASSWORD_LENGTH_MIN
  • Line 210: open(file_path) should specify encoding
  • Lines 205–206: error is printed twice (once explicitly, then by _handle_error)
  • json parameter (line 170) is unused
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/setup.py` around lines 270 - 285, Replace calls to ManageUsers.get_user
and ManageRoles.get_role inside load_users with direct ORM queries against the
User and Role models using the existing app context (avoid calling
create_app()/test_request_context inside ManageUsers/ManageRoles to eliminate
per-user app creation); also change the f-string "f\"Skipping user:
missing...\"" to a plain string, replace the hardcoded password min length 6
with config.PASSWORD_LENGTH_MIN, open files with explicit encoding (e.g.
open(file_path, encoding="utf-8")), remove the redundant explicit print of
errors so only _handle_error prints/logs them, and remove the unused json
parameter from the function signature and any callers. Ensure you reference
ManageUsers.get_user and ManageRoles.get_role to locate the affected calls and
update load_users and the function definition that has the json parameter.


user_data['role'] = rid

# Validate password length for internal users
if auth_source == INTERNAL:
if len(user_data['newPassword']) < 6:
print(f"Skipping user '{user_data['username']}': "
f"password must be at least 6 characters")
error_count += 1
continue

# Create the user
status, msg = create_user(user_data)
if status:
print(f"Created user: {user_data['username']}")
created_count += 1
else:
print(f"Error creating user '{user_data['username']}'"
f": {msg}")
error_count += 1

except Exception as e:
print(f"Error processing user entry: {str(e)}")
error_count += 1

print('----------')
print(f"Users created: {created_count}")
print(f"Users skipped (already exist): {skipped_count}")
print(f"Errors: {error_count}")
print('----------')

@app.command()
@update_sqlite_path
def add_user(email: str, password: str,
Expand Down
Loading