Skip to content

fix: add is_admin and full_name fields as optional in the request when updating user details #2736

Merged
crivetimihai merged 3 commits intomainfrom
2693-fix-update-users-through-api
Feb 7, 2026
Merged

fix: add is_admin and full_name fields as optional in the request when updating user details #2736
crivetimihai merged 3 commits intomainfrom
2693-fix-update-users-through-api

Conversation

@marekdano
Copy link
Copy Markdown
Collaborator

@marekdano marekdano commented Feb 6, 2026

🐛 Bug-fix PR

Closes #2693

📌 Summary

Updating admin privileges through the API requires mandatory fields (password, full_name), which results in unintended overwriting of existing user details.

The full_name and is_admin fields should be optional and should update the user's full_name and is_admin when they are included in the request

🔁 Reproduction Steps

  1. Call the admin user update API without providing all required fields.
  2. The API returns a "required field missing" error.
  3. Provide all mandatory fields and update the user.

Example API Call:

curl -X 'PUT' \
  'http://localhost:4444/auth/email/admin/users/user_admin%40example.com' \
  -H "Authorization: Bearer $TOKEN" \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -d '{
  "email": "user_admin@example.com",
  "password": "abcd1235",
  "is_admin": true
}'

In this case, the user.full_name is going to be None and it'll be displayed as N/A in UI.

🐞 Root Cause

email, password, is_admin, and full_name are treated as mandatory fields.

If full_name is not included in the request payload, the existing full name is overwritten and becomes null / NA.

💡 Fix Description

  1. Theupdate_user function in mcpgateway/routers/email_auth.py
  • Replace EmailRegistrationRequest with AdminUserUpdateRequest to have fields optional.
  • Handle full_name and is_admin fields from the request as optional and update them in the DB when they are available.
  • Add to catch PasswordValidationError exception when the password does not meet the requirements
  1. The AdminUserUpdateRequest in mcpgateway/schemas.py`

Add the optional password field to AdminUserUpdateRequest with basic min_length=8, and let the existing service validation handle the rest!

  1. Add more unit tests to cover the change

🧪 Verification

Check Command Status
Lint suite make lint
Unit tests make test
Coverage ≥ 80 % make coverage
Manual regression no longer fails steps / screenshots

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

…quest

Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
@crivetimihai crivetimihai force-pushed the 2693-fix-update-users-through-api branch from b1ea9b7 to 18817f5 Compare February 7, 2026 11:36
Copy link
Copy Markdown
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

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

Rebased onto latest main (18abdde) — no conflicts, all tests pass.

Review notes:

  • Logic is correct. Optional fields default to None and are only applied when explicitly provided. Password validation is properly enforced through auth_service.validate_password() before hashing.
  • No security concerns — schema provides baseline min_length=8, service layer enforces full complexity rules. PasswordValidationError is caught and returned as HTTP 400.
  • No performance impact.

Minor observations (non-blocking):

  1. hasattr() is redundant — on a Pydantic model, hasattr(user_request, "full_name") is always True since the field is declared. The is not None check alone suffices. Cosmetic — functionality is correct either way.
  2. is_active not handledAdminUserUpdateRequest defines is_active, but the endpoint ignores it. Pre-existing issue (not introduced by this PR); the UI uses separate /activate and /deactivate endpoints.
  3. Consistency — other endpoints (e.g., rbac.py) use .dict(exclude_unset=True) for partial updates. This PR uses is not None checks. Both work correctly for this use case.
  4. Test coverage — could add a test for single-field partial updates (e.g., only full_name or only is_admin), but existing tests cover the main scenarios well.

LGTM overall — the fix correctly addresses the reported bug.

@crivetimihai crivetimihai added this to the Release 1.0.0-RC1 milestone Feb 7, 2026
@crivetimihai crivetimihai merged commit 26f1f3e into main Feb 7, 2026
51 checks passed
@crivetimihai crivetimihai deleted the 2693-fix-update-users-through-api branch February 7, 2026 11:49
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
…n updating user details (IBM#2736)

* fix: add password and full_name fields as optional for update user request

Signed-off-by: Marek Dano <mk.dano@gmail.com>

* fix: add is_admin field as optional for the update user request

Signed-off-by: Marek Dano <mk.dano@gmail.com>

* fix: lint issue when running make flake8

Signed-off-by: Marek Dano <mk.dano@gmail.com>

---------

Signed-off-by: Marek Dano <mk.dano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Unable to Update User via Admin UI & API Requires Mandatory Fields Causing Full Name Loss

2 participants