-
Notifications
You must be signed in to change notification settings - Fork 183
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
Improve ntfy documentation on authorization and/or add authorization header #673
Comments
Dear Raffael, thank you very much for your swift report. You are right on the spot, as I've listed token-based authentication for ntfy as the last leftover backlog item at #607 (comment). You went the extra mile on providing excellent documentation content for variant A, and fabulously outlined how to bring in header-based bearer token propagation for variant B, so I do not have to conduct any research at all about it. We will bring in corresponding improvements on behalf of the next development iteration. Thank you so much! With kind regards, P.S.: Please let me know if there is any blocker for you in here. As I understand, you can already use variant A without needing any further modifications, and variant B would be a nice to have in order to make the client implementation "complete" in this regard. Right? |
Hi @amotl , I'm sorry, if I had seen #607 I hadn't opened this issue. Next time I'll try harder searching the existing issues. ;-) And you're right - it's no blocker for me, as I go with solution A for now, which works quite nice. Talking about blockers: #672 is more of blocker for me. (Right now I manually patched my proposed solution, which works out as expected, but I would love to see this in the official image.) |
Dear Raffael, apologies for not following up on this properly. I think both suggestions you've outlined have not been implemented yet, right? Just to reiterate in a compressed manner, to check if I understood your suggestions correctly:
That sounds easy. Please correct me if I'm wrong or missed an important detail. With kind regards, |
Hi @amotl , you got it right. Both solutions will work. Solution A is less work for you and solution B may look a little bit cleaner, but at the cost of additional effort. |
Introduction
Ntfy supports two authorization types: username + password and tokens. (See: https://docs.ntfy.sh/publish/#authentication ) For server to server or application communication (as in the case of mqttwarn) tokens are the means of choice. Currently, the documentation of the mqttwarn ntfy service only assumes username + password (see: https://mqttwarn.readthedocs.io/en/latest/notifier-catalog.html#ntfy).
The login method shown in the documentation (via url-property: https://:@/) does not work for token authorization.
Proposal
I suggest adding another example that demonstrates how to use token authentication.
Solution A
(This variant works without the need to customize the ntfy service.)
Add something like the following to the documentation:
Solution B
(This variant needs work on the ntfy service.)
Instead of using the
auth
-query parameter authorization information could be passed via theAuthorization
http header. Unfortunately the ntfy-service doesn't pass this header. My proposal for solution B) would be addauthorization
to the list of allowed fields inNTFY_FIELD_NAMES
(https://github.com/mqtt-tools/mqttwarn/blob/main/mqttwarn/services/ntfy.py#LL32C1-L32C17)Then add the following example to the documention:
The text was updated successfully, but these errors were encountered: