search

OpenConext / OpenConext-engineblock

OpenConext SAML 2.0 IdP/SP Gateway
14 stars 22 forks source link

Setting NameId from attribute manipulation broken #303

Closed thijskh closed 8 years ago

thijskh commented 8 years ago

The attribute manipulation functionality allows to override attributes but also the SAML NameId by setting the $subjectId variable. Since EB 5.0 setting the $subjectId variable no longer has an effect and the original NameId is passed through unchanged.

Manipulation of specific attributes through the $attributes array remains working.

This regression was observed once before in #142 (but I don't know if the causes are related).

DRvanR commented 8 years ago

After some research into the code, what strikes me most is that all code involved hasn't changed in the last two years... Thinking further (since this is executed in a closure, with a lot of by reference juggling) I think this has more to do with the move to PHP5.6 than actual changes in the code. I'm going to try and verify that first, before attempting to debug the whole attribute manipulation logic (that seemingly hasn't changed...)

What has changed further is the introduction of the Attribute Aggregation. This is executed before consent, but after manipulations on behalf of the IdP. Therefor if any of the aggregations overwrite manipulations done on behalf of the IdP, that might lead to conflicts as well. @thijskh : Were the manipulations run on behalf of the IdP or the SP? If on behalf of the IdP this might be a second cause to investigate...

thijskh commented 8 years ago

This is an SP-bound Attribute Manipulation. The SPs I tested with do not have the coin:attribute_aggregation_required setting enabled.

DRvanR commented 8 years ago

Ok, that leaves the changes in by-reference and closure handling of PHP as possible causes (for now). Am writing tests that test this behavior to see if I can reproduce the issue. Thanks.

thijskh commented 8 years ago

There is more to it. Adding eduPersonTargetedID to the ARP of the SP reveals more information. This is for an SP that simply has $subjectId = "NOOT"; as the AM.

The Subject NameID generated by EB is wrong: it's my original, unspecified NameID with a Format persistent (so not a persistent NameID as EB normally generates itself). So I assume that EB is somehow outputting the requested 'unspecified' NameID but not with the manipulation applied and also with the wrong Format specification.

<saml:Subject>
    <saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">urn:collab:person:surfnet.nl:kinkhorst</saml:NameID>
…
</saml:Subject>

However, the epTID, supposedly a copy of the subject NameID, seems to give the expected output: the modified NameID with the correct Format specification:

<saml:Attribute Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
    <saml:AttributeValue>
        <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">NOOT</saml:NameID>
    </saml:AttributeValue>
</saml:Attribute>
DRvanR commented 8 years ago

Resolved through #308