Merge pull request from GHSA-8q8j-7wc4-vjg5

jira: hide passwords in API response and admin portal

Author: madchap

dojo/api.py

@@ -111,6 +111,7 @@ def get_fields(cls, fields=None, excludes=None):
                 res_field = fields.get(django_field.name, None)
                 if res_field:
                     res_field.blank = True
+
         return fields
 
 
@@ -493,6 +494,7 @@ class Meta:
         }
         authentication = DojoApiKeyAuthentication()
         authorization = DjangoAuthorization()
+        excludes = ['password', 'ssh', 'api_key']
         serializer = Serializer(formats=['json'])
 
         @property
@@ -851,6 +853,7 @@ class Meta:
         }
         authentication = DojoApiKeyAuthentication()
         authorization = DjangoAuthorization()
+        excludes = ['password']
         serializer = Serializer(formats=['json'])
 
         @property

dojo/api_v2/serializers.py

@@ -401,6 +401,11 @@ class ToolConfigurationSerializer(serializers.ModelSerializer):
     class Meta:
         model = Tool_Configuration
         fields = '__all__'
+        extra_kwargs = {
+            'password': {'write_only': True},
+            'ssh': {'write_only': True},
+            'api_key': {'write_only': True},
+        }
 
 
 class ToolProductSettingsSerializer(serializers.ModelSerializer):
@@ -531,6 +536,9 @@ class JIRAConfSerializer(serializers.ModelSerializer):
     class Meta:
         model = JIRA_Conf
         fields = '__all__'
+        extra_kwargs = {
+            'password': {'write_only': True},
+        }
 
 
 class JIRASerializer(serializers.ModelSerializer):

dojo/models.py

@@ -918,6 +918,40 @@ def __str__(self):
         return self.name
 
 
+# declare form here as we can't import forms.py due to circular imports not even locally
+class ToolConfigForm_Admin(forms.ModelForm):
+    password = forms.CharField(widget=forms.PasswordInput, required=False)
+    api_key = forms.CharField(widget=forms.PasswordInput, required=False)
+    ssh = forms.CharField(widget=forms.PasswordInput, required=False)
+
+    # django doesn't seem to have an easy way to handle password fields as PasswordInput requires reentry of passwords
+    password_from_db = None
+    ssh_from_db = None
+    api_key_from_db = None
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if self.instance:
+            # keep password from db to use if the user entered no password
+            self.password_from_db = self.instance.password
+            self.ssh_from_db = self.instance.ssh
+            self.api_key = self.instance.api_key
+
+    def clean(self):
+        # self.fields['endpoints'].queryset = Endpoint.objects.all()
+        cleaned_data = super().clean()
+        if not cleaned_data['password'] and not cleaned_data['ssh'] and not cleaned_data['api_key']:
+            cleaned_data['password'] = self.password_from_db
+            cleaned_data['ssh'] = self.ssh_from_db
+            cleaned_data['api_key'] = self.api_key_from_db
+
+        return cleaned_data
+
+
+class Tool_Configuration_Admin(admin.ModelAdmin):
+    form = ToolConfigForm_Admin
+
+
 class Network_Locations(models.Model):
     location = models.CharField(max_length=500, help_text="Location of network testing: Examples: VPN, Internet or Internal.")
 
@@ -2634,6 +2668,33 @@ def get_priority(self, status):
             return 'N/A'
 
 
+# declare form here as we can't import forms.py due to circular imports not even locally
+class JIRAForm_Admin(forms.ModelForm):
+    password = forms.CharField(widget=forms.PasswordInput, required=True)
+
+    # django doesn't seem to have an easy way to handle password fields as PasswordInput requires reentry of passwords
+    password_from_db = None
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if self.instance:
+            # keep password from db to use if the user entered no password
+            self.password_from_db = self.instance.password
+            self.fields['password'].required = False
+
+    def clean(self):
+        # self.fields['endpoints'].queryset = Endpoint.objects.all()
+        cleaned_data = super().clean()
+        if not cleaned_data['password']:
+            cleaned_data['password'] = self.password_from_db
+
+        return cleaned_data
+
+
+class JIRA_Conf_Admin(admin.ModelAdmin):
+    form = JIRAForm_Admin
+
+
 class JIRA_Issue(models.Model):
     jira_id = models.CharField(max_length=200)
     jira_key = models.CharField(max_length=200)
@@ -3481,11 +3542,11 @@ def enable_disable_auditlog(enable=True):
 admin.site.register(IPScan)
 admin.site.register(Alerts)
 admin.site.register(JIRA_Issue)
-admin.site.register(JIRA_Conf)
+admin.site.register(JIRA_Conf, JIRA_Conf_Admin)
 admin.site.register(JIRA_PKey)
 admin.site.register(GITHUB_Conf)
 admin.site.register(GITHUB_PKey)
-admin.site.register(Tool_Configuration)
+admin.site.register(Tool_Configuration, Tool_Configuration_Admin)
 admin.site.register(Tool_Product_Settings)
 admin.site.register(Tool_Type)
 admin.site.register(Cred_User)

dojo/unittests/test_rest_framework.py

@@ -10,8 +10,7 @@
     ScansViewSet, StubFindingsViewSet, TestsViewSet, \
     ToolConfigurationsViewSet, ToolProductSettingsViewSet, ToolTypesViewSet, \
     UsersViewSet, ImportScanView, NoteTypeViewSet, AppAnalysisViewSet, \
-    EndpointStatusViewSet, SonarqubeIssueViewSet, SonarqubeIssueTransitionViewSet, \
-    SonarqubeProductViewSet, NotesViewSet
+    EndpointStatusViewSet, SonarqubeIssueViewSet, NotesViewSet
 from json import dumps
 from django.urls import reverse
 from rest_framework.authtoken.models import Token
@@ -74,6 +73,11 @@ def test_detail(self):
             relative_url = self.url + '%s/' % current_objects['results'][0]['id']
             response = self.client.get(relative_url)
             self.assertEqual(200, response.status_code)
+            # sensitive data must be set to write_only so those are not returned in the response
+            # https://github.com/DefectDojo/django-DefectDojo/security/advisories/GHSA-8q8j-7wc4-vjg5
+            self.assertFalse('password' in response.data)
+            self.assertFalse('ssh' in response.data)
+            self.assertFalse('api_key' in response.data)
 
         @skipIfNotSubclass('DestroyModelMixin')
         def test_delete(self):
@@ -90,9 +94,12 @@ def test_update(self):
                 relative_url, self.update_fields)
             for key, value in self.update_fields.items():
                 # some exception as push_to_jira has been implemented strangely in the update methods in the api
-                if key != 'push_to_jira':
+                if key not in ['push_to_jira', 'ssh', 'password', 'api_key']:
                     self.assertEqual(value, response.data[key])
             self.assertFalse('push_to_jira' in response.data)
+            self.assertFalse('ssh' in response.data)
+            self.assertFalse('password' in response.data)
+            self.assertFalse('api_key' in response.data)
             response = self.client.put(
                 relative_url, self.payload)
             self.assertEqual(200, response.status_code)