State Processors: Hashing the User Password

Hey @Hugues!

Hmm, interesting! Let's think about this. If you don't return something from your processor, then API Platform will ultimately serialize the $data argument that you passed in. In your case, unless something else is going on, the inner processor, after saving, returns $data right back to you. So... there shouldn't be any difference: if you return nothing, then $data (your entity) is serialized. But by returning $this->innerProcessor->process()... that should result in returning the same object! If you dump $data and $this->innerProcessor->process(), they must be different. Are they? If so, what are the differences? I would be interested to know - it smells like a mystery :).

Cheers!

Hi @weaverryan,
Thank you for your answer !
You're totally right $data and $this->innerProcessor->process() return the same entity.
I use Symfony 6.4 with api-platform/core v3.2.7.
If I debug my UserHashPasswordProcessor method I have this call stack :

App\State\UserHashPasswordProcessor->process (~my-project/src/State/UserHashPasswordProcessor.php:34)
ApiPlatform\State\CallableProcessor->process (~my-project/vendor/api-platform/core/src/State/CallableProcessor.php:46)
ApiPlatform\State\Processor\WriteProcessor->process (~my-project/vendor/api-platform/core/src/State/Processor/WriteProcessor.php:45)
...

// vendor/api-platform/core/src/State/Processor/WriteProcessor.php
final class WriteProcessor implements ProcessorInterface
{
    // ...

    public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = [])
    {
        // some code ...
        
        // $this->processor is an instance of SerializeProcessor
        // $this->callableProcessor is an instance of our UserHashPasswordProcessor
 +       return $this->processor->process($this->callableProcessor->process($data, $operation, $uriVariables, $context), $operation, $uriVariables, $context);
    }
}

if I've not returned anything from my processor $data will be null in the process method of SerializeProcessor.

// vendor/api-platform/core/src/State/Processor/SerializeProcessor.php
final class SerializeProcessor implements ProcessorInterface
{
    // ...

    public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = [])
    {
        // ...
    
        $serialized = $this->serializer->serialize($data, $request->getRequestFormat(), $serializerContext);
        // aie I don't have anything...    
        // ...
        
        return $this->processor->process($serialized, $operation, $uriVariables, $context);
    }
}

perhaps the code has changed in the meantime?
have a good weekend and I hope the conferences in Brussels were fascinating...

Hey @Hugues!

Hmm, indeed! The WriteProcessor stuff is new in API Platform 3.2... and while its addition shouldn't have changed anything, by looking at your debugging, it seems like you're right. If you don't return anything, nothing gets serialized. Well, actually, this is likely because you're using the new "mode" of API Platform without event listeners - https://api-platform.com/docs/core/upgrade-guide/#event-listeners - I'm guessing the "old" mode still works. But really, returning something from your processor is more correct anyway. I'll add a note to the tutorial to help others.

I hope the conferences in Brussels were fascinating...

It was GREAT. Many friends, many great conversations and ideas. Excited about the future :)

Cheers!

Hi @weaverryan,
I suspect a regression in API Platform 3.3.
If I run the tests in the "finish" folder after upgrading Symfony to 6.4 and API Platform to 3.3, then the patch() calls return null again.
If I revert only API Platform to 3.2 and keep Symfony to 6.4, the tests pass flawlessly.
In 3.2, event_listeners_backward_compatibility_layer: false breaks the tests, while when set to true they pass.
In 3.3, either value breaks the tests, so I guess the issue comes from somewhere else.

1) App\Tests\Functional\DragonTreasureResourceTest::testPostToCreateTreasure<br />Expected "(null)" to be the same as "A shiny thing".

2) App\Tests\Functional\DragonTreasureResourceTest::testPatchToUpdateTreasure<br />Expected "(null)" to be the same as "12345".

... 3 more errors like these. Do you have any suggestion to debug the source of the issue ? Thanks!

EDIT : fixed
The issue was that I added the return of $this->innerProcessor->process() only to one of the two StateProcessors, adding the return to all of them, changing the method return type to mixed solves the bug. event_listeners_backward_compatibility_layer is not concerned.

Hey @Tobias-B!

Your thinking on this is absolutely correct. For me, it was a trade-off between complexity (the API Platform official way is more complex) vs potential performance problems. So, the final decision is subjective, but since PersistProcessor is only called during POST/PUT/PATCH operations and it will only be called once (I would be more concerned if PersistProessor were called many times during a single request) and the logic inside of UserHashPasswordProcessor is really simple in those cases (if not User, it exits immediately), I think the performance issue is non-material. So, I went for simplicity :). But I think the other approach is 110% valid - so you can choose your favorite.

Cheers!

I'm building this tutorial not in the project but in a custom bundle.
Since it took me some time to find the solution for my case, I'd like to post it here for others that might struggle with that.

I did NOT set the #[AsDecorator(PersistProcessor::class)] attribute in UserHashPasswordProcessor

In the User Entity I added processor: UserHashPasswordProcessor::class to Put, Post and Patch.
Example:

new Put(
            security: 'is_granted("ROLE_USER_EDIT")',
            processor: UserHashPasswordProcessor::class
)

In the bundles services.xml I added:

<service id="Acme\MyBundle\State\UserHashPasswordProcessor" autowire="true" autoconfigure="true">
            <bind key="$processor" id="api_platform.doctrine.orm.state.persist_processor" type="service"/>
</service>

Since this Processor is called from the User Entity now, I also modified the process method in UserHashPasswordProcessor a litte:

public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): void
    {
        if ($data->getPlainPassword()) {
            $hashedPassword = $this->userPasswordHasher->hashPassword($data, $data->getPlainPassword());
            $data->setPassword($hashedPassword);
            $data->eraseCredentials();
        }

        $this->processor->process($data, $operation, $uriVariables, $context);
    }

If this isn't smart, please correct me :-)

Hey @Sebastian-K!

Hmmm. How have you set things up so that the "registration" endpoint has a firstName field but without User having a firstName property? Usually I WOULD have this as a property on User, or I might make a DTO for this specific operation if you've got things split up.

But anyway, this is an interesting problem! The JSON is ready from the request and passed directly to the serializer here: https://github.com/api-platform/core/blob/main/src/Symfony/EventListener/DeserializeListener.php#L98-L101

The problem is that, if your "resource class" for this operation is User and it doesn't have a firstName property, then that field from the JSON is simply ignored. I think the only way to get the firstName field would be to grab the $request->getContent() and json_decode() it manually. But... I really hope we can find a better way :).

Cheers!

Hey @Frederic

How did you set up the UserHashPasswordStateProcessor? What Ryan did was to decorate the main "persist" processor service, which would surprise me to throw an error. But anyway, the docs way seems better to me because you don't want to check the object's type every time

Cheers!

I can't confirm I did exactly the same as Ryan, I followed the script (no video, because not paid membership (yet...)), maybe I missed something in the video that isn't shown in the script and I might also just have missed something obvious on my part.

I manage to make it work the Ryan's way. Don't know what went wrong the first time
But I noticed something else, I let the dump('ALIVE') statement at the beginning of my process() method, and I discovered it behaves differently between both ways of wiring :
using AsDecorator, "ALIVE" gets dump to the console during test,
using the service config file, it doesn't.
Everything else happens the same (post, patch, hash, persist)

I see, if it is working now it's very likely that you missed something.
The difference between the config file and using PHP attributes is that attributes are not environment-specific, they will work on any your app is running, but config files are bound to an environment (dev, test, prod). If you configure the decorator service in a config/services_test.yaml file, does it work?

Cheers!

Hey @Sylvain-B!

Hmm, yea, I think I know what's going on - and my tip was misleading! If you wanted to simply inject/autowire the api_platform.doctrine.orm.state.persist_processor into some other service, using PersistProcessor::class (which is a service alias) would work great. But with decoration, it doesn't - and that's my fault. Explanation:

• The actual id of the core service is api_platform.doctrine.orm.state.persist_processor. Other services specifically reference this id to have this service injected into them.
• When you decorate api_platform.doctrine.orm.state.persist_processor, you effectively "replace" this service with your own. So anyone that was previously asking for this service id to be passed to them, will now be passed your service.
• But when you decorate PersistProcessor::class, you are decorating a service alias. So IF anyone asked for PersistProcessor::class to be passed to them, they would receive your service. But in reality, everyone asks for api_platform.doctrine.orm.state.persist_processor... which remains the original, core service.

Thanks for mentioning this - I wanted people to be aware of the new, shorter way of referencing the service - but in decoration, it's incorrect! I'm going to remove the note.

Cheers!

Hey @Fedale!

That's a great question. I can think of 2 options, and they're both totally fine imo:

• 1) A doctrine listener/subscriber as you were mentioning. These are a bit magic and they're hard to debug if you make a mistake while setting them up (they might just "not work" but no errors). But they work great and are very robust (once you get them working, it's not going to suddenly break).
• 2) Duplicate the hashing logic: do it in ApiPlatform like we do, then do it in EasyAdmin by, for example, overriding persistEntity() in your controller. Duplication sounds lame... but password hashing logic is already SO simple (it's just 1 line basically) that you are not really duplicating much.

Cheers!