Skip to content
This repository has been archived by the owner. It is now read-only.

Added (default enabled) option to name exported VPN config based on V… #872

Closed
wants to merge 2 commits into from
Closed

Added (default enabled) option to name exported VPN config based on V… #872

wants to merge 2 commits into from

Conversation

@funkypenguin
Copy link

@funkypenguin funkypenguin commented May 8, 2015

…PN description

We use the client-export utility to create VPN configs for our users, and the
default naming convention of --- is a little
unfriendly. I modified the web UI and the function which generates the config
to pass a flag (default enabled) which will instead name the VPN

…PN description

We use the client-export utility to create VPN configs for our users, and the
default naming convention of <hostname>-<protocol>-<port>-<user> is a little
unfriendly. I modified the web UI and the function which generates the config
to pass a flag (default enabled) which will instead name the VPN
@@ -44,20 +44,17 @@ $current_openvpn_version_rev = "03";

function openvpn_client_export_install() {
global $current_openvpn_version;
conf_mount_rw();
$tarpath = "/usr/local/pkg/openvpn-client-export-{$current_openvpn_version}.tgz";
$phpfile = "vpn_openvpn_export.php";

I have not looked hard, but why does this line need to be added?
And why do you need to take all this code here out of the $pfs_version tests - maybe so it works for 2.3 and onwards?

Copy link
Author

@funkypenguin funkypenguin May 8, 2015

Err... that's actually a mistake. My bad - github newbie here. I think this is because my version of the file is based on what's currently found on a live pfsense box, and it looks as if the git master branch has a newer version. I'll update my branch with an amendment, which (I believe) should update this pull request.

… master

There are some unexpected diffs produced, which I can't explain. However,
these diffs don't actually change anything (see line 72 vs 73). I think this
may be an artifact of the git diff. I tried vi and atom, with the same results.
@@ -807,7 +816,7 @@ function openvpn_client_export_sharedkey_config($srvid, $useaddr, $proxy, $zipco
}
$conf .= "http-proxy {$proxy['ip']} {$proxy['port']} ";
}
if ($proxy['proxy_type'] == "socks")
if ($proxy['proxy_type'] == "socks")
Copy link
Author

@funkypenguin funkypenguin May 8, 2015

This seems to be caused by atom/vi stripping blank spaces at the end of the lines. Since this actually improves the code (there are obviously no extra spaces on any other lines, else they would have suffered the same treatment), I suggest letting these minor corrections through as part of the pull.

Yes, that should be fine. The pfSense developer style guide is at https://doc.pfsense.org/index.php/Developer_Style_Guide
It specifies no white space at EOL.
And other stuff that is randomly used in code - like right here there are "if(" and "if (" used at random. "if (" is the standard. I am standardizing a lot of the core PHP code at the moment. Maybe there will be a day to do the same to all the package code :)
But in general commits/pull requests for white space change should be done separately to those for real functional change.

@rbgarga
Copy link
Member

@rbgarga rbgarga commented Sep 24, 2015

In order to have patches accepted you must sign CLA as described at https://www.pfsense.org/about-pfsense/#legal

@funkypenguin
Copy link
Author

@funkypenguin funkypenguin commented Sep 28, 2015

I've signed an ICLA as above :)

D

@rbgarga rbgarga added CLA and removed missing CLA labels Sep 29, 2015
@funkycopter
Copy link

@funkycopter funkycopter commented Nov 9, 2015

Noob here - how do I resolve the conflicts reported above?

@funkypenguin
Copy link
Author

@funkypenguin funkypenguin commented Nov 9, 2015

Oops, that was me, above, signed into a different account.

@jim-p
Copy link
Contributor

@jim-p jim-p commented Dec 9, 2015

It would be best to redo the original edits against the new version of this package over in https://github.com/pfsense/FreeBSD-ports/tree/devel/security/pfSense-pkg-openvpn-client-export
See also here: https://forum.pfsense.org/index.php?topic=103481.0

@jim-p jim-p closed this Dec 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants