SpotifyAPI icon indicating copy to clipboard operation
SpotifyAPI copied to clipboard

AuthorizationManager should deauthorize if app access was removed via Spotify

Open Mohamad-Kredly opened this issue 1 year ago • 1 comments

While looking into Issue https://github.com/Peter-Schorn/SpotifyAPI/issues/71, I ran into a new issue after revoking the app's access from Spotify Account -> Manage Apps (https://www.spotify.com/ca-en/account/apps/).

What happens here is that the refresh token is revoked, but the package doesn't handle this error which leads to a negative UX when the user wants to sign in again and naturally, the API calls will be running into errors. The access token, however, remains valid till it expires.

Encountered with AuthCodeFlowManager (proxy and client), untested with other auth methods.

Log Trace

[spotifyDecode: trace: decodeSpotifyErrors(data:httpURLResponse:) line 67] will try to decode data from URL 'https://accounts.spotify.com/api/token' into error objects: {"error":"invalid_grant","error_description":"Refresh token revoked"} refresh tokens completion: failure(SpotifyWebAPI.SpotifyAuthenticationError(error: "invalid_grant", errorDescription: Optional("Refresh token revoked")))

Steps to reproduce

  1. Authorize Spotify
  2. Go to https://www.spotify.com/ca-en/account/apps/ and remove the app's access
  3. Attempt to refresh the token

This will also be reflected when restarting the app and seeing the log out button instead of sign in when token has expired

Mohamad-Kredly avatar Dec 27 '24 17:12 Mohamad-Kredly

I applied a small patch that is currently working for me but I won't be doing a pull request for it as I haven't extensively tested it and I'm not familiar with Combine:

AuthorizationCodeFlowManager.swift

// Code...
public extension AuthorizationCodeFlowBackendManager {
// Code...
func refreshTokens(
        onlyIfExpired: Bool,
        tolerance: Double = 120
    ) -> AnyPublisher<Void, Error> {
// Code...
                    .handleEvents(
                        // Once this publisher finishes, we must
                        // set `self.refreshTokensPublisher` to `nil`
                        // so that the caller does not receive a publisher
                        // that has already finished.
                        receiveCompletion: { completion in
                            switch completion {
                            case .failure(let error):
                                // If a user revokes access using Spotify, the refresh token will be revoked
                                if let authError = error as? SpotifyAuthenticationError,
                                   authError.errorDescription == "Refresh token revoked" {
                                    Self.logger.warning("Access was revoked")
                                    // Clear the stored tokens since they're no longer valid
                                    DispatchQueue.main.async {
                                        self.deauthorize()
                                    }
                                }
                            case .finished:
                                break
                            }
                            self.refreshTokensPublisher = nil
                        }
                    )
// Code...

SpotifyAPI+Wrappers.swift

extension SpotifyAPI {
// Code..
func apiRequest(
        url: URL,
        httpMethod: String,
        makeHeaders: @escaping (_ accessToken: String) -> [String: String],
        bodyData: Data?,
        requiredScopes: Set<Scope>
    ) -> AnyPublisher<(data: Data, response: HTTPURLResponse), Error> {
// Code...
        return self.authorizationManager.refreshTokens(
            onlyIfExpired: true, tolerance: 120
        )
        .catch { error -> AnyPublisher<Void, Error> in
            // Handle revoked token error specifically
            if let authError = error as? SpotifyAuthenticationError,
               authError.errorDescription == "Refresh token revoked" {
                self.logger.warning("unauthorized: access was revoked")
                
                // Convert to unauthorized error for consistent handling
                return SpotifyGeneralError.unauthorized(
                    "unauthorized: access was revoked"
                ).anyFailingPublisher()
            }
            // Pass through other errors
            return error.anyFailingPublisher()
        }
        .tryMap { () -> String in
// Code...

A similar patch will likely have to be implemented in PKCE. I considered calling deauthorize() on the level of SpotifyAPI to simplify the code to just one file modification but refreshTokens() is public so this made more sense

Mohamad-Kredly avatar Dec 27 '24 20:12 Mohamad-Kredly