Fix issue when deserializing GithubIssueTemplate with hidden secret (#1008)

Author: chkeita

src/api-service/tests/test_secrets.py

@@ -14,6 +14,9 @@
     JobTemplateNotification,
 )
 from onefuzztypes.models import (
+    ADOTemplate,
+    GithubAuth,
+    GithubIssueTemplate,
     JobConfig,
     Notification,
     NotificationConfig,
@@ -27,13 +30,14 @@
 from __app__.onefuzzlib.orm import hide_secrets
 
 
+def hider(secret_data: SecretData) -> SecretData:
+    if not isinstance(secret_data.secret, SecretAddress):
+        secret_data.secret = SecretAddress(url="blah blah")
+    return secret_data
+
+
 class TestSecret(unittest.TestCase):
     def test_hide(self) -> None:
-        def hider(secret_data: SecretData) -> SecretData:
-            if not isinstance(secret_data.secret, SecretAddress):
-                secret_data.secret = SecretAddress(url="blah blah")
-            return secret_data
-
         notification = Notification(
             container=Container("data"),
             config=TeamsTemplate(url=SecretData(secret="http://test")),
@@ -47,11 +51,6 @@ def hider(secret_data: SecretData) -> SecretData:
             self.fail(f"Invalid config type {type(notification.config)}")
 
     def test_hide_nested_list(self) -> None:
-        def hider(secret_data: SecretData) -> SecretData:
-            if not isinstance(secret_data.secret, SecretAddress):
-                secret_data.secret = SecretAddress(url="blah blah")
-            return secret_data
-
         job_template_index = JobTemplateIndex(
             name="test",
             template=JobTemplate(
@@ -100,12 +99,33 @@ def test_roundtrip_github_issue(self) -> None:
             f"{current_path}"
             + "/../../../contrib/onefuzz-job-github-actions/github-issues.json"
         ) as json_file:
-            b = json.load(json_file)
-            b["container"] = "testing"
-            c = NotificationCreate.parse_obj(b)
-            d = c.json()
-            e = json.loads(d)
-            NotificationCreate.parse_obj(e)
+            notification_dict = json.load(json_file)
+            notification_dict["container"] = "testing"
+            notification1 = NotificationCreate.parse_obj(notification_dict)
+
+            assert isinstance(notification1.config, GithubIssueTemplate)
+            self.assertIsInstance(
+                notification1.config.auth.secret, GithubAuth, "Invalid secret type"
+            )
+
+            notification2 = NotificationCreate.parse_obj(
+                json.loads(notification1.json())
+            )
+
+            assert isinstance(notification2.config, GithubIssueTemplate)
+            self.assertIsInstance(
+                notification2.config.auth.secret, GithubAuth, "Invalid secret type"
+            )
+
+            hide_secrets(notification2, hider)
+
+            notification3 = NotificationCreate.parse_obj(
+                json.loads(notification2.json())
+            )
+            assert isinstance(notification3.config, GithubIssueTemplate)
+            self.assertIsInstance(
+                notification3.config.auth.secret, SecretAddress, "Invalid secret type"
+            )
 
     def test_roundtrip_team_issue(self) -> None:
         a = """
@@ -115,21 +135,59 @@ def test_roundtrip_team_issue(self) -> None:
                 }
 
         """  # noqa
-        b = json.loads(a)
-        c = NotificationCreate.parse_obj(b)
-        d = c.json()
-        e = json.loads(d)
-        NotificationCreate.parse_obj(e)
+        notification_dict = json.loads(a)
+        notification_dict["container"] = "testing"
+        notification1 = NotificationCreate.parse_obj(notification_dict)
+
+        assert isinstance(notification1.config, TeamsTemplate)
+        self.assertIsInstance(
+            notification1.config.url.secret, str, "Invalid secret type"
+        )
+
+        notification2 = NotificationCreate.parse_obj(json.loads(notification1.json()))
+        assert isinstance(notification2.config, TeamsTemplate)
+        self.assertIsInstance(
+            notification2.config.url.secret, str, "Invalid secret type"
+        )
+
+        hide_secrets(notification2, hider)
+
+        notification3 = NotificationCreate.parse_obj(json.loads(notification2.json()))
+        assert isinstance(notification3.config, TeamsTemplate)
+        self.assertIsInstance(
+            notification3.config.url.secret, SecretAddress, "Invalid secret type"
+        )
 
     def test_roundtrip_ado(self) -> None:
         current_path = pathlib.Path(__file__).parent.absolute()
         with open(
             f"{current_path}"
             + "/../../../contrib/onefuzz-job-azure-devops-pipeline/ado-work-items.json"  # noqa
         ) as json_file:
-            b = json.load(json_file)
-            b["container"] = "testing"
-            c = NotificationCreate.parse_obj(b)
-            d = c.json()
-            e = json.loads(d)
-            NotificationCreate.parse_obj(e)
+            notification_dict = json.load(json_file)
+            notification_dict["container"] = "testing"
+            notification1 = NotificationCreate.parse_obj(notification_dict)
+            assert isinstance(notification1.config, ADOTemplate)
+            self.assertIsInstance(
+                notification1.config.auth_token.secret, str, "Invalid secret type"
+            )
+
+            notification2 = NotificationCreate.parse_obj(
+                json.loads(notification1.json())
+            )
+            assert isinstance(notification2.config, ADOTemplate)
+            self.assertIsInstance(
+                notification2.config.auth_token.secret, str, "Invalid secret type"
+            )
+
+            hide_secrets(notification2, hider)
+
+            notification3 = NotificationCreate.parse_obj(
+                json.loads(notification2.json())
+            )
+            assert isinstance(notification3.config, ADOTemplate)
+            self.assertIsInstance(
+                notification3.config.auth_token.secret,
+                SecretAddress,
+                "Invalid secret type",
+            )

src/pytypes/onefuzztypes/models.py

@@ -511,15 +511,26 @@ class GithubIssueTemplate(BaseModel):
     # validator needed for backward compatibility
     @validator("auth", pre=True, always=True)
     def validate_auth(cls, v: Any) -> SecretData:
+        def try_parse_GithubAuth(x: dict) -> Optional[GithubAuth]:
+            try:
+                return GithubAuth.parse_obj(x)
+            except Exception:
+                return None
+
         if isinstance(v, GithubAuth):
             return SecretData(secret=v)
         elif isinstance(v, SecretData):
             return v
         elif isinstance(v, dict):
-            try:
-                return SecretData(GithubAuth.parse_obj(v))
-            except Exception:
-                return SecretData(GithubAuth.parse_obj(v["secret"]))
+            githubAuth = try_parse_GithubAuth(v)
+            if githubAuth:
+                return SecretData(secret=githubAuth)
+
+            githubAuth = try_parse_GithubAuth(v["secret"])
+            if githubAuth:
+                return SecretData(secret=githubAuth)
+
+            return SecretData(secret=v["secret"])
         else:
             raise TypeError(f"invalid datatype {type(v)}")
 

src/pytypes/tests/test_models.py

@@ -6,8 +6,8 @@
 import unittest
 
 from onefuzztypes.models import Scaleset, SecretData, TeamsTemplate
-from onefuzztypes.requests import NotificationCreate
 from onefuzztypes.primitives import PoolName, Region
+from onefuzztypes.requests import NotificationCreate
 from pydantic import ValidationError