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

Does not follow reject Promise calling acquireTokenSilent if in Iframe #902

Closed
3 tasks
binaryjam opened this issue Aug 1, 2019 · 8 comments
Closed
3 tasks
Labels
enhancement Enhancement to an existing feature or behavior. feature Feature requests.

Comments

@binaryjam
Copy link

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X ] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Browser:

  • [X ] Chrome version Version 76.0.3809.87 (Official Build) (64-bit)
  • Firefox version XX
  • IE version XX
  • [X ] Edge version DevEdge 77.0.230.2 (Official build) dev (64-bit)
  • Safari version XX

Library version


Library version: 1.1.1

Current behavior

When making a call to msal.acquireTokenSilent, if the page is embedded in an Iframe it does not work and neither does it return an error.

Expected behavior

Now I know that you do not want to support IFRAMEs in this library from earlier tickets, but when this is called it needs to obey the promise pattern and either succeed or fail.

In this case it should return an error, preferably with a new error code to say we do not support iframes as part of the promise.

In the UserAgentApplication.ts the decorator resolveTokenOnlyIfOutOfIframe
checks if in Iframe and returns a new promise, but does nothing with that promise subsequently, if not in an iframe a differnt promise is called and resolves or rejects correctly.
That promise needs to be rejected with an iframe error warning please.

Minimal reproduction of the problem with instructions

If you take the QuickStart Code and make that work stand alone, then embed that page inside an Iframe in another Website (pref not both localhost, I have not tried on localhost or same websites so this is how I set it up). you can actually login it will popup the Dialog box, but it will not do anything.

@pkanher617
Copy link
Contributor

@binaryjam We are currently working on updating the iframe logic to support msal.js within iframes. @sameerag Can you give more information here?

@DarylThayil DarylThayil added enhancement Enhancement to an existing feature or behavior. feature Feature requests. labels Aug 2, 2019
@DarylThayil
Copy link
Contributor

I am into this suggestion, I wonder if this should be a breaking change or not
I don't think it is ....

@sameerag
Copy link
Member

sameerag commented Aug 2, 2019

I asked @binaryjam to open this. If there is a way we can send an error with our current behavior if a token is not resolved in an iframe, that can be at least informative. What I need to investigate is if this can be achieved today and the redundancy of the same once we lift the condition in MSAL.

DarylThayil added a commit that referenced this issue Aug 2, 2019
... with an appropriate error.
* Get rid of the decorator function because it is only used once
* Add EnvironmentError
* tests

resolves #902
@DarylThayil
Copy link
Contributor

#909
@binaryjam / @sameerag please review

@sameerag
Copy link
Member

sameerag commented Aug 2, 2019

Ah! I see @DarylThayil, This makes sense. I was more trying to find a condition at resolution. This sounds good.

@sameerag
Copy link
Member

sameerag commented Aug 2, 2019

@binaryjam Can you also please test #909 in your environment to verify?

@binaryjam
Copy link
Author

binaryjam commented Aug 2, 2019 via email

@DarylThayil
Copy link
Contributor

we believe this will be solved in #899, unfortunately my pr does not take into account the full scenario, and we believe removing the iframe in iframe restriction will. closing this ticket

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement to an existing feature or behavior. feature Feature requests.
Projects
None yet
4 participants