Azure-Samples / ms-identity-dotnetcore-maui

MAUI sample using MSAL library
MIT License
41 stars 16 forks source link

Change to remove use of Client Id-based Return URI #12

Open J0nKn1ght opened 1 year ago

J0nKn1ght commented 1 year ago

Firstly, thank you for creating these sample projects, which are a really useful resource.

The code currently uses a return URI in the format $"msal{ClientId}://auth", which is the default option when creating a new app registration for the Azure AD B2C tenant in the Azure Portal. However, this makes it difficult to switch B2C tenants when building the app, as the value has to be updated in the appsettings.json file, an also in the Android MsalActivity.cs file IntentFilter.

Would it be possible to change this to use a fixed return URI (for example "ms.identity.donetcore.maui://auth"), and then changing the instructions in the readme.md file to indicate that you must add this return URI when creating the app registration in the portal?

Also, can the 2 IntentFilters (which also currently require the return URI to be entered) be removed from the AndroidManifest.xml file? I presume they are not required, as the IntentFilter from the MsalActivity.cs file is added at build time, and when running on Android this seemed to cause the system dialog to be displayed asking you to pick which application you wanted to use to service the request, with the app listed multiple times.

This issue is for a:

- [ ] bug report -> please search issues before submitting
- [x] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Minimal steps to reproduce

Any log messages given by the failure

Expected/desired behavior

Changing the values in the appsettings.json files should be sufficient to switch between Azure AD B2C tenants, without also having to edit other files in the project.

OS and Version?

Windows 10. (Android 12 - API 31 in the emulator)

Versions

Mention any other details that might be useful

I've made changes to my local copy, based on this Stack Overflow post, to use constants for the host, scheme, and return uri, so that these can be set where required in the code, and also changed my appsettings to allow multiple tenant details to be included, and it all works great. This allows me to change an 'environment' appsetting, and it will then pull out the details for the Azure AD B2C tenant that I want to target, without making any other code changes.

petertangelder commented 1 year ago

I ran into the same issues just last week and manage to fix both of them, maybe not in the most elegant way.

I noticed too that, when logging in, the app opens a browser and after completing the login flow in the browser, the system dialog asks you with what application you would like to service the request, indeed my own app was displayed twice here. To me, that's not a very good user experience (why would the end user be asked what app to open after logging in?). I worked around this issue by adding .WithUseEmbeddedWebView(true) in the call to AcquireTokenInteractive in the SignInUserInteractivelyAsync method of the MSALClientHelper class. This keeps the flow inside the app and won't open a separate browser app and won't display the open app dialog anymore.

As for the second issue, I also struggled with the hard coded client id in the AndroidManifest and MsalActivity files, as my client id is different between Test en Production. To fix this, I used a bit of a hack:

Maybe not the nicest solution, but it works!

J0nKn1ght commented 1 year ago

Hi @petertangelder. Thanks for your input.

With respect to the system dialog being displayed, I think that this is caused by the AndroidManifest.xml in the project source containing the intent filters for the activities microsoft.identity.client.BrowserTabActivity and MauiAppBasic.Platforms.Android.Resources.MsalActivity. I'm not an Android developer (so hopefully one will correct me if this is incorrect), but I believe that when the project builds, the Android code is scanned, and any intent filters found in activities are injected into the AndroidManifest.xml file. You can see this if you check the output version in the 'obj\Debug\net7.0-android' folder.

Because the project file contains multiple intent filters that have the same data host and scheme, when the return URI is called by the AD B2C processing, Android finds 3 activities that can all handle the same URI, and so therefore prompts the user for which activity to call. I think removing the activities from the AndroidManifest.xml file in the project, and letting the build process add the correct one, will fix this issue. I don't know whether it causes any unwanted side-effects, so maybe someone can let me know if this isn't a valid approach.

Secondly, you mentioned changing your code to use .WithUseEmbeddedWebView(true). This is the specific reason that I was using this sample to sign in with AD B2C, because Google have tightened their security policies to prevent signing in from an embedded WebView. If I use the embedded WebView, and try and sign in with Google, I get the following error:

Google Error

This might not be an issue for your use case, but I thought that it was worth highlighting.