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

[Angular] Move MsalRedirectComponent To APP_INITIALIZER #4036

Open
mindingdata opened this issue Sep 8, 2021 · 3 comments
Open

[Angular] Move MsalRedirectComponent To APP_INITIALIZER #4036

mindingdata opened this issue Sep 8, 2021 · 3 comments
Labels
feature Feature requests. msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package tracked-internally Bugs that are tracked by Msft internally

Comments

@mindingdata
Copy link

Core Library

MSAL.js v2 (@azure/msal-browser)

Wrapper Library

MSAL Angular (@azure/msal-angular)

Description

I've been working with MSAL v2 with the Angular wrapper recently on a project, and for the most part, I like having the MsalRedirectComponent as a way, out of the box to handle the redirect observable.

Where things started to go awry was where various parts of the app would start requesting tokens while interaction was in progress. I know that in general, I should redirect back to a page that has no authentication required, but other developers aren't so knowledgeable and often added in things to root components (For example, something in the app.component.ts that calls an API etc).

I solved almost all of my issues instantly by using an APP_INITIALIZER as described here : https://tutorialsforangular.com/2019/12/05/using-the-app_initializer-token-to-bootstrap-your-application/

An APP_INITIALIZER is basically a set of code that you can run before any other piece of code in your application, synchronously. Including any components/guards/routing etc.

By creating an APP_INITIALIZER that simply ran the following (Note in this case, I wanted to return a promise but it also handles observables).

await this.injector.get(MsalService).handleRedirectObservable().toPromise();

This ensured that no matter the route I got redirect back to, no matter which guards were in place, no matter which code was in app.component etc, this would always run before all of them and synchronously too (Getting rid of having to wait on InProgress being none).

Anyway, the point is, is that I personally believe this to be a much cleaner and simpler way to get around many gotchas of using the MsalRedirectComponent and I would be happy to create a PR if it's something we should use.

Source

External (Customer)

@mindingdata mindingdata added the feature Feature requests. label Sep 8, 2021
@github-actions github-actions bot added msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package labels Sep 8, 2021
@jasonnutter
Copy link
Contributor

@mindingdata Great suggestion, seems like something we should consider. You are welcome to open a PR to demonstrate how you think this could work, and we can go from there. Thanks!

@sameerag sameerag added the tracked-internally Bugs that are tracked by Msft internally label Apr 15, 2022
@yksht
Copy link
Contributor

yksht commented Oct 3, 2022

@jasonnutter
Hi, I have some time to continue with this task.

Your last message from the closed pr:

Please let us know if you would still like to explorer this idea, because I think it is interesting (preferably in a backwards compatible way).

Im not sure I have ideas to not break compatability.
Did you mean just keep MsalRedirectComponent and implement new solution as an extra way of having msalBroadcastService.inProgress$ subscription?

@yksht
Copy link
Contributor

yksht commented Jan 19, 2023

Found out APP_INITIALIZER is not ideal solution here. MsalModule can be imported by lazy loaded module and APP_INITIALIZER provider registered in MsalModule won't be invoked in such case.
There is feature request in angular project to add MODULE_INITIALIZER which can perfectly fit for this scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature requests. msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package tracked-internally Bugs that are tracked by Msft internally
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants