Open OIDF-automation opened 6 months ago
Thanks for your thoughts, Gabriel. A few reactions to your suggestions…
value
can be merged with one_of
provided that the value
value is in the one_of
set of values. (Although since it would result in additional code, I’d want to be convinced that this is a worthwhile addition.)add
can be combined with superset_of
provided that the result satisfies the superset_of
.default
being used to override another default
. This would violate the “Equal Opportunity“ principal at https://openid.bitbucket.io/connect/openid-federation-1_0.html#name-principles.essential
, I tend to favor your interpretation (a) - that it’s not an error if both true
and false
values are in the chain, but that if anywhere in the chain, the value true
occurs then the result of the merge is true
. Although my read of the spec currently is that it specifies (b).Thanks again!
On value
:
one_of
was just an example, I would say value
can be combined with all value checks, i.e. one_of
, subset_of
, superset_of
, if the value from value
is valid for those parameters. I don’t think that this would (generally) result in additional code, since the value check must be done anyways.subset_of
: here we must make sure that only the value check part of this operator is done, not the value modifier part, i.e. implementations must explicitly check that the value from value
is a subset of subset_of
before/instead of applying the subset_of
operator. For one_of
and superset_of
I would argue that no additional consistency check is required, since applying the operator already does the check (and does not modify the value).value
with one_of
and superset_of
should not be a problem, and not require additional code; I can see that it would be reasonable to not allow combination with subset_of
, because of the additionally needed code, however, it also feels a bit inconsistent to me. default
, also no code is needed for that, value
will be applied first, then default
does nothing. I can see your point on default
overwriting default
.
For essential
, I originally assumed interpretation (a), but it was not clear to me what the spec currently says, I don’t care too much what it will be in the end, it just should be more clear.
@zachmann Hi Gabriel,
I studied PR https://github.com/openid/federation/pull/111 - "Policy operators: no restrictions when combining add and superset_of".
Every operator definition follows an identical template. This template includes (among other things):
It is correct that if a software doesn't implement the combination checks for add
+ superset_of
the order of applying superset_of
after add
will enforce the rule nevertheless. The combination spec, as description or result, is still valid however:
add: MAY be combined with subset_of, in which case the values of add MUST be a subset of the values of subset_of.
subset_of: MAY be combined with add, in which case the values of add MUST be a subset of the values of subset_of.
The current combination specs were worded like this to make it easier for policy designers to reason about operators and how they may be combined without having to also consider the order of the operator application. If we remove the conditionals ("... in which case...") this feature will be lost to the reader.
I see two ways going forward:
Keep the combo specs like there are and leave it up to implementers to figure out and decide not to implement the combo check (and rely only on the application order). E.g. for efficiency or because it's easier to code.
Edit all combination specs to delineate clearly between the check that must absolutely be implemented and what is just a "description" of the allowed combination.
I'm not in favour of completely removing the normative description, because this will make it more difficult for the reader to figure out the allowed combinations.
I understand the reasoning. I would also say in general it is fine as it currently is.
In the case of combining add
and superset_of
together I found it too restrictive, as only the result after applying add
but not already the add
value(s) have to be a superset of superset_of
.
When submitting the PR I considered wording it something like that, but found it too complex, if it also just could be left out.
But for the sake of consistency it makes sense to add a condition, I'll update the PR.
I'm kind of sympathetic to what is raised here.
I think the standard could be simplified and processing be made more secure by simply stating that the final metadata MUST be consistent with all policy operators, or policy processing MUST fail.
Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/2156
Original Reporter: gzachmann
Hi,
while going through the metadata policies operators of the newest spec version and found some things to comment on. I wanted to hear your opinion on them, if those are valid points or might already have been discussed before.
value
: Why can it only be combined withessential
? I think it should be possible to combine with all value checks, e.g. one IA might set aone_of
policy value check and another IA/TA sets avalue
, this could still work perfectly fine.add
: Combination withsuperset_of
: I don't think we should have the requirement that the values fromadd
MUST be a superset ofsuperset_of
. Only afteradd
is done the result MUST be a superset of the values insuperset_of
. I'd argue we could just strip the stated requirement, since the consistency with value checks is checked anyway later.default
: Merging: Personally, I would like to have the possibility to mergedefault
in the sense that superiors overwrite subordinate policies. This would enable a national federation to set another default than an intra-national-fed. Since default is rather weak (it's just a default, if the value is something else it's still fine) - I don't feel like a "conflict" between different IAs, is something critical/incompatible)superset_of
: Combination withadd
: See aboveessential
: Merging: The spec states that "If a Superior has specifiedessential=true
, then a Subordinate MUST NOT change that." It's not completely clear to me what the meaning of 'MUST NOT change' is:a)
essential
is true as soon as any entity in the chain says so, i.e. subordinates cannot overwrite true with false - if they try it does not matter, the chain is still validb) if a subordinate defines
essential=false
and a superior definedessential=true
this MUST result in a policy error.