Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth2 support #205

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

OAuth2 support #205

wants to merge 9 commits into from

Conversation

ebogdanov
Copy link

@ebogdanov ebogdanov commented Mar 14, 2018

Closing issues: #193, #184, #181.
Since Intuit do not issue OAuth1 keys anymore, it can be very handy for business to migrate their code without any hastle in the future.

Code comments:
Unfortuanelly, I had to split current OAuth1 code into base & child class.
In other places tried to followed original code style and logic as much as possible.

@ebogdanov
Copy link
Author

Hi.

Thanks for your project.

This pull request been a while in review state, may be I need to fix something here?
I'd happy to discuss this.

@ebogdanov
Copy link
Author

ebogdanov commented Apr 9, 2018 via email

Copy link

@kpirbhai kpirbhai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 3181 in Sql.php. In making the column request_datetime, it is set to NOT NULL with no default but the field isn't filled in at least when initially saving the token. This causes the request to fail.

ebogdanov and others added 6 commits April 10, 2018 15:48
My previous commit was a bit hasty, there is an additional parameter that must be passed before user.
The CodeIgniter web connector example passes null for this parameter by default, and that seems to work in my application.
Remove create_function to improve php 7.2 compatibility
@WiredWonder
Copy link

Has this pull request been merged? Keen to get OAuth 2.

Thanks,
Jason

@ebogdanov
Copy link
Author

@galapogos01 No, it's not, I did not got any feedback about it.
I think it will be great if you can help with testing. If you insist on latest version I can rebase my cloned repo.

@WiredWonder
Copy link

Thanks @ebogdanov.

I have just had a look at this and can't work out how to switch my App to using OAuth 2 in the QuickBooks Developer Portal. Maybe they sent me the OAuth deprecation email a bit early because there is nowhere I can enable OAuth 2 in my App.

@tangofour
Copy link

Galapogos01,

I sent email to quickbooks support because I couldn't find the new keys for my app at the developer.intuit.com. The existing account was setup before oauth2 and doesn't have the new keys. Turns out you either have to create a new developer account or migrate your existing account.

I created a new account to test this code. I didn't know how to pull the changes, so I screen scraped all he changes from the 14 changed files. Seems to be working OK. I can connect with my new sandbox using oauth2 creds.

Any idea when this will be merged?
Thanks,
Jim.

@ebogdanov
Copy link
Author

@galapogos01
The oldest app I've, is still on OAuth1 and Intuit did not sent any notifications when migration is scheduled. I think that they will do it one day for sure :)

If you'll try to create new app, the only one choice is OAuth2.

@tangofour
Thanks for your efforts.
To test those changes you can just clone my repo https://github.com/ebogdanov/quickbooks-php, not the original one.

@tangofour
Copy link

I found a couple of issues when I tried to integrate my changes into the existing auth1.0 app.

The config.php was already using $token for the application token in auth1.0.
$token = new QuickBooks_IPP_OAuth2($oauth2_client_id, $oauth2_client_secret);
$helper = new QuickBooks_IPP_OAuth2Helper($token);
caused a conflict on 1.0 app $token.
I changed to code to be
$oauth2_token = new QuickBooks_IPP_OAuth2($oauth2_client_id, $oauth2_client_secret);
$helper = new QuickBooks_IPP_OAuth2Helper($oauth2_token);

I didn't like clearing out the oauth2 key when using auth1.0 so I added a flag instead.
I'd rather make the decision on the fly, but there is no way to know the version before logging into intuit.

if ( ! $UsingAuth2 )
{
$oauth_version = QuickBooks_IPP::AUTHMODE_OAUTH;
$IntuitAnywhere = new QuickBooks_IPP_IntuitAnywhere($dsn, $encryption_key, $oauth_consumer_key, $oauth_consumer_secret, $quickbooks_oauth_url, $quickbooks_success_url);
}
else
{
$oauth_version = QuickBooks_IPP::AUTHMODE_OAUTH2;
$oauth2_token = new QuickBooks_IPP_OAuth2($oauth2_client_id, $oauth2_client_secret);
$helper = new QuickBooks_IPP_OAuth2Helper($oauth2_token);
$IntuitAnywhere = new QuickBooks_IPP_IntuitAnywhereOAuth2($dsn, $encryption_key, $oauth2_client_id, $oauth2_client_secret, $quickbooks_oauth_url, $quickbooks_success_url);
// Refresh token if expired
$IntuitAnywhere->refresh_expired_token($the_username, $the_tenant);
if ($IntuitAnywhere->errorNumber())
{
echo 'Unable to refresh access token: ' . $IntuitAnywhere->errorMessage();
}
$quickbooks_oauth_url = $helper->getAuthorizationURL($quickbooks_oauth_url);
}

I can use the same code for oauth1.0 test and production
and oauth2.0 test, I don't have the production keys yet.

Thanks for making these changes, really helped me out.
Jim.

@Jxckaroo
Copy link

I have tried and i cannot get this to work at all. After trying to "connect" to a company in QB, i just get stuck in a continuous loop of selecting a company and then clicking continue... Help?

@ebogdanov
Copy link
Author

@Jxckaroo As I understand this can happen if there is problem with DB structure or records in it.

quickbooks-php/QuickBooks/IPP/IntuitAnywhereOAuth2.php
function handle, something happens here.

Can you verify this way, please?

@Jxckaroo
Copy link

Jxckaroo commented Jan 23, 2019

@ebogdanov I have debugged throughout the code and it came down to scopes.

I changed this:

$quickbooks_oauth_url = $helper->getAuthorizationURL($quickbooks_oauth_url);

To this:

// Production scopes
// $scope = 'com.intuit.quickbooks.accounting,com.intuit.quickbooks.payment'; 

// Dev scopes
$scope = 'com.intuit.quickbooks.payment';

$quickbooks_oauth_url = $helper->getAuthorizationURL($quickbooks_oauth_url, $scope);

I think this is something to do with the dev apps that QB provide dont have the right scopes available? Or could just be QB account setup, havent managed to try in prod yet. Either way, it doesnt seem to like the default of the accounting scope, which is why i set my own and pass them through.

This is the default it doesnt seem to like:

public function handle($app_username, $app_tenant, $scope = QuickBooks_IPP_OAuth2Helper::SCOPE_ACCOUNTING, $status = '')

Would it not be better to fail and display a failure than defaulting scopes that the user may not have?

@ebogdanov
Copy link
Author

@Jxckaroo Thanks for your research and feedback.
Yes, this makes sense. But this is totaly linked with your application settings or account which you're trying to connect.
Code worked for sure with "com.intuit.quickbooks.accounting" scope for me (this code is used in production) and peoples who tried this before you.

A bit more info about how to start with Quickbooks API can be found here: https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0#download-the-oauth-library

I'm quoting here all available scopes for reference:

com.intuit.quickbooks.accounting — QuickBooks Online API
com.intuit.quickbooks.payment — QuickBooks Payments API
com.intuit.quickbooks.payroll — QuickBooks Payroll API (whitelisted beta apps only)
com.intuit.quickbooks.payroll.timetracking — QuickBooks Payroll API for for access to compensation (whitelisted beta apps only)
com.intuit.quickbooks.payroll.benefits — QuickBooks Payroll API for access to benefits/pension/deduction (whitelisted beta apps only)

@Jxckaroo
Copy link

@ebogdanov No problem - Yes, I got the scope which i needed from the API docs, thanks. Would it bet better if this was handled via failures rather than a default? - Id be more than happy to make the change on your repo and submit a pull request 😄

@ebogdanov
Copy link
Author

@Jxckaroo Sure, you're welcome to send pull request. 👍
Thanks.

@ebogdanov
Copy link
Author

@consolibyte
Copy link
Owner

Heads-up -- we should have this stuff working sometime next week.

@aik099
Copy link
Contributor

aik099 commented Feb 1, 2019

Heads-up -- we should have this stuff working sometime next week.

Thanks @consolibyte . Will there be any migration guide so that as library user we'll know how to update our code as well?

@camflan
Copy link

camflan commented Feb 11, 2019

@consolibyte Any updates?

@WiredWonder
Copy link

@consolibyte Keith any update? @ebogdanov did Keith accept your pull request?

@WiredWonder
Copy link

@ebogdanov I think Keith is referring to non-OAuth2 commits in https://github.com/consolibyte/quickbooks-php/pull/205/commits

Hoping to get some progress on this soon. QB are doing a drive to have people converted by May.

@ebogdanov
Copy link
Author

@galapogos01 Those commits are from the main branch and git able to supress them automatically during merge.

@WiredWonder
Copy link

I got sick of waiting for upstream to merge this pull so I tried your repo @ebogdanov . Also the temptation of the Quickbooks hoodie got me motivated.

Upon granting access and being redirected back to my App I am seeing the following errors and no connection:

`Notice: unserialize(): Error at offset 11 of 736 bytes in /wp-content/plugins/tiper-quickbooks/QuickBooks/Encryption/Aes.php on line 82

Warning: current() expects parameter 1 to be array, boolean given in /wp-content/plugins/tiper-quickbooks/QuickBooks/Encryption/Aes.php on line 83

Notice: unserialize(): Error at offset 11 of 736 bytes in /wp-content/plugins/tiper-quickbooks/QuickBooks/Encryption/Aes.php on line 82

Warning: current() expects parameter 1 to be array, boolean given in /wp-content/plugins/tiper-quickbooks/QuickBooks/Encryption/Aes.php on line 83`

Any tips appreciated.

@ebogdanov
Copy link
Author

Hoodie? :) Intuits sends hoodies for developers who will do early migration? I've to force employer to do this fast... :)

  1. Have you created new DB tables?
  2. Can you try to set encryption key to be empty and try again?
  3. If 2. is not acceptable, I suspect that something wrong with encryption.
    quickbooks-php/QuickBooks/Encryption/Aes.php have commented code to debug this.

let me know if this will help or not, please.

@WiredWonder
Copy link

WiredWonder commented May 1, 2019

This is what the latest Intuit Developer emails have been saying!

  1. Yes (I had to trash all tables to force the plugin to create quickbooks_oauth2; not sure how I could have avoided this).
  2. This worked! No more errors.
  3. It does not sound ideal to skip encryption! Any ideas what to look for? It is failing unserializing $decrypted but I am not clear on what format this string should be or why it is not unserialize-able.

edit: so $decrypted is an 800+ char string, truncated at 736 chars. Not sure if this is a limitation of mcrypt

Thanks for your help.

Cheers,
Jason

@ebogdanov
Copy link
Author

Yes, sounds like there is issue with encryption and truncation.
Since oauth2 keys are longer, such issue appeared. I feel like I've found it during development, but decide that fixing encryption in this branch is too much "changes" :)

I will research it on weekend and let know, either by commit here, or via comment if for some reason there is not backwards-compatible solution.

@WiredWonder
Copy link

Thanks Evgeniy. I couldn't find any obvious reason for the truncation but I'm no encryption expert!

@busbyjon
Copy link

busbyjon commented May 2, 2019

Hey! @ebogdanov and @galapogos01 - I know this is off topic on this PR - but the link is here if you're app needs updating and want a hoodie!

https://partners.intuit.com/

@ebogdanov
Copy link
Author

@busbyjon Thanks for pointing.

@ebogdanov
Copy link
Author

@galapogos01
I've tested this. On my machine it works correct. The same behaviour as you've described happens only if company was linked, and after this encryption key was changed. Decryption will fail and warnings will be shown. So that I can't confirm this and need more info.

consolibyte pushed a commit that referenced this pull request May 6, 2019
@consolibyte
Copy link
Owner

consolibyte commented May 6, 2019

Quick heads-up -- we're merging in support for OAuth2. Should have things for new apps ready tomorrow. Some commits were merged in today for the initial pieces. More later today.

A migration process from OAuth1 to OAuth2 should follow this week.

@ebogdanov
Copy link
Author

@consolibyte
Thanks for great news.

Do you merge your own implementation or I should solve conflicts?

@sapangupta2009
Copy link

Any update on OAuth2.0 support? Can i clone this repo and use it for oAuth2.0 app?

@sapangupta2009
Copy link

I have cloned this repo and getting following error:

Fatal error: Error Num.: 1054 Error Msg.:Unknown column 'oauth_state' in 'field list' SQL: INSERT INTO quickbooks_oauthv2 ( app_tenant, oauth_state, request_datetime ) VALUES ( '12345', 'b5663992657ee74a14f77e449fe3rr35', '2019-05-09 08:58:42' ) Database: /quickbook in /opt/lampp/htdocs/quickbooks-php/QuickBooks/Driver/Sql/Mysqli.php on line 374

@bairdj1
Copy link

bairdj1 commented May 12, 2019

Three fields are missing from quickbooks_oauthv2 when they are created. oauth_state, last_access_datetime, and last_refresh_datetime. Once you add those fields, the db errors go away. I was able to get connected to my sandbox company, but I am getting ERROR_XML when I try to query the customer info or item list. This is the same code that works with oauth1. I didn't think I would have to change things that far downstream from getting connected. Are there parameter changes etc that I need to do to get this to work?

@consolibyte
Copy link
Owner

@bairdj1 @sapangupta2009 We'll have detailed instructions tomorrow morning on how to migrate/cut things over. Am aware that the SQL schema is missing currently (we got held up slightly here because all of Intuit's sandboxes were down Thurs/Fri).

@consolibyte
Copy link
Owner

Initial instructions are here; still cleaning them up:
#181

@consolibyte
Copy link
Owner

consolibyte commented May 16, 2019

The 3.6 release has working OAuth v2.0 support now. OpenID Connect and an automated OAuth v1 to v2 migration process are coming soon.

https://github.com/consolibyte/quickbooks-php/releases/tag/v3.6

Manual migration (e.g. you have to reconnect via the OAuth flow) instructions are here:
https://github.com/consolibyte/quickbooks-php/blob/master/README_OAUTHV1_TO_OAUTHV2.md

@consolibyte
Copy link
Owner

ALSO -- Migrate your app to OAuth 2.0 before May 31 and you could win a limited edition Intuit hoodie, PLUS if you tell Intuit about your migration plans, you’ll gain entry to win the entertainment system of your dreams. Find out more here: https://intuit.me/OAuth2

@WiredWonder
Copy link

Worked first go. Staging & Prod now migrated. Thanks Keith!

@consolibyte
Copy link
Owner

@galapogos01 Glad to here things are working! Make sure you let Intuit know so you get your hoodie/get entered into their drawing!

@bairdj1
Copy link

bairdj1 commented May 21, 2019 via email

@WiredWonder
Copy link

@consolibyte how do I let them know exactly?

@consolibyte
Copy link
Owner

@galapogos01 Fill out the form here: https://partnerportal.intuit.com/migration/

@consolibyte
Copy link
Owner

@bairdj1 You too! Fill out the form here to let Intuit know! https://partnerportal.intuit.com/migration/

You'll be entered into a drawing and could win some cool stuff!

@whobutsb
Copy link

Thank you @consolibyte and everyone else on who put in work to get this package updated. It has been a huge help.

How are you guys handling updating the token? Do we really need to login to QuickBooks every hour to keep the token alive?

@consolibyte
Copy link
Owner

@whobutsb If you're using our code, the token updating happens automatically. You DO NOT have to have the person log in again, ever. The access/refresh tokens are updated whenever the access token expires and the new values get written to MySQL/MariaDB in the quickbooks_oauth2 table.

aik099 pushed a commit to intechnic/quickbooks-php that referenced this pull request Jul 23, 2019
* master:
  Update to README for OAuthv2.
  Fixes consolibyte#181, adds OAuth v2 support.
  Work on consolibyte#181, clean up of migration to OAuthv2.
  Work on consolibyte#181, converted first example over to OAuthv2.
  Work on consolibyte#181, converted first example over to OAuthv2.
  Work on consolibyte#181, auth and refresh/access token renewal.
  Work on consolibyte#181, auth and HTTP requests via OAuth2 actually working now.
  Work on consolibyte#181, changes to allow OAuth2 requests.
  Work on consolibyte#181, initial OAuth2 authentication flow working; still need to do the actual request signing part of things and OpenID Connect.
  Work on consolibyte#205, initial support for OAuth2.
  Adding support for updating estimates in QBO.

# Conflicts:
#	QuickBooks/IPP.php
#	QuickBooks/IPP/IntuitAnywhere.php
@aik099
Copy link
Contributor

aik099 commented Jul 23, 2019

@consolibyte, I was testing OAuth2 integration and stumbled upon couple of Fatal Errors. See #269.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.