Skip to content

Commit

Permalink
Merge pull request #525 from GhostManager/hotfix/replace-blanks-filter
Browse files Browse the repository at this point in the history
Added `replace_blanks` filter for templates
  • Loading branch information
chrismaddalena authored Sep 25, 2024
2 parents cb7f13e + b2d1be3 commit ec0293b
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 14 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

# CHANGELOG

## [4.3.1] – 25 Sep 2024

### Added

* Added a `replace_blanks` filter to the report template engine to replace blank values in a dictionary with a specified string
* This filter is useful when sorting a list of dictionaries with an attribute that may have a blank value
* Added an option in the change search in the findings library to search findings attached to reports (Closes #400)
* Instead of matches from the library, the search will return results for findings attached to reports to which the user has access

### Changed

* Changed the serializer for report context to replace null values with a blank string (`""`) to help prevent errors when generating reports
* **Note:** This change may affect templates that rely on null values to trigger conditional logic, but most conditional statements should not be affected
* **Example:** The condition `{% if not X %}` will still evaluate to `True` if `X` is `None` or `""`
* Changed the report form to allow users with the `admin` or `manager` roles to change the report's project (Closes #368)
* This change allows a report to be moved from one project to another (e.g., you make a copy for a follow-up assessment)
* This feature is only available to users with the `admin` or `manager` roles to prevent accidental data leaks

### Fixed

* Fixed an edge case with the Namecheap sync task that could lead to a domain remaining marked as expired after re-purchasing it or renewing it during the grace period

## [4.3.0] – 23 Sep 2024

### Added
Expand Down
31 changes: 21 additions & 10 deletions ghostwriter/modules/custom_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ def __init__(self, *args, exclude=None, **kwargs):
self.fields.pop(field)
super().__init__(*args, **kwargs)

def to_representation(self, instance):
"""
Override the default method to ensure empty strings are returned for null values. The null values will
cause Jinja2 rendering errors with filters and expressions like `sort()`.
"""
data = super().to_representation(instance)
for key, value in data.items():
try:
if not value:
data[key] = ""
except KeyError:
pass
return data


class OperatorNameField(RelatedField):
"""Customize the string representation of a :model:`users.User` entry."""
Expand Down Expand Up @@ -113,7 +127,7 @@ class ExtraFieldsSerField(serializers.Field):
def __init__(self, model_name, **kwargs):
self.model_name = model_name
self.root_ser = None
kwargs['read_only'] = True
kwargs["read_only"] = True
super().__init__(**kwargs)

def bind(self, field_name, parent):
Expand All @@ -130,7 +144,9 @@ def to_representation(self, value):
if not hasattr(self.root_ser, "_extra_fields_specs") or self.root_ser._extra_fields_specs is None:
self.root_ser._extra_fields_specs = {}
if self.model_name not in self.root_ser._extra_fields_specs:
self.root_ser._extra_fields_specs[self.model_name] = ExtraFieldSpec.objects.filter(target_model=self.model_name)
self.root_ser._extra_fields_specs[self.model_name] = ExtraFieldSpec.objects.filter(
target_model=self.model_name
)

# Populate output
for field in self.root_ser._extra_fields_specs[self.model_name]:
Expand Down Expand Up @@ -514,10 +530,7 @@ class DomainHistorySerializer(CustomModelSerializer):
exclude=["id", "project", "domain"],
)

extra_fields = ExtraFieldsSerField(
Domain._meta.label,
source="domain.extra_fields"
)
extra_fields = ExtraFieldsSerField(Domain._meta.label, source="domain.extra_fields")

class Meta:
model = History
Expand Down Expand Up @@ -567,10 +580,7 @@ class ServerHistorySerializer(CustomModelSerializer):
exclude=["id", "project", "static_server", "transient_server"],
)

extra_fields = ExtraFieldsSerField(
StaticServer._meta.label,
source="server.extra_fields"
)
extra_fields = ExtraFieldsSerField(StaticServer._meta.label, source="server.extra_fields")

class Meta:
model = ServerHistory
Expand Down Expand Up @@ -756,6 +766,7 @@ class Meta:

class FullProjectSerializer(serializers.Serializer):
"""Serialize :model:`rolodex:Project` and related entries."""

project = ProjectSerializer(source="*")
client = ClientSerializer()
contacts = ProjectContactSerializer(source="projectcontact_set", many=True, exclude=["id", "project"])
Expand Down
2 changes: 2 additions & 0 deletions ghostwriter/modules/reportwriter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def __iter__(self):
def __bool__(self):
self._record()
return super().__bool__()

undefined = RecordUndefined
else:
undefined = jinja2.make_logging_undefined(logger=logger, base=jinja2.Undefined)
Expand All @@ -55,6 +56,7 @@ def __bool__(self):
env.filters["get_item"] = jinja_funcs.get_item
env.filters["regex_search"] = jinja_funcs.regex_search
env.filters["filter_tags"] = jinja_funcs.filter_tags
env.filters["replace_blanks"] = jinja_funcs.replace_blanks

if debug:
return env, undefined_vars
Expand Down
23 changes: 23 additions & 0 deletions ghostwriter/modules/reportwriter/jinja_funcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,26 @@ def mk_evidence(context: jinja2.runtime.Context, evidence_name: str) -> Markup:

def raw_mk_evidence(evidence_id) -> Markup:
return Markup('<span data-gw-evidence="' + html.escape(str(evidence_id)) + '"></span>')


def replace_blanks(list_of_dicts, placeholder=""):
"""
Replace blank strings in a dictionary with a placeholder string.
**Parameters**
``dict``
Dictionary to replace blanks in
"""

try:
for d in list_of_dicts:
for key, value in d.items():
if value is None:
d[key] = placeholder
except (AttributeError, TypeError) as e:
logger.exception("Error parsing ``list_of_dicts`` as a list of dictionaries: %s", list_of_dicts)
raise InvalidFilterValue(
"Invalid list of dictionaries passed into `replace_blanks()` filter; must be a list of dictionaries"
) from e
return list_of_dicts
14 changes: 14 additions & 0 deletions ghostwriter/reporting/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
get_item,
regex_search,
strip_html,
replace_blanks,
)
from ghostwriter.reporting.templatetags import report_tags

Expand Down Expand Up @@ -2508,6 +2509,19 @@ def test_filter_tags_with_invalid_dict(self):
with self.assertRaises(InvalidFilterValue):
filter_tags(findings, ["xss", "T1659"])

def test_replace_blanks(self):
example = [
{"example": "This is a test"},
{"example": None},
{"example": "This is another test"},
]
res = replace_blanks(example, "BLANK")
self.assertEqual(
res, [{"example": "This is a test"}, {"example": "BLANK"}, {"example": "This is another test"}]
)
with self.assertRaises(InvalidFilterValue):
replace_blanks("Not a list", "BLANK")


class LocalFindingNoteUpdateTests(TestCase):
"""Collection of tests for :view:`reporting.LocalFindingNoteUpdate`."""
Expand Down
2 changes: 1 addition & 1 deletion ghostwriter/shepherd/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def __init__(self, *args, **kwargs):
for field in self.fields:
self.fields[field].widget.attrs["autocomplete"] = "off"
self.fields["name"].widget.attrs["placeholder"] = "ghostwriter.wiki"
self.fields["registrar"].widget.attrs["placeholder"] = "NameCheap"
self.fields["registrar"].widget.attrs["placeholder"] = "Namecheap"
self.fields["domain_status"].empty_label = "-- Select Status --"
self.fields["whois_status"].empty_label = "-- Select Status --"
self.fields["health_status"].empty_label = "-- Select Status --"
Expand Down
17 changes: 17 additions & 0 deletions ghostwriter/shepherd/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ def fetch_namecheap_domains():
domain_queryset = Domain.objects.filter(registrar="Namecheap")
expired_status = DomainStatus.objects.get(domain_status="Expired")
burned_status = DomainStatus.objects.get(domain_status="Burned")
available_status = DomainStatus.objects.get(domain_status="Available")
health_burned_status = HealthStatus.objects.get(health_status="Burned")
for domain in domain_queryset:
# Check if a domain in the library is _not_ in the Namecheap response
Expand Down Expand Up @@ -799,6 +800,18 @@ def fetch_namecheap_domains():
domain=domain,
note="Automatically set to Expired because the domain did not appear in Namecheap during a sync.",
)
# Catch domains that were marked as expired but are now back in the Namecheap data
else:
if domain.expired:
logger.info("Domain %s is marked as expired but is now back in the Namecheap data", domain.name)
domain_changes["updates"][domain.id] = {}
domain_changes["updates"][domain.id]["domain"] = domain.name
domain_changes["updates"][domain.id]["change"] = "renewed"
domain.expired = False
if domain.domain_status == expired_status:
domain.domain_status = available_status
domain.save()

# Now, loop over every domain returned by Namecheap
for domain in domains_list:
logger.info("Domain %s is now being processed", domain["Name"])
Expand Down Expand Up @@ -840,8 +853,12 @@ def fetch_namecheap_domains():
] = "<p>Namecheap has locked the domain. This is usually the result of a legal complaint related to phishing/malicious activities.</p>"

# Set AutoRenew status
# Ignore Namecheap's `AutoRenew` value if the domain is expired (both can be true)
if domain["AutoRenew"] == "false" or domain["IsExpired"] == "true":
entry["auto_renew"] = False
# Ensure the domain's auto-renew status in the database matches Namecheap
elif domain["AutoRenew"] == "true":
entry["auto_renew"] = True

# Convert Namecheap dates to Django
entry["creation"] = datetime.strptime(domain["Created"], "%m/%d/%Y").strftime("%Y-%m-%d")
Expand Down
6 changes: 3 additions & 3 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ djangorestframework-api-key==2.2.0
django-tinymce==3.4.0 # Deprecated, but kept here for legacy migrations
docutils==0.18.1
docxtpl==0.12.0
dnspython==2.2.0
jinja2==3.1.3
dnspython==2.6.1
jinja2==3.1.4
python-docx==0.8.11
python-nmap==0.7.1
python-pptx==0.6.21
requests==2.31.0
requests==2.32.0
django-timezone-field==4.2.3
pyjwt==2.7.0
psutil==5.9.4
Expand Down

0 comments on commit ec0293b

Please sign in to comment.