[api/mixins] Fix filtering of unrelated fields in FilterSerializerByOrganization #319

[Stoiicc]

This pull request fixes the issue described in #319.
The FilterSerializerByOrganization mixin was applying organization-based filtering to every serializer field that exposed a queryset, regardless of whether the related model actually contained an organization relation.This behavior was inconsistent with the expected logic of the mixin and caused unintended filtering in modules where certain related models are not tied to organizations.

This patch ensures that queryset filtering is applied only to fields whose related model actually contains an organization relation.Fields that do not have such a relation are left untouched.The behavior for fields pointing directly to the Organization model remains unchanged, and the existing include_shared logic and filtering rules for non-superusers continue to work as before.

Fixes #319.

[nemesifier]

Thanks @Stoiicc for tackling #319. The underlying goal is valid: the mixin should not apply organization filtering to related fields whose model has no organization relation. But this is a security-critical multitenancy mixin, and the rewrite introduces a pattern I cannot accept as-is.

Blocker: fail-open exception handling

The new code wraps the queryset filtering in broad except Exception: pass:

try:
    field.queryset = queryset.filter(conditions).distinct()
except Exception:
    pass

If anything goes wrong building or applying the filter, this silently leaves the field's queryset unfiltered, i.e. exposing every organization's objects in the browsable API and writable choices. For a mixin whose entire job is org isolation, that is a fail-open data leak. There are several such blocks. Please make filtering fail closed: if the intended filter cannot be applied, the field should end up empty or raise, never wide open. Also replace the broad excepts with the specific exceptions you actually expect.

Other concerns:

1. Auto-detecting org fields is a heuristic. _get_org_related_fields treats any FK to Organization as the filter field and ORs across all of them. A model with more than one Organization relation (or an indirect one) could be filtered on the wrong field. Please confirm this matches the real models in play and add tests for the multi-relation case.
2. Overlap with #444. PR #444 heavily rewrites this same api/mixins.py. The two will conflict and overlap conceptually. Let's sequence them: #444 is the canonical multitenancy/shared-objects work, so I would rebase this on top of (or fold it into) #444 rather than land them independently.
3. This needs comprehensive tests proving org isolation is preserved for every case (the #319 scenario, the org field itself, shared objects, and crucially that no path leaves a field unfiltered).

Right problem, but the fail-open handling is a blocker and it must be coordinated with #444.

[openwisp-companion]

Hi @Stoiicc 👋,

This is a friendly reminder that this pull request has had no activity for 7 days since changes were requested.

We'd love to see this contribution merged! Please take a moment to:

• Address the review feedback
• Push your changes
• Let us know if you have any questions or need clarification

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 7 more days, the linked issue will be unassigned to allow other contributors to work on it.

Thank you for your contribution! 🙏

[openwisp-companion]

Hi @Stoiicc 👋,

This pull request has been marked as stale due to 14 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:

• The issue will be reassigned to you
• Your contribution is still very welcome

If you need more time or have questions about the requested changes, please let us know. We're happy to help! 🤝