IsraelOrtuno / pipedrive

Complete Pipedrive API client for PHP
MIT License
166 stars 58 forks source link

PipedriveToken class constructor changes token values #95

Closed ziimk closed 3 years ago

ziimk commented 3 years ago

👋 A dev from Pipedrive here. We are working on changing the token format in the near future and came across an issue in this library during testing.

When new PipedriveToken([...]) is called here with values from Pipedrive's OAuth /token endpoint response then the values are run through array_map('camel_case', ...).

This will result in broken access/refresh tokens assigned to the class properties if the first symbol happens to be in upper case.

For example, access token Foo will be transformed to foo and will obviously fail validation when used to fetch data from Pipedrive API.

The token format should be opaque to the clients so any kind of manipulation with the tokens is risky :)

I think the easiest fix is to just remove the call to camel_case. As it works only on the array values, I don't quite understand why it's there at all 🤔

IsraelOrtuno commented 3 years ago

@daniti since you implemented this part, would you mind to help here?

daniti commented 3 years ago

I think the idea was to affect the keys and not the values, to be able to instantiate it with snake case attributes - like the ones coming from the API response - but the implementation is clearly a mistake.

Removing this call shouldn't negatively affect anything. I just made a PR with this line removed.

Thank you @ziimk

IsraelOrtuno commented 3 years ago

Fixed PR #97