Lines are not propertly cutoff

[tvdijen]

See selected area.. Gotta translate encoded characters before cutting off the line.

[khlr]

Hello Tim!

I may be wrong, but it think that's just the encoded content of that claim. Did you notice that the value starts with a <saml:NameID ...?

But could it be possible that content is malformed since it doesn't end with an encoded closing tag?

[tvdijen]

It does end with a > but the line is cutoff right in the middle of that code... Or am I just missing it? I guess it's a bit of an edge-case because the value is a NameID here...

[khlr]

Is there a way for me to reproduce this?

[tvdijen]

Sure! Go to https://engine.openconext.moo-archive.nl/authentication/sp/debug, select SimpleSAMLphp and login using khlr/Welcome01 to generate this SAMLreponse

[khlr]

Thank you for the test-account!

It's ok, that the attribute value is HTML-encoded. It's just the eduPersonTargetedID's value. It's due to the window's size, that the line in your screenshot ends with an ampersand. If you resize it, the linebreak will be on another position.

The claim's value is:

<saml:NameID xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" SPNameQualifier="https://engine.foo.nl/authentication/sp/metadata">f1248bc07c1895a3e679c1b4c614602b40a85e60</saml:NameID>

Hence I'd expect this attribute value in the token (the HTML-encoded representation of the above value):

<saml:AttributeValue xsi:type="xs:string">&lt;saml:NameID xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" SPNameQualifier="https://engine.foo.nl/authentication/sp/metadata"&gt;f1248bc07c1895a3e679c1b4c614602b40a85e60&lt;/saml:NameID&gt;</saml:AttributeValue>

But unfortunately thats's not the case! The attribute looks like this (cropped):

<saml:AttributeValue xsi:type="xs:string">&lt;saml:NameID xmlns:saml="urn:...metadata"&gt;f1248b...85e60&</saml:NameID></saml:AttributeValue>

The difference is, that the line ends with 85e60&</saml:NameID></saml:AttributeValue> whereas it should end with 85e60&lt;/saml:NameID&gt;</saml:AttributeValue>!

So SAML tracer seems to not encode the closing tag correctly. I'll take a closer look at that soon!

[khlr]

@tvdijen could you please re-enable the test-account? I'd like to debug this a bit more.

[tvdijen]

It's done @khlr!

[khlr]

Thanks, Tim! But unfortunately there's an error when I try to log in: 76b54a0604 Authentication processing filter without name given.

[tvdijen]

I'm very sorry! It has been fixed now.. As you can see, I use this setup for all kinds of testing / PoC's / development, so this could happen again!

[khlr]

Well, I think this could be quite easily fixed by replacing this old method (which only replaced the first occurrence of a character)

function xmlEntities(string) {
  string = string.replace('&', '&', 'g');
  string = string.replace('"', '"', 'g');
  string = string.replace("'", ''', 'g');
  string = string.replace(/</g, '&lt;');
  string = string.replace('>', '&gt;', 'g');
  return string;
}

with this updated version (which replaces all occurrences of a character):

function xmlEntities(string) {
  string = string.replace(/&/g, '&');
  string = string.replace(/"/g, '"');
  string = string.replace(/'/g, ''');
  string = string.replace(/</g, '&lt;');
  string = string.replace(/>/g, '&gt;');
  return string;
}

But I'm wondering if that makes sense... As you can see from the screenshot some posts above, the actual content of the claim is another un-encoded claim. And that's exactly what travels over the wire. So from my understanding the claim value should be encoded (since it's xsi:type="xs:string"), but that shouldn't be done by SAML Tracer - I think that should be done by the IdP before issuing the token. Or am I getting this wrong?

If SAML Tracer would replace the <saml:AttributeValue><saml:NameID>f1248bc0</saml:NameID></saml:AttributeValue> with <saml:AttributeValue>&lt;saml:NameID&gt;f1248bc0&lt;/saml:NameID&gt;</saml:AttributeValue> it would distort the original claim value which patently doesn't have that value encoded.

Maybe it's just too late or it's my lack of SAML knowledge 😉 Perhaps @thijskh or @jaimeperez could shed some light on this?

[khlr]

This should be fixed for quite a while now, right? #53 has remedied this, hasn't it, @tvdijen ?

Hence I'm closing this issue now. Feel free to reopen it if I missed something.