search

emersion / hydroxide

A third-party, open-source ProtonMail CardDAV, IMAP and SMTP bridge
MIT License
1.63k stars 126 forks source link

feat: Reuse access tokens #247

Closed ydylla closed 4 months ago

ydylla commented 1 year ago

Hi, looks like a new access token is fetched on each action (e.g. smtp login), instead of reusing it as long as its valid.
For my account the access tokens currently are valid for 24 hours, so reusing them could save some requests.

emersion commented 4 months ago

The intent of the current behavior is to refresh the token while we can, to push back the expiration time while we can.

ydylla commented 4 months ago

But the current code does not reuse the token at all, it seems to just fetch a new one every time. Or am I missing something?

My use case is an app that opens multiple short smtp session to send a single email in quick succession (one email per http request). In that case it's better to reuse the token instead of risking triggering some rate limiting on login requests. I use a custom build hydroxide with this commit without problems since I opened the PR.

Edit: Ah I think the current behavior is for a use case with smtp sessions that are open for longer. You hope that some other session starts in the meantime which would then also refresh the token for the already open session.

emersion commented 4 months ago

The current logic is designed to avoid ending up with expired tokens.

With this patch, if a user opens an SMTP session once a day, we'd be very likely to never call AuthRefresh while the token is still valid. Additionally IMAP sessions are long-lived, so we don't want to keep going without refreshing the token if it's only going to be valid for a few minutes.

I suppose the real solution here would be to set up a timer to refresh the token before it expires? It would be nice to test if the refresh token can be used after the access token expires (and how long after).

ydylla commented 4 months ago

Yes either a timer which refreshes the token in background. Or the token must be checked and potentially refreshed closer to its usage and not just during session login. Maybe somehow in setRequestAuthorization but I don't know the code base good enough to really evaluate this.
I will close this PR because apparently it's not a good solution for all use cases. It just fixes my use case.

emersion commented 4 months ago

To fix your use-case without regressing other cases we could check if the token has been refreshed recently, e.g. less thna 5 min ago.