IdentityServer / IdentityServer4.WsFederation

Sample for implementing WS-Federation IdP support for IdentityServer4
Apache License 2.0
67 stars 53 forks source link

NameIdentifier claim added twice #5

Closed janih78 closed 6 years ago

janih78 commented 6 years ago

I'm using IdentityServer4 + WS-Federation sample from this repo + ASP.NET Identity and getting error "InvalidOperationException: ID4139: No suitable Saml2NameIdentifier could be created for the SAML2:Subject because more than one Claim of type NameIdentifier was supplied." while login. Without ASP.NET Identity everything works as supposed (e.g. by using TestUserStore). I identified this as a bug in SignInResponseGenerator.cs -> CreateSubjectAsync-method where NameIdentifier is added twice.

First in here

var nameid = new Claim(ClaimTypes.NameIdentifier, result.User.GetSubjectId());  
nameid.Properties[ClaimProperties.SamlNameIdentifierFormat] = result.RelyingParty.SamlNameIdentifierFormat;
var outboundClaims = new List<Claim> { nameid };

And after that in foreach-loop

var outboundClaim = new Claim(result.RelyingParty.ClaimMapping[claim.Type], claim.Value);
if (outboundClaim.Type == ClaimTypes.NameIdentifier)
{
    outboundClaim.Properties[ClaimProperties.SamlNameIdentifierFormat] = result.RelyingParty.SamlNameIdentifierFormat;
}
outboundClaims.Add(outboundClaim);

I can fix this locally, but it's a good idea to fix your sample too.

BTW: Do you have any plans to make a WS-Federation nuget package (IdentityServer4 plugin) based on this sample?

leastprivilege commented 6 years ago

Feel free to open a PR with a more defensive approach.

NameIdentifier is the equivalent to the sub claim - there should be only one.