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

Refresh Tokens + Web App Example? #84

Closed
bryanveloso opened this issue Oct 3, 2013 · 24 comments
Closed

Refresh Tokens + Web App Example? #84

bryanveloso opened this issue Oct 3, 2013 · 24 comments
Labels

Comments

@bryanveloso
Copy link

I've been trying to wrap my head around refresh tokens and the OAuth2 web example (https://requests-oauthlib.readthedocs.org/en/latest/examples/real_world_example.html). Would it be possible to provide an example usage of refresh tokens in that context? Where in the code it would go, etc?

Just a suggestion! 😁

@ib-lundgren
Copy link
Member

We should definitely have an example of refreshing in a "real world" app. Think it might be good to have a new extended example that includes refreshing as to not overwhelm the reader.

As I see it there are (at least) two ways to go about refreshing in a web app. Both happening possibly long after the initial OAuth dance in the middle of normal API interactions.

  1. Validate the token with the provider (if they provide such a feature)(e.g. Github). If valid continue else refresh. This could be done first thing in a view where the token is used.
  2. Utilize the expires_in token attribute to guess if its valid or not. This would happen "behind the scenes" in requests-oauthlib assuming expires_in is provided correctly.

Note that even if the token has not expired it might have been revoked. This is more likely in 2 than 1 (manifest as a race condition after validation in 1). Unfortunately, there is no telling whether this has happened in terms of a pre-defined OAuth 2 error. One approach would be to "catch" a response code of 401 and use 1 to validate the token and refresh. Naturally this should be covered in a/the example too.

@bryanveloso would this help you think? any other bits that need detailing?

@shazow
Copy link
Contributor

shazow commented Oct 3, 2013

+1 to @bryanveloso, I'm currently debugging why my refresh stuff isn't working and it's not obvious (and the intricacies are not very documented).

Google API uses a refresh token, if you need an API to build an example around.

The prescribed method, "Define automatic token refresh and update" is supposed to refresh the token automagically, no? If so, I'm not sure what you mean by your questions, @ib-lundgren.

@bryanveloso Not sure if this is helpful to you, as I haven't been able to get refreshing working properly myself, but my approach has been to pass a closure into token_updater which includes the necessary context necessary to save the changed token. Specifically, mine looks something like this (using SQLAlchemy):

def make_token_updater(token_obj):
    def wrapped(new_token):
        token_obj.token = new_token
        Session.commit()
    return wrapped

...

oauth = OAuth2Session(..., token_updater=make_token_updater(current_token))

@ib-lundgren
Copy link
Member

@shazow the refresh is a bit fragile in that the only way requests-oauthlib/oauthlib can tell whether a refresh is needed is whether the time from s = OAuth2Session(..., token={...., expires_in=X}) to when the token is used, i.e. s.get(protected) is if the time delta between the two is larger than X. Thus updating expires_in falls on the user of requests-oauthlib. Something which is a bit unclear and would be much better explained in a web app like example.

@shazow
Copy link
Contributor

shazow commented Oct 3, 2013

@ib-lundgren So what does the automatic token refresh do?

Fwiw, the way Google's oauth2 client handles it is it immediately converts the expires_in time to a unix timestamp and stores that instead of the relative expires_in time. This way it can tell whether it's expired or not at request time. Any reason why we can't do that?

@ib-lundgren
Copy link
Member

@shazow That is exactly what I intended to do in the example. It is also what is happening in the automatic token refresh but in a more awkward way than necessary. Currently requests-oauthlib honors the expires_in but what might be better would be an expires_at. I remember starting to look into that but got interrupted and frankly forgot about it since.

The pseudo code for the expires in approach would be

token = sess.fetch_token(....)
token[expires_at] = seconds(token[expires_in]) + now()
save_token(token)

# take a break

token = load_token()
token[expires_in] = token[expires_at] - now()
sess = OAuth2Session(..., token=token)

We could make that saner by having requests-oauthlib return the expires_at and accept an expires_at timestamp as you suggest.

@shazow
Copy link
Contributor

shazow commented Oct 3, 2013

Ah, that is a little hacky but makes sense, thanks!

+1 on doing it more transparently. :)

Seems oauthlib uses datetime objects rather than just epoch int. Dunno why.

@ib-lundgren
Copy link
Member

@shazow because I just happened to write it that way. Think unix timestamps would be nicer so can do a little cross library refactor of it.

@ib-lundgren
Copy link
Member

Will try and get these examples sorted and add expires_at support by the end of this week.

@shazow would it be helpful to send the OAuth2Session into the updater perhaps? and/or the original token?

@bryanveloso
Copy link
Author

Thank you guys, this sounds perfect. 🤘

@shazow
Copy link
Contributor

shazow commented Oct 3, 2013

@ib-lundgren Hrm, not sure, I don't see much benefit. I feel like you'll need a closure no matter what you do. Would love to see counterexamples. :)

@ib-lundgren
Copy link
Member

@bryanveloso @shazow when/if you have a moment (no rush) please take a look at the w.i.p. refresh example at https://gist.github.com/ib-lundgren/6823954 and let me know if it is useful and what needs work.

The doc strings will have more details about use-cases and what we have talked about here. I hope to find time to add an example showing token revocation -> unauthorized API interaction -> token validation -> obtain new token. That could be included either here or in a separate example.

Note that with oauthlib master version a new attribute expires_at is added to all tokens (unix timestamp).

@shazow
Copy link
Contributor

shazow commented Oct 4, 2013

A few thoughts:

  • What's the deal with docstring_templator?
  • I'd put the focus on automatic_refresh as that's the recommended usage in the docs (and really most practical). But the name is a bit misleading, as you don't need to manually automatically refresh. :P I'd call the two methods something like query_with_auto_refresh and query_with_manual_refresh. Make sure the manual refresh example is symmetric—that is, it does a query, catches the need to refresh, does the refresh, etc.
  • Fwiw, Google's OAuth2 clients don't subtract any seconds from the expires_at time. Probably safe to not do it (I suspect there's a small buffer, serverside).

Definitely a huge improvement from what we have now. Thanks for this. :)

@ib-lundgren
Copy link
Member

Thanks for the feedback :-)

The templator is just a temporary thing to save me some time while manually
testing the tutorial parts. Will remove it as the example "matures".

I think I'll remove the "manual" example and the validation bit. Maybe move
them to another example that shows how to catch the need to refresh/restart
based on validation and not time.

The expires_at replacement (not subtract) is to force refresh rather than
sleep for an hour :-)
On 4 Oct 2013 14:47, "Andrey Petrov" notifications@github.com wrote:

A few thoughts:

  • What's the deal with docstring_templator?
  • I'd put the focus on automatic_refresh as that's the recommended
    usage in the docs (and really most practical). But the name is a bit
    misleading, as you don't need to manually automatically refresh. :P I'd
    call the two methods something like query_with_auto_refresh and
    query_with_manual_refresh. Make sure the manual refresh example is
    symmetric—that is, it does a query, catches the need to refresh, does the
    refresh, etc.
  • Fwiw, Google's OAuth2 clients don't subtract any seconds from the
    expires_at time. Probably safe to not do it (I suspect there's a small
    buffer, serverside).


Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-25699535
.

@shazow
Copy link
Contributor

shazow commented Oct 4, 2013

Ah, in that case, I'd fix it to use a real example that people will inevitably copypasta into their code, and mention in a comment how to override it to force a refresh now.

The default should be real working code. Don't want getting people confused why their tokens are getting refreshed on every load. :)

@ib-lundgren
Copy link
Member

Good point. Will make it a comment.
On 4 Oct 2013 15:17, "Andrey Petrov" notifications@github.com wrote:

Ah, in that case, I'd fix it to use a real example that people will
inevitably copypasta into their code, and mention in a comment how to
override it to force a refresh now.

The default should be real working code. Don't want getting people
confused why their tokens are getting refreshed on every load. :)


Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-25701691
.

@bryanveloso
Copy link
Author

Apologies for being a bother; has any progress been made?

@shazow
Copy link
Contributor

shazow commented Oct 24, 2013

@bryanveloso If it's any help, I've been using this:

import time
from requests_oauthlib import OAuth2Session

def _clean_token(token):
    """Strip out all the stuff we don't need, and add an `expires_at` field."""
    return {
        'access_token': token['access_token'],
        'token_type': token['token_type'],
        'refresh_token': token['refresh_token'],
        'expires_at': int(time.time() + token['expires_in']),
    }

def _token_updater(old_token):
    def wrapped(new_token):
        token = _clean_token(new_token)

        # Do what you need to persist the new token. Save it into the DB or whatever.
        old_token.update(token)
        Session.commit()

    return wrapped

def auth_session(request, token=None, state=None):
    if token and 'expires_at' in token:
        token['expires_in'] = int(token['expires_at'] - time.time())

    return OAuth2Session(
        oauth_config['client_id'],
        redirect_uri=request.route_url('account_connect'),
        scope=oauth_config['scope'],
        auto_refresh_url=oauth_config['token_url'],
        auto_refresh_kwargs=_dict_view(oauth_config, ['client_id', 'client_secret']),
        token_updater=_token_updater(token),
        token=token,
        state=state,
    )


def auth_url(oauth):
    return oauth.authorization_url(
        oauth_config['auth_url'],
        access_type='offline', approval_prompt='force',
    )

def auth_token(oauth, response_url):
    token = oauth.fetch_token(
        oauth_config['token_url'],
        authorization_response=response_url,
        client_secret=oauth_config['client_secret'],
    )
    return _clean_token(token)

This is using Pyramid's request and SQLAlchemy's Session, should be pretty straight forward to port to Django or what have you.

@bryanveloso
Copy link
Author

This worked great @shazow, thank you. ❤️

@ib-lundgren
Copy link
Member

@bryanveloso Sorry about not following through, been very swamped lately and frankly forgot about this completely. It is back on the todo list now so on the next free slot Ill sort it out :)

@akavlie
Copy link

akavlie commented Nov 18, 2013

Is there still a need to update the expires_in parameter?

Current oauthlib is setting and checking an expires_at param. Not sure when that was added.

@ib-lundgren
Copy link
Member

@akavlie if you store and supply the token that contains expires_at then no it should not be needed. However I've not been looking at this code for a long time now with life keeping my way to busy I can't remember exactly and would need to refresh my memory/experiment a bit.

@akavlie
Copy link

akavlie commented Dec 9, 2013

I can verify that the expires_in param is being updated internally by the library.

@ib-lundgren
Copy link
Member

Cheers.

On Mon, Dec 9, 2013 at 10:51 PM, Aaron Kavlie notifications@github.comwrote:

I can verify that the expires_in param is being updated internally by the
library.


Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-30182503
.

@ib-lundgren
Copy link
Member

Available at http://requests-oauthlib.readthedocs.org/en/latest/examples/real_world_example_with_refresh.html since a little while back. Closing :)

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

No branches or pull requests

4 participants