Improve Oauth2 API for handling tokens persistence with SQL

Open
#207 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
authentication

Research direction

Start in boxsdk/auth/oauth2.py at the existing refresh_lock uses in refresh and revoke, then compare the RedisManagedOAuth2 implementation. Determine how a lock_tokens context manager could expose both token retrieval and locking while remaining overrideable for SQL SELECT ... FOR UPDATE. Done means SQL-backed token persistence can use the API without rewriting refresh or revoke.

Written by the indexing model from the issue text.

Description

enhancement

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 (https://github.com/box/box-python-sdk/blob/6597e930929f3ee49d4930a53304277167c0a4db/boxsdk/auth/oauth2.py#L201-L202 and https://github.com/box/box-python-sdk/blob/6597e930929f3ee49d4930a53304277167c0a4db/boxsdk/auth/oauth2.py#L305-L306) 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()
Dominant language
Python
Stars
460
Forks
223
Avg merge
14h 18m
Merged PRs (30d)
21

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from box/box-python-sdk

All issues in box/box-python-sdk

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.