search

Sustainsys / Saml2

Saml2 Authentication services for ASP.NET
Other
960 stars 602 forks source link

CreateClaims "Sequence contains more than one element" #792

Closed freelancer42 closed 2 years ago

freelancer42 commented 7 years ago

I'm trying to use this with a Nexus Hybrid Access Gateway IdP. Everything seems to work fine when I'm just using password authentication at the IdP, but when I'm switching to authentication using a chained IdP I'm getting this exception:

[InvalidOperationException: Sequence contains more than one element] System.Linq.Enumerable.SingleOrDefault(IEnumerable1 source) +5923580 Kentor.AuthServices.Saml2P.<CreateClaims>d__57.MoveNext() +951 System.Collections.Generic.List1..ctor(IEnumerable1 collection) +536 System.Linq.Enumerable.ToList(IEnumerable1 source) +80 Kentor.AuthServices.Saml2P.Saml2Response.GetClaims(IOptions options) +309 Kentor.AuthServices.WebSso.AcsCommand.ProcessResponse(IOptions options, Saml2Response samlResponse, StoredRequestState storedRequestState) +48 Kentor.AuthServices.WebSso.AcsCommand.Run(HttpRequestData request, IOptions options) +1160 Kentor.AuthServices.HttpModule.Saml2AuthenticationModule.OnAuthenticateRequest(Object sender, EventArgs e) +367 System.Web.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +92 System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +165

Looking at the code, it seems to me the code in Saml2Response.cs is expecting there to be exactly zero or one instances of Saml2AuthenticationStatement in the response, but in this case there is more than one. The code should account for this possibility.

AndersAbel commented 6 years ago

Yes, the code for extracting SessionNotOnOrAfte is expecting there to be at most one AuthenticationStatement.

In your scenario, does both of them contain a SessionNotOnOrAfter setting? I think that the right way to handle this would be to get the earlieast expiration if there are multiple.

freelancer42 commented 6 years ago

Yes they do (though maybe it would be best not to assume so?), and I agree that sounds like the best way to handle it.

AndersAbel commented 6 years ago

I'll put this on the todo list. But as you might have noticed it's quite extensive. If you need this feature completed, please consider submitting a PR or contact me at anders@sustainsys.com to sponsor the work.