[GH-ISSUE #276] Allow loading an expired token from the cache #82

Closed
opened 2026-02-27 20:23:01 +03:00 by kerem · 3 comments
Owner

Originally created by @DusterTheFirst on GitHub (Oct 22, 2021).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/276

Is your feature request related to a problem? Please describe.
When writing an unattended repetitive short-lived program, that will persist its credentials in the token cache that rspotify creates and subsequently load it back on the next execution as to prevent the need for re-authentication, the current implementation of the read_token_cache function is limiting. The read_token_cache function will return Ok(None) if the stored token is past its expiration rather than returning the token to the caller. This prevents such an app from loading the refresh token and using it during the automatic re-authentication that rspotify does.

Describe the solution you'd like
read_token_cache should return expired tokens rather than None. This could be as simple as the removal of token.is_expired() from the line https://github.com/ramsayleung/rspotify/blob/master/src/clients/oauth.rs#L54. The function signature would stay the same, returning an Option, since is expected behavior that read_token_cache would not load a token with mismatched scopes to that of the client.

Describe alternatives you've considered
Since this functionality may be depended on, this may be a breaking change. It may be more favorable to instead have a different method that would allow the token to be invalid or even refresh it automatically on load.

Additional context
As I currently understand it, the refresh token does not have a set expiration time and instead will stay valid until the user revokes permission to the spotify app. The expiry date stored in the cache only applies to the token and not to the refresh token as the refresh token would need to be used past the expiration of the normal token to obtain a new token.

Originally created by @DusterTheFirst on GitHub (Oct 22, 2021). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/276 **Is your feature request related to a problem? Please describe.** When writing an unattended repetitive short-lived program, that will persist its credentials in the token cache that rspotify creates and subsequently load it back on the next execution as to prevent the need for re-authentication, the current implementation of the `read_token_cache` function is limiting. The `read_token_cache` function will return `Ok(None)` if the stored token is past its expiration rather than returning the token to the caller. This prevents such an app from loading the refresh token and using it during the automatic re-authentication that rspotify does. **Describe the solution you'd like** `read_token_cache` should return expired tokens rather than None. This could be as simple as the removal of `token.is_expired()` from the line https://github.com/ramsayleung/rspotify/blob/master/src/clients/oauth.rs#L54. The function signature would stay the same, returning an Option, since is expected behavior that `read_token_cache` would not load a token with mismatched scopes to that of the client. **Describe alternatives you've considered** Since this functionality may be depended on, this may be a breaking change. It may be more favorable to instead have a different method that would allow the token to be invalid or even refresh it automatically on load. **Additional context** As I currently understand it, the refresh token does not have a set expiration time and instead will stay valid until the user revokes permission to the spotify app. The expiry date stored in the cache only applies to the token and not to the refresh token as the refresh token would need to be used past the expiration of the normal token to obtain a new token.
kerem 2026-02-27 20:23:01 +03:00
Author
Owner

@marioortizmanero commented on GitHub (Oct 22, 2021):

Hmm, if you take automatic reauthentication into account it does make sense to be able to read expired tokens. Otherwise it makes no sense when combining cached and refreshing tokens.

This code is probably from back when we didn't have self-refreshing tokens, in which case not reading expired tokens can be helpful in order to do something different if the read_token_cache call fails. But it might be useful to read expired tokens even here, because that way you know you can just refresh it, rather than perform the whole auth process from scratch (which is what you have to do if the scopes don't match). And there's no way to differentiate between expired tokens and invalid tokens in order to do that.

I think reading expired tokens makes sense in both use-cases, so it should be established as the default behavior. The breaking changes would be fine as long as we release it as v0.12.0, which is completely fine (moreso knowing we are in beta and we should focus on the future interface).

If we consider that the current behavior is acceptable in some cases, perhaps it'd better to add an allow_expired parameter to that function, and an explanation of this in the docs? That way the user is forced to check the docs and make sure it's working as intended.

As I currently understand it, the refresh token does not have a set expiration time and instead will stay valid until the user revokes permission to the spotify app. The expiry date stored in the cache only applies to the token and not to the refresh token as the refresh token would need to be used past the expiration of the normal token to obtain a new token.

You are correct. But the refresh token will also "expire" after requesting a new token in PKCE.

<!-- gh-comment-id:949778303 --> @marioortizmanero commented on GitHub (Oct 22, 2021): Hmm, if you take automatic reauthentication into account it does make sense to be able to read expired tokens. Otherwise it makes no sense when combining cached and refreshing tokens. This code is probably from back when we didn't have self-refreshing tokens, in which case not reading expired tokens can be helpful in order to do something different if the `read_token_cache` call fails. But it might be useful to read expired tokens even here, because that way you know you can just *refresh* it, rather than perform the whole auth process from scratch (which is what you have to do if the scopes don't match). And there's no way to differentiate between expired tokens and invalid tokens in order to do that. I think reading expired tokens makes sense in both use-cases, so it should be established as the default behavior. The breaking changes would be fine as long as we release it as `v0.12.0`, which is completely fine (moreso knowing we are in beta and we should focus on the future interface). If we consider that the current behavior is acceptable in some cases, perhaps it'd better to add an `allow_expired` parameter to that function, and an explanation of this in the docs? That way the user is forced to check the docs and make sure it's working as intended. > As I currently understand it, the refresh token does not have a set expiration time and instead will stay valid until the user revokes permission to the spotify app. The expiry date stored in the cache only applies to the token and not to the refresh token as the refresh token would need to be used past the expiration of the normal token to obtain a new token. You are correct. But the refresh token will also "expire" after requesting a new token in PKCE.
Author
Owner

@DusterTheFirst commented on GitHub (Oct 22, 2021):

If we consider that the current behavior is acceptable in some cases, perhaps it'd better to add an allow_expired parameter to that function, and an explanation of this in the docs? That way the user is forced to check the docs and make sure it's working as intended.

That would make a lot of sense since when a user updates and the interface changes they will have to look into that parameter rather than the interface staying the same but a slight functionality change from under them.

If it would be helpful I could create a PR proposing the changes as mentioned here.

<!-- gh-comment-id:949882028 --> @DusterTheFirst commented on GitHub (Oct 22, 2021): > If we consider that the current behavior is acceptable in some cases, perhaps it'd better to add an allow_expired parameter to that function, and an explanation of this in the docs? That way the user is forced to check the docs and make sure it's working as intended. That would make a lot of sense since when a user updates and the interface changes they will have to look into that parameter rather than the interface staying the same but a slight functionality change from under them. If it would be helpful I could create a PR proposing the changes as mentioned here.
Author
Owner

@marioortizmanero commented on GitHub (Oct 22, 2021):

That would make a lot of sense since when a user updates and the interface changes they will have to look into that parameter rather than the interface staying the same but a slight functionality change from under them.

Yup, but if it doesn't make much sense to not load expired tokens in any case then it's still a better idea to just break backward compatibility for the future's sake.

If it would be helpful I could create a PR proposing the changes as mentioned here.

Sure!

<!-- gh-comment-id:949897786 --> @marioortizmanero commented on GitHub (Oct 22, 2021): > That would make a lot of sense since when a user updates and the interface changes they will have to look into that parameter rather than the interface staying the same but a slight functionality change from under them. Yup, but if it doesn't make much sense to not load expired tokens in any case then it's still a better idea to just break backward compatibility for the future's sake. > If it would be helpful I could create a PR proposing the changes as mentioned here. Sure!
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/rspotify#82
No description provided.