Merge pull request #4542 from StackStorm/fix/st2-apikey-load-dup

Fix CLI 'st2 apikey load' idempotence and API '/api/v1/apikeys' not honoring 'ID' for the new record creation
StackStorm · Feb 12, 2019 · 456b3e0 · 456b3e0
2 parents f45200c + 097fc0d
commit 456b3e0
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 2 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -59,6 +59,9 @@ Fixed
 
   Reported by @johandahlberg (bug fix) #4533 4534
 
+* Fix CLI ``st2 apikey load`` not being idempotent and API endpoint ``/api/v1/apikeys`` not
+  honoring desired ``ID`` for the new record creation. #4542
+
 2.10.0 - December 13, 2018
 --------------------------
 

diff --git a/st2api/tests/unit/controllers/v1/test_auth_api_keys.py b/st2api/tests/unit/controllers/v1/test_auth_api_keys.py
@@ -197,6 +197,7 @@ def test_post_delete_key(self):
 
     def test_post_delete_same_key_hash(self):
         api_key = {
+            'id': '5c5dbb576cb8de06a2d79a4d',
             'user': 'herge',
             'key_hash': 'ABCDE'
         }
@@ -207,8 +208,9 @@ def test_post_delete_same_key_hash(self):
         # drop into the DB since API will be masking this value.
         api_key_db = ApiKey.get_by_id(resp1.json['id'])
 
+        self.assertEqual(resp1.json['id'], api_key['id'], 'PK ID of created API should match.')
         self.assertEqual(api_key_db.key_hash, api_key['key_hash'], 'Key_hash should match.')
-        self.assertEqual(api_key_db.user, api_key['user'], 'Key_hash should match.')
+        self.assertEqual(api_key_db.user, api_key['user'], 'User should match.')
 
         resp = self.app.delete('/v1/apikeys/%s' % resp1.json['id'])
         self.assertEqual(resp.status_int, 204)

diff --git a/st2common/st2common/models/api/auth.py b/st2common/st2common/models/api/auth.py
@@ -143,11 +143,13 @@ def from_model(cls, model, mask_secrets=False):
 
     @classmethod
     def to_model(cls, instance):
+        # If PrimaryKey ID is provided, - we want to work with existing ST2 API key
+        id = getattr(instance, 'id', None)
         user = str(instance.user) if instance.user else None
         key_hash = getattr(instance, 'key_hash', None)
         metadata = getattr(instance, 'metadata', {})
         enabled = bool(getattr(instance, 'enabled', True))
-        model = cls.model(user=user, key_hash=key_hash, metadata=metadata, enabled=enabled)
+        model = cls.model(id=id, user=user, key_hash=key_hash, metadata=metadata, enabled=enabled)
         return model
 
 

diff --git a/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml b/st2tests/st2tests/fixtures/generic/apikeys/apikey1.yaml
@@ -1,4 +1,5 @@
 ---
+id: 58e3f3330c0517062a3fda43
 user: bill
 key_hash: "ec81d4a56f5987b0ae1cff6e152459986e873d6604637fc70d85c0a0daf131b0a830ccd5b6454cc0c95c0ba6e6655933c993325eb3a28bc43af6c1d801a7c1e8"  # 1234
 metadata:

diff --git a/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml b/st2tests/st2tests/fixtures/generic/apikeys/apikey2.yaml
@@ -1,4 +1,5 @@
 ---
+id: 5c5ddd776cb8de530e0a1391
 user: dilbert
 key_hash: "17f858ea0bb108feaa91b8eee524c7382e0218ff541783d45996a1149d50dfde4bc19f2e6a591028a2ea08de4211893b246d4eda61dd3c9cf294a2405184ac4b" # 5678
 metadata: