clarify merging of essential policy operators #110
Conversation
selfissued
requested review from
ve7jtb,
rohe,
peppelinux,
vdzhuvinov and
selfissued
| met, this MUST result in a policy error. | ||
| Operator value merge: The result of merging the values of two | ||
| <spanx style="verb">essential</spanx> operators is the logical | ||
| disjunction (<spanx style="verb">OR</spanx>) of the operator values. |
There was a problem hiding this comment.
The proposed replacement is not equivalent.
The (inclusive) logical disjunction of:
- Superior policy: essential=true
- Subordinate policy: essential=false
is: essential=true
For the above condition the spec (last sentence in paragraph) describes a policy violation error, not a successful merge.
There was a problem hiding this comment.
I wasn't completely sure what should be done in this case, but I agree that as it is currently phrased this is the most likely meaning (and you confirmed it).
However, the spec also states that
If the essential operator is omitted, this is equivalent to including it with a value of
false.
Therefore, I believe my suggestion is the only feasible approach. Otherwise either omitting is not the same as providing essential=false, or essential=true could not be merged, only if all entities provide essential=true.
There was a problem hiding this comment.
This shows that we have a bug in the essential spec. Good catch!
Would you be able to add the customary history entry to this PR?
Examples here:
https://openid.net/specs/openid-federation-1_0-39.html#appendix-D
As noted in my issue #11 I think the current phrasing on merging the
essentialpolicy operator is not clear enough.This is my proposal on how to simplify the phrasing, as I think the semantic should be.