[admin/ipam] Add export all subnets CSV action #104#203
Conversation
Added an export button to the subnet admin page to export all subnets in CSV format . The export is ordered by creation time, optimized with select_related and covered by tests verifying multi- subnet CSV output. Fixes openwisp#104
nemesifier
left a comment
There was a problem hiding this comment.
Thanks @Stoiicc, the feature is welcome, but there is a multi-tenancy blocker that must be fixed before this can merge, plus some cleanup.
Blocker: it exports every organization's subnets
export_all_view uses Subnet.objects.all() and is gated on change_subnet, but it does not filter by the requesting user's organizations. A non-superuser operator who has change_subnet in their own org could export the subnets (and IP addresses) of every organization in the system. That is a cross-tenant data leak. Please restrict the export to the organizations the user manages, using the same org-scoped queryset the admin already applies via the multitenant mixin. Superusers should still see all.
Cleanup:
- Dead code. The
fields = [IpAddress._meta.get_field("ip_address"), ...]list is built but never used. Please remove it. - Lint. There is trailing whitespace on
writer.writerow([])and on the blank line afterreturn render(...). That will fail flake8. - Permission. Export is a read operation;
view_subnetis arguably more appropriate thanchange_subnet. Please consider which permission should gate it. - Format. Concatenating per-subnet CSV blocks separated by empty rows produces an unusual file. Confirm this is the intended format; a single consistent header + rows is usually friendlier for spreadsheet import.
Please fix the org scoping (the blocker), clean up the dead code and lint, and add a test asserting that an operator cannot export another org's subnets.
|
Hi @Stoiicc 👋, This is a friendly reminder that this pull request has had no activity for 8 days since changes were requested. We'd love to see this contribution merged! Please take a moment to:
If you're busy or need more time, no worries! Just leave a comment to let us know you're still working on it. Note: within 6 more days, the linked issue will be unassigned to allow other contributors to work on it. Thank you for your contribution! 🙏 |
|
Hi @Stoiicc 👋, This pull request has been marked as stale due to 15 days of inactivity after changes were requested. As a result, any linked issues are being unassigned from you so other contributors can pick them up. However, you can still continue working on this PR! If you push new commits or respond to the review feedback:
If you need more time or have questions about the requested changes, please let us know. We're happy to help! 🤝 |
Checklist
Reference to Existing Issue
Closes #104 .
Description of Changes
Please describe these changes.
This PR adds an Export Subnets action to the Subnet admin page, allowing administrators to export all subnets in CSV format.
The exported CSV uses the same structure as the existing import template, ensuring consistency between import and export workflows.
Ensured export order by sorting subnets by creation time.