MessageHandlers and SOAP faults

[Brent Putman]

On 6/11/15 11:46 PM, Cantor, Scott wrote:
> On 6/11/15, 11:10 PM, "dev on behalf of Brent Putman" <dev-bounces at shibboleth.net on behalf of putmanb at georgetown.edu> wrote:
>
>
> Well, maybe, but if it's not exposed to the deployer, then it's not configurable either. I didn't want fault codes to depend on our code, but on a mapping table. SOAP faults aren't terribly interesting anyway, but it made more difference for SAML statuses.

Right not configurable, at lest not in SWF, etc.  But it wouldn't need
or want to be configurable, see next comment.

>> But that is close to what I had in mind:  1) the emitter sets the Fault in the context tree somewhere 2) it then throws 3) (IIUC) something up above in SWF is catching that exception, and mapping it to an action to run (e.g. AddSOAPFault) 4) that action, as one possible strategy, can pull the previously-set fault out and use it directly.
> The question is how it knows what Fault to put into the context tree.

In case it wasn't clear, what I'm talking about here is MessageHandlers
that are SOAP processing components, period.  So they are SOAP-aware,
and they are by definition going to know what kind of SOAP Fault to
return under various conditions.  That's basically a fundamental part
of what they do. 

I'm sure you know, the various WS-* specs define specific fault codes. 
Take a look for example at the constants defined in WSSecurityConstants
and WSAddressingConstants.

For example, with the SAML Assertion token processing one I'm working
on, it would in some cases want to return a
WSSecurityConstants.SOAP_FAULT_INVALID_SECURITY_TOKEN. It knows to do
that because it just does. In another case it knows to just return a
FaultCode.SERVER, because of say an internal processing error.

Another example: if we wanted to implement replay detection of
WS-Addressing MessageID, the handler might want to emit a
WSAddressingConstants.SOAP_FAULT_DUPLICATE_MESSAGE_ID.  It might event
want to put the duplicate ID info into the FaultString.

Some more advanced ones might even want to add FaultActor or Detail.

In short, it seems to me that the SOAP processing component is the
thing that ought to build the Fault, since that's where all the info is.


>
>> If by that, you mean having SWF catch thrown Exceptions in the appropriate places and transitioning to the AddSOAPFault action, I agree.
> No, I specifically meant mapping an event to a fault rather than hardcoding it. I tried to avoid that.

Right I get that was the use case so far.  You have potentially
non-SOAP-aware code that either throws exceptions or emits an Event,
and you want to in a configurable manner map those exceptions and/or
events to fault codes.

This is really the opposite case.  You have SOAP aware components that
literally can't emit an Event, b/c the API and dependency hierarchy is
wrong.  They could theoretically throw an exception that somewhere gets
mapped to a Fault, but I'm not keen on the idea of having dozens of
specialized exception subtypes, one for each code.  And with that you
still wouldn't get all the other fault info.  We could, as we have done
elsewhere, actually attach the Fault to the exception. That is probably
the other way to go here, and is the other thing I might propose.  But
that changes up the processing model from what you already have in
AddSOAPFault. (Actually *something* would still have to get the Fault
from the exception and supply to AddSOAPFault, so I don't think that
really buys anything...)

(Also don't forgot about the SOAP client and SP half-duplex cases, for
which the profile API stuff is problematic and/or flat out doesn't work.)

The simplest, more conservative approach for the IdP seems to me to
just allow the Fault to be specified explicitly in the MessageContext.




Moving on:

>> I guess I'm not clear how that could happen. If everything were generic-ed, I don't think you could.
> If it's left untyped (meaning the actions themselves stay generic and not typed), 

Right, but they shouldn't be, that's my point. 


> I'm not sure what it buys (and I'm not sure that's any different from declaring them without type parameters at all, 


There's certainly Actions and/or other genericed components like
MessageHandlers where the type carried in the getMessage() slot is
irrelevant. E.g. ones that are *just* operating on or populating other
context data. So if you were declaring those in code, you'd use a '?'
wildcard I think, to indicate that the type doesn't matter.


> since either way the type's gone at runtime).

Sure, but the win with generics is compile-time type checking.  As long
as you *don't* have non-generic-ed code putting things into or pulling
things out of generically typed structures (so as Objects, what they
are at runtime), then it's guaranteed that you can not have a
ClassCastException.  If that assumption is being violated, then there's
a problem.  And that's kind of what I'm noting here.  In the SOAP fault
action, the PRC isn't genericed, so it's (illegally) allowed to put a
MessageContext<Fault> into the PRC.  As if the type is fungible, when
it really shouldn't be.


>
>> Which I think can only happen if you have non-generic code operating on the generic types.  Which I guess I'm saying I didn't think we'd have.  I'm sure the big picture is complicated though.
> We had code that wasn't SAML specific operating on SAMLObject originally. 

That I think is part of the problem.  If we had that, it was wrong.  I
think all of this would hang together if generics were used
"correctly", but if some things are specified too narrowly, or without
the proper use wildcards and "extends" and "super", then there would
certainly be a problem, yes.  Remember my recent fixing of the generics
as relates to X509Credential TrustEngine stuff...


> Something had to change, I just probably went too far with it because it was in my way and wasn't really helping me do anything.


I'm not convinced that this couldn't be made to work.  But I don't
necessarily want to rathole on this right now.  If we were going to
change anything here, it likely would be a Future timescale thing.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20150612/ca775812/attachment.html>