[Federation] Policy language: disambiguation about subset_of and essential operators

[Pasquale Barbaro]

from https://openid.net/specs/openid-connect-federation-1_0.html, section 5.1.2:

The specs for subset_of operator say: "If the metadata parameter has no value and essential is set to false, then the subset_of check MUST be ignored."
And then right after it says:
"Otherwise, this operator MUST compute the intersection of the values in the directive and the values of parameter. If the resulting intersection is non-empty the parameter MUST be set to the values of the intersection"

The word “Otherwise” at the beginning of the sentence makes me understand that in this case the essential is true ( because in the previous sentence essential was false, so “Otherwise” must mean the opposite) and so it almost seems that the intersection must be computed only in this case (when essential is true), but the intersection is always computed as shown in the examples in the Table 1

[Giuseppe De Marco]

Ciao Pasquale, thank you for this issue, we’ll find with your help if any clarification is needed.

You said the intersection is always computed as shown in the examples in the Table 1 and that’s true since the value of the metadata is present, as [a,e]
to [a], since If the metadata parameter has no value and essential is set to false, then the subset_of check MUST be ignored and in that case the metadata has value so it was not ignored.

Is there something I'm missing or any proposal by your side to improve the text thank to your proofreading experience?

[Pasquale Barbaro]

Thanks for your reply, Giuseppe. You’re right, the specs are clear about that, for some reason I missed the part “… and has no value …” while reading, and that is the important part, because if it has no value and essential is false, the check must indeed be ignored.

However, I don’t find the part with “Otherwise …” to be immediately easy to understand. After a hurried reading, it almost seems to me that there is only a case in wich the policy check must be performed, while there are three of them.

Therefore, I suggest to put a little table to clarify when the check must be performed or not, based on essential being true or false AND the metadata presence, maybe it could be something like this:

‌

[Michael Jones]

We discussed this on the 26-Jun-23 Connect call. We will discuss possibly adding the requested table during Friday's Federation editors' call.

[Giuseppe De Marco]

@Pasquale what about adding the column check to the existing Table 1 for the resolution of this issue?

[Pasquale Barbaro]

@Giuseppe De Marco I don’t know... One one hand the specs would keep one table instead of two, but on the other hand, I think that maybe adding a column on Table 1 would make it less clear because as of now it only have cols which map to data while check column would be a behaviour instead of a data. Furthermore the current Table 1 has 7 example rows, and check would be set to “yes” only for the last row (essential false and input null or missing), so maybe it would be a little redundant.

[Giuseppe De Marco]

I’m trying to find a solution to avoid to have another table, since as you said the check is valid just for the last row, or differently as the table that you have proposed: where the check must not be done just for the first row.

What about adding this information, as a remark, and additionally to the normative text we have already in the paragraph, in the description text (caption) of the Table 1?

What do you think about a text like the one below, in the Table 1 caption?

Table 1: Map of the outputs produced by combining metadata policies with different input values.  
The subset_of check is ignored where the metadata parameter has no value and 
essential is set to false, as shown in the last row. 

‌

[Pasquale Barbaro]

It seems a good solution to me.
This extra sentence would strenghten what is already written at the beginning of operators one_of / subset_of / superset_of, while keeping everything tidy and compact.

[Giuseppe De Marco]

then Ok!

the PR below resolves this issue
https://bitbucket.org/openid/connect/pull-requests/553

thank you Pasquale

[Giuseppe De Marco]

Resolved by https://bitbucket.org/openid/connect/pull-requests/553