Bulk upload: auto_renew - Bug

[lez0sec]

The current CSV template doesn't include the auto_renew field, despite it being present in the code.
Also, if you include it on purpose, it is not parsed properly (it seems the logic is flawed)

Here's a diff that works for me if you want patch the code:

diff --git a/ghostwriter/shepherd/views.py b/ghostwriter/shepherd/views.py
index 16777ef..1926deb 100644
--- a/ghostwriter/shepherd/views.py
+++ b/ghostwriter/shepherd/views.py
@@ -489,9 +489,11 @@ def import_domains(request):
                 domain_status = DomainStatus.objects.get(
                     domain_status='Available')
                 entry['domain_status'] = domain_status
-            # Accept any auto_renew value (True, X, Yes, ...) to mean True
-            if 'auto_renew' in entry:
+            # Accept any auto_renew value (True, Enabled, Yes, ...) to mean True
+            if any(yes_option in entry['auto_renew'].lower() for yes_option in ["yes", "enabled", "true"]):
                 entry['auto_renew'] = True
+            else:
+                entry['auto_renew'] = False
             # The last_used_by field will only be set by Shepherd at check-out
             if 'last_used_by' in entry:
                 entry['last_used_by'] = None

[chrismaddalena]

@lez0sec Thank you for reporting this! Sorry it has taken me so long to acknowledge this. The world had other plans for my priorities over these past couple of months, but a big update will be coming soon 🙂

Your observation is correct; the logic does not work as intended. The intention was for any entry for auto_renew to be interpreted as True. That's flawed on its own because a No/Disabled/False/etc entry would be treated as "True," but I had meant to revisit that later. The logic was incorrect because it checked if auto_renew was in the row, which it always would be if you had the column in your csv file. It was not checking if it was blank or not after that.

I also did some additional testing with various inputs that showed untrimmed spaces caused errors as well. The csv entries are now stripped before any value checks.

I am using your suggested solution. I slightly expanded it to include a pair of other potential options. This will be in the next update I am bringing in as a PR shortly:

            # Accept a variety of "True" values to mean True
            # Thanks to @lez0sec for fixing this logic:
            #   https://github.com/GhostManager/Ghostwriter/issues/73
            if 'auto_renew' in entry:
                if any(yes_option in entry['auto_renew'].lower().strip().strip() for yes_option in ['yes', 'enabled', 'true', 'x', 'enable']):
                    entry['auto_renew'] = True
                else:
                    entry['auto_renew'] = False