Proxy Tokens Server

[Peter-Schorn]

I'm opening this issue in order to discuss all of the changes to proxy-tokens-server in one place. @Ivorforce

[Ivorforce]

Linking #14, #16, #17

[Peter-Schorn]

See https://github.com/Ivorforce/SpotifyAPI/pull/2#issue-623769413

[Peter-Schorn]

We should only merge changes into master that have been finalized in proxy-tokens-server. This is why I'm willing to merge #17, but not #16. The changes to the authorization managers are not finalized because I've barely started on the documentation.

[Ivorforce]

Fair. I'll try to see if there's anything else worth extracting already, or contestable to work on.

[Peter-Schorn]

@Ivorforce Please comment on the current structure of the authorization managers (make sure you pull the latest changes):

ClientCredentialsFlowManager, AuthorizationCodeFlowManager, and AuthorizationCodeFlowPKCEManager have each been split into two separate classes: a base class that is generic over the backend and a concrete subclass that uses the client backend. This makes the library much simpler to use for clients who don't need to implement a custom backend.

[Ivorforce]

I'm not seeing this change in your branch right now. But I would argue against concrete subclasses.

Instead, I would typealias commonly used subtypes under that name and, if desired, add convenience inits and extensions. Although I would refrain from doing this too much since it duplicates a lot of code and docstring.

As a heads-up: I might take a while going through all this (tomorrow), as I'm sure you notice it's a lot of changes. I'll still be focusing on trying to separate out individual PRs that can reduce this one's complexity. But I will definitely also have more comments.

[Peter-Schorn]

Just look at the latest commit on proxy-tokens-server. See Below. Don't pay attention to the documentation comments. I haven't updated them yet.

https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/ClientCredentialsFlowManager.swift#L17 https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/ClientCredentialsFlowManager.swift#L674-L676

https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/AuthorizationCodeFlowManager.swift#L61-L64 https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/AuthorizationCodeFlowManager.swift#L562-L564

https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/AuthorizationCodeFlowPKCEManager.swift#L89-L92 https://github.com/Peter-Schorn/SpotifyAPI/blob/aad995f50bfd194e7895aaaab6636dec5be2cc4c/Sources/SpotifyWebAPI/Authorization/AuthorizationCodeFlowPKCEManager.swift#L668-L670

[Ivorforce]

Ah, thanks for the links. I think it's good to keep backwards compatibility through this, and shield a bit of the complexity from the user. Yet my comment holds. In light of the new adaptability, I would change the nature of these from concrete subclasses to typealias.

[Peter-Schorn]

Thanks for the feedback, but I'm going to overrule you here and keep the concrete subclasses. By the way, do you have your own backend server setup that the new proxy backends can use? If so, I'll show you how to run some of my integration tests against it. It's important that the server returns the correct status code and error responses from spotify as well as the successful responses.

[Ivorforce]

I use the default one someone provided using Glitch. Integration tests would probably be useful for this, I agree.

[Peter-Schorn]

I've added a test plan, SpotifyAPIProxyServer, which runs all the tests that use a proxy server. More specifically, it runs tests for AuthorizationCodeFlowPKCEClientBackend, AuthorizationCodeFlowProxyBackend, and ClientCredentialsFlowProxyBackend. If your server only works with one of the authorization flows, then just disable the tests for the other ones.

Make sure you have http://localhost:8080 registered as a redirect URI.

First save your client id and secret to a file in the following (JSON) format:

{
    "client_id": "abcabcabcabcabcabcabcabcabcabcabcabc",
    "client_secret": "abcabcabcabcabcabcabcabcabcabcabca"
}

Then, open the test plan at Tests/SpotifyAPIProxyServer.xctestplan and select the configuration tab and assign the path to this file to the spotify_credentials_path environment variable. Next, configure the following environment variables based on which authorization flows your server supports. Don't forget to check the checkboxes.

• SPOTIFY_AUTHORIZATION_CODE_FLOW_TOKENS_URL
• SPOTIFY_AUTHORIZATION_CODE_FLOW_REFRESH_TOKENS_URL
• SPOTIFY_AUTHORIZATION_CODE_FLOW_PKCE_TOKENS_URL
• SPOTIFY_AUTHORIZATION_CODE_FLOW_PKCE_REFRESH_TOKENS_URL
• SPOTIFY_CLIENT_CREDENTIALS_FLOW_TOKENS_URL

Select the SpotifyAPI-Package scheme. Then open the test navigator and, at the very top, set the active test plan to SpotifyAPIProxyServer. From the menu bar, select product>run in order to run the tests.

Dozens of times, an authorization URL will be opened in your browser. After you login and are redirected, paste the url into the console. If you find this too tedious, then there's a way to run a local server which will listen for the redirect: Set this package as your working directory and run the following script:

python3 enable_testing.py true

Then clean and rebuild the project.

[Peter-Schorn]

Please let me know if you are able to run the tests.

[Ivorforce]

It looks like you have merged your own branch of the proxy server implementation. Thank you for all the hard work :)

I haven't been able to test recently due to being caught up in work. If you'd still like me to test your implementation of the proxy server, I'm happy to help - I'll be back on it in a week or so.

[Peter-Schorn]

Yes, I would like your help testing the proxy server. Just let me know when you have time.

[Peter-Schorn]

I've just updated the Runing the Unit Tests wiki page. Let me know if you are able to follow the instructions there and run the tests.

[Ivorforce]

Alright, I'm back finally! \o/

I ran the tests with your instructions. Here's the results:

I guess I didn't deny any of the auth requests, so it's natural that test failed. lol Not sure about the invalid tokens. But the rest seems to work! That's awesome!

I have to add that in addition to your instructions, I disabled the rest of the tests manually, since my server doesn't cover PKCE / Client Credentials functionality. Also, it was a bit unfortunate I had to add the client secret to the env variables, since technically it wouldn't be required for the proxy token tests. But for a test suite i don't think it matters much.

[Ivorforce]

Update: Tested in my app by switching to 1922ae6 and AuthorizationCodeFlowProxyBackend.

Works excellently! Thanks again for all your effort in this. I'd consider this issue resolved.

[Peter-Schorn]

Also, it was a bit unfortunate I had to add the client secret to the env variables, since technically it wouldn't be required for the proxy token tests.

If the tests you run don't require the client secret, then you should be able to set a dummy value, although I haven't tried this.

Can you show me all the logs of the tests that failed?

[Peter-Schorn]

It would also be useful to see the server that you are using.

[Peter-Schorn]

testRefreshWithInvalidRefreshToken likely failed because it's expecting an error produced by my server (VaporServerError). I've since updated it so that it no longer expects this specific error.

testDenyAuthorizationRequest requires a special environment variable that I have yet to document. I've since updated it to be skipped if this variable is not present.

Please pull the latest changes and re-run the tests.

Be sure to send the full logs of any failed tests.

[Peter-Schorn]

Version 2.0.0 has been officially released with all the latest changes.