Can't PATCH User entity's 'mail' field via JSON API [#2885372]

Hi! I encountered a problem with user editing via JSON API. If I edit other users, there are no problems. And if I edit my own account there are no problems too until I want to choose my email or username. Server responds with 422 status and "Your current password is missing or incorrect" message.

So I can't understand how to send current password with data. I tryed something like this:

{
              "data": {
                "type": "user--user",
                "id" :  "5a87f646-053a-4d88-8aaa-407c2675160e",
                "attributes": {
                  "mail": "support@co.co",
                  "preferred_langcode": "ru",
                  "pass": "admin"
                }               
              }
            }

But it doesn't work

UPD

Just figured out, what should I do. I need a separate normalizer for user entities. The most important thing that user password validates against its hashed value. So solution of problem is in this piece of code:

public function denormalize($data, $class, $format = NULL, array $context = []) {

    /* @var \Drupal\user\UserInterface $user */
    $user = parent::denormalize($data, $class, $format, $context);

    $user->setExistingPassword($user->pass->value);

    return $user;
  }

Now I organised this normalizer in my custom module and it works for me. So I need some advice, what should I do with my solution? Should I publish it as separate module, or may by it will be part of jsonapi or jsonapi_extras module?

Comment	File	Size	Author
#11	2885372-jsonapi-user-protected-fields-4.patch	1.02 KB	gun_dose
#11	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass
#9	2885372-jsonapi-user-protected-fields-3.patch	4.51 KB	gun_dose
#9	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass
#8	2885372-jsonapi-user-protected-fields-2.patch	4.49 KB	gun_dose
#8	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply
#7	2885372-jsonapi-user-protected-fields-1.patch	4.49 KB	gun_dose
#7	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply
#4	2885372-jsonapi-user-protected-fields.patch	4.47 KB	gun_dose
#4	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

12 June 2017 at 10:22

gun_dose created an issue. See original summary.

Comment #2

gun_dose CreditAttribution: gun_dose commented 12 June 2017 at 13:07

Issue summary:

View changes

Comment #3

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso at Lullabot commented 9 July 2017 at 06:07

I think that submitting a patch here is a good way to fix this problem for other people.

Comment #4

gun_dose CreditAttribution: gun_dose commented 9 July 2017 at 13:22

File	Size
2885372-jsonapi-user-protected-fields.patch	4.47 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass

Finally created patch for this issue. There is a separate normalizer class for user entities. And also some changes in src/Controller/EntityResource.php - for validating of 'current_pass' field.

By the way, for performing of PATCH request of user entities you should include field, named 'current_pass', in your payload, if you are going to change user email or password. Something like

{
  "data": {
    "type": "user--user",
    "id" :  "5a87f646-053a-4d88-8aaa-407c2675160e",
    "attributes": {
      "mail": "test@test.com",     
      "current_pass":  "old_pass",            
      "pass": "new_pass"
    }               
  }
}

Comment #5

gun_dose CreditAttribution: gun_dose commented 9 July 2017 at 14:21

Component:	Miscellaneous	» Code
Status:	Active	» Needs review

Comment #6

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso at Lullabot commented 22 July 2017 at 05:32

Status:

Needs review

» Needs work

Thanks for the patch! It looks really good.

My major concern is. Did you check how REST core supports patching user entities? Are they doing something similar? Do they also have a dedicated normalizer for the entity?

Also, some code comments:

+++ b/src/Controller/EntityResource.php
@@ -246,6 +250,10 @@ class EntityResource {
+    if ($entity->getEntityTypeId() == 'user' && isset($current_pass)) {
+      $entity->setExistingPassword($current_pass);
+    }

Shouldn't it be enough to set it in UserEntityNormalizer::denormalize().

I'd like to remove any special case handling from this file. All custom code should be in the new normalizer.

+++ b/src/Normalizer/UserEntityNormalizer.php
@@ -0,0 +1,77 @@
+    if (!empty($current_pass)) $user->setExistingPassword($current_pass);

Please fix coding standards.

Comment #7

gun_dose CreditAttribution: gun_dose commented 22 July 2017 at 15:30

File	Size
2885372-jsonapi-user-protected-fields-1.patch	4.49 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply

@e0ipso, I looked how this case implemented in core rest module, and there are the same error - if you patch your owh email or password, you'll get error, that you not typed current password, and with 'current_pass' field in payload it causes error with a message "A fatal error occurred: Field current_pass is unknown." But 'current_pass' field is standart field in user profile form, so I guess, that we should use the same field in payload.

And some comments about your questions

1. Unfortunately, it's impossible to move it to denormalizer, because validation of payload fields occurs in EntityResource, and data fields are given directly from request, so we can't alter this data in denormalizer. May be there are another solution to do it, but I don't know how. If you have any ideas, say me and I will try to do something with it.
2. I attached to this comment patch with coding standards fix

Comment #8

gun_dose CreditAttribution: gun_dose commented 22 July 2017 at 16:36

File	Size
2885372-jsonapi-user-protected-fields-2.patch	4.49 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply

1 file was hidden/shown/deleted

File	Size
2885372-jsonapi-user-protected-fields-1.patch	4.49 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply

Sorry, for incorrect patch, it was for old version. And here is right patch

UPD: lol, this patch is also wrong, sorry :)

Comment #9

gun_dose CreditAttribution: gun_dose commented 22 July 2017 at 17:23

File	Size
2885372-jsonapi-user-protected-fields-3.patch	4.51 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass

Comment #10

gun_dose CreditAttribution: gun_dose commented 22 July 2017 at 17:27

1 file was hidden/shown/deleted

File	Size
2885372-jsonapi-user-protected-fields-2.patch	4.49 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 Patch Failed to Apply PHP 7 & MySQL 5.5, D8.4 Patch Failed to Apply

Comment #11

gun_dose CreditAttribution: gun_dose commented 23 July 2017 at 10:00

File	Size
2885372-jsonapi-user-protected-fields-4.patch	1.02 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.3 314 pass PHP 7 & MySQL 5.5, D8.4 307 pass

Just figured out where is root of problem. Any processings in normalizer does not affect anything at all. We need to have 'current_pass' field in request payload, but we must not process this field in entity validation. So any place where we can do something is 'patchIndividual' method in EntityResource.php, because it takes data directly from request. So I removed all unnecessary from my patch and attached result to this comment.

BTW, @e0ipso, I understand your position about patching EntityResource, but user protected fields is very specific case, that doesn't occur in another entity types, but at the same time it is very important thing to let user change it's own password.

Comment #12

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 12 December 2017 at 12:40

Title:	Can't patch user mail	» Can't PATCH User entity's 'mail' field via JSON API
Status:	Needs work	» Postponed (maintainer needs more info)
Issue tags:		+API-First Initiative, +Needs tests

See \Drupal\Tests\rest\Functional\EntityResource\User\UserResourceTestBase::testPatchDxForSecuritySensitiveBaseFields().

The only thing that should be needed, is changing

{
              "data": {
                "type": "user--user",
                "id" :  "5a87f646-053a-4d88-8aaa-407c2675160e",
                "attributes": {
                  "mail": "support@co.co",
                  "preferred_langcode": "ru",
                  "pass": "admin"
                }               
              }
            }

{
              "data": {
                "type": "user--user",
                "id" :  "5a87f646-053a-4d88-8aaa-407c2675160e",
                "attributes": {
                  "mail": "support@co.co",
                  "preferred_langcode": "ru",
                  "pass": {
                     "existing": "admin"
                  }
                }               
              }
            }

Then JSON API's entity denormalizer should set the existing value, then \Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraintValidator() will do the necessary work, including checking the existing password.