From 41a6daca75c103b7654edea238554b666e296284 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Sat, 22 May 2021 11:57:09 +0300 Subject: [PATCH] Metadata API: Add id to Key This simplifies life for API users as usually a key needs its identifier: this is already visible in how update() becomes simpler in the API. The downside is that 'from_dict()' now has two arguments (so arguably the name is not great anymore but it still does _mostly_ the same job as other from_dicts). This is an API change, if a minor one. Signed-off-by: Jussi Kukkonen --- tests/test_api.py | 14 +++++++------- tuf/api/metadata.py | 19 +++++++++++-------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 2a2ebc997f..183685c9f3 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -373,18 +373,18 @@ def test_key_class(self): # Testing that the workflow of deserializing and serializing # a key dictionary doesn't change the content. test_key_dict = key_dict.copy() - key_obj = Key.from_dict(test_key_dict) + key_obj = Key.from_dict("id", test_key_dict) self.assertEqual(key_dict, key_obj.to_dict()) # Test creating an instance without a required attribute. for key in key_dict.keys(): test_key_dict = key_dict.copy() del test_key_dict[key] with self.assertRaises(KeyError): - Key.from_dict(test_key_dict) + Key.from_dict("id", test_key_dict) # Test creating a Key instance with wrong keyval format. key_dict["keyval"] = {} with self.assertRaises(ValueError): - Key.from_dict(key_dict) + Key.from_dict("id", key_dict) def test_role_class(self): @@ -413,7 +413,7 @@ def test_role_class(self): test_role_dict = role_dict.copy() del test_role_dict[role_attr] with self.assertRaises(KeyError): - Key.from_dict(test_role_dict) + Key.from_dict("id", test_role_dict) # Test creating a Role instance with keyid dublicates. # for keyid in role_dict["keyids"]: role_dict["keyids"].append(role_dict["keyids"][0]) @@ -433,7 +433,7 @@ def test_metadata_root(self): keyid = root_key2['keyid'] - key_metadata = Key(root_key2['keytype'], root_key2['scheme'], + key_metadata = Key(keyid, root_key2['keytype'], root_key2['scheme'], root_key2['keyval']) # Assert that root does not contain the new key @@ -441,7 +441,7 @@ def test_metadata_root(self): self.assertNotIn(keyid, root.signed.keys) # Add new root key - root.signed.add_key('root', keyid, key_metadata) + root.signed.add_key('root', key_metadata) # Assert that key is added self.assertIn(keyid, root.signed.roles['root'].keyids) @@ -453,7 +453,7 @@ def test_metadata_root(self): # Try adding the same key again and assert its ignored. pre_add_keyid = root.signed.roles['root'].keyids.copy() - root.signed.add_key('root', keyid, key_metadata) + root.signed.add_key('root', key_metadata) self.assertEqual(pre_add_keyid, root.signed.roles['root'].keyids) # Remove the key diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 034eeceec9..a00ffb9a5f 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -417,6 +417,7 @@ class Key: """A container class representing the public portion of a Key. Attributes: + keyid: An identifier string keytype: A string denoting a public key signature system, such as "rsa", "ed25519", and "ecdsa-sha2-nistp256". scheme: A string denoting a corresponding signature scheme. For example: @@ -428,6 +429,7 @@ class Key: def __init__( self, + keyid: str, keytype: str, scheme: str, keyval: Dict[str, str], @@ -435,19 +437,20 @@ def __init__( ) -> None: if not keyval.get("public"): raise ValueError("keyval doesn't follow the specification format!") + self.keyid = keyid self.keytype = keytype self.scheme = scheme self.keyval = keyval self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {} @classmethod - def from_dict(cls, key_dict: Dict[str, Any]) -> "Key": + def from_dict(cls, keyid: str, key_dict: Dict[str, Any]) -> "Key": """Creates Key object from its dict representation.""" keytype = key_dict.pop("keytype") scheme = key_dict.pop("scheme") keyval = key_dict.pop("keyval") # All fields left in the key_dict are unrecognized. - return cls(keytype, scheme, keyval, key_dict) + return cls(keyid, keytype, scheme, keyval, key_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dictionary representation of self.""" @@ -558,7 +561,7 @@ def from_dict(cls, root_dict: Dict[str, Any]) -> "Root": roles = root_dict.pop("roles") for keyid, key_dict in keys.items(): - keys[keyid] = Key.from_dict(key_dict) + keys[keyid] = Key.from_dict(keyid, key_dict) for role_name, role_dict in roles.items(): roles[role_name] = Role.from_dict(role_dict) @@ -584,10 +587,10 @@ def to_dict(self) -> Dict[str, Any]: return root_dict # Update key for a role. - def add_key(self, role: str, keyid: str, key_metadata: Key) -> None: - """Adds new key for 'role' and updates the key store.""" - self.roles[role].keyids.add(keyid) - self.keys[keyid] = key_metadata + def add_key(self, role: str, key: Key) -> None: + """Adds new signing key for delegated role 'role'.""" + self.roles[role].keyids.add(key.keyid) + self.keys[key.keyid] = key def remove_key(self, role: str, keyid: str) -> None: """Removes key from 'role' and updates the key store. @@ -863,7 +866,7 @@ def from_dict(cls, delegations_dict: Dict[str, Any]) -> "Delegations": keys = delegations_dict.pop("keys") keys_res = {} for keyid, key_dict in keys.items(): - keys_res[keyid] = Key.from_dict(key_dict) + keys_res[keyid] = Key.from_dict(keyid, key_dict) roles = delegations_dict.pop("roles") roles_res = [] for role_dict in roles: