security: harden Spotify OAuth state and token lifecycle - #146
Open
Blackspirits wants to merge 11 commits into
Open
security: harden Spotify OAuth state and token lifecycle#146Blackspirits wants to merge 11 commits into
Blackspirits wants to merge 11 commits into
Conversation
Blackspirits
marked this pull request as ready for review
August 9, 2026 09:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens the Spotify authorization and token lifecycle without changing the public card API.
statevalue to the authorization request.stateto an HttpOnly, SameSite=Lax cookie and validates it withhmac.compare_digestin the callback.expired_tswhen the initial access token is issued, avoiding an unnecessary refresh on first card access.refresh_tokenfrom a refresh response.invalid_grant) from transient/malformed refresh failures, so temporary Spotify errors do not delete stored authorization or incorrectly tell users to re-authenticate.Security rationale
Spotify's Authorization Code flow recommends using
stateto protect the redirect flow against CSRF. Bearer access tokens are credentials and should not be emitted to application logs. Spotify refresh responses may omit a new refresh token, so the existing refresh token must be retained unless a replacement is returned. A refresh failure other thaninvalid_grantis not proof that the user's stored authorization is invalid and must not delete those credentials.References:
Compatibility
/api/viewparameters and normal rendering behavior are unchanged.expired_tsis additive token metadata.api/view.svg.pyis not modified by this PR.api/view.py/ the.svgendpoint. If fix local dev setup + local file playback crashes #144 lands first, this branch should be rebased and the token-lifecycle changes re-audited against the resulting endpoint implementation before merge.Tests
Added/updated tests cover:
access_denied) authorization without token exchange.codeanderror.expires_inmetadata.Review / CI status
The complete diff has been re-audited after the follow-up fixes. The PR is currently mergeable and contains changes to six files only.
The upstream
Python Flask CI/CDworkflow is still reported asaction_required: GitHub created the workflow run but did not start jobs because this fork contribution requires maintainer approval. The current head therefore still needs the upstream pytest/CI run before merge.