IsraelOrtuno / pipedrive

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

OAuth support for (Guzzle >= 6) #40

Closed daniti closed 6 years ago

daniti commented 6 years ago

Fixed previous PR.

Pipedrive OAuth can be instantiated like this

$pipedrive = Pipedrive::OAuth([
    'clientId' => 'xxxxx',
    'clientSecret' => 'xxxxx',
    'redirectUrl' => 'xxxxx',
    'storageClass' => new PipedriveTokenIO()
]);

PipedriveTokenIO is the user's own implementation of the PipedriveTokenStorage interface, with the two setToken and getToken methods.

setToken accepts an instance of PipedriveToken, which has these attributes

protected $access_token;
protected $expires_at; // expires AT, not IN (calculated as time() + expires_in, coming from Pipedrive
protected $refresh_token;

PipedriveToken silently takes care of refreshing the token when needed, and updates it through the setToken method.

PipedriveClient checks if the token is valid. If it's not (or it's not set yet), it calls $pipedrive->OAuthRedirect();. On the callback page, you can call $pipedrive->authorize($_GET['code']); to get the first token (also automatically stored through the setToken method).

TODO:

Here's a basic, not-so-great implementation of PipedriveTokenStorage, just to get the idea:

class PipedriveTokenIO implements \Devio\Pipedrive\PipedriveTokenStorage
{
    public function setToken(\Devio\Pipedrive\PipedriveToken $token)
    {
        $_SESSION['token'] = serialize($token); // or encrypt and store in the db, or anything else...
    }

    public function getToken()
    {
        return isset($_SESSION['token']) ? unserialize($_SESSION['token']) : null;
    }
}
IsraelOrtuno commented 6 years ago

Should we really provide support for Guzzle < 6?

IsraelOrtuno commented 6 years ago

What if we make Version 2 to OAuth only? So we can clean the complexity of maintaining both. Not sure about this, just a thought.

IsraelOrtuno commented 6 years ago

Or maybe separate Pipedrive class for the regular API access and then have an OAuthPipedrive for the OAuth implementation. I think this way it will be easier to maintain, what do you think?

Will clone your PR this weekend and play with it. Thanks a lot for such a contribution!

daniti commented 6 years ago

Sure! It's a pleasure.

I personally think Guzzle < 6 could be dropped. Most Pipedrive's integrations and apps are new and Guzzle 6 was released already ~3 years ago.

About the API token, it's probably going to stay around for a while, so I would keep it. But I agree that we could split it into two different libraries, which could share a common core since the endpoints are the same.

Also, in the future, PipedriveOAuth (or PipedriveApp...?) could have methods that would be useless for the API token one (for example for ratings, reviews, etc.).

Yes, please... Play around with it and see if everything is ok :)

If you want to start somewhere, this is what I used for testing the flow, token refresh etc. https://github.com/daniti/temp-pipedrive-oauth-test

Do you already have a Pipedrive sandbox account to create an OAuth app?

IsraelOrtuno commented 6 years ago

I do have my own Pipedrive account but was just checking how to create an App and cannot do it without sending an e-mail. I see that yo work at Pipedrive, could you create a sandbox app for me?

daniti commented 6 years ago

Yeah, you'll need a sandbox account with access to the Marketplace Manager to create an app.

Please, fill in this form https://pipedrivewebforms.com/form/c204c5aaab52ad56893c22f28a8434682563550 and I'll make sure we give you one right away.

If you need help to create a sandbox app or anything else, send me an email at [removed]

:)

daniti commented 6 years ago

Any news about this? :) @IsraelOrtuno

IsraelOrtuno commented 6 years ago

@daniti I am back into it, have just forked your PR and will try to clean up for a v2

daniti commented 6 years ago

that's great news! Let me know if I can help. P.S.: Sorry for the close/reopen. Clicked the wrong button...

IsraelOrtuno commented 6 years ago

Working on this since now @daniti

Will add this for a v2 version

IsraelOrtuno commented 6 years ago

@daniti would you mind to PR to the docs about how to use this part?

daniti commented 6 years ago

Sure. Will work on it soon.

daniti commented 6 years ago

@IsraelOrtuno I added the documentation for OAuth support.