Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] VPN templates conflict #973

Open
nemesifier opened this issue Jan 31, 2025 · 6 comments · May be fixed by #977
Open

[bug] VPN templates conflict #973

nemesifier opened this issue Jan 31, 2025 · 6 comments · May be fixed by #977
Labels

Comments

@nemesifier
Copy link
Member

Describe the bug
Assigning a variation of a OpenVPN client template while unassigning another one causes the resulting configuration to become bugged: the configuration variables of the certificates are not resolved.

Steps To Reproduce
Steps to reproduce the behavior:

  1. Create a VPN-client template using OpenVPN
  2. Create another VPN-client template using the same server, but with slightly different configurations
  3. Assign the template created in step 1, save
  4. Now unassign the template created in step 1 while at the same time assign the template created in step 2, save

Expected behavior
The resulting configuration should work normally.

Screenshots

Image

System Informatioon:

  • OS: Debian bookworm/sid
  • Python Version: 3.10.12
  • Django Version: 4.2
@shwetd19
Copy link

shwetd19 commented Feb 1, 2025

@nemesifier can I work on this ? would love to move forward with it!

@nemesifier
Copy link
Member Author

@shwetd19 sure, you can try! Can you replicate the issue?

@pandafy
Copy link
Member

pandafy commented Feb 5, 2025

@shwetd19 are you still working on this?

@shwetd19
Copy link

shwetd19 commented Feb 5, 2025

Yes I am, we'll raise PR in some time

@pandafy
Copy link
Member

pandafy commented Feb 5, 2025

This bug occurs due to the following code:

if action == 'post_add':
vpn_list = instance.templates.filter(type='vpn').values_list('vpn')
instance.vpnclient_set.exclude(vpn__in=vpn_list).delete()
# when adding or removing specific templates
for template in templates.filter(type='vpn'):
if action == 'post_add':
if vpn_client_model.objects.filter(
config=instance, vpn=template.vpn
).exists():
continue
client = vpn_client_model(
config=instance,
vpn=template.vpn,
auto_cert=template.auto_cert,
)
client.full_clean()
client.save()
elif action == 'post_remove':
for client in instance.vpnclient_set.filter(vpn=template.vpn):
client.delete()

The m2m field sends the following signals after the operation in Step 4:

  1. post_add: Triggered when adding template 2. This creates a new VpnClient object (referred to as VpnClient2) for this configuration, but it also deletes the existing VpnClient object created by template 1. Refer to this line:

if action == 'post_add':
vpn_list = instance.templates.filter(type='vpn').values_list('vpn')
instance.vpnclient_set.exclude(vpn__in=vpn_list).delete()

  1. post_remove: Triggered when removing template 1. This deletes all VpnClients associated with the VPN related to template 1.

elif action == 'post_remove':
for client in instance.vpnclient_set.filter(vpn=template.vpn):
client.delete()

As a result, the configuration ends up with no VpnClient object, causing the VPN configuration variables to remain unresolved.

@shwetd19 shwetd19 linked a pull request Feb 5, 2025 that will close this issue
4 tasks
@shwetd19
Copy link

shwetd19 commented Feb 5, 2025

Hey @nemesifier @pandafy I've made a PR for this can you pls have a look, also I had a question as, Currently the fix focuses on preserving VPN clients when their VPN is still in use by other templates. Do we need additional validation to ensure this doesn't create any inconsistencies with other VPN-related features in OpenWISP?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants