search

Sustainsys / Saml2

Saml2 Authentication services for ASP.NET
Other
951 stars 601 forks source link

NullReferenceException at Sustainsys.Saml2.Saml2P.Saml2PSecurityTokenHandler.ValidateToken #1336

Closed ethan-agencyQ closed 2 years ago

ethan-agencyQ commented 2 years ago

We found out that when the SAML response doesn't have the , the library would throw a NullReferenceException at Sustainsys.Saml2.Saml2P.Saml2PSecurityTokenHandler.ValidateToken (See detail stack trace below).

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidateConditions(Saml2SecurityToken samlToken, TokenValidationParameters validationParameters)
   at Sustainsys.Saml2.Saml2P.Saml2PSecurityTokenHandler.ValidateToken(String token, TokenValidationParameters validationParameters, SecurityToken& validatedToken)
   at Sustainsys.Saml2.Saml2P.Saml2Response.CreateClaims(IOptions options, IdentityProvider idp)+MoveNext()
   at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Sustainsys.Saml2.Saml2P.Saml2Response.GetClaims(IOptions options, IDictionary`2 relayData)
   at Sustainsys.Saml2.WebSso.AcsCommand.ProcessResponse(IOptions options, Saml2Response samlResponse, StoredRequestState storedRequestState, IdentityProvider identityProvider, String relayState)
   at Sustainsys.Saml2.WebSso.AcsCommand.Run(HttpRequestData request, IOptions options)
   at Sustainsys.Saml2.AspNetCore2.Saml2Handler.HandleRequestAsync()

However, this node should be optional meaning the samlToken.Assertion.Conditions can be null, the latest Microsoft.IdentityModel.Tokens.Saml2 library has fixed its bug. So I tried the latest Microsoft library, but Sustainsys would throw another NullReferenceException at Saml2PSecurityTokenHandler (See below) trying to access samlToken.Assertion.Conditions.NotOnOrAfter

https://github.com/Sustainsys/Saml2/blob/c466d3bbf12479944e350b649f5b5c825616007b/Sustainsys.Saml2/SAML2P/Saml2PSecurityTokenHandler.cs#L88

AndersAbel commented 2 years ago

Yes, this is a bug. The WebSSO Profile states that the NotOnOrAfter in the SubjectConfirmationData is mandatory, but not in the Conditions. So the library should look at that instead.

AndersAbel commented 2 years ago

Won't fix in v2. Hope I'll get it right in develop/v3