Skip to content

Commit 44904bb

Browse files
jeremigonzalesedwin1123
authored andcommitted
feat(spp_dci_client): mask OAuth2 client secret in UI
Add a computed display field that masks the stored secret with asterisks and only writes through when a new value is provided. Prevents accidental secret exposure when viewing the data source form. Includes formatting fixes from prettier.
1 parent 535bbd1 commit 44904bb

File tree

3 files changed

+276
-38
lines changed

3 files changed

+276
-38
lines changed

spp_dci_client/models/data_source.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ class DCIDataSource(models.Model):
2222
_description = "DCI Data Source"
2323
_order = "name"
2424

25+
_SECRET_MASK = "********"
26+
2527
name = fields.Char(
2628
required=True,
2729
string="Name",
@@ -70,7 +72,15 @@ class DCIDataSource(models.Model):
7072
oauth2_client_secret = fields.Char(
7173
string="OAuth2 Client Secret",
7274
groups="base.group_system",
73-
help="OAuth2 client secret (visible only to system administrators)",
75+
copy=False,
76+
help="OAuth2 client secret (internal storage, not displayed in UI)",
77+
)
78+
oauth2_client_secret_display = fields.Char(
79+
string="OAuth2 Client Secret",
80+
compute="_compute_oauth2_client_secret_display",
81+
inverse="_inverse_oauth2_client_secret_display",
82+
groups="base.group_system",
83+
help="OAuth2 client secret (write-only for security - value is masked after saving)",
7484
)
7585
oauth2_scope = fields.Char(
7686
string="OAuth2 Scope",
@@ -177,6 +187,19 @@ class DCIDataSource(models.Model):
177187
help="Cached token expiration timestamp (internal use only)",
178188
)
179189

190+
@api.depends("oauth2_client_secret")
191+
def _compute_oauth2_client_secret_display(self):
192+
for record in self:
193+
record.oauth2_client_secret_display = self._SECRET_MASK if record.oauth2_client_secret else False
194+
195+
def _inverse_oauth2_client_secret_display(self):
196+
for record in self:
197+
value = record.oauth2_client_secret_display
198+
if value and value != self._SECRET_MASK:
199+
record.oauth2_client_secret = value
200+
elif not value:
201+
record.oauth2_client_secret = False
202+
180203
@api.constrains("code")
181204
def _check_code_unique(self):
182205
"""Ensure code is unique across all data sources."""

spp_dci_client/tests/test_data_source.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,3 +629,88 @@ def test_default_values(self):
629629
self.assertEqual(ds.search_endpoint, "/registry/sync/search")
630630
self.assertEqual(ds.subscribe_endpoint, "/registry/subscribe")
631631
self.assertEqual(ds.auth_endpoint, "/oauth2/client/token")
632+
633+
def test_secret_display_field_masks_value(self):
634+
"""Test that oauth2_client_secret_display returns masked value"""
635+
ds = self.DataSource.create(
636+
{
637+
"name": "Test CRVS",
638+
"code": "test_crvs_mask",
639+
"base_url": "https://crvs.example.org/api",
640+
"auth_type": "oauth2",
641+
"oauth2_token_url": "https://auth.example.org/token",
642+
"oauth2_client_id": "client123",
643+
"oauth2_client_secret": "secret456",
644+
"our_sender_id": "openspp.example.org",
645+
}
646+
)
647+
# Display field should show mask, stored field should have real value
648+
self.assertEqual(ds.oauth2_client_secret_display, "********")
649+
self.assertEqual(ds.oauth2_client_secret, "secret456")
650+
651+
def test_secret_display_field_empty_when_no_secret(self):
652+
"""Test that oauth2_client_secret_display is empty when no secret is set"""
653+
ds = self.DataSource.create(
654+
{
655+
"name": "Test CRVS",
656+
"code": "test_crvs_empty",
657+
"base_url": "https://crvs.example.org/api",
658+
"auth_type": "none",
659+
}
660+
)
661+
self.assertFalse(ds.oauth2_client_secret_display)
662+
self.assertFalse(ds.oauth2_client_secret)
663+
664+
def test_secret_display_write_updates_stored_field(self):
665+
"""Test that writing a new value through display field updates the stored secret"""
666+
ds = self.DataSource.create(
667+
{
668+
"name": "Test CRVS",
669+
"code": "test_crvs_write",
670+
"base_url": "https://crvs.example.org/api",
671+
"auth_type": "oauth2",
672+
"oauth2_token_url": "https://auth.example.org/token",
673+
"oauth2_client_id": "client123",
674+
"oauth2_client_secret": "old_secret",
675+
"our_sender_id": "openspp.example.org",
676+
}
677+
)
678+
# Write a new secret through the display field
679+
ds.write({"oauth2_client_secret_display": "brand_new_secret"})
680+
self.assertEqual(ds.oauth2_client_secret, "brand_new_secret")
681+
# Invalidate cache to force recomputation of the display field
682+
ds.invalidate_recordset(["oauth2_client_secret_display"])
683+
self.assertEqual(ds.oauth2_client_secret_display, "********")
684+
685+
def test_secret_display_mask_value_does_not_overwrite(self):
686+
"""Test that writing the mask value does not overwrite the real secret"""
687+
ds = self.DataSource.create(
688+
{
689+
"name": "Test CRVS",
690+
"code": "test_crvs_nooverwrite",
691+
"base_url": "https://crvs.example.org/api",
692+
"auth_type": "oauth2",
693+
"oauth2_token_url": "https://auth.example.org/token",
694+
"oauth2_client_id": "client123",
695+
"oauth2_client_secret": "real_secret_value",
696+
"our_sender_id": "openspp.example.org",
697+
}
698+
)
699+
# Writing the mask value should not change the stored secret
700+
ds.write({"oauth2_client_secret_display": "********"})
701+
self.assertEqual(ds.oauth2_client_secret, "real_secret_value")
702+
703+
def test_secret_display_clear_removes_secret(self):
704+
"""Test that clearing the display field removes the stored secret"""
705+
ds = self.DataSource.create(
706+
{
707+
"name": "Test CRVS",
708+
"code": "test_crvs_clear",
709+
"base_url": "https://crvs.example.org/api",
710+
"auth_type": "none",
711+
"oauth2_client_secret": "secret_to_clear",
712+
}
713+
)
714+
# Clear the secret through the display field
715+
ds.write({"oauth2_client_secret_display": False})
716+
self.assertFalse(ds.oauth2_client_secret)

0 commit comments

Comments
 (0)