Skip to content
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

dns/ddclient - add "get" protocol in custom service type #3523

Merged
merged 13 commits into from
Sep 2, 2023

Conversation

DaCookie4u
Copy link
Contributor

I am loving this new "custom POST" option, but my provider unfortunately only supports GET-requests. POST-requests fail with a CSRF validation.

Since I am unable to change the code on my providers side I decided to implemented a "custom GET" option. This does the same work only with a GET-request instead of POST.

I tried to keep the changes as small as possible. Not sure if that is wanted, but this way 99% of the current code is used and only the request-type changes.

@DaCookie4u DaCookie4u reopened this Aug 8, 2023
@fichtner
Copy link
Member

@DaCookie4u thanks, at first glance this looks good. @AdSchellevis will let you know about cleanups in a bit. He also mentioned we could add PUT as well while at it.

Cheers,
Franco

Copy link
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, LGTM

@DaCookie4u
Copy link
Contributor Author

I liked the PUT idea, so I put it in. Hope that was OK.

I also switched the getattr() method to the general request method. I have the feeling that is cleaner and somehow safer.

@DaCookie4u
Copy link
Contributor Author

BTW, how long does it for plugins to be packaged to the main repository after being committed to master? I couldn't find any info on the exact process that leads from master to package.

@fichtner
Copy link
Member

master branch merges land in the next development version of the upcoming release. If we can wrap up tomorrow that would be 23.7.2 for early next week.

But I think these changes are simple enough to be pushed to stable as well so they will be released directly in 23.7.2.

@AdSchellevis AdSchellevis merged commit 61333a7 into opnsense:master Sep 2, 2023
@AdSchellevis
Copy link
Member

@DaCookie4u looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants