search

sureshchahal / angular2-adal

Angular 2 wrapper for adal.js
MIT License
41 stars 50 forks source link

Fails on acquiretoken if using adaljs 1.0.16 #85

Open kphung138 opened 6 years ago

kphung138 commented 6 years ago

In Adaljs 1.0.16, the way handleWindowCallback works has change in terms of how the _renewStatuses variable is retrieved from the iframe. My work around for now is to add a yarn.lock or npm-shrinkwrap.json to tell it to get v1.0.14 and not 1.0.16 because the package.json allows for minor upgrad eg. ^1.0.14

Previously the logic to get the _renewStatuses from the iframe parent in RequestInfo. But now this code has moved out to handleWindowCallback and ng2-adal implements it's own handleWindowCallback .

skorunka commented 6 years ago

@kphung138 I had a same issue adal 1.0.16 - the acquire toked sopped to work. Version 1.0.15 works just fine. Thank you for sharing.

skorunka commented 6 years ago

Do you guys know what to change to make angular2-adal work with adal 1.0.16?

kphung138 commented 6 years ago

Thanks for that. I didnt have time to check if a higer version worked. I was initially using adal-angular4 as lm using ng4 and that npm started off as 1.0.16. Looking at ng2-adal package.json it was coded against 1.0.14. So l left it as that and used yarn.lock and npmshrinkwrap.json to lock down the version to 1.0.14. Because npm/yarn install will always get the latest. U might hit this issue. I think most of these angular2+ wrappers are based on similar code where handlewindowcallback needs to be refactored.

skorunka commented 6 years ago

@kphung138 Exactly, you are right. I'm already trying to figure out what changes were made to methods in handleWindowCallback between 1.0.15 and 1.0.16 so I can refactor the wrapper. For now, I'm using v1.0.15.

kphung138 commented 6 years ago

I initially posted an issue on adaljs and the author was kind enough to higlight the code that was changed in adaljs. We need similar code in handlewindowcallback. This code of getting the renewalstautuses was initally in requestinfo but was moved out.

skorunka commented 6 years ago

@kphung138 Can you share the code or the link to the post? Thank you.

kphung138 commented 6 years ago

See if this link works https://github.com/AzureAD/azure-activedirectory-library-for-js/issues/698. The issue has been closed as it's a bug with the ng2-adal.

claudiuconstantin commented 6 years ago

@kphung138, that link is dead, the correct one is: https://github.com/AzureAD/azure-activedirectory-library-for-js/issues/698

stamler commented 6 years ago

@kphung138 thanks for isolating this. I'm using adal-angular5 and it seems to have the same issue. @grumar has already updated the dependencies to 1.0.16 but it seems to break token acquisition that was working on 1.0.15 so I've locked it to the old version for now. SessionStorage shows invalid adal states (tested using a separate app ID for Angular and the server, in my case falcon/python. Hoping to get a better look at this in 10 days if someone doesn't get to it before me.