openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.38k stars 2.53k forks source link

luci-app-openvpn: FileUpload fields not shown #2305

Closed mwarning closed 5 years ago

mwarning commented 5 years ago

In the OpenVPN LuCI interface, an instance created from a template has many FileUpload fields that are not shown despite being present in the uci config object (e.g. dh or ca). They can be added explicitly so that they appear. But removal does not seem to be possible.

dibdot commented 5 years ago

@jow- I could fix that with the "unsafeupload" property but it looks like some css magic is needed (at least in bootstrap theme): OpenWrt theme: upload1

Bootstrap theme: upload2

Thanks!

mwarning commented 5 years ago

Would that help? I think this fixes some things.

--- a/applications/luci-app-openvpn/luasrc/model/cbi/openvpn-advanced.lua
+++ b/applications/luci-app-openvpn/luasrc/model/cbi/openvpn-advanced.lua
@@ -247,6 +247,10 @@ for _, option in ipairs(params) do

        o.optional = true

+       if option[1] == FileUpload then
+           o.datatype = "file"
+       end
dibdot commented 5 years ago

@mwarning No, that will already checked by the used FileUpload function. Appropriate changes for your issue are in #2307

mwarning commented 5 years ago

As a heads up, I applied the changes from #2307 to the current nightly build of OpenWrt and it did not seem to work yet.

dibdot commented 5 years ago

@mwarning maybe you could elaborate what "did not seem to work" mean?

2307 add the opportunity to remove existing entries ... it doesn't change the (desired) FileUpload behaviour that only valid file handles are shown.

mwarning commented 5 years ago

Sorry, I meant that FileUpload settings from the template, like dh and ca, are not shown despite being preset with dummy values. I thought you meant that this problem was fixed in #2307.

dibdot commented 5 years ago

That's a feature, not a bug. The FileUpload types are only visible if they reference a valid file (typically uploaded by the user in /etc/luci-uploads/....)

mwarning commented 5 years ago

I tried to set secret and thought that would work. But OpenVPN did not start because the ca field etc. contains a dummy file path. But since I do not see that field, I do not know that I have to add (to make it visible) and then remove that field.

dibdot commented 5 years ago

@mwarning Ah OK, I think I've got it ... please test latest #2307, all FileUpload entries should be shown (even with default values).

mwarning commented 5 years ago

@dibdot looks good. I can see all fields that are set now. When I clear the dh filed and hit "Save & Apply", the field is removed. (But hitting the "Save" button does not remove an empty field - is that intentional?)

dibdot commented 5 years ago

Yes, that's the usual behaviour - only "apply" will finally update/commit the changes to the uci file itself.

mwarning commented 5 years ago

Alright. And is it intentional that luci-app-openvpn does not depend on package openvpn?

dibdot commented 5 years ago

yep - I will merge #2307 now. Thanks for your input & testing efforts.

mwarning commented 5 years ago

ok, thanks for your efforts.