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

Ignore messages that are not from Auth0's iframe #406

Closed
sandrinodimattia opened this issue Mar 31, 2017 · 44 comments
Closed

Ignore messages that are not from Auth0's iframe #406

sandrinodimattia opened this issue Mar 31, 2017 · 44 comments
Assignees

Comments

@sandrinodimattia
Copy link
Member

So renewAuth is not working for me with postMessage enabled. The request is getting cancelled in the browser. In the iframe it looks like other events are being sent (before the code reaches parseHash) which causes the iframe to be destroyed.

Now I have this when running locally (because of an event from webpack) and when running a deployed version (because of an empty event). Here is what fixed it for me in the iframe handler:

IframeHandler.prototype.messageEventListener = function (e) {
  if (!e.data || e.data === '' || e.data.type === 'webpackOk') {
    return;
  }
  this.destroy();
  this.callbackHandler(e.data);
};
@luisrudge
Copy link
Contributor

This is in our backlog. We need to figure it out a way to ignore posted messages that are not from our iframe. I'll not add anything specific to webpack at the moment, but this will be fixed in the near future

@luisrudge luisrudge changed the title renewAuth requests with postMessage are getting cancelled Ignore messages that are not from Auth0's iframe Mar 31, 2017
@luisrudge luisrudge self-assigned this Mar 31, 2017
@oyvinmar
Copy link

oyvinmar commented Apr 5, 2017

We are having the same issue with Redux Devtools, so a general solution would be appreciated.

@luisrudge
Copy link
Contributor

It will be general. We'll probably prefix our own messages with auth:[...] and check for that prefix before destroying.

@bragma
Copy link

bragma commented Apr 12, 2017

Just found out this huge problem. It seems that facebook's SDK causes renewAuth's callback to be triggered without error but the response contains invalid data.

@lina128
Copy link

lina128 commented Apr 14, 2017

Same issue here.

@luisrudge
Copy link
Contributor

Hey.. Can any of you create a repro project so I can test this? I know what the problem is, but I can't reproduce it 😭

@lina128
Copy link

lina128 commented Apr 14, 2017

I use this react redux starter kit: https://github.com/davezuko/react-redux-starter-kit
The message includes data of the following format:
{ ..., source: '@devtools-extension' } or { ..., source: '@devtools-page' }

I'll try to create a repro project.

@luisrudge
Copy link
Contributor

Thanks. I don't know if we can effectively fix this at all. We don't have control on how the messages are posted...

    // this code goes inside the callback.html file (or your callback route)
    auth0.parseHash(window.location.hash, function (err, data) {
      parent.postMessage(err || data, "http://localhost:3000/");
    });

Since this code lives outside of auth0.js, there's no way to know if the message is coming from the callback itself or from elsewhere.

@lina128
Copy link

lina128 commented Apr 14, 2017

Is it possible to enrich the data somehow? Like data.source = 'auth0'. It will be along the line of your previous thoughts: 'prefix our own messages with auth:[...]'.

@luisrudge
Copy link
Contributor

nope, because this code lives outside of auth0.js. This is custom code that users write.

@lina128
Copy link

lina128 commented Apr 14, 2017

This code is auth0's code, right?
IframeHandler.prototype.messageEventListener = function (e) {
this.destroy();
this.callbackHandler(e.data);
};

why not check the data source before destroying? Like Sandrinodimattia add this line before destroying:
if (!e.data || e.data === '' || e.data.type === 'webpackOk') {
return;
}

In my case, I need to add this:
if (!e.data || e.data === '' || e.data.source === '@devtools-extension' || e.data.source === '@devtools-page') {
return;
}

For this to be solved for general cases, shouldn't the code just add a data.source or something and check that before destroying?

@luisrudge
Copy link
Contributor

This is auth0's code, yes.
But here's the thing.

For a webpack fix, we'd need to add:
if (!e.data || e.data === '' || e.data.type === 'webpackOk')

For a redux dev tools fix, we'd need to add:
if (!e.data || e.data === '' || e.data.source === '@devtools-extension' || e.data.source === '@devtools-page')

This won't scale, because lots of iframes sends data via postMessage. There's no way we blacklist every possible type of message that can happen.

@bragma
Copy link

bragma commented Apr 14, 2017

You have no control on others' code but the silent callback page is under control of the developer so he can enrich (ex: Add a string) the posted message. Then you can allow to specify the same string on renewAuth's options. When handling posted data, if the configured data is missing you ignore it.
There are a few assumptions here about parsability of unknown messages, but with a try/catch you can ignore errors.

@luisrudge
Copy link
Contributor

Yeah, that will work, but the best way is to receive something from the back end that we can check, so the developer won't need to do anything.
There's some working being made in the back end related to this. Once that is done, we can properly implement this check.

@bragma
Copy link

bragma commented Apr 15, 2017

Good to know it will be fixed soon! At the moment I had to delay Facebook's SDK initialization to avoid getting unwnated posts.
I am not sure it's a good idea, but maybe the "state" field of the request may be leveraged? It is required for it to be something unique and the response should already be ignored if the state is unknown, so maybe it can be used to know if the post is required, i.e. if the state matches.

@luisrudge
Copy link
Contributor

Yeah.. there are a few alternatives to fix this in our client, but this will be better handled in the back end. I'm not sure what's the ETA on that, but it's coming.

@buccfer
Copy link

buccfer commented Apr 20, 2017

Mmm the data comes from the parseHash function. Maybe it should include in the data object a "source" property with some specific value. And in the iframe handler check the source before destroying.

web-auth/index.js

function buildParseHashResponse(qsParams, appStatus, token) {
  return {
    accessToken: qsParams.access_token || null,
    idToken: qsParams.id_token || null,
    idTokenPayload: token || null,
    appStatus: appStatus || null,
    refreshToken: qsParams.refresh_token || null,
    state: qsParams.state || null,
    expiresIn: qsParams.expires_in ? parseInt(qsParams.expires_in, 10) : null,
    tokenType: qsParams.token_type || null,
    _source: 'auth0js'
  };
}

helper/iframe-handler.js

IframeHandler.prototype.messageEventListener = function (e) {
  if (e.data && e.data._source === 'auth0js') {
    this.destroy();
    this.callbackHandler(e.data);
  }
};

I tested it a little bit and seems to be working. Of course It might be necessary to add this special field on parseHash error response. But I'm not really sure.

:)

@rochdev
Copy link

rochdev commented May 9, 2017

I've seen a similar pattern to what @buccfer is proposing in several other libraries. Here is an example. With this pattern both data and err would get an isAuth0 property to true.

@aaronchilcott
Copy link
Contributor

I have just discovered this issue too.

My $0.02:

Allow the Iframe helper to accept an event processor which return true if the desired event was found or false otherwise. I don't think it's really the responsibility of the Iframe helper to decide if the event is the one we're looking for or not. Instead, the main auth0 logic should do that.

It would be super nice if we could overload that too, so we could customise it a bit more...

@luisrudge
Copy link
Contributor

This was fixed in #432 - it's still not documented though.

@gilesbutler
Copy link

Hey @luisrudge any update on documentation around this?

I followed @oliverlloyd's suggestion from #429 but that didn't work.

We're still seeing react-devtools and webpack responses in the authResult on auth0-js 8.8.0.

@luisrudge
Copy link
Contributor

luisrudge commented Jul 3, 2017

@gilesbutler Sorry, this is not in the docs yet. You have to use the postMessageDataType option.

var auth0 = new auth0.WebAuth({
  domain: "{YOUR_AUTH0_DOMAIN}",
  clientID: "{YOUR_AUTH0_CLIENT_ID}"
});

var options = {
  //other options
  postMessageDataType: 'my-custom-data-type'
};
auth0.renewAuth(options, (err, authResult)=> {
  console.log(err, authResult);
});

And then in your callback page, you have to call it like this:

<!DOCTYPE html>
<html>

<head>
  <script type="text/javascript">
    parent.postMessage({
        hash: window.location.hash,
        type: 'my-custom-data-type'
    }, "https://localhost:3000/");
  </script>
</head>

<body></body>

</html>

@rochdev
Copy link

rochdev commented Jul 3, 2017

What is the benefit of using usePostMessage: true?

@luisrudge
Copy link
Contributor

luisrudge commented Jul 4, 2017

we listen to different events based on that param:

eventListenerType: usePostMessage ? 'message' : 'load',
which are then checked here:
case 'message':

@gilesbutler
Copy link

Thanks for getting back to me @luisrudge unfortunately still can't get this to work. We're using React-router so our callback page is just a route that triggers a SilentCallback component...

{
setupAuth0() {
    this.auth0 = new auth0.WebAuth({
      domain: config.auth0.domain,
      clientID: config.auth0.clientID,
      postMessageDataType: 'auth0:silent-authentication',
      ...
    });
  }
}

<Route path="/auth/renew" component={SilentCallback} />

// SilentCallback.js
export default () => {
  const data = {
    hash: window.location.hash,
    type: 'auth0:silent-authentication',
  };

  window.parent.postMessage(data, "*");

  return null
}

This is how we're calling renewAuth, but the redirectUri is never actually hit.

const nonce = this.randomString(16);

this.auth0.renewAuth({
      redirectUri: `${window.location.origin}/auth/renew`,
      usePostMessage: true,
      nonce,
    }, (err, authResult) => {
      if (err) {
        console.error(err);
        // The access_token is probably bad, remove it so we don't keep trying to renew it.
        localStorage.removeItem('access_token');
      } else if (hasProp(authResult, 'idToken') && decodeJwt(authResult.idToken).nonce !== nonce) {
        // Something fishy is going on.
        console.error('The nonce string returned did not match the one sent.')
        // Ensure user is asked to login again
        localStorage.removeItem('access_token');
      } else if (hasProp(authResult, 'idToken')) {
        // Save the returned session data to LS
        this.setSession(authResult);
      } else {
        console.log('Unexpected authResult:', authResult);
      }
    });

So authResult is still...

{
    source: "react-devtools-detector", 
    reactBuildType: "development"
}

@luisrudge
Copy link
Contributor

does your route gets called at all? does it work if you have just a static html page instead of letting react router handle that?

@buccfer
Copy link

buccfer commented Jul 5, 2017

@luisrudge This is not working for us neither.

  1. package.json
    "auth0-js": "^8.8.0"

  2. In our app:

const webAuth = new auth0.WebAuth({
  domain: 'ourDomain.auth0.com',
  clientID: 'OUR_CLIENT_ID',
  postMessageDataType: 'custom-type'
});
  1. In the callback handler page (a complete separate page)
<!DOCTYPE html>
<html>
  <head>
    <script type="text/javascript">
      parent.postMessage({
        hash: window.location.hash,
        type: 'custom-type'
      }, window.location.origin.concat('/'));
    </script>
  </head>
  <body></body>
</html>

And still getting the iframe closed by other browser extensions.
Auth result: Object {messageSource: "AUGURY_INSPECTED_APPLICATION", messageId: "2554fb99997e9", serialize: 0, messageType: 7, content: Object}

Can you please reopen this issue at least until the documentation is updated?
Thanks.

@luisrudge
Copy link
Contributor

Can you build a sample repo where I can reproduce this issue? It'd be good to have the dependencies that use postMessage as well so I can reproduce on my machine too.

@buccfer
Copy link

buccfer commented Jul 6, 2017

I think it would be better if you can give us a working example, since we don't really know how we should implement the renew auth in the new flow.

The thing is that is you have a browser extension like vue-dev-tools or similar, the iframe is still being closed by the browser extension with the authReault having useless data.

@aaronchilcott
Copy link
Contributor

@buccfer, If your development extensions are causing issues then I recommend turning them off while developing a prototype.

Here's some snippets from some working code I used when I developed the original patch for this functionality:

Callback html page:

<!DOCTYPE html>
<html>
<head>
  <script type="text/javascript">
    var data = {
      type: 'auth0:silent-authentication',
      hash: window.location.hash
    };

    // This page is designed to be loaded in an Iframe
    parent.postMessage(data, "*");
  </script>
</head>
<body></body>
</html>

This part is written in typescript, but the general gist of the code should be clear:

  public loginSso(): void {
    var data = {
      audience: `https://${AUTH_CONFIG.domain}/userinfo`,
      scope: 'openid',
      responseType: 'id_token token',
      usePostMessage: true,
      postMessageDataType: 'auth0:silent-authentication',
      redirectUri: AUTH_CONFIG.silentCallbackURL,
    };

    this.notifications.show(
      'Checking for an existing SSO Session',
      NotificationService.TYPES.INFO
    );

    this.auth0.renewAuth(data, (err, authResult)=>{

      console.log(data, err, authResult);


      if (err && err.error == 'login_required') {
        this.notifications.show(
          'No existing SSO sessions found, redirecting you to sign-on server.',
          NotificationService.TYPES.WARNING
        );

        setTimeout(() => {
          this.login()
        }, 3000);
      } else if (err) {
        this.notifications.show(
          'Something went wrong while attempting to perform SSO Signon',
          NotificationService.TYPES.ERROR
        );
        return
      }

      if (authResult && authResult.accessToken && authResult.idToken) {
        this.setSession(authResult);

        this.notifications.show(
          'Existing SSO Session found! You are now authenticated .',
          NotificationService.TYPES.INFO
        );

      } else if (err && err.error == 'login_required') {
        this.notifications.show(
          'No existing SSO sessions found, redirecting you to sign-on server.',
          NotificationService.TYPES.WARNING
        );

        setTimeout(() => {
          this.login()
        }, 3000);
      } else {
        // There is a possible bug in the auth0 library. For now we don't want to show this.
        this.notifications.show(
          'An unexpected error has occurred.',
          NotificationService.TYPES.ERROR,
          3000
        );
      }
    });
  }

@shorn
Copy link

shorn commented Jul 6, 2017

Just some feedback to let people know luisrudge's code sample above worked perfectly for me.
I was getting an iframe error reported because of "webpackOk" message before implementing the 'postMessageDataType' attribute.
Using auth0-lock v10.18.0.

@luisrudge
Copy link
Contributor

@buccfer @gilesbutler I noticed you both are not using usePostMessage: true,. Maybe that's the issue? Let's track this down so we can either improve docs or fix the bug if there is one.

@luisrudge
Copy link
Contributor

although @shorn said my example works and I didn't use it as well, so that may not be the issue.

@buccfer
Copy link

buccfer commented Jul 6, 2017

@luisrudge code:

var auth0 = new auth0.WebAuth({
  domain: "{YOUR_AUTH0_DOMAIN}",
  clientID: "{YOUR_AUTH0_CLIENT_ID}",
  postMessageDataType: 'my-custom-data-type'
});

@aaronchilcott code:

var data = {
      audience: `https://${AUTH_CONFIG.domain}/userinfo`,
      scope: 'openid',
      responseType: 'id_token token',
      usePostMessage: true,
      postMessageDataType: 'auth0:silent-authentication',
      redirectUri: AUTH_CONFIG.silentCallbackURL,
    };

    this.auth0.renewAuth(data, (err, authResult)=>{

I see they are different. In one the postMessageDataType is in the auth0.WebAuth constructor and in the other it is in the renewAuth options.
I'll see if the second approach works, since the first one doesn't. But I have to wait till the afternoon for that.

@shorn
Copy link

shorn commented Jul 6, 2017

@luisrudge - sorry, I just meant the postMessageDataType works. My code that calls renewAuth() does have usePostMessage:true.

@buccfer
Copy link

buccfer commented Jul 7, 2017

@shorn Can you please share your implementation? Since none of the examples found so far work for me. Have you tested it with browser extensions like vue-dev-tools or augury? They still close the iframe with useless data.

@aaronchilcott What's the point in disabling the extensions? Shouldn't auth0js work independently of the browser extensions the user has enabled? That's something you cannot control in production.

@luisrudge Is there any chance to update the doc?

@luisrudge
Copy link
Contributor

@buccfer it should work despite extensions. Please create a sample with a list of extensions so I can reproduce the issue. Please include the steps I have to follow to cause the issue (like: click to open the popup, then click the XYZ button in the extension etc)

@buccfer
Copy link

buccfer commented Jul 7, 2017

@luisrudge Finally I got something different. Now the renewAuth gives me an error:

{error: "invalid_token", errorDescription: "Algorithm HS256 is not supported. (Expected algs: [RS256])"}

Do I have to configure the option to switch how the id_tokens are signed from HS256 to RS256 in our client?

@luisrudge
Copy link
Contributor

Correct. There's a similar discussion here: #303 (comment)

@dimitrovs
Copy link

The postMessageDataType has to be in the renewAuth options, not in auth0.WebAuth . Then it works with 8.8.0 .

@devpascoe
Copy link

☝️ yes @dimitrovs that <3

@luisrudge
Copy link
Contributor

@dimitrovs you're totally write. I'll edit my snippet to reflect that. thanks!

@skunkwerk
Copy link

Thank God I found this thread. Spent 9 hours trying to debug this issue with renewAuth. Turns out it was one of the chrome developer tools - likely Vue.js debugger. Turn it off, and it works fine!

@Geczy
Copy link

Geczy commented Sep 29, 2017

This was very annoying, thank you for the direction. I've submitted a PR to fix this in the auth0 examples too auth0-samples/auth0-react-samples#44

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