[WIP] IdentityModel dependency update for OpenIdConnect, WsFed, and Bearer #21
Conversation
|
Looking forward to this... a number of packages in our app have moved to 5.x and we can't build our our Azure AD integration without this update. |
Tratcher
force-pushed
the
tratcher/identitymodel
branch
from
2c1734e to
3048975
Compare
|
Updated with WsFed and Jwt |
|
Compare with dev...lovemaths:5.2.0 for additional changes around tokens vs keys. |
Tratcher
force-pushed
the
tratcher/identitymodel
branch
from
3048975 to
bd7e480
Compare
|
@lovemaths @brentschmaltz I've incorporated your recommendations. Please review. |
|
@Tratcher The change looks good to me. |
| @@ -19,6 +19,6 @@ internal class IssuerSigningKeys | |||
| /// <summary> | |||
| /// Signing tokens. | |||
| /// </summary> | |||
| public IEnumerable<X509SecurityToken> Tokens { get; set; } | |||
| public IEnumerable<SecurityKey> Keys { get; set; } | |||
There was a problem hiding this comment.
@Tratcher my apologizes. This is one of the best commits I've ever read. But I can't find any reference/article for migration from System.IdentityModel to Microsoft.IdentityModel Could you please provide us more information about this? We've exciting code that needs to migrated
There was a problem hiding this comment.
You should take that up with @lovemaths and @brentschmaltz, most of this was done on their advice.
There was a problem hiding this comment.
There was a problem hiding this comment.
@ysmoradi we are developing the updated IdentityModel changes here: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/tree/dev
Tratcher
changed the title
| { | ||
| issuer = entityDescriptor.EntityId.Id; | ||
| } | ||
| var serializer = new WsFederationMetadataSerializer(); |
There was a problem hiding this comment.
nicely simplified :-)
| @@ -254,6 +240,11 @@ public string SignInAsAuthenticationType | |||
| public ISecureDataFormat<AuthenticationProperties> StateDataFormat { get; set; } | |||
|
|
|||
| /// <summary> | |||
| /// Gets or sets the <see cref="ISecurityTokenValidator"/> used to validate identity tokens. | |||
| /// </summary> | |||
| public ISecurityTokenValidator SecurityTokenValidator { get; set; } | |||
There was a problem hiding this comment.
Should not be able to set this to null.
| using Microsoft.IdentityModel.Protocols; | ||
| using Microsoft.IdentityModel.Protocols.OpenIdConnect; |
There was a problem hiding this comment.
do we need the using Microsoft.IdentityModel.Protocols above?
There was a problem hiding this comment.
Yes, ConfigurationManager.
| } | ||
|
|
||
| // what to do here? | ||
| if (principal == null) |
There was a problem hiding this comment.
it could be the case the validator returned null OR no validator could read the token.
I agree in either case, you need to throw.
| Options.SecurityTokenHandlers = new Collection<ISecurityTokenValidator> | ||
| { | ||
| new Saml2SecurityTokenHandler(), | ||
| // new SamlSecurityTokenHandler(), |
There was a problem hiding this comment.
SamlSecurityTokenHandler is now available.
Tratcher
force-pushed
the
tratcher/identitymodel
branch
from
c818f25 to
99b749c
Compare
Tratcher
force-pushed
the
tratcher/identitymodel
branch
from
99b749c to
5d58dab
Compare
#7 Do not merge.
This is an experiment on what it would take to update Katana to IdetityModel v5. I only did OpenIdConnect for now.
Other packages that need updating:
Q: