MessageHandlers and SOAP faults

[Cantor, Scott]

On 6/11/15, 8:48 PM, "dev on behalf of Brent Putman" <dev-bounces at shibboleth.net on behalf of putmanb at georgetown.edu> wrote:

>I'm still trying to fully grok how all the flows and event stuff works in the IdP.  But I think I get that the idea for this so far is that the AddSOAPFault action is extracting info via the injected functions and building the Fault.

Yes. Most things that got pushed down to OpenSAML had to be heavily function-fied to make them work.

>The issue I see is that the default FaultCodeMappingFunction is really profile-centric and based on the use of EventContext. I don't think that a stricly implemented MessageHandler can make use of that from a dependency point of view - that's in profile-api and I generally see that as strictly off-limits for the message-oriented SOAP stuff. (The SAML Assertion one technically *could* b/c it's in saml-impl, but I don't think it's right).

I guess my take is that if it's ultimately a profile flow running your handler here, just have the MessageHandler throw an exception that will get mapped to an Event in the surrounding profile flow and then AddSOAPFault could be used in the "outer" flow that's actually invoking the handler. That's basically how the ECP stuff works I think.

>So I suppose one could inject a different impl of Function<ProfileRequestContext,QName> there and also a Function<ProfileRequestContext,String> for the fault string.  I note though that that approach doesn't allow the Fault emitter to specify the full range of Fault data, like FaultActor and Detail.

Isn't the problem that you don't have a ProfileRequestContext at all? If you have one, you can just move the FaultCodeMappingFunction out of the MessageHandler into some place that's not "disallowed" from knowing about the EventContext and inject it with Spring when the time comes. The default function could be something more neutral.

>The way I did it in the delegation extension was to have the fault-emitting code store an actual Fault object in a message subcontext, and then something later pulls it out.  Can you see any reason not to augment the AddSOAPFault action with an additional Function<ProfileRequestContext,Fault>,
> which if non-null and it produces anything, bypasses the rest?  Or is there a better way?

That sounds fine too, or just create a different action that suits your needs. But more to the point, I wouldn't really want to not be reusing the same fault-mapping logic in the IdP that's already there.

>Also, I'm just noticing: I think there's some generics issues with some of this.  We don't see it because it's all Spring and reflection.  But for example in AddSOAPFault, I see:
>
>profileRequestContext.setOutboundMessageContext(new MessageContext<Fault>());
>
>Since the PRC ought to be generically parameterized with an OutboundMessageType (that certainly isn't going to be a Fault, for a SAML flow it would likely be SAMLObject or even more specific), that really isn't kosher.

The actual use of generics in the PRC is basically unworkable because in any given case, you may get an invalid message in there that has to be detected and gracefully handled and turning that into class-cast errors in the code was not a better option. So a lot of the stuff just got changed and I left the PRC declared with no types in all the actions.

>Wondering whether all our actions are like that.

They are, because a lot of stuff crashes if it's actually nailed down. Basically if the generic type is made non-generic, that introduces assumptions that the actual code does not need to impose.

>  Anyway, with careful wiring it doesn't actually break I guess, it's just perhaps unsettling...

I considered at one point just removing the generics entirely because it just doesn't work to use it in many places without turning controllable error checking into runtime exceptions needlessly.

Basically, when you're using generics to avoid casts, you'd better be damn sure that the objects can't ever be anything else, and these flows are never really sure of that because that's just what getting bad input means.

-- Scott