auth: using headers is optional #155
Conversation
tcely
changed the title
tcely
force-pushed
the
patch-1
branch
4 times, most recently
from
c32b032 to
6d9876d
Compare
The POST data now contains: - txt - subdomain - password This still checks `X-Api-Key` and `X-Api-User` for backwards compatibility. If a user, is provided we'll still check it, but it is now optional.
After looking at the code before my changes, I'm not sure why this case would make it past the auth handler before.
NXDOMAIN shouldn't really impact normal usage, but waiting an hour for a new configuration record to show up is more acceptable. Let resolvers cache the TXT records for a short time to help with tight checking loops.
|
This seems like a good way of doing it. Essentially just makes "subdomain" the username, which is fine since the only API endpoint that requires authentication needs you to provide the subdomain anyway. |
|
Thanks for the PR! There's a lot to review, so this is going to take me a while but initially it looks good! |
|
@joohoi how's it going? |
|
The Register API is still going to need to return a username for backwards compatibility, and I would be concerned that it is going to make the model more confusing to have to document that a username is part of the return value, as it will seem like you are supposed to use it somehow (I started thinking about this when looking at the docs file and the example of testing out register and getting a return value--if I were reading that and saw a username in the json, I'd assume I'm supposed to do something with it). One thought I had to mitigate the confusion is that register could return a username that is set to the value of subdomain. This way, at least it doesn't look like something new and important. If we were to do that, it might appear as if username and subdomain are interchangeable monikers for the same thing. I still don't think it's ideal, but if there is benefit to this change, maybe it helps for the new user who hasn't followed the history. |
|
I would also point out that any acme client that supports acme-dns already (and even new clients) will probably have to consider the presence of existing servers that support the header version of the update api or at least be prepared that some servers will fail on updates if they don't. So it begs the question of whether the clients will also support the new version. At some level, I think we're stuck with username. |
The POST data now contains:
This still checks
X-Api-KeyandX-Api-Userfor backwards compatibility. If a user is provided we'll still check it, but it is now optional.This should address most of #77 too.