Skip to content

Improve Oauth2 API for handling tokens persistence with SQL #207

Open

Description

@dtheodor

I'm trying to accommodate storing my tokens in an SQL database, and trying to extend the OAuth2 class to enable storing and retrieving the tokens from the database. The _refresh_lock is something that is naturally also managed by the same database. (You have a conceptually similar implementation for redis with RedisManagedOAuth2)

What you would typically do with SQL in order to lock a specific row to update it (exactly what we want to do here) is use the SELECT ... FOR UPDATE command, which locks and retrieves the data at the same time

The existing code where the lock is used (

with self._refresh_lock:
access_token, refresh_token = self._get_and_update_current_tokens()
and
with self._refresh_lock:
access_token, refresh_token = self._get_and_update_current_tokens()
) looks like this :

def refresh():
  with self.refresh_lock:
    tokens = self.get_tokens()
    # ... proceed with logic to update them

What I'm looking for is this

def refresh():
  with self.select_for_update(...) as tokens:
    # ... proceed with logic

Well, I can't really extend the class to achieve that unless I rewrite the whole methods refresh and revoke. I think a better API to express the need to "lock tokens and retrieve them" is with its own context manager function, that can have a default implementation that can be overridden. This contextmanager can be trivially rewritten to use select for update

def refresh():
  with self.lock_tokens(...) as tokens:
    # ... proceed with logic

@contextmanager
def lock_tokens():
    with self.refresh_lock:
      yield self.get_tokens()

Activity

  1. Jeff-Meadows commented on Jun 12, 2017

    @Jeff-Meadows
    Contributor

    I think you can accomplish this with the current API, by creating a custom lock-like object that calls your SELECT .. FOR UPDATE and saves the tokens as state on the object. Entering the context manager via with self._refresh_lock: will issue your SQL command and make the tokens available until __exit__ is called. You could then override _get_tokens to simply return the stored tokens.

    class SQLOAuth2(OAuth2):
        def __init__(self, *args, **kwargs):
            refresh_lock = SelectForUpdateLock(self)
            super(SQLOAuth2, self).__init__(*args, refresh_lock=refresh_lock, **kwargs)
    
        def _get_tokens(self):
            return self._refresh_lock.tokens
    
        class SelectForUpdateLock(object):
            def __init__(self, sql_auth):
                super(SelectForUpdateLock, self).__init__()
                self._sql_auth = sql_auth
                self.tokens = None
    
            def select_for_update(self):
                """Implementation""""
    
            def __enter__(self):
                with self.select_for_update() as tokens:
                    self.tokens = tokens
    
            def __exit__(self, *args):
                self.tokens = None
    
  2. dtheodor commented on Jun 12, 2017

    @dtheodor
    Author

    Yes that's right. I'm merely suggesting what I think is an improvement that looks like the right abstraction to me and can avoid a hidden state solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions