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

Doesnt seem to work when use popUp = true #25

Open
slubowsky opened this issue Jan 27, 2017 · 9 comments
Open

Doesnt seem to work when use popUp = true #25

slubowsky opened this issue Jan 27, 2017 · 9 comments

Comments

@slubowsky
Copy link

slubowsky commented Jan 27, 2017

Looks like AdalService.handleWindowCallback isn't called when using popup (and cant call it manually as hash wont be found) so updateDataFromCache is never called and userInfo never gets set to be authenticated. Recalling AdalService.init from config.callback to pick up the info from storage seems to be a workaround for now.

@kaleemxii
Copy link

I made it work without re-initializing adal but by subscribing to route change event.

This works because when you set the config as below with poup and isangular to true, it changes the main window url and appends hash to it (you can code of this in adal.js, search for isAngular context)

 conf = {
            instance: 'https://login.microsoftonline.com/',
            tenant: 'common',
            clientId: client ID
            redirectUri: window.location.origin + '/',
       
            popUp: true,
            isAngular: true
        };

I basically subscribe to route change event before calling adalService.login() and unsubscribe immediately after the route change, it goes something like this

private routeChangeSubForAdal: Subscription;

Signin(){
   if (!this.routeChangeSubForAdal || this.routeChangeSubForAdal.closed) {
            this.routeChangeSubForAdal = this.router.events.subscribe((event) => {
                
                if (!this.adalService.userInfo.isAuthenticated) {
                    if (event.url.length > 100 && !!window.location.hash) {
                        console.log("route changed ....");
                         this.adalService.handleWindowCallback();                        
                    }
                    this.routeChangeSubForAdal.unsubscribe();
                }
            });
        }
        this.adalService.login();
}

Tested couple of scenarios and it holds good as of now..

@slubowsky
Copy link
Author

@kaleemxii That seems like a lot more work then just passing in a callback and calling init again from the callback

@mazhisai
Copy link

@slubowsky do you mind sharing your implementation? i am running into the same issue. thx!

@mazhisai
Copy link

@slubowsky @kaleemxii i tried both your approaches and it didnt work for me. Basically i can see the callback trigger back but inside the callback, it seems the user is not authenticated yet! apprecaite some suggestions here. thanks!

@slubowsky
Copy link
Author

@mazhisai I no longer do it that way, Instead I have added a function to the adalService (I find that I have to rebuild it anyway or it causes problems)
public refreshDataFromCache() { this.updateDataFromCache(this.adalContext.config.loginResource); }
and I call that when logged in.

In any event here's my code with the old way calling adalService.init added back but commented out and replaced with a call to adalService.refreshDataFromCache. I tried to leave only the minimum necessary and removed all my other code. Hopefully whats left actually works..

@Injectable()
export class AuthService {
  public userInfo: OAuthData;
  private adalConfig; Object;
 
  constructor(private adalService: AdalService,
    @Inject(APP_CONFIG) private config: IAppConfig) {
    
    this.adalConfig = Object.assign({
      redirectUri: window.location.origin + '/welcome.html',
      postLogoutRedirectUri: window.location.origin + '/',
      popUp: true,
      callback: this.loggedIn.bind(this)
    }, config.adalConfig);

    this.adalService.init(this.adalConfig);
    this.adalService.handleWindowCallback();
    this.checkForAuthenticatedUser();
  }

  private loggedIn(): void {
    // this.adalService.init(this.adalConfig); // handleWindowCallback was never called - and cant be, hash is not on window...

    // added this function to ng2-adal. Using popup handleWindowCallback wont work.
    // isAngular congig option might be able to help here??
    this.adalService.refreshDataFromCache();
    this.checkForAuthenticatedUser();
  }

  private checkForAuthenticatedUser(): void {
    if (this.adalService.userInfo.isAuthenticated) {
      this.userInfo = this.adalService.userInfo; 
	  // logged in at this point
    } else {
      // not logged in
    }
  }

  login(): void {
    this.adalService.login();
  }

  logout(): void {
    this.adalService.logOut();
  }
}

The config object being passed in looks something like this:

adalConfig: {
        tenant: 'my tenant',
        clientId: 'my client id',
        endpoints: {endpoint1: 'value', endpoint2: 'value'}
    }

@mazhisai
Copy link

@slubowsky thanks a lot for your response, much appreciated. let me give it a try and let you know if that fixed my issue :)

@mazhisai
Copy link

@slubowsky thanks again for the help, i had to fork the code as well to add refreshDataFromCache() to get past the issue.

@mazhisai
Copy link

mazhisai commented Mar 20, 2017

I have put together a sample app that shows how to use the popUp library. https://github.com/mazhisai/ng2-adal-QuickStart. Also the change needed in the library is merged as part of #35 PR

@MikeLallemont
Copy link

Will this work inside an ionic app using cordova? Is the popup you are referring to an HTML popup window? I'm trying to authenticate using this library inside a hybrid app and it opens an instance of the default browser app on the device, which cannot redirect back to the actual app that needs authentication. Anyone know how to do this? I know I can use Cordova In App Browser plugin, but the Adal js library knows nothing about it, so it doesn't work.

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

4 participants