Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web SDK can take a custom OAuth token handler function #116

Closed
wants to merge 2 commits into from

Conversation

nzoschke
Copy link
Contributor

@nzoschke nzoschke commented Jun 24, 2021

Before, a Web Playback SDK developer could only use the internal in PKCE auth and refresh flow, which seems to always require an auth prompt.

Now, a developer can pass in a custom getOAuthToken function which allows them to customize the auth and refresh flow.

The use case is for Spotify apps that use the standard Authorization Code flow and manage access and refresh tokens with a server side component.

Fixes #112

TODO:

  • Docs

@nzoschke nzoschke changed the title Web SDK can take a custom handler function Web SDK can take a custom OAuth token handler function Jun 24, 2021
@nzoschke
Copy link
Contributor Author

nzoschke commented Jun 24, 2021

I'm splitting up #114 into smaller parts. This addresses No 2 from #114 (comment)

  1. can this library be enhanced to take a custom token callback to connectToSpotifyRemote for a scenarios where you already have a valid token and perform the refresh with a custom server side component?

I tested that the standard usage of this library still works with:

{
    var scope = [
      'app-remote-control',
      'playlist-modify-public',
      'playlist-read-collaborative',
      'playlist-read-private',
      'streaming',
      'user-library-modify',
      'user-library-read',
      'user-modify-playback-state',
      'user-read-currently-playing',
      'user-read-email',
      'user-read-playback-state',
      'user-read-private',
      'user-top-read',
    ].join(',');

    var token = await SpotifySdk.getAuthenticationToken(
      clientId: clientID,
      redirectUrl: redirectURI,
      scope: scope,
    );

    await SpotifySdk.connectToSpotifyRemote(
      clientId: clientID,
      redirectUrl: redirectURI,
      accessToken: token,
      scope: scope,
    );
  }

I also tested that my custom token handler works with:

{
    // get access token from server
    final res = await http.post(...);
    final token = jsonDecode(res.body)['access_token'] as String;

    SpotifySdkPlugin.getOAuthToken = () async {
      print('getOAuthToken custom cb $token');
      return token;
    };

    await SpotifySdk.connectToSpotifyRemote(
      clientId: clientID,
      redirectUrl: redirectURI,
      playerName: 'JukeLab',
      accessToken: token,
      scope: scope,
    );

Due to having a server side access / refresh token, my app loads the player with no auth prompt!

@fotiDim
Copy link
Collaborator

fotiDim commented Jun 29, 2021

In general looks good. Let's also update the readme to indicate this new feature, how developers can use it and clearly state that it is "web only".

@@ -735,6 +745,16 @@ external set _onSpotifyWebPlaybackSDKReady(void Function()? f);
@JS('onSpotifyWebPlaybackSDKReady')
external void Function()? get _onSpotifyWebPlaybackSDKReady;

/// Allows assigning the function getOAuthToken
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Allows assigning the function getOAuthToken
/// Allows getting the function getOAuthToken

Since this is for the getter shouldn't that be getting?

@fotiDim
Copy link
Collaborator

fotiDim commented Jun 29, 2021

@nzoschke something that came to mind, if in an alternative implementation, you only passed a "refresh token" endpoint instead of a function would that be enough for you? I am thinking that on the iOS SDK there is an alternative authentication flow that works like this. Also other generic oAuth libraries that I have used work in similar way. If you set the refresh endpoint then they use it. We could potentially align the APIs.

My counter suggestion would look like this:

SpotifySdkPlugin.tokenSwapURL = '';
SpotifySdkPlugin.tokenRefreshURL = '';

@nzoschke
Copy link
Contributor Author

Yes I think that would work.

My server side components were built with this token swap pattern in mind.

Support for this in the iOS SDK would be great, and I see how this could work with the web SDK for my use case too.

Here's another reference doc about the token swap:

https://developer.spotify.com/documentation/ios/guides/token-swap-and-refresh/

@fotiDim
Copy link
Collaborator

fotiDim commented Jun 29, 2021 via email

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 1, 2021

Here is a fairly complete example. This is using:

As I do intend to support iOS for my app, I did end up following the token swap pattern as documented in:

I think this pattern makes sense for both iOS and web.

  Future permissionCheck() async {
    var apiURI = 'http://localhost:8000';
    var clientID = '<REDACTED>';
    var redirectURI = 'http://localhost:8686/callback.html';

    var scope = [
      'app-remote-control',
      'playlist-modify-public',
      'playlist-read-collaborative',
      'playlist-read-private',
      'streaming',
      'user-library-modify',
      'user-library-read',
      'user-modify-playback-state',
      'user-read-currently-playing',
      'user-read-email',
      'user-read-playback-state',
      'user-read-private',
      'user-top-read',
    ].join(',');

    final url = Uri.https('accounts.spotify.com', '/authorize', {
      'client_id': clientID,
      'redirect_uri': redirectURI,
      'response_type': 'code',
      'scope': scope,
    });

    final result = await FlutterWebAuth.authenticate(
      url: url.toString(),
      callbackUrlScheme: redirectURI,
    );

    // swap code for token
    final res = await http.post(
      Uri.parse('$apiURI/api/v1/spotify/token'),
      body: {
        'code': Uri.parse(result).queryParameters['code'],
        'redirect_uri': redirectURI,
      },
    );

    var out = jsonDecode(res.body);
    var now = (DateTime.now().millisecondsSinceEpoch / 1000).round();

    var token = SpotifyToken(
      accessToken: out['access_token'] as String,
      clientId: clientID,
      expiry: now + out['expires_in'] as int,
      refreshToken: out['refresh_token'] as String,
    );

    SpotifySdkPlugin.getOAuthToken = () async {
      // refresh token if expired
      if (token.expiry <= DateTime.now().millisecondsSinceEpoch / 1000) {
        print('getOAuthToken refresh ${token.expiry}');

        var rt = token.refreshToken;
        final res = await http.post(
          Uri.parse('$apiURI/api/v1/spotify/refresh'),
          body: {
            'refresh_token': rt,
          },
        );

        var out = jsonDecode(res.body);
        var now = (DateTime.now().millisecondsSinceEpoch / 1000).round();

        token = SpotifyToken(
          accessToken: out['access_token'] as String,
          clientId: clientID,
          expiry: now + out['expires_in'] as int,
          refreshToken: rt,
        );
      }

      print('getOAuthToken ${token.accessToken}');
      return token.accessToken;
    };

    var ok = await SpotifySdk.connectToSpotifyRemote(
      clientId: clientID,
      redirectUrl: redirectURI,
      accessToken: token.accessToken,
      scope: scope,
    );

    print('connect $ok');
  }

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 1, 2021

So I too could see a design that uses the token swap settings for the web SDK.

We introduce these:

SpotifySdkPlugin.tokenSwapURL = '';
SpotifySdkPlugin.tokenRefreshURL = '';

So if the token URLs are not set, it does the existing Authorization Code with PKCE auth flow and refresh.

If these are set, it does an Authorization Code (without PKCE) to get a code, then follows the well defined token swap spec (docs) to swap the code for an access token and to refresh the token.

The pros are:

  • similar design to the iOS SDK (I'm sure we'd want to get the iOS token swap stuff supported too)
  • less work on a developer to implement OAuth and refresh themselves

The cons are:

  • unnatural design for the Web Playback SDK vs supplying your own getOAuthToken in the docs
  • forces a developer to implement their server side component a certain way

So @fotiDim given all of the above do you have a preference between (1) this existing PR for getOAuthToken or (2) making a new PR with the token swap pattern for the Web SDK?

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 1, 2021

Here's a first pass at what doing the token swap stuff in the web SDK might look like. I think it's feeling good to bake this in, certainly simpler than all the stuff I had to figure out on my own to do the flow and pass the refresh function down.

One thing is I'll want to get the access token out to use with the web API client.

diff --git a/lib/spotify_sdk_web.dart b/lib/spotify_sdk_web.dart
index bd0475d..9654661 100644
--- a/lib/spotify_sdk_web.dart
+++ b/lib/spotify_sdk_web.dart
@@ -83,6 +83,17 @@ class SpotifySdkPlugin {
   static const String DEFAULT_SCOPES =
       'streaming user-read-email user-read-private';
 
+  static String? _tokenSwapURL;
+  static String? _tokenRefreshURL;
+
+  static set tokenSwapURL(String s) {
+    _tokenSwapURL = s;
+  }
+
+  static set tokenRefreshURL(String s) {
+    _tokenRefreshURL = s;
+  }
+
   /// constructor
   SpotifySdkPlugin(
       this.playerContextEventController,
@@ -403,19 +414,36 @@ class SpotifySdkPlugin {
   }
 
   /// Authenticates a new user with Spotify and stores access token.
-  Future<String> _authorizeSpotify(
-      {required String clientId,
-      required String redirectUrl,
-      required String? scopes}) async {
+  Future<String> _authorizeSpotify({
+    required String clientId,
+    required String redirectUrl,
+    required String? scopes,
+  }) async {
+    print('_authorizeSpotify $_tokenSwapURL');
     // creating auth uri
     var codeVerifier = _createCodeVerifier();
     var codeChallenge = _createCodeChallenge(codeVerifier);
     var state = _createAuthState();
-    var authorizationUri =
-        'https://accounts.spotify.com/authorize?client_id=$clientId&response_type=code&redirect_uri=$redirectUrl&code_challenge_method=S256&code_challenge=$codeChallenge&state=$state&scope=$scopes';
+
+    var params = {
+      'client_id': clientId,
+      'redirect_uri': redirectUrl,
+      'response_type': 'code',
+      'state': state,
+      'scope': scopes,
+    };
+
+    if (_tokenSwapURL == null) {
+      params['code_challenge_method'] = 'S256';
+      params['code_challenge'] = codeChallenge;
+    }
+
+    final authorizationUri =
+        Uri.https('accounts.spotify.com', 'authorize', params);
 
     // opening auth window
-    var authPopup = window.open(authorizationUri, 'Spotify Authorization');
+    var authPopup =
+        window.open(authorizationUri.toString(), 'Spotify Authorization');
     String? message;
     var sub = window.onMessage.listen(allowInterop((event) {
       message = event.data.toString();
@@ -461,20 +489,38 @@ class SpotifySdkPlugin {
     }
     await sub.cancel();
 
-    // exchange auth code for access and refresh tokens
     dynamic authResponse;
+
+    // build request to exchange auth code with PKCE for access and refresh tokens
+    var req = RequestOptions(
+      path: 'https://accounts.spotify.com/api/token',
+      method: 'POST',
+      data: {
+        'client_id': clientId,
+        'grant_type': 'authorization_code',
+        'code': parsedMessage.queryParameters['code'],
+        'redirect_uri': redirectUrl,
+        'code_verifier': codeVerifier
+      },
+      contentType: Headers.formUrlEncodedContentType,
+    );
+
+    // or build request to exchange code with token swap
+    // https://developer.spotify.com/documentation/ios/guides/token-swap-and-refresh/
+    if (_tokenSwapURL != null) {
+      req = RequestOptions(
+        path: _tokenSwapURL!,
+        method: 'POST',
+        data: {
+          'code': parsedMessage.queryParameters['code'],
+          'redirect_uri': redirectUrl,
+        },
+        contentType: Headers.formUrlEncodedContentType,
+      );
+    }
+
     try {
-      authResponse = (await _authDio.post(
-              'https://accounts.spotify.com/api/token',
-              data: {
-                'client_id': clientId,
-                'grant_type': 'authorization_code',
-                'code': parsedMessage.queryParameters['code'],
-                'redirect_uri': redirectUrl,
-                'code_verifier': codeVerifier
-              },
-              options: Options(contentType: Headers.formUrlEncodedContentType)))
-          .data;
+      authResponse = (await _authDio.fetch(req)).data;
     } on DioError catch (e) {
       print('Spotify auth error: ${e.response?.data}');
       rethrow;
@@ -492,15 +538,36 @@ class SpotifySdkPlugin {
   /// Refreshes the Spotify access token using the refresh token.
   Future<dynamic> _refreshSpotifyToken(
       String? clientId, String? refreshToken) async {
+    print('refresh?');
+    // build request to refresh PKCE for access and refresh tokens
+    var req = RequestOptions(
+      path: 'https://accounts.spotify.com/api/token',
+      method: 'POST',
+      data: {
+        'grant_type': 'refresh_token',
+        'refresh_token': refreshToken,
+        'client_id': clientId,
+      },
+      contentType: Headers.formUrlEncodedContentType,
+    );
+
+    // or build request to refresh code with token swap
+    // https://developer.spotify.com/documentation/ios/guides/token-swap-and-refresh/
+    if (_tokenRefreshURL != null) {
+      req = RequestOptions(
+        path: _tokenRefreshURL!,
+        method: 'POST',
+        data: {
+          'refresh_token': refreshToken,
+        },
+        contentType: Headers.formUrlEncodedContentType,
+      );
+    }
+
     try {
-      return (await _authDio.post('https://accounts.spotify.com/api/token',
-              data: {
-                'grant_type': 'refresh_token',
-                'refresh_token': refreshToken,
-                'client_id': clientId,
-              },
-              options: Options(contentType: Headers.formUrlEncodedContentType)))
-          .data;
+      var d = (await _authDio.fetch(req)).data;
+      d['refresh_token'] = refreshToken;
+      return d;
     } on DioError catch (e) {
       print('Token refresh error: ${e.response?.data}');
       rethrow;

@fotiDim
Copy link
Collaborator

fotiDim commented Jul 5, 2021

As I do intend to support iOS for my app, I did end up following the token swap pattern as documented in:

That is great I was about to suggest that. My current thinking is:

  • We should strive for API consistency. If we adopt the getOAuthToken callback then this is doomed to be web only. If we instead adopt tokenSwapURL and tokenRefreshURL then this can at least be used for iOS and Web. If we go now with getOAuthToken we could end up eventually having both getOAuthTokenand tokenSwapURL / tokenRefreshURL in the API in order to support iOS and Web and I believe this would make the API confusing.
  • We already has a couple of requests for exposing tokenSwapURL and tokenRefreshURL on iOS.
  • If we adopt tokenSwapURL and tokenRefreshURL we will have to introduce some business logic in the SDK which will be the first time this is done and it kind of contradicts the purpose of this library which is meant to be a conveniency wrapper over the native SDKs.
  • The standardization of the swap/refresh service actually is imposed by Spotify themselves. This is what they suggest for their iOS SDK so following the same pattern for another platform I guess wouldn't hurt it will allow more devs to adopt this new platform with less effort.

Given the above my vote goes for adopting tokenSwapURL and tokenRefreshURL given that we keep the introduced business logic to the absolute minimum. I will value API consistency in that case.

@brim-borium since this is a strategic decision that we need to make, do you have an opinion on this?

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 5, 2021

See #121 for a PR implementing the token swap pattern for web

@fotiDim
Copy link
Collaborator

fotiDim commented Jul 6, 2021

Shall we close this in favour of #121?

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 7, 2021

Yes closing in favor of #121.

@nzoschke nzoschke closed this Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On Web the SDKs always open the Spotify consent popup
2 participants