Re: [PATCH] ddns.cgi: Drop static provider list for token based auth.

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3688 bytes --]

Hi,

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 <stefan.schantl(a)ipfire.org> 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 <stefan.schantl(a)ipfire.org>
>>> ---
>>> 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 = 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 = 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 = 1;
>>> 			$password = $username;
>>> 
>>> -- 
>>> 2.20.1
>>>