nwunderly / starlette-discord

"Login with Discord" support for Starlette and FastAPI
https://starlette-discord.rtfd.io
MIT License
32 stars 11 forks source link

DiscordOAuthSession Lost access token should be preserved considering its longevity. #8

Closed chrisdewa closed 3 years ago

chrisdewa commented 3 years ago

When the DiscordOAuthSession instance fetches the access_token on callback, the information contained in the token is this:

{'access_token': '...', 'expires_in': 604800, 'refresh_token': '...', 'scope': ['identify', 'guilds'], 'token_type': 'Bearer', 'expires_at': 1621962039.1969795}.

(tokens removed for obvious reasons).

This information is lost in the examples supplied but can be preserved in this way:

client = DiscordOAuthClient(CLIENT_ID, CLIENT_SECRET, REDIRECT_URI)
....

@app.get('/callback')
async def callback(code: str):    
    async with client.session(code) as session:
        token = session._discord_token  # notice protected attribute access

The expires_in field gives the amount of seconds that the token should last before it expires, in this case its 604800 which amounts to one week.

As follows, its unreasonable that the same token information cannot be used in following requests.

The DiscordOAuthSession instance must be able to be constructed from a valid token instead of only with the code

# following the client's construction
client = DiscordOAuthClient(CLIENT_ID, CLIENT_SECRET, REDIRECT_URI)

# sessions should be able to be constructed from the exchanged code on login redirect
session = client.session(code='dhjaksdhjadskhdaskjda')
# or with a stored, valid token
session = client.session(token='dsjkdsaljdaskldas')

Aditionally, it would also be useful if there was a token_updater method that would make use of the supplied refresh_token.

chrisdewa commented 3 years ago

I'm working on a PR to solve this issue.

nwunderly commented 3 years ago

I honestly wasn't aware tokens could last up to a week. From what I'd seen in the docs it was my understanding that the tokens should be discarded after use. This is also why I made the token a private attribute.

The ability to construct a session from a valid session is definitely something worth adding. Should probably add a property for accessing the token as well. I'll take a look at your PR tomorrow, I'm sure we can get something worked out.

Appreciate the help!

nwunderly commented 3 years ago

Fixed in most recent PR. 👌