From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH] ddns.cgi: Drop static provider list for token based auth. Date: Wed, 02 Dec 2020 18:23:09 +0100 Message-ID: In-Reply-To: <6c824d99f62dc3e6c2b0a5d3c6774517988d7b4b.camel@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4480560518575227605==" List-Id: --===============4480560518575227605== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Stefan, Would it be possible to create some form of simple database file where all kn= own suppliers are listed in together with what inputs they expect. Then you would know which items to check for in the WUI. Maybe you could even= only show the required input boxes once the supplier has been chosen from th= e drop-down box. Regards, Adolf. On 02/12/2020 17:02, Stefan Schantl wrote: > Mhm, >=20 > you are right this will only move the problem from the developers (us) > to the users. >=20 > This is a really hard problem because there is no easy solution for > this issue. >=20 > We simple can take care about the users input and verify if the > providers supports what has been configured or we even do not. >=20 > If we decide to safe the users from themself, we need something to > verify against and that would be something like such a crappy static > list. >=20 > Do I miss something or think to narrow? >=20 > @List followers, what are your opinions? >=20 > Best regards, >=20 > -Stefan >> >> You are moving the problem from the developer to the user. >> >> I think that might be more messy. >> >> I am also not sure how this will affect existing setups? >> >> -Michael >> >>> On 2 Dec 2020, at 15:35, Stefan Schantl >>> wrote: >>> >>> Hello Michael, >>> >>> the intension of this patch was, that a supported provider of DDNS >>> has >>> changed it's API to support now token based authentication. The >>> code >>> already has been changed so here we are safe by now. >>> >>> Also in case a new dynamic DNS provider will be added to DDNS, we >>> do >>> not have to do anything, because the GUI automatically will be >>> filled >>> with the new provider. >>> >>> But there is one big exception in this: >>> >>> If a new or existing provider has supports authentication tokens, >>> this >>> provider has to be added to this list, which is not very intuitive. >>> >>> We also have to ship the GUI each time for new, if this happened. >>> >>> (This was the former reason, why we created the "ddns list- >>> providers" >>> command to prevent from this.) >>> >>> My change simplifies, the check if token based authentication >>> should be >>> used or not. It removes the check against the static list of >>> providers >>> which supports token based authentications. (This is the list, >>> which >>> needs to be keep up to date by hand.) >>> >>> For the user nothing changes, because if he speciefies "token" as >>> username, still the token base auth will be used for the choosen >>> provider. >>> >>> The only way an user may be affected is, if he configures a token >>> based >>> auth for a provider which does not supports it. This now would be >>> possible and will result in an authentication error against the >>> provider - But Hey, if he fills in incorrect data the same will be >>> happen. >>> >>> The only bigger problem would be if the valid username of a user is >>> "token". In this case he will be affected in the same way and this >>> could not be fixed in an easy way. >>> >>> IMHO, the benefits are bigger for us, because the maintain work >>> will >>> become much easier and one stupid error source will get eliminated. >>> >>> Best regards, >>> >>> -Stefan >>>> Hello, >>>> >>>> I do not understand exactly what this patch is trying to achieve. >>>> >>>> Unfortunately we have no choice than doing it this way with the >>>> current UI. >>>> >>>> I do not think it is worth altering the UI for this, and I do not >>>> know how we could do it without having a list again? >>>> >>>> -Michael >>>> >>>>> On 2 Dec 2020, at 11:30, Stefan Schantl < >>>>> stefan.schantl(a)ipfire.org> >>>>> wrote: >>>>> >>>>> This is really hard to maintain when adding new or altering >>>>> existing >>>>> providers. >>>>> >>>>> Reference #12415. >>>>> >>>>> Signed-off-by: Stefan Schantl >>>>> --- >>>>> html/cgi-bin/ddns.cgi | 8 ++++---- >>>>> 1 file changed, 4 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/html/cgi-bin/ddns.cgi b/html/cgi-bin/ddns.cgi >>>>> index 715c37290..024eaf7f6 100644 >>>>> --- a/html/cgi-bin/ddns.cgi >>>>> +++ b/html/cgi-bin/ddns.cgi >>>>> @@ -665,13 +665,13 @@ sub GenerateDDNSConfigFile { >>>>> >>>>> my $use_token =3D 0; >>>>> >>>>> - # Handle token based auth for various >>>>> providers. >>>>> - if ($provider ~~ ["dns.lightningwirelabs.com", >>>>> "entrydns.net", "regfish.com", >>>>> - "spdns.de", "zzzz.io"] && >>>>> $username >>>>> eq "token") { >>>>> + # Check if token based auth is configured. >>>>> + if ($username eq "token") { >>>>> $use_token =3D 1; >>>>> + } >>>>> >>>>> # Handle token auth for freedns.afraid.org and >>>>> regfish.com. >>>>> - } elsif ($provider ~~ ["freedns.afraid.org", >>>>> "regfish.com"] && $password eq "") { >>>>> + if ($provider ~~ ["freedns.afraid.org", >>>>> "regfish.com"] >>>>> && $password eq "") { >>>>> $use_token =3D 1; >>>>> $password =3D $username; >>>>> >>>>> --=20 >>>>> 2.20.1 >>>>> --===============4480560518575227605==--