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

[Feature Request] Unity on Windows not well supported #2305

Closed
najadojo opened this issue Dec 14, 2020 · 16 comments · Fixed by #2475
Closed

[Feature Request] Unity on Windows not well supported #2305

najadojo opened this issue Dec 14, 2020 · 16 comments · Fixed by #2475

Comments

@najadojo
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Windows Unity support missing suggests that Unity on Windows isn't well tested.

Describe the solution you'd like
I'd like to see sample code that uses Unity on Windows (both UWP and Standalone).

Describe alternatives you've considered
Using WAM in these places is possible but the API is much more complicated to use and isn't as well supported as MSAL. For example api-version 2.0 isn't used by default.

Additional context
Unity is one of the most popular game engines and uses .net as its scripting framework. Applications that wish to use Microsoft Identity struggle with MSAL issues.

@bgavrilMS
Copy link
Member

MSAL now has support for WAM on its UWP target. Can that be used? See https://aka.ms/msal-net-wam for details and a sample.

@najadojo
Copy link
Contributor Author

I was unaware of this newer feature so I tried it out. I started testing with 4.24, Unity doesn't support net461 so we are unable to use this feature in editor in Standalone builds. We've worked around this limitation by developing a native dll that we then pinvoke our exported functions to handle the WAM flow.

When using vanilla MSAL in Unity Editor on this version we see:

NotImplementedException: The method or operation is not implemented.
System.Net.ServicePoint.set_ConnectionLeaseTimeout (System.Int32 value) (at <ef151b6abb5d474cb2c1cb8906a8b5a4>:0)
Microsoft.Identity.Client.Platforms.net45.Http.DnsSensitiveClientHandler.AddConnectionLeaseTimeout (System.Uri endpoint) (at <a924e7e4a2694c87b10f6573abcf213c>:0)
Microsoft.Identity.Client.Platforms.net45.Http.DnsSensitiveClientHandler+<SendAsync>d__2.MoveNext () (at <a924e7e4a2694c87b10f6573abcf213c>:0)

We do not see this in 4.11 our current LKG. This type of issue is similar to the kinds of issues we encounter trying to consume MSAL; #2304 is another such issue.

When I use

#if !UNITY_EDITOR && UNITY_WSA
            builder = builder.WithExperimentalFeatures().WithBroker();
#endif

and then compile and deploy the UWP application we still get errors right away:

JsonSerializationException: Unable to find a constructor to use for type Microsoft.Identity.Client.Instance.Discovery.InstanceDiscoveryResponse. A class should either have a default constructor, one constructor with arguments or a constructor marked with the JsonConstructor attribute. Path 'tenant_discovery_endpoint', line 1, position 29.
  at Microsoft.Identity.Json.Serialization.JsonSerializerInternalReader.CreateNewObject (Microsoft.Identity.Json.JsonReader reader, Microsoft.Identity.Json.Serialization.JsonObjectContract objectContract, Microsoft.Identity.Json.Serialization.JsonProperty containerMember, Microsoft.Identity.Json.Serialization.JsonProperty containerProperty, System.String id, System.Boolean& createdFromNonDefaultCreator) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.Serialization.JsonSerializerInternalReader.CreateObject (Microsoft.Identity.Json.JsonReader reader, System.Type objectType, Microsoft.Identity.Json.Serialization.JsonContract contract, Microsoft.Identity.Json.Serialization.JsonProperty member, Microsoft.Identity.Json.Serialization.JsonContainerContract containerContract, Microsoft.Identity.Json.Serialization.JsonProperty containerMember, System.Object existingValue) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal (Microsoft.Identity.Json.JsonReader reader, System.Type objectType, Microsoft.Identity.Json.Serialization.JsonContract contract, Microsoft.Identity.Json.Serialization.JsonProperty member, Microsoft.Identity.Json.Serialization.JsonContainerContract containerContract, Microsoft.Identity.Json.Serialization.JsonProperty containerMember, System.Object existingValue) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.Serialization.JsonSerializerInternalReader.Deserialize (Microsoft.Identity.Json.JsonReader reader, System.Type objectType, System.Boolean checkAdditionalContent) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.JsonSerializer.DeserializeInternal (Microsoft.Identity.Json.JsonReader reader, System.Type objectType) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.JsonSerializer.Deserialize (Microsoft.Identity.Json.JsonReader reader, System.Type objectType) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.JsonConvert.DeserializeObject (System.String value, System.Type type, Microsoft.Identity.Json.JsonSerializerSettings settings) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.JsonConvert.DeserializeObject[T] (System.String value, Microsoft.Identity.Json.JsonSerializerSettings settings) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.JsonConvert.DeserializeObject[T] (System.String value) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Client.Utils.JsonHelper.DeserializeFromJson[T] (System.String json) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Client.OAuth2.OAuth2Client.CreateResponse[T] (Microsoft.Identity.Client.Http.HttpResponse response, Microsoft.Identity.Client.Internal.RequestContext requestContext) [0x00000] in <00000000000000000000000000000000>:0 
  at Microsoft.Identity.Json.Utilities.ThreadSafeStore`2[TKey,TValue]..ctor (System.Func`2[T,TResult] creator) [0x00000] in <00000000000000000000000000000000>:0 

Unity can support a lot of .net stuff but care needs to be taken on which platform features are used and how their implementations are provided.

@najadojo
Copy link
Contributor Author

#2231

@Now-you-see-me-007
Copy link

Now-you-see-me-007 commented Jan 31, 2021

// The MSAL Public client app
private static IPublicClientApplication PublicClientApp;

// private static string MSGraphURL = "https://graph.microsoft.com/v1.0/";
private static AuthenticationResult authResult;

private void Start()
{
    accessTokenText = GameObject.FindGameObjectWithTag("at").GetComponent<Text>();
    idTokenText = GameObject.FindGameObjectWithTag("it").GetComponent<Text>();
    FetchAccessToken();
}
private static async void FetchAccessToken()
{
    UnityEngine.Debug.Log("Entered Fetch token");
    AuthenticationResult x = await SignInUserAndGetTokenUsingMSAL(scopes);
    accessTokenText.text = x.AccessToken;
    idTokenText.text = x.IdToken;
    UnityEngine.Debug.Log(x);
}

private static async Task<AuthenticationResult> SignInUserAndGetTokenUsingMSAL(string[] scopes)
{
    UnityEngine.Debug.Log("Entered the task Method.........!!!");

    // Initialize the MSAL library by building a public client application
    PublicClientApp = PublicClientApplicationBuilder.Create(ClientId)
        .WithAuthority(Authority)
        .WithRedirectUri("https://login.microsoftonline.com/common/oauth2/nativeclient")
         .WithLogging((level, message, containsPii) =>
         {
            //System.Diagnostics.Debug.WriteLine($"MSAL: {level} {message} ");
             UnityEngine.Debug.Log($"MSAL: {level} {message} ");
         }, LogLevel.Warning, enablePiiLogging: false, enableDefaultPlatformLogging: true)
        .Build();

    // It's good practice to not do work on the UI thread, so use ConfigureAwait(false) whenever possible.
    IEnumerable<IAccount> accounts = await PublicClientApp.GetAccountsAsync().ConfigureAwait(false);
    IAccount firstAccount = accounts.FirstOrDefault();

    try
    {
        authResult = await PublicClientApp.AcquireTokenSilent(scopes, firstAccount)
                                          .ExecuteAsync();
    }
    catch (MsalUiRequiredException ex)
    {
        // A MsalUiRequiredException happened on AcquireTokenSilentAsync. This indicates you need to call AcquireTokenAsync to acquire a token
       // System.Diagnostics.Debug.WriteLine($"MsalUiRequiredException: {ex.Message}");
        UnityEngine.Debug.Log($"MsalUiRequiredException: {ex.Message}");
        authResult = await PublicClientApp.AcquireTokenInteractive(scopes)
                                          .ExecuteAsync()
                                          .ConfigureAwait(false);

    }
    UnityEngine.Debug.Log(authResult.AccessToken);

    return authResult;
}

This is my code and I have a couple of Questions:

  1. If at all this works, when I build for UWP, In case of Acquire Token interactively : Does it open the login window automatically and gets the access token? When I built the sample Native UWP project from azure portal it does get the access token successfully after the login and permission allow
    2.When I use the "Microsoft.Identity.Client" and use the above code for getting the access token , it is throwing the below error:

"JsonSerializationException: Unable to find a constructor to use for type Microsoft.Identity.Client.Instance.Discovery.InstanceDiscoveryResponse. A class should either have a default constructor, one constructor with arguments or a constructor marked with the JsonConstructor attribute. Path 'tenant_discovery_endpoint', line 1, position 29."

I need help regarding the Azure AD login and API Authentication, any help would be highly appreciated

@najadojo were you able to solve this?

@jmprieur
Copy link
Contributor

jmprieur commented Jan 31, 2021

@Now-you-see-me-007
Copy link

I am not targeting the Hololens but UWP Desktop. I have added the following lines to the Assets/Links.xml:

The error still persists.

@jmprieur
Copy link
Contributor

jmprieur commented Feb 7, 2021

See also #2343, which is in progress

@jmprieur
Copy link
Contributor

@pmaytak @bgavrilMS : I believe we can mark this one as Fixed ?

@pmaytak
Copy link
Contributor

pmaytak commented Mar 11, 2021

@jmprieur Yes, I think so.

@jmprieur jmprieur added the Fixed label Mar 11, 2021
@jmprieur jmprieur added this to the 4.27.0 milestone Mar 11, 2021
@jmprieur
Copy link
Contributor

Thanks for confirming @pmaytak

@najadojo
Copy link
Contributor Author

I wouldn't consider this issue resolved as there are still several items that I would like solutions for:

@bgavrilMS bgavrilMS reopened this Mar 15, 2021
@bgavrilMS
Copy link
Member

bgavrilMS commented Mar 15, 2021

net45 is almost out of support and you should consider using net461 target.

I can remove the usage of Sha256Cng from the public client code path (PR - https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/2475/files)

http://localhost redirect URI is perfectly safe for a public client application and MSAL regularly undergoes security audits. MSAL will open a local port and start listening on it and then it will start the system browser. When the user completes authentication, the system browser redirects to http://localhost:1234/code=xyz and this is how MSAL reads the auth code. There is no https with sockets. By the way, Visual Studio uses this auth mechanism, as well as Git Credential Manager, Azure CLI and PowerShell.

image

If you want to provide a different mechanism for opening a browser and reading the auth code, you can - use ICustomWebUi extensibiliy - see https://docs.microsoft.com/en-us/dotnet/api/microsoft.identity.client.extensibility.icustomwebui?view=azure-dotnet

System browser or WAM should be usable via Mono.

@StLoch
Copy link

StLoch commented Apr 19, 2021

@bgavrilMS , I tried 4.29 today to see if the Sha256Cng/il2cpp problem has been resolved, and I can no longer get Microsoft.Identity.Client working in the unity editor (so I didn't make it far enough to test il2cpp). Current error is:

NotImplementedException: The method or operation is not implemented.
System.Net.ServicePoint.set_ConnectionLeaseTimeout (System.Int32 value) (at :0)
Microsoft.Identity.Client.Platforms.net45.Http.DnsSensitiveClientHandler.AddConnectionLeaseTimeout (System.Uri endpoint) (at :0)
Microsoft.Identity.Client.Platforms.net45.Http.DnsSensitiveClientHandler+d__2.MoveNext () (at :0)

I know this issue is marked as closed, but the core problem seems to have gotten worse. Would you like me to open a new issue?

@StLoch
Copy link

StLoch commented Apr 19, 2021

Actually, looking at the code, I see:

try { ServicePointManager.FindServicePoint(endpoint) .ConnectionLeaseTimeout = (int)HttpClientConfig.ConnectionLifeTime.TotalMilliseconds; } catch (NotImplementedException) { // Unity doesn't implement see https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/2537 }

I'll try building and testing on latest.

Issue following up on the latest here for reference:

#2537

@bgavrilMS
Copy link
Member

We should release MSAL 4.30 with this change in the next few days.

@StLoch
Copy link

StLoch commented Apr 20, 2021

@bgavrilMS FYI, I pulled the repo and built myself. The above issue was resolved, which got things working in editor again, but it still did not work in il2cpp builds. For that, I had to remove the HAVE_CAD define, otherwise the NewtonSoft code would return DynamicCodeGeneration as true and fail at runtime. After removing that, everything works, and I'm unblocked for now, but it may be good to take a look at that. The NewtonSoft-For-Unity project (which I saw referenced in the identity code and does work out of the box) had that define disabled as well.

And thanks for following up with all of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants