Fix user setting API to use allowlist for updatable fields

[dale053]

What problem does this PR solve?

The PATCH /users/me endpoint uses a denylist to filter out sensitive fields, which means any new or unexpected fields in the request body are accepted and written to the database by default. This allows users to potentially overwrite protected fields that weren't explicitly blocked.

This PR replaces the denylist with an allowlist (nickname, avatar, language, color_schema, timezone), so only explicitly permitted fields can be updated. This is a safer default — new fields are denied unless intentionally added to the allowlist.

Fixes #14861

Type of change

• Bug Fix (non-breaking change which fixes an issue)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 62d25c19-7ea6-4380-8571-a4328c87563c

📥 Commits

Reviewing files that changed from the base of the PR and between 51f69d2 and 1be827e.

📒 Files selected for processing (2)

• api/apps/restful_apis/user_api.py
• test/testcases/test_web_api/test_user_app/test_user_app_unit.py

🚧 Files skipped from review as they are similar to previous changes (1)

• api/apps/restful_apis/user_api.py

---

📝 Walkthrough

Walkthrough

The PATCH /users/me handler now uses an allowlist (nickname, avatar, language, color_schema, timezone) to build the update payload, logs any other non-password fields that were ignored, and tests were updated to capture and assert the warning and excluded fields.

Changes

User Settings Update Security Fix

Layer / File(s)	Summary
Allowlist-based user field update api/apps/restful_apis/user_api.py	setting_user constructs update_dict from an explicit allowlist of safe fields, collects unsupported non-password keys into ignored_fields, logs a warning containing the current user id and sorted ignored fields, then calls UserService.update_by_id with the filtered payload.
Tests capture audit warnings and assert excluded fields test/testcases/test_web_api/test_user_app/test_user_app_unit.py	The unit test monkeypatches logging.warning to collect calls, asserts update_by_id is called without theme, email, and status, and verifies the logged warning includes the current user id and the omitted-field list.

Sequence Diagram

sequenceDiagram
  participant Client
  participant UserAPI as PATCH_users_me
  participant UserService
  participant Database
  participant Logger

  Client->>UserAPI: PATCH /users/me with request body
  UserAPI->>UserAPI: filter request fields using ALLOWED_USER_UPDATE_FIELDS
  UserAPI->>UserService: update_by_id(user_id, filtered_update_dict)
  UserService->>Database: persist allowed fields
  UserAPI->>Logger: warning(user_id, ignored_fields)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through code with careful paws,

Only safe fields pass my laws.
Warnings noted, tests attest,
Now user updates are more blessed.
Hooray—no secrets left to gnaw!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: switching the user settings API from a blocklist to an allowlist for field updates.
Description check	✅ Passed	The description fully addresses the template with a clear problem statement and correct type of change classification as a bug fix.
Linked Issues check	✅ Passed	The PR implements the exact allowlist security fix requested in issue #14861: restricting PATCH /users/me to only {nickname, avatar, language, color_schema, timezone} and logging ignored fields.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the security fix: API logic refactoring to use allowlist and corresponding unit test updates to verify the new behavior and logging.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/apps/restful_apis/user_api.py`:
- Around line 336-339: The loop building update_dict drops unexpected keys
silently; modify the code around ALLOWED_USER_UPDATE_FIELDS / request_data /
update_dict to collect any request_data keys that are not in
ALLOWED_USER_UPDATE_FIELDS and emit an audit log (e.g., using the existing
logger or current_app.logger) containing the ignored field names (not values)
and context (user id or request id if available) at info/warning level so client
misuse or abuse is visible; keep the existing behavior of not including values
and still only adding allowlisted keys to update_dict.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e46a6854-b47c-4b82-a087-24093d601e64

📥 Commits

Reviewing files that changed from the base of the PR and between 5e46457 and 51f69d2.

📒 Files selected for processing (1)

• api/apps/restful_apis/user_api.py

[dale053]

Hi, @JinHai-CN
When you have a moment, could you please review this PR?