Reverse attribute mapping..

Cantor, Scott cantor.2 at osu.edu
Mon Sep 23 11:50:24 EDT 2013


On 9/21/13 11:55 AM, "Rod Widdowson" <rdw at steadingsoftware.com> wrote:
>
>I should warn at the top that this can get quote confusing as you navigate
>between SAML attributes their values, formats  and names and IdP
>attribute,
>their values and names.  Further on in this mail I will confuse the issue
>more by throwing in references to RequestedAttributes (both sorts).

We currently use "IdPSession" for the primary session object, I think we
should move to IdPAttribute for the internal attribute case.

>At the lowest level, the work is done by an optional
>'AttributeMapperFactory' method to the attribute encoders.  This returns
>the
>analogous attribute mapper.  Hence the code which maps  IdPAttribute to
>SAMLAttribute can also produce a class whose job is to do the reverse
>(SAMLAttribute to IdPAttribute).

Reinforced by the fact that you actually call it IdPAttribute here. ;-)

>What concerns me most is the addition of the factory method to the
>encoders.
>As described above this seems OK, and it makes for quite clean code, but
>having code which works at this level of detail inside idp-saml-* 'feels
>wrong'.  This become particularly notably  when one realizes that in this
>case the attribute mapper is not from a SAML Attribute to an IdP Attribute
>but from a SAML RequestedAttribute to an IdP RequestedAttribute.

I don't think it's that bad. RequestedAttribute in SAML is really just a
wrapper for Attribute that adds the isRequired flag. It's basically the
same thing.

>The other nasty side effect of all this is in testing and dependency
>ordering.  In order to do sensible testing I wanted to build an XML
>description of an attribute resolver, create the AttributeResolver from
>it,
>and from that generate the mapper which I could then interrogate.  Indeed
>I
>have done this, but because of the new dependencies introduced I cannot
>make
>idp-mapper-impl depend on idp-resolver-spring without introducing
>circularity and so the test has actually been added in
>idp-resolver-spring.

Is it circular if it's just a test dependency?

>2) Move some of idp-attribute-mapper-* into idp-saml-* (or move the
>attribute encoders into idp-attribute-mapper).  This may well introduce
>other circularity (the resolver definition needs to depend on the
>attribute
>encoders, but the attribute mapper depends on the resolver definition).

Well, I was going to say that I'm not sure the mapper functionality right
now shouldn't be in idp-saml anyway. Is it really general, or is it based
on the SAML syntax?

>3) Move the factory method out from idp-saml-* and replace it with a big
>case statement (on class type) inside the mapper code, or possibly
>polymorphic methods.  Yuck.

I'd hold judgement pending (2). If it's truly general, then I think it's
appropriate. If not, I think a subinterface for AttributeEncoder might be
needed.

-- Scott




More information about the dev mailing list