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

Do not merge : When rendered in an iframe, silent rejects #909

Closed
wants to merge 1 commit into from

Conversation

DarylThayil
Copy link
Contributor

... with an appropriate error.

  • Get rid of the decorator function because it is only used once
  • Add EnvironmentError
  • tests

resolves #902

... with an appropriate error.
* Get rid of the decorator function because it is only used once
* Add EnvironmentError
* tests

resolves #902
return new EnvironmentError(code, desc);
}


Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove extra space

Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

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

I would like to see end-to-end tests once though on this. As discussed I have a suspicion this will be thrown no matter what as per the current implementation.

@DarylThayil DarylThayil changed the title When rendered in an iframe, silent rejects Do not merge : When rendered in an iframe, silent rejects Aug 2, 2019
@DarylThayil
Copy link
Contributor Author

This causes issues with the way that pages are rerendered within popup on return from aad

@binaryjam
Copy link

Is there a way I can help test this, as I get the code from a cdn normally and am not sure what to do here. I would have difficulty compiling and building this locally.

@DarylThayil
Copy link
Contributor Author

@binaryjam no problem. I think there are unfortunately some architectural issues with that prevent us from making thing change right now

@sameera is looking to remove the reliance on not allowing to be rendered in an iframe. Sameera this is something we should consider because when you remove the restriction, and the original app is rendered in the iframe to get the token response, there may be another ATS call and create an endless loop

@sameerag
Copy link
Member

sameerag commented Aug 5, 2019

@DarylThayil Yes, I am considering that and indeed it is a well-known issue in ADAL which needed the developer to add a condition to prevent the loop, that was remedied by this decorator in MSAL.

@jasonnutter
Copy link
Contributor

Closing, as this will be addressed by the work to support iframes in #899.

@jasonnutter jasonnutter closed this Aug 6, 2019
@sameerag sameerag deleted the resolve-error branch November 10, 2020 01:43
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.

Does not follow reject Promise calling acquireTokenSilent if in Iframe
4 participants