Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust and simplify policy operator combinations #177

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vdzhuvinov
Copy link
Collaborator

@vdzhuvinov vdzhuvinov commented Jan 15, 2025

Addresses issues #11 , #129 , #180.

The chief aim of this PR is to make it easier for architects to devise metadata policies in federations with multiple Trust Anchors or federations with complex topologies. In a single-anchored federation the current, limited operator combinations were okay, because one can simply lookup the policies of the Superior(s) and tweak the local policy where necessary. When dealing with multiple Trust Anchors the limited combinations become a problem. This PR fixes that. It also fixes the value + essential combination, which current spec may lead to policy conflict (#180).

This PR incorporates the contributions of @zachmann from PRs #111 and #112 (thanks!), with slight edits , plus several additional combination changes.

The proposed combinations were implemented in the Nimbus OIDC / OAuth SDK and were tested, including tests against a suite of several thousand generated test vectors: https://connect2id.com/blog/metadata-policy-test-vectors-openid-federation

The proposed combinations changes + fix, as a table:

image

The combinations in draft 41, for comparison. Notice that the updated version has more green or yellow squares for policies to "land", as it covers all logical combinations.

image

This PR also tightens the language in a few places.

@zachmann
Copy link
Collaborator

zachmann commented Jan 16, 2025

There is an error in the image: The "new" table states the check for value + add is The value of "value" MUST be a subset of the values "add". It should be the other way round: The value of "add" MUST be a subset of the values of "value".

The spec text is fine.

@vdzhuvinov
Copy link
Collaborator Author

The "new" table states the check for value + add is The value of "value" MUST be a subset of the values "add". It should be the other way round: The value of "add" MUST be a subset of the values of "value".

@zachmann Thanks for spotting this, I fixed the table.

@vdzhuvinov
Copy link
Collaborator Author

Please, ignore this PR for the time being. I had a chat with Roland and we also want to investigate an alternative change where in combinations of the value operator it always takes precedence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants