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

OAuth token refreshing doesn't work #5

Closed
delabere opened this issue Sep 23, 2017 · 29 comments
Closed

OAuth token refreshing doesn't work #5

delabere opened this issue Sep 23, 2017 · 29 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@delabere
Copy link

Hi there,

Thanks again for the help with the previous issue.

Is there a way for me to restart my machine and then use MonzoAPI() without getting another Auth_Code for my Client?

Thanks,
Jack

@delabere
Copy link
Author

Update: I restarted and it did still work.

However it didn't before. Is there an expiry on the refresh token of some sort?

Thanks

@pawelad pawelad added the question Further information is requested label Sep 23, 2017
@bartonp
Copy link
Contributor

bartonp commented Sep 23, 2017

If you have updated the module to the latest one, then the token file got changed which meant that the easiest route fix that change was to reauth. I have had a refresh token used a week later without any issues.

You can also read the Monzo documentation - https://monzo.com/docs - for information on the API itself, or ask in their Slack - https://developers.monzo.com/.

@pawelad
Copy link
Owner

pawelad commented Sep 23, 2017

Yeah, basically what @bartonp said. In theory as long as you set up the OAuth client correctly and the file is on the disk it should work automatically by refreshing the token when it expires (that is, on the first request after it expires). That said, I think I had a similar situation a couple of times and couldn't really pin point the reason.

The whole OAuth part of current Monzo API is generally less than ideal and I'm mostly hoping that they'll fix it soon and do provide us with a stable 1.0 version of it.

Feel free to leave comments here if you think the refreshing doesn't work 100% of the times, but please be as descriptive about the situation as you can so I can hopefully reproduce it and find the problem.

@pawelad
Copy link
Owner

pawelad commented Sep 23, 2017

If you have updated the module to the latest one, then the token file got changed which meant that the easiest route fix that change was to reauth.

And this is also true and could be the reason if you updated pymonzo to version 0.10.0 - I decided to drop shelve module that was responsible for the weird .db extensions and rely on JSON, which meant the the file needed to be recreated.

@bartonp
Copy link
Contributor

bartonp commented Sep 23, 2017

By the way, the only re-auth problem I have had is in _get_response where the response wasn't raising an error and just continued on as if the response was the correct one. I just added this check in after response = getattr(self._session, method)(url, params=params):

if response.status_code == 401:
    raise TokenExpiredError()

@pawelad
Copy link
Owner

pawelad commented Sep 23, 2017

@bartonp hm, interesting, and definitely not how it's suppose to work. Could you please open another GitHub issue describing when was this happening? Thanks!

@delabere
Copy link
Author

This was before I updated so that wasn't causing the issue.
I have now updated and re-authed now though so hopefully it doesn't happen again - if it does I'll let you know.

@pawelad pawelad closed this as completed Sep 24, 2017
@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

Feel free to reopen this if the problem occurs again.

@delabere
Copy link
Author

Not sure how to re-open but I have had this issue again.

Came to try out my script again and needed to re-auth - again it lasted about a day.

I might try this on my pc and see if I have the same issue with the token dying.

@pawelad pawelad reopened this Sep 24, 2017
@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

So could you go step by step through it?

You successfully authenticated via. OAuth 2, ~/.pymonzo-token file was created and you could initialize MonzoAPI() without passing any arguments, even after restarting shell (getting rid of environment variables).

But on the next day it didn't work - could you paste the error? Did it happen on MonzoAPI() initialization or when trying to access any actual data? Could you check if ~/.pymonzo-token file exists on your machine?

@delabere
Copy link
Author

delabere commented Sep 24, 2017

Sure - it has actually just happened again.

You successfully authenticated via. OAuth 2, ~/.pymonzo-token file was created and you could initialize MonzoAPI() without passing any arguments, even after restarting shell (getting rid of environment variables).

Correct, I authorised by passing the arguments directly into the MonzoAPI() like this:

from pymonzo import MonzoAPI
monzo = monzoAPI(
client_id='oauthclient_XXXXXXXX',    		
client_secret='L+dlCHdSIH3r0GC0p9kjaN3XXXXXXXX'
auth_code='XXxxXXXXxXXXXXXxXX'
)

...and everything works fine. Survives shell and computer restart and works from any directory.

Token still exists:

Jacks-MBP:~ jackrickards$ ls ~/.pymonzo-token*
/Users/jackrickards/.pymonzo-token	/Users/jackrickards/.pymonzo-token.db

When I run again a day or so later without arguments I get the following error:

>>> monzo = MonzoAPI()
>>> monzo
<class 'pymonzo.monzo_api.MonzoAPI'>({'_token': {u'message': u'Provide client authentication', 
u'error_description': u'Provide client authentication', u'error': u'invalid_client'}, '_session': 
<requests_oauthlib.oauth2_session.OAuth2Session object at 0x10f8c14d0>})

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

OK, thanks for the info, I appreciate it and will look into this.

@pawelad pawelad added bug Something isn't working and removed question Further information is requested labels Sep 24, 2017
@pawelad pawelad changed the title Question: Should the token survive the shell restart? OAuth token refreshing doesn't work 'the next day' Sep 24, 2017
@delabere
Copy link
Author

No problem - appreciate the help.

I noticed from this:

Jacks-MBP:~ jackrickards$ ls ~/.pymonzo-token*
/Users/jackrickards/.pymonzo-token	/Users/jackrickards/.pymonzo-token.db

That I had two token files.

So I have deleted them both and re-authorised. Now I only have one:

Jacks-MBP:~ jackrickards$ ls ~/.pymonzo-token*
/Users/jackrickards/.pymonzo-token

I will let you know if/when it fails again or if this solved it. Though I'm not sure why it would have changed it's mind over which one to use.

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

So I have deleted them both and re-authorised. Now I only have one:

That's expected and should't be related - it's because of the token file format change from shelve to JSON.

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

@delabere could you try one thing for me?

Could you authenticate using OAuth, save somewhere the contents of ~/.pymonzo-token, then run MonzoAPI()._refresh_oath_token() and compare current ~/.pymonzo-token with the one before that command was run? You don't need to paste the contents here, you can just tell me:
a) if it worked (no errors, you could access data after running that
b) did the contents of ~/.pymonzo-token change

Somewhat related - I should probably add some debug logging to the application : )

@delabere
Copy link
Author

delabere commented Sep 24, 2017

I get this error:

>>> MonzoAPI()._refresh_oath_token()
{u'message': u'Provide client authentication', u'error_description': u'Provide client authentication', 
u'error': u'invalid_client'}

@delabere
Copy link
Author

So it seems it happened again even faster this time. I need to re-authenticate and didn't realise.

I will do that and then tell you what happens with the refresh.

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

OK, then for some reason you can't refresh the token at all.

First things first - could you check for me if you marked the created OAuth client as confidential in Monzo API playground? : -)

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

@delabere you can check it by going to https://developers.monzo.com/apps/home and selecting your application:

screenshot 2017-09-24 15 07 40

@delabere
Copy link
Author

image

Sorry that took a while, the email didn't come through.

@pawelad
Copy link
Owner

pawelad commented Sep 24, 2017

OK, so I'm currently out of ideas. Let me test a couple things out.

@bartonp
Copy link
Contributor

bartonp commented Sep 25, 2017

The refresh token is looking for self._client_token and self._client_secret which aren't set in the __init__ when you go through the 'token file route'. The only one out of the two of them that needs to be set should be self._client_secret as the client_id is stored within the token, which currently isn't the case. You are getting the invalid client_id because your client is currently set to none when you haven't passed it in.

If you pass in the client_id and client_secret, and then add this code to the __init__ above the a) choice, you'll fix the error but not in the best way:

        if client_secret is not None:
            self._client_secret = client_secret
        if client_id is not None:
            self._client_id = client_id

The documentation on refreshing the token can be seen here - https://monzo.com/docs/#refreshing-access - for reference.

@delabere
Copy link
Author

@bartonp

Thanks I've given that a go. I will report back in a day or so to see if my token holds up.

Question: Do I need to set the remaining IF's to ELIF's in the init class?

It didn't seem to cause any problems when running MonzoAPI() so I'm guessing now.

@pawelad
Copy link
Owner

pawelad commented Sep 25, 2017

@bartonp ah, you're right, I must've messed that up during recent refactoring - sorry about that. I'll try to have a bugfix release later today.

@bartonp
Copy link
Contributor

bartonp commented Sep 26, 2017

@delabere The rest of the code stays the same in the init, only add what I mentioned above. You don't need to change anything else there. Let us know how it went, the non-refreshing part of the token I believe only lasts a couple of hours anyway :)

@delabere
Copy link
Author

delabere commented Sep 26, 2017

@bartonp
So I can confirm it has worked for the longest time I can remember. Will confirm back in another 24 hours.

I also sent the refresh command mentioned earlier and it worked without error, and the token information changed.

I think you've sorted it.

@delabere
Copy link
Author

delabere commented Oct 3, 2017

Ooops - never came back.

Can say that it's been working sine the @bartonp fix.

Happy to test if there is a new release available .

@pawelad pawelad changed the title OAuth token refreshing doesn't work 'the next day' OAuth token refreshing doesn't work Oct 6, 2017
@pawelad
Copy link
Owner

pawelad commented Oct 6, 2017

Closed via #8

@pawelad pawelad closed this as completed Oct 6, 2017
@pawelad pawelad added this to the v0.10.2 milestone Oct 6, 2017
@pawelad
Copy link
Owner

pawelad commented Oct 6, 2017

This should be fixed in v0.10.2 - thanks again for the help @bartonp!

@pawelad pawelad self-assigned this Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants