search

coopTilleuls / CoopTilleulsForgotPasswordBundle

Provides a "forgot password" complete feature for your API through a Symfony bundle
MIT License
79 stars 25 forks source link

Issue with naming of token and tokenValue parameter #100

Closed Cruiser13 closed 2 years ago

Cruiser13 commented 2 years ago

Describe the bug The API and Routing in the current version does expect the path to be something like /forgot_password/{tokenValue} But the parameter in the OpenAPI specs is token only: https://github.com/coopTilleuls/CoopTilleulsForgotPasswordBundle/blob/main/Bridge/ApiPlatform/OpenApi/OpenApiFactory.php#L123

This leads to a 404 because the route /forgot_password/{tokenValue} will be called instead of /forgot_password/345234234

You can rename the route to /forgot_password/{token} easily. But this will break the recognition of the token as the RequestEventListener will look for tokenValue at the moment: https://github.com/coopTilleuls/CoopTilleulsForgotPasswordBundle/blob/main/EventListener/RequestEventListener.php#L108

To Reproduce Setup a project with the current defaults

Expected behavior Use eighter token or tokenValue through the whole bundle.

Additional context I'd be happy to provide a PR renaming all instances of token to tokenValue or the other way around - depends on your preferences.

vincentchalamon commented 2 years ago

Hi @Cruiser13,

Thanks for opening this ticket. Could you open a PR to fix it please?

vincentchalamon commented 2 years ago

After investigation, it's totally normal to have a tokenValue as a request attribute, because the token request attribute is built from the RequestEventListener. This naming prevents erasing the original attribute.

The issue was on the OpenApiFactory parameter declaration, it should be tokenValue instead of token.

But I don't understand how you got a 404 while calling this route. Can you provide more context please @Cruiser13?

Cruiser13 commented 2 years ago

@vincentchalamon the open api docs (and the client generator of api platform) will not replace tokenValue in the request url as token is requested and not tokenValue. This leads to a request of /forgot_password/{tokenValue}?token=sometoken which will trigger a 404 since the route /forgot_password/{tokenValue} does not exist. I hope that has been a better explanation?

vincentchalamon commented 2 years ago

Can you try replacing 'token' by 'tokenValue' in the OpenApiFactory to check if it solves your issue please?

Cruiser13 commented 2 years ago

@vincentchalamon that would work the same way. That's the reason I asked wether I should be renaming all instances of token to tokenValue or the other way around in the first place :-)

vincentchalamon commented 2 years ago

OK I misunderstood the original issue, my bad! The parameter name must be tokenValue. The token parameter is just for internal usage as a PasswordToken object retrieved from the token value.