Conversation
* add method comments * simplify the hook script * better handle passed parameters
| # | ||
| # It iterates over DNSimple domains looking for a domain match | ||
| # | ||
| # Note: This verification fails if you own the domain `foo.com` and you attempt to verify `e-foo.com` |
There was a problem hiding this comment.
The more I think about it, the less comfortable I am with this edge case. It can get the users into a situation where they add a TXT record into DNSimple that will never be verified by Let's Encrypt.
My suggestion is to first use a routine (maybe domain-name?) to split the challenge_fqdn into the domain and subdomain, and then only compare the domain to what is available in DNSimple for the auth token via an equality check.
There was a problem hiding this comment.
I think there are two issues with the original code. 1) it may not work creating a cert for a top-level domain and 2) your comment about string matching means we should rewrite this, e.g. using include will find 'foo.com' in 'nofoo.com'.
There was a problem hiding this comment.
it may not work creating a cert for a top-level domain
Interesting. I didn't see that in my read through. Is that because of how we call the dnsimple api?
There was a problem hiding this comment.
I just haven't run through it and assume the string concatenation needs to be tightened up. I found a gem to pull out the domain and will update this tomorrow.
| account_id = account.data[0].id | ||
|
|
||
| domain_hash = find_domain(account_id, full_domain_name) | ||
| if domain_hash["domain_name"] != "" |
There was a problem hiding this comment.
I also want to double check that this if statement wasn't needed because of the way you are using a user auth token instead of an account auth token.
It works for me, but that means... nada.
Clean up dns parsing and verification
|
Hope to have cycles to test this on Friday |
|
SO.... how should i be installing these gems? I cd'ed into the hooks/dnsimple directory, and ran what am i doing wrong here? |
|
ok, so i went back and ran |
* PublicSuffix should use parse, not domain * fix an argument name mismatch
|
note, i didn't see your previous PR #12 , so the changes in |
Not 100% on best practices. If you have a better way, feel free to fix it. |
|
I want to say this PR is on an older branch that is stale. Is this still relevant- sorry for the delayed reply |
2 participants
The script is currently producing errors when calling the
exit_hook, and has some unnecessary logic left over from previous testing.This Pull Request will resolve the logical errors, and improve on the script in general to make it more maintainable in the future.