microsoft-authentication-library-for-dotnet icon indicating copy to clipboard operation
microsoft-authentication-library-for-dotnet copied to clipboard

Allow non-GUID client id with PublicClientApplicationBuilder

Open eberlekhaufe opened this issue 1 year ago • 6 comments

MSAL client type

Public

Problem statement

MSAL.NET: 4.60.0 .NET 8

I use the PublicClientApplicationBuilder to create a msal app to authenticate with a 3rd party identity provider which issues client ids in the form "Some GUID@somesuffix"

Unfortunately PublicClientApplicationBuilder.Build fails on such a client id, as it expects a proper GUID.

Can that be changed to support non-GUID client ids? Even if validation was changed, would there be down-stream errors with a non-GUID client id?

Here is my code var clientBuilder = PublicClientApplicationBuilder.Create("my-non-GUID-clientid") .WithDefaultRedirectUri() .WithExperimentalFeatures() .WithOidcAuthority("some url");

var msalClient = clientBuilder.Build(); // throws here var result = await msalClient.AcquireTokenInteractive(scopes: ["api", "api:concurrent_access", "email", "oidc", "profile"]).ExecuteAsync();

Calling Build throws a Microsoft.Identity.Client.MsalClientException Message: Error: ClientId is not a GUID. Stack Trace: at Microsoft.Identity.Client.PublicClientApplicationBuilder.Validate() at Microsoft.Identity.Client.AbstractApplicationBuilder`1.BuildConfiguration() at Microsoft.Identity.Client.PublicClientApplicationBuilder.BuildConcrete() at Microsoft.Identity.Client.PublicClientApplicationBuilder.Build() ...

Thanks a lot in advance, Krischan

Proposed solution

PublicClientApplicationBuilder and PublicClientApplication should support non-GUID client ids, if possible.

Alternatives

Any workaround would also be appreciated, if exists :)

eberlekhaufe avatar Mar 28 '24 16:03 eberlekhaufe

What identity provider are you using @eberlekhaufe?

localden avatar Apr 02 '24 23:04 localden

It is an identity Provider implemented in the acumatica ERP system. It is implemented based on https://identityserver4.readthedocs.io/en/latest/

It supports oauth2/oidc.

eberlekhaufe avatar Apr 03 '24 08:04 eberlekhaufe

It is an identity Provider implemented in the acumatica ERP system. It is implemented based on https://identityserver4.readthedocs.io/en/latest/

It supports oauth2/oidc.

By the way, when attempting to use a generic oidc Identity Provider, it is better to use WithOidcAuthority(). (It may still miss the non-GUID support at this time, but it will eventually be implemented.)

rayluo avatar Apr 03 '24 19:04 rayluo

I'll assign to myself for the time being - I think there is value in this being a generic client ID, but likely this will fall a bit lower in the priority stack of things we need to do in the short-term.

localden avatar Apr 03 '24 20:04 localden

Non-guid client id:s are already supported for ADFS: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/1107c3e157bfa6efc50ae6419ae9da38d309ae89/src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs#L380-L384 And if I change that to also allow "Generic" authorities, I was able to use Auth0 and a non-guid client id with PublicClientApplicationBuilder. Is there anything else that needs to be changed?

jwikberg avatar Apr 29 '24 08:04 jwikberg

Hi @jwikberg , in my setup neither ADFS nor Auth0 is used (see code snippet above in the issue). Using the Builder as above, causes the client id must be guid error.

eberlekhaufe avatar May 02 '24 08:05 eberlekhaufe

Hi @jwikberg , in my setup neither ADFS nor Auth0 is used (see code snippet above in the issue). Using the Builder as above, causes the client id must be guid error.

My question was directed at Microsoft.

jwikberg avatar May 07 '24 11:05 jwikberg

Non-guid client id:s are already supported for ADFS:

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/1107c3e157bfa6efc50ae6419ae9da38d309ae89/src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs#L380-L384

And if I change that to also allow "Generic" authorities, I was able to use Auth0 and a non-guid client id with PublicClientApplicationBuilder. Is there anything else that needs to be changed?

Feel free to propose a change. There is an AuthorityType.Generic type.

bgavrilMS avatar May 07 '24 13:05 bgavrilMS

Hi @bgavrilMS, hope you and the team will consider the PR I've created which fixes the issue. PR just modifies the section mentioned by @jwikberg and there is also a new test added (not sure if the test location is correct - it uses Selenium).

kurtanr avatar May 07 '24 18:05 kurtanr