SC.connect doesn't make sure that oauth_token is valid

Open
#32 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
javascript
Domain
api, authentication

Research direction

Start by tracing SC.connect, SC.isConnected, and config.set, focusing on how an existing oauth_token is handled and how the /me request reports a 401. Confirm that an invalid token can be cleared or rejected without reloading the page, then verify the reconnect flow and existing authentication behavior.

Written by the indexing model from the issue text.

Description

With the previous SDK version, I could avoid a login popup every pageload by doing this:

var params = {
    client_id:    '...',
    redirect_uri: '...',
    scope:        'non-expiring'
};
var auth = localStorage.getItem('auth-token');
if (auth) {
    params.access_token = auth;
}
SC.initialize(params);

// ...

function connectionCallback() {
    localStorage.setItem('auth-token', SC.accessToken());
    onConnected();
    SC.get('/me', gotUserdata);
}

if (SC.isConnected()) {
    // make sure we really are
    SC.get('/me', function(user, err) {
        if (!!err) {
            // Error exists, connectify
            SC.connect(connectionCallback);
        } else {
            // We're in
            onConnected();
            gotUserdata(user);
        }
    });
} else {
    SC.connect(connectionCallback);
}

However now isConnected is simply

  isConnected () {
    return config.get('oauth_token') !== undefined;
  },

and connect does this:

  // resolve immediately when oauth_token is set
  const oauth_token = config.get('oauth_token');
  if (oauth_token) {
    return new Promise((resolve) => { resolve({oauth_token}); });
  }

I thought I could get around this by checking for a 401 when fetching /me, but unfortunately I can't null out the oauth_token config value:

  set(key, value) {
    if (value) {
      config[key] = value;
    }
  }

The best I can think of now is to delete the localStorage value and reload the page, but that's not very nice.

Dominant language
JavaScript
Stars
388
Forks
76
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from soundcloud/soundcloud-javascript

All issues in soundcloud/soundcloud-javascript

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.