Take order into account for certain cases

After we have dropped OrderedDict in e3b267e
we are relying on python3.7+ default behavior to preserve the insertion
order, but there is one caveat.
When comparing dictionaries the order is still irrelevant compared to
OrderedDict. For example:
>>> OrderedDict([(1,1), (2,2)]) == OrderedDict([(2,2), (1,1)])
False
>>> dict([(1,1), (2,2)]) == dict([(2,2), (1,1)])
True

There are two special attributes, defined in the specification, where
the order makes a difference when comparing two objects:
- Metadata.signatures
- Targets.delegations.roles.
We want to make sure that the order in those two cases makes a
difference when comparing two objects and that's why those changes
are required inside two __eq__ implementations.

Signed-off-by: Martin Vrachev <[email protected]>

Author: MVrachev

tests/test_metadata_eq_.py

@@ -10,9 +10,12 @@
 import os
 import sys
 import unittest
-from typing import Any, ClassVar, Dict
+from typing import Any, ClassVar, Dict, List
+
+from securesystemslib.signer import SSlibSigner
 
 from tests import utils
+from tests.repository_simulator import RepositorySimulator
 from tuf.api.metadata import (
     TOP_LEVEL_ROLE_NAMES,
     DelegatedRole,
@@ -60,6 +63,35 @@ def test_metadata_eq_(self, test_case_data: Any) -> None:
         setattr(md_2, self.case_name, test_case_data)
         self.assertNotEqual(md, md_2)
 
+    def test_md_eq_signatures_reversed_order(self) -> None:
+        # Test comparing objects with same signatures but different order.
+
+        # Remove all signatures and create new ones.
+        md = Metadata.from_bytes(self.metadata["snapshot"])
+        md.signatures = {}
+        md_2 = copy.deepcopy(md)
+        signatures: List[SSlibSigner] = []
+        # Assign the new signatures to the md object in the straight order.
+        for i in ["0", "1", "2"]:
+            _, signer = RepositorySimulator.create_key()
+            signature = signer.sign(md.to_bytes())
+            signatures.append(signature)
+            md.signatures[i] = signature
+
+        # Assign the new signatures in a reverse order for the md_2 object.
+        for j in [2, 1, 0]:
+            md_2.signatures[str(j)] = signatures[j]
+
+        # Assert that both objects are not the same because of signatures order.
+        self.assertNotEqual(md, md_2)
+
+        # but if we fix the signatures order they will be equal
+        md_2.signatures = {}
+        for j in [0, 1, 2]:
+            md_2.signatures[str(j)] = signatures[j]
+
+        self.assertEqual(md, md_2)
+
     def test_md_eq_special_signatures_tests(self) -> None:
         # Test that metadata objects with different signatures are not equal.
         md = Metadata.from_bytes(self.metadata["snapshot"])
@@ -284,6 +316,54 @@ def test_delegations_eq_(self, test_case_data: Any) -> None:
         setattr(delegations_2, self.case_name, test_case_data)
         self.assertNotEqual(delegations, delegations_2)
 
+    def test_delegations_eq_roles_reversed_order(self) -> None:
+        # Test comparing objects with same delegated roles but different order.
+        role_one_dict = {
+            "keyids": ["keyid1"],
+            "name": "a",
+            "terminating": False,
+            "paths": ["fn1"],
+            "threshold": 1,
+        }
+        role_two_dict = {
+            "keyids": ["keyid2"],
+            "name": "b",
+            "terminating": True,
+            "paths": ["fn2"],
+            "threshold": 4,
+        }
+        role_one = DelegatedRole.from_dict(role_one_dict)
+        role_two = DelegatedRole.from_dict(role_two_dict)
+
+        delegations_dict = {
+            "keys": {
+                "keyid2": {
+                    "keytype": "ed25519",
+                    "scheme": "ed25519",
+                    "keyval": {"public": "bar"},
+                }
+            },
+            "roles": [],
+        }
+        delegations = Delegations.from_dict(copy.deepcopy(delegations_dict))
+        delegations.roles["a"] = role_one
+        delegations.roles["b"] = role_two
+
+        # Create a second delegations obj with reversed roles order
+        delegations_2 = Delegations.from_dict(delegations_dict)
+        delegations_2.roles["b"] = copy.deepcopy(role_two)
+        delegations_2.roles["a"] = copy.deepcopy(role_one)
+
+        # Both objects are not the equal because of delegated roles order.
+        self.assertNotEqual(delegations, delegations_2)
+
+        # but if we fix the delegated roles order they will be equal
+        delegations_2.roles = {}
+        delegations_2.roles["a"] = copy.deepcopy(role_one)
+        delegations_2.roles["b"] = copy.deepcopy(role_two)
+
+        self.assertEqual(delegations, delegations_2)
+
     # Common attributes between TargetFile and MetaFile doesn't need testing.
     targetfile_attributes_modifications: utils.DataSet = {
         "path": "",

tuf/api/metadata.py

@@ -131,6 +131,15 @@ def __eq__(self, other: Any) -> bool:
         if not isinstance(other, Metadata):
             return False
 
+        # Check that the order of the signatures is the same as dictionaries
+        # are insensitive to order in comparisons (see issue #1788).
+        if (
+            isinstance(self.signatures, dict)
+            and isinstance(other.signatures, dict)
+            and list(self.signatures.keys()) != list(other.signatures.keys())
+        ):
+            return False
+
         return (
             self.signatures == other.signatures
             and self.signed == other.signed
@@ -1428,6 +1437,15 @@ def __eq__(self, other: Any) -> bool:
         if not isinstance(other, Delegations):
             return False
 
+        # Check that the order of the roles is the same as dictionaries
+        # are insensitive to order in comparisons (see issue #1788).
+        if (
+            isinstance(self.roles, dict)
+            and isinstance(other.roles, dict)
+            and list(self.roles.keys()) != list(other.roles.keys())
+        ):
+            return False
+
         return (
             self.keys == other.keys
             and self.roles == other.roles