fix: add is_admin and full_name fields as optional in the request when updating user details #2736
Merged
crivetimihai merged 3 commits intomainfrom Feb 7, 2026
Merged
Conversation
…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>
b1ea9b7 to
18817f5
Compare
crivetimihai
reviewed
Feb 7, 2026
Member
crivetimihai
left a comment
There was a problem hiding this comment.
Rebased onto latest main (18abdde) — no conflicts, all tests pass.
Review notes:
- Logic is correct. Optional fields default to
Noneand are only applied when explicitly provided. Password validation is properly enforced throughauth_service.validate_password()before hashing. - No security concerns — schema provides baseline
min_length=8, service layer enforces full complexity rules.PasswordValidationErroris caught and returned as HTTP 400. - No performance impact.
Minor observations (non-blocking):
hasattr()is redundant — on a Pydantic model,hasattr(user_request, "full_name")is alwaysTruesince the field is declared. Theis not Nonecheck alone suffices. Cosmetic — functionality is correct either way.is_activenot handled —AdminUserUpdateRequestdefinesis_active, but the endpoint ignores it. Pre-existing issue (not introduced by this PR); the UI uses separate/activateand/deactivateendpoints.- Consistency — other endpoints (e.g.,
rbac.py) use.dict(exclude_unset=True)for partial updates. This PR usesis not Nonechecks. Both work correctly for this use case. - Test coverage — could add a test for single-field partial updates (e.g., only
full_nameor onlyis_admin), but existing tests cover the main scenarios well.
LGTM overall — the fix correctly addresses the reported bug.
crivetimihai
approved these changes
Feb 7, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🐛 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_nameandis_adminfields should be optional and should update the user'sfull_nameandis_adminwhen they are included in the request🔁 Reproduction Steps
Example API Call:
In this case, the
user.full_nameis going to beNoneand it'll be displayed asN/Ain UI.🐞 Root Cause
email,password,is_admin, andfull_nameare treated as mandatory fields.If
full_nameis not included in the request payload, the existing full name is overwritten and becomesnull/NA.💡 Fix Description
update_userfunction inmcpgateway/routers/email_auth.pyEmailRegistrationRequestwithAdminUserUpdateRequestto have fields optional.full_nameandis_adminfields from the request as optional and update them in the DB when they are available.PasswordValidationErrorexception when the password does not meet the requirementsAdminUserUpdateRequest inmcpgateway/schemas.py`Add the optional password field to
AdminUserUpdateRequestwith basic min_length=8, and let the existing service validation handle the rest!🧪 Verification
make lintmake testmake coverage📐 MCP Compliance (if relevant)
✅ Checklist
make black isort pre-commit)