-
Notifications
You must be signed in to change notification settings - Fork 647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated Cloudflare to use token auth and update ttl #1726
Conversation
| @@ -251,6 +256,15 @@ class updatedns | |||
| $this->_error(9); | |||
| } | |||
| break; | |||
| case 'cloudflare-token': | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent seems off
| @@ -271,6 +285,7 @@ class updatedns | |||
| case 'linode-v6': | |||
| case 'regfish-v6': | |||
| case 'route53-v6': | |||
| case 'cloudflare-token-v6': | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent seems off
| @@ -319,6 +334,8 @@ class updatedns | |||
| case 'citynetwork': | |||
| case 'cloudflare': | |||
| case 'cloudflare-v6': | |||
| case 'cloudflare-token': | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent
| $baseUrl = 'https://api.cloudflare.com/client/v4'; | ||
| $fqdn = str_replace(' ', '', $this->_dnsHost); | ||
| $recordType = ($this->_useIPv6) ? 'AAAA' : 'A'; | ||
| $ttlData = intval($this->_dnsTTL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be simplified (including block below), e.g.
$ttlData = intval($this->_dnsTTL) < 1 ? 1 : intval($this->_dnsTTL);
omitting intval() will likely lead to the same result by the way
| "Authorization: Bearer {$this->_dnsPass}", | ||
| 'Content-Type: application/json' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
psr-12: } else { (https://www.php-fig.org/psr/psr-2/)
| @@ -1322,12 +1357,14 @@ class updatedns | |||
| break; | |||
| case 'cloudflare': | |||
| case 'cloudflare-v6': | |||
| case 'cloudflare-token': | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent.
| @@ -114,6 +114,8 @@ function is_dyndns_username($uname) | |||
| switch ($pconfig['type']) { | |||
| case 'cloudflare': | |||
| case 'cloudflare-v6': | |||
| case 'cloudflare-token': | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AdSchellevis Thanks for reviewing.
I have fixed the issues.
If I should have missed smth, please note or correct if necessary ;-)
Thank you 👍 :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styling looks good now, thanks, only issue left seems to be validating the ttl value.
|
@andreas-rupper looks good to me, just to be sure, did you test the code with correct and faulty inputs? I'm not using the plugin myself, i it works on your end, I'm ok with merging it now. |
@AdSchellevis Yes, I did test it including validation and it worked as expected. |
|
@andreas-rupper your welcome, thanks for confirming, I'll merge the PR |
| @@ -139,6 +139,10 @@ function is_dyndns_username($uname) | |||
| $input_errors[] = gettext("The username contains invalid characters."); | |||
| } | |||
|
|
|||
| if ((string)((int)$pconfig['ttl']) != $pconfig['ttl']) { | |||
| $input_errors[] = gettext("The TTL value needs to be a valid integer number."); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broken. Not all services use this field. Those that don't throw a hissy fit and won't allow to save. e.g. the custom service type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NOYB @AdSchellevis
Please check my pull request #1903 (below)
No description provided.