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

No authentication success signals w/ fragment response, standard flow, and iFrame check #22

Open
jheinnic opened this issue Sep 21, 2017 · 1 comment

Comments

@jheinnic
Copy link

I'm currently attempting to use this library with the following init options:

   this.initOptions = {
      adapter: 'default',
      flow: 'standard',
      responseMode: 'fragment',
      checkLoginIframe: true,
      checkLoginIframeInterval: 5
    };

With the latest 0.9.1 release, I've no problem getting a call to login() to redirect to my authentication form and return with an access token, but the application never detects the successful authentication by firing true value down either authenticatedObs or authenticateSuccessObs.

Digging into the code a little bit, I tracked the problem into the URIParser utility used by the callback to extract the required input from the browser's current location. The "oauth" structure it returns currently looks like this:

{ 
    url: "http://www.foo.com/path/is/correct",
    state: "state",
    code: "code"
}

It looks like a fairly easily corrected error near the bottom of the URIParser file. In the loop where it is copying values from the hash of parsed fragment key/value pairs it is simply assigning the keys it is iterating over instead of their de-referenced values.

I hacked my workspace by changing line 126 of this block:

       for (const param in fragmentParams) {
           if (param) {
              oauth[param] = param;
           }
       }

... to copy the value for each required key instead of setting the key itself ...

       for (const param in fragmentParams) {
           if (param) {
                oauth[param] = fragmentParams[param];
           }
       }

I'm having a little trouble working out how to build and run tests for this project, or I would have tried to attach a test case along with this issue and the associated pull request, but didn't want that to keep me from offering the tip. Would be great to be able to drop the one liner patch from my README file--so far the rest of my experience with this library has been very smooth. Thanks for putting it up!

@ebondu
Copy link
Owner

ebondu commented Sep 22, 2017

Hi,
Tanks for your contribution. I will provide a new version including this fix.
I agree with you, some relevant unit tests should be added to the project ;)

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

No branches or pull requests

2 participants