optionsfw.cgi: Centered 'Save...' buttons - added missing lang strings

Message ID 20201228134820.2201-1-matthias.fischer@ipfire.org
State Dropped
Headers show
Series optionsfw.cgi: Centered 'Save...' buttons - added missing lang strings | expand

Commit Message

Matthias Fischer Dec. 28, 2020, 1:48 p.m. UTC
I had these already used in my previous DNS/NTP-patch, that's why I missed
them.

Now added for 'next' and the 'centered buttons' patch.

Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
---
 langs/de/cgi-bin/de.pl | 1 +
 langs/en/cgi-bin/en.pl | 1 +
 2 files changed, 2 insertions(+)

Comments

Michael Tremer Dec. 29, 2020, 10:22 a.m. UTC | #1
Hi,

> On 28 Dec 2020, at 14:48, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
> 
> I had these already used in my previous DNS/NTP-patch, that's why I missed
> them.

That’s what I thought.

But I do not understand how we can change the label on the button, because it does not restart the firewall and we unfortunately cannot do that here.

This would probably be more confusing. Is your intention to have the firewall restart entirely at this point when the user clicks the button?

-Michael

> Now added for 'next' and the 'centered buttons' patch.
> 
> Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
> ---
> langs/de/cgi-bin/de.pl | 1 +
> langs/en/cgi-bin/en.pl | 1 +
> 2 files changed, 2 insertions(+)
> 
> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> index 87181c184..9d403b883 100644
> --- a/langs/de/cgi-bin/de.pl
> +++ b/langs/de/cgi-bin/de.pl
> @@ -1110,6 +1110,7 @@
> 'fw settings dropdown' => 'Alle Netzwerke auf Regelerstellungsseite anzeigen',
> 'fw settings remark' => 'Anmerkungen in Regeltabelle anzeigen',
> 'fw settings ruletable' => 'Leere Regeltabellen anzeigen',
> +'fw settings save and restart' => 'Speichern und Neustart',
> 'fwdfw ACCEPT' => 'Akzeptieren (ACCEPT)',
> 'fwdfw DROP' => 'Verwerfen (DROP)',
> 'fwdfw MODE1' => 'Alle Pakete verwerfen',
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index 625c6899f..476d3304c 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -1136,6 +1136,7 @@
> 'fw settings dropdown' => 'Show all networks on rulecreation site',
> 'fw settings remark' => 'Show remarks in ruletable',
> 'fw settings ruletable' => 'Show empty ruletables',
> +'fw settings save and restart' => 'Save and Restart',
> 'fwdfw ACCEPT' => 'ACCEPT',
> 'fwdfw DROP' => 'DROP',
> 'fwdfw MODE1' => 'Drop all packets',
> -- 
> 2.18.0
>
Matthias Fischer Dec. 29, 2020, 11:19 a.m. UTC | #2
On 29.12.2020 11:22, Michael Tremer wrote:
> Hi,

Hi,

>> On 28 Dec 2020, at 14:48, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
>> 
>> I had these already used in my previous DNS/NTP-patch, that's why I missed
>> them.
> 
> That’s what I thought.

;-)

> But I do not understand how we can change the label on the button, because it does not restart the firewall and we unfortunately cannot do that here.

I wanted to keep the 'Save' button in case the user wants to make
additional changes. After choosing 'Save' the usual errormessage/notice
appears. But it should be clear that clicking 'Save' is NOT enough for
the DNS/NTP changes to take effect. This could be adapted to read 'For
changes to take effect you have to choose Save and Restart', e.g.

The problem with the DNS/NTP rules was that the usual error message
leads to the firewall GUI, where clicking 'Apply changes' (only)
triggers '/usr/local/bin/firewallctrl' which is not enough in this case.
The DNS/NTP rules are not applied. Since they reside in
'/etc/rc.d/init.d/firewall', the execution of '/etc/rc.d/init.d/firewall
restart' is needed here.

So I added a *second* button ('Save and Restart') which triggers a
*complete* re*start* of the firewall rules. And because of that I needed
the new binary 'optionsfwctrl' which does the job.

As far as I can see, adding this 'Restart'-functionality avoids a
(former needed) complete reboot if you changed some options on
'optionsfw.cgi'. Thats how I interpret the message 'Some options need a
reboot to take effect'. Which options are these, anyway?

I hope I could make myself clear...?

> This would probably be more confusing. Is your intention to have the firewall restart entirely at this point when the user clicks the button?

Yes - that is what must be done now by adding or deleting the DNS/NTP-rules.

Best,
Matthias

> -Michael
> 
>> Now added for 'next' and the 'centered buttons' patch.
>> 
>> Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
>> ---
>> langs/de/cgi-bin/de.pl | 1 +
>> langs/en/cgi-bin/en.pl | 1 +
>> 2 files changed, 2 insertions(+)
>> 
>> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
>> index 87181c184..9d403b883 100644
>> --- a/langs/de/cgi-bin/de.pl
>> +++ b/langs/de/cgi-bin/de.pl
>> @@ -1110,6 +1110,7 @@
>> 'fw settings dropdown' => 'Alle Netzwerke auf Regelerstellungsseite anzeigen',
>> 'fw settings remark' => 'Anmerkungen in Regeltabelle anzeigen',
>> 'fw settings ruletable' => 'Leere Regeltabellen anzeigen',
>> +'fw settings save and restart' => 'Speichern und Neustart',
>> 'fwdfw ACCEPT' => 'Akzeptieren (ACCEPT)',
>> 'fwdfw DROP' => 'Verwerfen (DROP)',
>> 'fwdfw MODE1' => 'Alle Pakete verwerfen',
>> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
>> index 625c6899f..476d3304c 100644
>> --- a/langs/en/cgi-bin/en.pl
>> +++ b/langs/en/cgi-bin/en.pl
>> @@ -1136,6 +1136,7 @@
>> 'fw settings dropdown' => 'Show all networks on rulecreation site',
>> 'fw settings remark' => 'Show remarks in ruletable',
>> 'fw settings ruletable' => 'Show empty ruletables',
>> +'fw settings save and restart' => 'Save and Restart',
>> 'fwdfw ACCEPT' => 'ACCEPT',
>> 'fwdfw DROP' => 'DROP',
>> 'fwdfw MODE1' => 'Drop all packets',
>> -- 
>> 2.18.0
>> 
>
Michael Tremer Dec. 29, 2020, 11:28 a.m. UTC | #3
Hi,

> On 29 Dec 2020, at 12:19, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
> 
> On 29.12.2020 11:22, Michael Tremer wrote:
>> Hi,
> 
> Hi,
> 
>>> On 28 Dec 2020, at 14:48, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
>>> 
>>> I had these already used in my previous DNS/NTP-patch, that's why I missed
>>> them.
>> 
>> That’s what I thought.
> 
> ;-)
> 
>> But I do not understand how we can change the label on the button, because it does not restart the firewall and we unfortunately cannot do that here.
> 
> I wanted to keep the 'Save' button in case the user wants to make
> additional changes. After choosing 'Save' the usual errormessage/notice
> appears. But it should be clear that clicking 'Save' is NOT enough for
> the DNS/NTP changes to take effect. This could be adapted to read 'For
> changes to take effect you have to choose Save and Restart', e.g.
> 
> The problem with the DNS/NTP rules was that the usual error message
> leads to the firewall GUI, where clicking 'Apply changes' (only)
> triggers '/usr/local/bin/firewallctrl' which is not enough in this case.
> The DNS/NTP rules are not applied. Since they reside in
> '/etc/rc.d/init.d/firewall', the execution of '/etc/rc.d/init.d/firewall
> restart' is needed here.
> 
> So I added a *second* button ('Save and Restart') which triggers a
> *complete* re*start* of the firewall rules. And because of that I needed
> the new binary 'optionsfwctrl' which does the job.
> 
> As far as I can see, adding this 'Restart'-functionality avoids a
> (former needed) complete reboot if you changed some options on
> 'optionsfw.cgi'. Thats how I interpret the message 'Some options need a
> reboot to take effect'. Which options are these, anyway?
> 
> I hope I could make myself clear...?

Yes you did, but this still doesn’t work.

There are plenty of temporary rules that are being created and which simply will get lost after restarting the firewall. Mainly this affects IPsec, but also QoS.

So you will kill all IPsec tunnels unless those are being shut down and brought up again.

Also the command was not part of this patch, so the button does not do what it says it would be doing.

-Michael

>> This would probably be more confusing. Is your intention to have the firewall restart entirely at this point when the user clicks the button?
> 
> Yes - that is what must be done now by adding or deleting the DNS/NTP-rules.
> 
> Best,
> Matthias
> 
>> -Michael
>> 
>>> Now added for 'next' and the 'centered buttons' patch.
>>> 
>>> Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
>>> ---
>>> langs/de/cgi-bin/de.pl | 1 +
>>> langs/en/cgi-bin/en.pl | 1 +
>>> 2 files changed, 2 insertions(+)
>>> 
>>> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
>>> index 87181c184..9d403b883 100644
>>> --- a/langs/de/cgi-bin/de.pl
>>> +++ b/langs/de/cgi-bin/de.pl
>>> @@ -1110,6 +1110,7 @@
>>> 'fw settings dropdown' => 'Alle Netzwerke auf Regelerstellungsseite anzeigen',
>>> 'fw settings remark' => 'Anmerkungen in Regeltabelle anzeigen',
>>> 'fw settings ruletable' => 'Leere Regeltabellen anzeigen',
>>> +'fw settings save and restart' => 'Speichern und Neustart',
>>> 'fwdfw ACCEPT' => 'Akzeptieren (ACCEPT)',
>>> 'fwdfw DROP' => 'Verwerfen (DROP)',
>>> 'fwdfw MODE1' => 'Alle Pakete verwerfen',
>>> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
>>> index 625c6899f..476d3304c 100644
>>> --- a/langs/en/cgi-bin/en.pl
>>> +++ b/langs/en/cgi-bin/en.pl
>>> @@ -1136,6 +1136,7 @@
>>> 'fw settings dropdown' => 'Show all networks on rulecreation site',
>>> 'fw settings remark' => 'Show remarks in ruletable',
>>> 'fw settings ruletable' => 'Show empty ruletables',
>>> +'fw settings save and restart' => 'Save and Restart',
>>> 'fwdfw ACCEPT' => 'ACCEPT',
>>> 'fwdfw DROP' => 'DROP',
>>> 'fwdfw MODE1' => 'Drop all packets',
>>> -- 
>>> 2.18.0
>>> 
>> 
>
Matthias Fischer Dec. 29, 2020, 12:37 p.m. UTC | #4
On 29.12.2020 12:28, Michael Tremer wrote:
> Hi,
> 
>> On 29 Dec 2020, at 12:19, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
>> 
>> On 29.12.2020 11:22, Michael Tremer wrote:
>>> Hi,
>> 
>> Hi,
>> 
>>>> On 28 Dec 2020, at 14:48, Matthias Fischer <matthias.fischer@ipfire.org> wrote:
>>>> 
>>>> I had these already used in my previous DNS/NTP-patch, that's why I missed
>>>> them.
>>> 
>>> That’s what I thought.
>> 
>> ;-)
>> 
>>> But I do not understand how we can change the label on the button, because it does not restart the firewall and we unfortunately cannot do that here.
>> 
>> I wanted to keep the 'Save' button in case the user wants to make
>> additional changes. After choosing 'Save' the usual errormessage/notice
>> appears. But it should be clear that clicking 'Save' is NOT enough for
>> the DNS/NTP changes to take effect. This could be adapted to read 'For
>> changes to take effect you have to choose Save and Restart', e.g.
>> 
>> The problem with the DNS/NTP rules was that the usual error message
>> leads to the firewall GUI, where clicking 'Apply changes' (only)
>> triggers '/usr/local/bin/firewallctrl' which is not enough in this case.
>> The DNS/NTP rules are not applied. Since they reside in
>> '/etc/rc.d/init.d/firewall', the execution of '/etc/rc.d/init.d/firewall
>> restart' is needed here.
>> 
>> So I added a *second* button ('Save and Restart') which triggers a
>> *complete* re*start* of the firewall rules. And because of that I needed
>> the new binary 'optionsfwctrl' which does the job.
>> 
>> As far as I can see, adding this 'Restart'-functionality avoids a
>> (former needed) complete reboot if you changed some options on
>> 'optionsfw.cgi'. Thats how I interpret the message 'Some options need a
>> reboot to take effect'. Which options are these, anyway?
>> 
>> I hope I could make myself clear...?
> 
> Yes you did, but this still doesn’t work.
> 
> There are plenty of temporary rules that are being created and which simply will get lost after restarting the firewall. Mainly this affects IPsec, but also QoS.

I couldn't test with IPSec. QoS could be an option, but I never used it
=> no experiences at all. If these two make this a showstopper, then it
should be. Or are there any chances to save these temporary rules and
apply them afterwards? If not, the only chance would be to leave this at
it was before - changes require a complete reboot.

> So you will kill all IPsec tunnels unless those are being shut down and brought up again.

I see: not good.

> Also the command was not part of this patch, so the button does not do what it says it would be doing.

The command 'optionsfwctrl' was part of a patchset I sent a few hours
earlier on the same day. "optionsfw.cgi: Forcing DNS and NTP requests to
use only local servers on GREEN/BLUE".
Commit => "New binary: optionsfwctrl - needed for new firewall DNS/NTP
options"

Its based on 'unboundctrl.c'.

Best,
Matthias

> -Michael
> 
>>> This would probably be more confusing. Is your intention to have the firewall restart entirely at this point when the user clicks the button?
>> 
>> Yes - that is what must be done now by adding or deleting the DNS/NTP-rules.
>> 
>> Best,
>> Matthias
>> 
>>> -Michael
>>> 
>>>> Now added for 'next' and the 'centered buttons' patch.
>>>> 
>>>> Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
>>>> ---
>>>> langs/de/cgi-bin/de.pl | 1 +
>>>> langs/en/cgi-bin/en.pl | 1 +
>>>> 2 files changed, 2 insertions(+)
>>>> 
>>>> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
>>>> index 87181c184..9d403b883 100644
>>>> --- a/langs/de/cgi-bin/de.pl
>>>> +++ b/langs/de/cgi-bin/de.pl
>>>> @@ -1110,6 +1110,7 @@
>>>> 'fw settings dropdown' => 'Alle Netzwerke auf Regelerstellungsseite anzeigen',
>>>> 'fw settings remark' => 'Anmerkungen in Regeltabelle anzeigen',
>>>> 'fw settings ruletable' => 'Leere Regeltabellen anzeigen',
>>>> +'fw settings save and restart' => 'Speichern und Neustart',
>>>> 'fwdfw ACCEPT' => 'Akzeptieren (ACCEPT)',
>>>> 'fwdfw DROP' => 'Verwerfen (DROP)',
>>>> 'fwdfw MODE1' => 'Alle Pakete verwerfen',
>>>> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
>>>> index 625c6899f..476d3304c 100644
>>>> --- a/langs/en/cgi-bin/en.pl
>>>> +++ b/langs/en/cgi-bin/en.pl
>>>> @@ -1136,6 +1136,7 @@
>>>> 'fw settings dropdown' => 'Show all networks on rulecreation site',
>>>> 'fw settings remark' => 'Show remarks in ruletable',
>>>> 'fw settings ruletable' => 'Show empty ruletables',
>>>> +'fw settings save and restart' => 'Save and Restart',
>>>> 'fwdfw ACCEPT' => 'ACCEPT',
>>>> 'fwdfw DROP' => 'DROP',
>>>> 'fwdfw MODE1' => 'Drop all packets',
>>>> -- 
>>>> 2.18.0
>>>> 
>>> 
>> 
>

Patch

diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
index 87181c184..9d403b883 100644
--- a/langs/de/cgi-bin/de.pl
+++ b/langs/de/cgi-bin/de.pl
@@ -1110,6 +1110,7 @@ 
 'fw settings dropdown' => 'Alle Netzwerke auf Regelerstellungsseite anzeigen',
 'fw settings remark' => 'Anmerkungen in Regeltabelle anzeigen',
 'fw settings ruletable' => 'Leere Regeltabellen anzeigen',
+'fw settings save and restart' => 'Speichern und Neustart',
 'fwdfw ACCEPT' => 'Akzeptieren (ACCEPT)',
 'fwdfw DROP' => 'Verwerfen (DROP)',
 'fwdfw MODE1' => 'Alle Pakete verwerfen',
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index 625c6899f..476d3304c 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -1136,6 +1136,7 @@ 
 'fw settings dropdown' => 'Show all networks on rulecreation site',
 'fw settings remark' => 'Show remarks in ruletable',
 'fw settings ruletable' => 'Show empty ruletables',
+'fw settings save and restart' => 'Save and Restart',
 'fwdfw ACCEPT' => 'ACCEPT',
 'fwdfw DROP' => 'DROP',
 'fwdfw MODE1' => 'Drop all packets',