questions re the ChainingMetadataProvider type

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

questions re the ChainingMetadataProvider type

Tom Scavo
I was surprised to find attributes and child elements mentioned on the
ChainingMetadataProvider page [1] but looking at the schema I see that
this is true.

The ChainingMetadataProvider type is based on the abstract
MetadataProviderType and so the former has all the attributes and
child elements of the latter. In the schema, you'll find this
sentence: "Note that metadata filters and the require valid metadata
flag expressed on this configuration overrides those settings on
member providers."

This raises two questions:

1) I understand the comment about the requireValidMetadata attribute
but what about the rest of the Common Attributes? Are they simply
ignored by the parent ChainingMetadataProvider?

2) I don't understand the comment about the metadata filters. Since
filter order matters, the parent ChainingMetadataProvider can't simply
inject a filter into a child provider. The only thing that makes sense
is some kind of override scheme such that if a filter on the parent
ChainingMetadataProvider also exists on a child provider, the former
takes precedence. Likewise if a filter on the parent
ChainingMetadataProvider is not present on *any* child provider, the
filter is ignored. Is that how it works?

Thanks,

Tom

[1] https://wiki.shibboleth.net/confluence/x/NgInAQ
--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Rod Widdowson
> child elements of the latter. In the schema, you'll find this
> sentence: "Note that metadata filters and the require valid metadata
> flag expressed on this configuration overrides those settings on
> member providers."

Sounds like we need an RFE to review the schema <documentation> Elements.  I don't have time to look at this in the V3 timeframe,
but it can be targeted for V4.
 
> 1) I understand the comment about the requireValidMetadata attribute
> but what about the rest of the Common Attributes? Are they simply
> ignored by the parent ChainingMetadataProvider?

As I recall we actually tend to warn if people use attributes which are meaningless particulsarly for the chaining provider

  >      log.warn("{} is not valid for ChainingMetadataProvider);

Which fires for:
        failFastInitialization
        requireValidMetadata
        satisfyAnyPredicates
        useDefaultPredicateRegistry
        criterionPredicateRegistryRef


 
> 2) I don't understand the comment about the metadata filters.

Filters apply hierarchically.  

The SP doesn’t allow them at all on Chaining providers and I'd like to do something like that in the IdP but it gets grubby because
there is an implied chaining filter at the top level of each <MetadataProvider> so you may not be getting what you expect.  I don't
have enough deployment experience to make the call, but when Scott gets back he will no doubt have some comments to offer..

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Cantor, Scott E.
In reply to this post by Tom Scavo
> The ChainingMetadataProvider type is based on the abstract
> MetadataProviderType and so the former has all the attributes and child
> elements of the latter. In the schema, you'll find this
> sentence: "Note that metadata filters and the require valid metadata flag
> expressed on this configuration overrides those settings on member
> providers."

Schema's wrong, neither is settable on that object. The parser doesn't outright block them, but it would just fail at startup time. So yes, that would be confusing, but it isn't possible.

-- Scott

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Cantor, Scott E.
In reply to this post by Rod Widdowson
> The SP doesn't allow them at all on Chaining providers and I'd like to do
> something like that in the IdP but it gets grubby because there is an implied
> chaining filter at the top level of each <MetadataProvider> so you may not be
> getting what you expect.  I don't have enough deployment experience to make
> the call, but when Scott gets back he will no doubt have some comments to
> offer..

We don't/can't easily disallow it in the schema, but the setter on the actual class throws. The SP doesn't have the schema to enforce it with, and it's the same there, it's just constructor-throws rather than property based. Obviously we could do more warnings if we wanted, but it's not crucial since we don't even intend people wire Chaining up any more.

-- Scott

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: questions re the ChainingMetadataProvider type

Tom Scavo
On Mon, May 14, 2018 at 9:52 AM, Cantor, Scott <[hidden email]> wrote:
>
> ... it's not crucial since we don't even intend people wire Chaining up any more.

If that's true, then you may as well deprecate
ChainingMetadataProvider. Regardless, chunks of the documentation need
to be rewritten in that case.

Let me push back on this a little bit. A sequence of providers wrapped
in a ChainingMetadataProvider is a welcome convenience due to the fact
that top-level providers must have a sortKey (if not, the order is
undefined). OTOH, a ChainingMetadataProvider implies the natural
order.

Note that I added some examples to the ChainingMetadataProvider topic.
[1] It's easier to talk about strategy in the presence of a
ChainingMetadataProvider since the order is the natural order. An
explicit sortKey is a nuisance.

Tom

[1] https://wiki.shibboleth.net/confluence/x/NgInAQ
--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Cantor, Scott E.
> If that's true, then you may as well deprecate ChainingMetadataProvider.

I thought we did, on the pages at least, but I think I'm mis-remembering because of how the metadata providers file works, it sort of has to explicitly be a Chaining provider at the top level I guess just because of the schema. We probably could do something about that if we wanted, but it's probably not worth it, we just pre-bake that in as a default. I'm just saying nobody's likely to configure one deliberately.

-- Scott

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: questions re the ChainingMetadataProvider type

Tom Scavo
In reply to this post by Rod Widdowson
On Mon, May 14, 2018 at 9:38 AM, Rod Widdowson <[hidden email]> wrote:
>
> Sounds like we need an RFE to review the schema <documentation> Elements.

Yes, that is needed, but that won't fix the basic problem. The
ChainingMetadataProvider type is not like the other metadata providers
yet it inherits from the same abstract base type
(MetadataProviderType). It really is its own animal.

Because of this anomaly, the docs are forced to tell "little white
lies" such as: "The Common Attributes are available on all metadata
provider types" or "The following child elements may be used with all
metadata provider types."

Tom
--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Rod Widdowson
> Yes, that is needed, but that won't fix the basic problem. The
> ChainingMetadataProvider type is not like the other metadata providers
> yet it inherits from the same abstract base type
> (MetadataProviderType). It really is its own animal.

Well that could be fixed, but I don't see that this has anything to do with the documentation.  The documentation should describe
what works and what doesn't and how.    The schema is a nicety to help some development and some people debug their configuration.
The structure of the code and the structure of the schema need not have any impact on the documentation.  

You specify an invalid configuration?  It's going to barf and if it doesn't it should.  If it doesn’t either the documentation is
wrong or the code is wrong, but the schema is irrelevant at that level.  UNLESS  you want to consider the schema as partial
documentation and for that reason only we ot behoves us to keep it as up to date as possible.



--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: questions re the ChainingMetadataProvider type

Tom Scavo
On Mon, May 14, 2018 at 11:33 AM, Rod Widdowson
<[hidden email]> wrote:
>> Yes, that is needed, but that won't fix the basic problem. The
>> ChainingMetadataProvider type is not like the other metadata providers
>> yet it inherits from the same abstract base type
>> (MetadataProviderType). It really is its own animal.
>
> Well that could be fixed, but I don't see that this has anything to do with the documentation.

It does affect the documentation. At the moment, the following
statements in the documentation are incorrect:

In MetadataConfiguration:

"The Common Attributes are available on all metadata provider types"

"The following child elements may be used with all metadata provider types"

In ChainingMetadataProvider:

"Any of the common attributes may be specified"

"Any of the common child elements can be specified"

I can fix these documentation errors but it points to a fundamental
problem with the ChainingMetadataProvider, that is, it is not a
metadata provider at all!

Tom
--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: questions re the ChainingMetadataProvider type

Tom Scavo
On Mon, May 14, 2018 at 12:15 PM, Tom Scavo <[hidden email]> wrote:

>
> ... the following statements in the documentation are incorrect:
>
> In MetadataConfiguration:
>
> "The Common Attributes are available on all metadata provider types"
>
> "The following child elements may be used with all metadata provider types"
>
> In ChainingMetadataProvider:
>
> "Any of the common attributes may be specified"
>
> "Any of the common child elements can be specified"

These doc bugs are fixed in the wiki. I don't think there's anything
particularly contentious to be reviewed but it is slightly more
complicated than it needs to be. I'll leave it at that.

I think there is a deployment issue here. Some federations document
the use of ChainingMetadataProvider while others do not. I know of at
least one federation document that does not use
ChainingMetadataProvider but neither does it use sortKey on the bare
metadata providers. AFAICT by reading *our* documentation, that
results in an unspecified search order. I'm pretty sure that's not
what the author of the federation document intended.

Tom
--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Cantor, Scott E.
> I think there is a deployment issue here. Some federations document the use of
> ChainingMetadataProvider while others do not. I know of at least one
> federation document that does not use ChainingMetadataProvider but neither
> does it use sortKey on the bare metadata providers. AFAICT by reading *our*
> documentation, that results in an unspecified search order. I'm pretty sure
> that's not what the author of the federation document intended.

Everybody with > 1 metadata source is chaining. That's just mandatory to make the config work (which is not exactly true in the SP, it can be implicit there).

I believe you're formally correct that we don't specify the order without the key. In practice it's always been what you'd expect, but not specifying it leaves us the ability to change something if we have to, and it's certainly a good idea to specify the key. And no, I don't do it myself, but that's the luxury of being part of the team that would decide to break it, so I'd have warning. I certainly would not advocate anybody document doing it that way (including our own defaults if that's the case).

-- Scott

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Rod Widdowson
In reply to this post by Tom Scavo
 > In MetadataConfiguration:
> "The Common Attributes are available on all metadata provider types"

About Common attributes..
 
> I can fix these documentation errors but it points to a fundamental
> problem with the ChainingMetadataProvider, that is, it is not a
> metadata provider at all!

That’s semantics, you say tomato, I say "one of the rare Solanaceae with edible fruit".  I don’t have time to spend on hair
splitting, but I'll observe that as one of the authors of that code it passes the duck test to me.

/R

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Rod Widdowson
In reply to this post by Cantor, Scott E.
> Everybody with > 1 metadata source is chaining. That's just mandatory to make the config work (which is not exactly true in the
SP, it can
> be implicit there).

Since this is dev: It can be implicit in the IdP if you have multiple provider files (which I guess is what you are saying) - and
_that_ is why the key is needed if you have multiple files...

For myself I would always specify the key, I had to dive into the documentation of Collctions.sort to see that it (currently,
remember Java has changed gears and is a potentially rapidly changing platform) guarantees stability..  There again I also over use
parentheses in boolean expressions when I'm writing C...

/R

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Cantor, Scott E.
> Since this is dev: It can be implicit in the IdP if you have multiple provider files
> (which I guess is what you are saying) - and _that_ is why the key is needed if
> you have multiple files...

Ah, right.

> For myself I would always specify the key, I had to dive into the documentation
> of Collctions.sort to see that it (currently, remember Java has changed gears
> and is a potentially rapidly changing platform) guarantees stability..  There
> again I also over use parentheses in boolean expressions when I'm writing C...

I'm pretty certain that I was accurate in saying we don't formally specify any sort order without the key even within a single file and an explicit Chaining provider. We obviously have a predictable order, but it's implementation dependent.

-- Scott

--
To unsubscribe from this list send an email to [hidden email]
Reply | Threaded
Open this post in threaded view
|

RE: questions re the ChainingMetadataProvider type

Rod Widdowson
> I'm pretty certain that I was accurate in saying we don't formally specify any sort order without the key even within a single
file and an
> explicit Chaining provider. We obviously have a predictable order, but it's implementation dependent.

No, we (I) explicitly added the documentation when I added the index:

[1] Metadata sources combined via a chain are searched in the order in which they occur in the chain, and the first entry matching
the entityID is returned.

I did so with an explicit knowledge of what I was doing so and with a not to watch this in future (Lists turning back into
Collections - which reminds me there is work to there still in V4 in the IdPAttribute#Getvalues()API...)

Pretty sure we discussed it too...



[1]
https://wiki.shibboleth.net/confluence/pages/viewpage.action?spaceKey=IDP30&title=MetadataConfiguration#MetadataConfiguration-Search
OrderingSearchOrdering

--
To unsubscribe from this list send an email to [hidden email]