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

Metadata API: enforce role name uniqueness in delegations #1426

Closed
jku opened this issue May 31, 2021 · 4 comments · Fixed by #1537
Closed

Metadata API: enforce role name uniqueness in delegations #1426

jku opened this issue May 31, 2021 · 4 comments · Fixed by #1537
Assignees
Labels
backlog Issues to address with priority for current development goals
Milestone

Comments

@jku
Copy link
Member

jku commented May 31, 2021

The spec does not say anything about role name uniqueness in a delegations object but I believe we cannot safely allow multiple roles with same role name in the roles array of a delegations object. If we did then the roles could have different keyids, and then we would end up in a situation where a metadata may be both a valid delegation and an invalid delegation at the same time, depending on how the role gets chosen and that does not seem like the intention of the design.

I don't think there are real uses cases for non-unique role names in delegations either -- and I assume that this situation is just an unintentional side-effect of making roles an ordered list -- so the spec should possibly clarify that uniqueness is required (although I guess theoretically some complex ordering issue could be solved by multiple roles per role name).

Regardless, metadata API should IMO enforce role name uniqueness on Delegations.roles unless a good case is made against that.

@jku
Copy link
Member Author

jku commented May 31, 2021

Potentially we could use roles: OrderedDict[str, DelegatedRole] to store the roles (and raise in from_dict() if input contains duplicate role names)?

As trishank mentioned on slack: from python 3.7 onwards Dict itself guarantees insert order so is deterministic -- for our uses OrderedDict still makes sense (and also informs the reader that the order matters)

@MVrachev
Copy link
Collaborator

I would suggest discussing and solving this issue in the spec repo first so that we make sure we are on the same page with the spec.
If there is a use case where role name duplicates are needed (which I doubt) then I hope they will be shared there.

Otherwise, a good catch, and I agree with the sentiment that from my perspective I see only problems in allowing duplicate role names in delegations.

@sechkova
Copy link
Contributor

sechkova commented Jun 7, 2021

I don't think there are real uses cases for non-unique role names in delegations either -- and I assume that this situation is just an unintentional side-effect of making roles an ordered list -- so the spec should possibly clarify that uniqueness is required (although I guess theoretically some complex ordering issue could be solved by multiple roles per role name).

The same is implied by the spec itself it just needs to be written explicitly I guess:

In order to accommodate prioritized delegations, the "roles" key in the DELEGATIONS object above points to an array of delegated roles, rather than to a hash table.

@MVrachev
Copy link
Collaborator

I want to work on this and will assign myself.
If somebody else is already working on it, please let me know.

@MVrachev MVrachev self-assigned this Aug 23, 2021
@sechkova sechkova added this to the Sprint 7 milestone Sep 1, 2021
@sechkova sechkova modified the milestones: Sprint 7, Sprint 8 Sep 15, 2021
@jku jku closed this as completed in #1537 Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants