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

[Investigation] Can MSAL.NET leverage Identity Model extension for .NET / Crypto #1453

Closed
jmprieur opened this issue Oct 17, 2019 · 4 comments

Comments

@jmprieur
Copy link
Contributor

jmprieur commented Oct 17, 2019

Is your feature request related to a problem? Please describe.
we have many issues about some certs not being supported on some platforms (some versions of .NET FW, or .NET Core).
Identity Model extension for .NET / Crypto has solved the problem

Can we leverage it?
Is it supported on all the platforms where we need it?

Additional context
Add any other context or screenshots about the feature request here.

Workaround
Users can use the signed assertions ...

@jmprieur jmprieur added this to the 4.6 milestone Oct 17, 2019
@jmprieur jmprieur modified the milestones: 4.6, 4.7 Oct 30, 2019
@jmprieur jmprieur modified the milestones: 4.8, 4.9 Nov 7, 2019
@bgavrilMS bgavrilMS self-assigned this Dec 4, 2019
@bgavrilMS
Copy link
Member

bgavrilMS commented Dec 4, 2019

We met with @GeoK and discussed about this - this is a summary of the discussion.

Where can Identity Model be used in MSAL

  1. Internally, in confidential client flows, MSAL needs to create and sign an assertion (a jwt) using an X509Certificate. Historically this has created lots of bugs because of the plethora of X509Certificate
    types out there. Identity Model knows about these details and is able to handle them across many platforms.

  2. In the public API of MSAL, for confidential clients, we expose .WithCertificate(X509Certificate cert). With the integration of Identity Model we could also support: .WithCertificate(SigningCredentials creds). The top level abstraction SigniningCredentials is very powerful as there are adaptors to KeyVault, EC certs etc. Some discussion needs to happen here on the exact method signature, errors etc.

  3. For PoP extensibility, to allow users to bring their own PoP key.

Changes needed for MSAL to reference Identity Model

There are 2 big rocks to consider in this transition:

  1. Identity Model 5.x has a single non-System reference - Newtonsoft.Json. We cannot bring any dependnecies, especially not Newtonsoft, into the transitive dependency graph of MSAL.

Solution: Identity Model to ship a version 6 where they break this dependency. This is already planned, but no ETA.

  1. Identity Model supports: net45, net451, net461 and netstandard1.4, netstandard2.0. This is incompatible with MSAL's netstandard1.3 target.

**Solution 1 **: MSAL to bump support of netstandard from 1.3 to (at least) 1.4 (I see no reason to not go to 1.6 - see table here)
This will require a major version upgrade of MSAL.

Solution 2: Identity Model to ship with netstandard 1.3 support in version 6

CC @henrik-me @brentschmaltz

@bgavrilMS
Copy link
Member

I believe this concludes the investigation.

@henrik-me
Copy link
Contributor

We should keep this as a goal and explore where things are as IdentityModel 6 ships.

@bgavrilMS
Copy link
Member

I think we now generally agree that we will not do this.

@pmaytak pmaytak removed this from the Future Major Version milestone Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants