ADFS HTTP-Artifact Signature

[egonk]

I've been debugging an issue on my test Server 2022 (I got urn:oasis:names:tc:SAML:2.0:status:Requester) and figured out the fix in this function (v0.4.13):

// Element returns an etree.Element representing the object in XML form.
func (r *ArtifactResolve) Element() *etree.Element {
	el := etree.NewElement("samlp:ArtifactResolve")
	el.CreateAttr("xmlns:saml", "urn:oasis:names:tc:SAML:2.0:assertion")
	el.CreateAttr("xmlns:samlp", "urn:oasis:names:tc:SAML:2.0:protocol")

	// Note: This namespace is not used by any element or attribute name, but
	// is required so that the AttributeValue type element can have a value like
	// "xs:string". If we don't declare it here, then it will be stripped by the
	// cannonicalizer. This could be avoided by providing a prefix list to the
	// cannonicalizer, but prefix lists do not appear to be implemented correctly
	// in some libraries, so the safest action is to always produce XML that is
	// (a) in canonical form and (b) does not require prefix lists.
	el.CreateAttr("xmlns:xs", "http://www.w3.org/2001/XMLSchema")

	el.CreateAttr("ID", r.ID)
	el.CreateAttr("Version", r.Version)
	el.CreateAttr("IssueInstant", r.IssueInstant.Format(timeFormat))
	if r.Issuer != nil {
		el.AddChild(r.Issuer.Element())
	}
	if r.Signature != nil {
		el.AddChild(r.Signature)
	}
	artifact := etree.NewElement("samlp:Artifact")
	artifact.SetText(r.Artifact)
	el.AddChild(artifact)
	return el
}

I put Signature before Artifact and it works now!

The idea comes from this blog post: https://www.wiktorzychla.com/2017/09/adfs-and-saml2-artifact-binding-woes.html

(this one was tricky)make sure the Signature goes between the Issuer and Artifact nodes. By taking a look into ADFS sources (the Microsoft.IdentityServer.dll) you could learn that ADFS has its own exclusive, hand-written algorithm of signature validation which doesn't use the .NET's SignedXml class at all. This manually coded algorithm expects this particular order of nodes and logs 'Element' is an invalid XmlNodeType (as shown at the screenshot above, not too helpful) if you put your signature as the very last node. Frankly, I don't see any sense in this limitation at all.

What do we do here? Just commit this fix or make an option ADFSCompat?