Adding Initial PKCE Auth Flow Support (#542)

* Added base funtionality for PKCE Authorization - i538

* fixed a mistake with the auth code

* fixed more misunderstandings. fixed grant_access_token to now call authorization if needed

* added comments and references to code verifier and code challenge

* removed debug print statement

* updated unit tests for new PKCE flow

* cleaned up username issues - added doc strings to class

* fixed import issue, added user endpoint tests

* forgot to commit this file

* linted

* clarified comment

* no longer generates code verifier or challenge in constructor, only when needed

* fixed flake8 complaints, added forgotten unit tests

* fixed linting with unit tests

* anotha one

* added python3.5 support

* linting

* added to CHANGELOG

* removed as_dict option from get_access_token()

Co-authored-by: tomCLANCC <26153156+tomCLANCC@users.noreply.github.com>

tests/integration/test_user_endpoints.py

@@ -5,7 +5,8 @@ from spotipy import (
     prompt_for_user_token,
     Spotify,
     SpotifyException,
-    SpotifyImplicitGrant
+    SpotifyImplicitGrant,
+    SpotifyPKCE
 )
 import unittest
 import requests
@@ -424,3 +425,36 @@ class SpotipyImplicitGrantTests(unittest.TestCase):
         c_user = self.spotify.current_user()
         user = self.spotify.user(c_user['id'])
         self.assertEqual(c_user['display_name'], user['display_name'])
+
+
+class SpotifyPKCETests(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        scope = (
+            'user-follow-read '
+            'user-follow-modify '
+        )
+        auth_manager = SpotifyPKCE(scope=scope, cache_path=".cache-implicittest")
+        cls.spotify = Spotify(auth_manager=auth_manager)
+
+    def test_user_follows_and_unfollows_artist(self):
+        # Initially follows 1 artist
+        current_user_followed_artists = self.spotify.current_user_followed_artists()[
+            'artists']['total']
+
+        # Follow 2 more artists
+        artists = ["6DPYiyq5kWVQS4RGwxzPC7", "0NbfKEOTQCcwd6o7wSDOHI"]
+        self.spotify.user_follow_artists(artists)
+        res = self.spotify.current_user_followed_artists()
+        self.assertEqual(res['artists']['total'], current_user_followed_artists + len(artists))
+
+        # Unfollow these 2 artists
+        self.spotify.user_unfollow_artists(artists)
+        res = self.spotify.current_user_followed_artists()
+        self.assertEqual(res['artists']['total'], current_user_followed_artists)
+
+    def test_current_user(self):
+        c_user = self.spotify.current_user()
+        user = self.spotify.user(c_user['id'])
+        self.assertEqual(c_user['display_name'], user['display_name'])

tests/unit/test_oauth.py

@@ -5,7 +5,7 @@ import unittest
 
 import six.moves.urllib.parse as urllibparse
 
-from spotipy import SpotifyOAuth, SpotifyImplicitGrant
+from spotipy import SpotifyOAuth, SpotifyImplicitGrant, SpotifyPKCE
 from spotipy.oauth2 import SpotifyClientCredentials, SpotifyOauthError
 from spotipy.oauth2 import SpotifyStateError
 
@@ -46,6 +46,10 @@ def _make_implicitgrantauth(*args, **kwargs):
     return SpotifyImplicitGrant("CLID", "REDIR", "STATE", *args, **kwargs)
 
 
+def _make_pkceauth(*args, **kwargs):
+    return SpotifyPKCE("CLID", "REDIR", "STATE", *args, **kwargs)
+
+
 class OAuthCacheTest(unittest.TestCase):
 
     @patch.multiple(SpotifyOAuth,
@@ -333,3 +337,135 @@ class TestSpotifyImplicitGrant(unittest.TestCase):
         parsed_url = urllibparse.urlparse(url)
         parsed_qs = urllibparse.parse_qs(parsed_url.query)
         self.assertTrue(parsed_qs['show_dialog'])
+
+
+class SpotifyPKCECacheTest(unittest.TestCase):
+
+    @patch.multiple(SpotifyPKCE,
+                    is_token_expired=DEFAULT, refresh_access_token=DEFAULT)
+    @patch('spotipy.oauth2.open', create=True)
+    def test_gets_from_cache_path(self, opener,
+                                  is_token_expired, refresh_access_token):
+        scope = "playlist-modify-private"
+        path = ".cache-username"
+        tok = _make_fake_token(1, 1, scope)
+
+        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        is_token_expired.return_value = False
+
+        spot = _make_pkceauth(scope, path)
+        cached_tok = spot.get_cached_token()
+
+        opener.assert_called_with(path)
+        self.assertIsNotNone(cached_tok)
+        self.assertEqual(refresh_access_token.call_count, 0)
+
+    @patch.multiple(SpotifyPKCE,
+                    is_token_expired=DEFAULT, refresh_access_token=DEFAULT)
+    @patch('spotipy.oauth2.open', create=True)
+    def test_expired_token_refreshes(self, opener,
+                                     is_token_expired, refresh_access_token):
+        scope = "playlist-modify-private"
+        path = ".cache-username"
+        expired_tok = _make_fake_token(0, None, scope)
+        fresh_tok = _make_fake_token(1, 1, scope)
+
+        token_file = _token_file(json.dumps(expired_tok, ensure_ascii=False))
+        opener.return_value = token_file
+        refresh_access_token.return_value = fresh_tok
+
+        spot = _make_pkceauth(scope, path)
+        spot.get_cached_token()
+
+        is_token_expired.assert_called_with(expired_tok)
+        refresh_access_token.assert_called_with(expired_tok['refresh_token'])
+        opener.assert_any_call(path)
+
+    @patch.multiple(SpotifyPKCE,
+                    is_token_expired=DEFAULT, refresh_access_token=DEFAULT)
+    @patch('spotipy.oauth2.open', create=True)
+    def test_badly_scoped_token_bails(self, opener,
+                                      is_token_expired, refresh_access_token):
+        token_scope = "playlist-modify-public"
+        requested_scope = "playlist-modify-private"
+        path = ".cache-username"
+        tok = _make_fake_token(1, 1, token_scope)
+
+        opener.return_value = _token_file(json.dumps(tok, ensure_ascii=False))
+        is_token_expired.return_value = False
+
+        spot = _make_pkceauth(requested_scope, path)
+        cached_tok = spot.get_cached_token()
+
+        opener.assert_called_with(path)
+        self.assertIsNone(cached_tok)
+        self.assertEqual(refresh_access_token.call_count, 0)
+
+    @patch('spotipy.oauth2.open', create=True)
+    def test_saves_to_cache_path(self, opener):
+        scope = "playlist-modify-private"
+        path = ".cache-username"
+        tok = _make_fake_token(1, 1, scope)
+
+        fi = _fake_file()
+        opener.return_value = fi
+
+        spot = SpotifyPKCE("CLID", "REDIR", "STATE", scope, path)
+        spot._save_token_info(tok)
+
+        opener.assert_called_with(path, 'w')
+        self.assertTrue(fi.write.called)
+
+
+class TestSpotifyPKCE(unittest.TestCase):
+
+    def test_generate_code_verifier_for_pkce(self):
+        auth = SpotifyPKCE("CLID", "REDIR")
+        auth.get_pkce_handshake_parameters()
+        self.assertTrue(auth.code_verifier)
+
+    def test_generate_code_challenge_for_pkce(self):
+        auth = SpotifyPKCE("CLID", "REDIR")
+        auth.get_pkce_handshake_parameters()
+        self.assertTrue(auth.code_challenge)
+
+    def test_code_verifier_and_code_challenge_are_correct(self):
+        import hashlib
+        import base64
+        auth = SpotifyPKCE("CLID", "REDIR")
+        auth.get_pkce_handshake_parameters()
+        self.assertEqual(auth.code_challenge,
+                         base64.urlsafe_b64encode(
+                                hashlib.sha256(auth.code_verifier.encode('utf-8'))
+                                .digest())
+                         .decode('utf-8')
+                         .replace('=', ''))
+
+    def test_get_authorize_url_doesnt_pass_state_by_default(self):
+        auth = SpotifyPKCE("CLID", "REDIR")
+
+        url = auth.get_authorize_url()
+
+        parsed_url = urllibparse.urlparse(url)
+        parsed_qs = urllibparse.parse_qs(parsed_url.query)
+        self.assertNotIn('state', parsed_qs)
+
+    def test_get_authorize_url_passes_state_from_constructor(self):
+        state = "STATE"
+        auth = SpotifyPKCE("CLID", "REDIR", state)
+
+        url = auth.get_authorize_url()
+
+        parsed_url = urllibparse.urlparse(url)
+        parsed_qs = urllibparse.parse_qs(parsed_url.query)
+        self.assertEqual(parsed_qs['state'][0], state)
+
+    def test_get_authorize_url_passes_state_from_func_call(self):
+        state = "STATE"
+        auth = SpotifyPKCE("CLID", "REDIR")
+
+        url = auth.get_authorize_url(state=state)
+
+        parsed_url = urllibparse.urlparse(url)
+        parsed_qs = urllibparse.parse_qs(parsed_url.query)
+        self.assertEqual(parsed_qs['state'][0], state)