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

Improve ntfy documentation on authorization and/or add authorization header #673

Open
codebude opened this issue May 19, 2023 · 4 comments
Assignees

Comments

@codebude
Copy link

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:

In addition to passing username+password via URI, you can authorize your requests with a token (or username + password), by appending the auth information to the auth parameter.

[config:ntfy]
targets  = {
    'test': 'http://localhost:5555/testdrive?auth=<auth_info>',
    }

The <auth_info> itself must be base64 encoded information in the following formats:

Solution B

(This variant needs work on the ntfy service.)

Instead of using the auth-query parameter authorization information could be passed via the Authorization http header. Unfortunately the ntfy-service doesn't pass this header. My proposal for solution B) would be add authorization to the list of allowed fields in NTFY_FIELD_NAMES (https://github.com/mqtt-tools/mqttwarn/blob/main/mqttwarn/services/ntfy.py#LL32C1-L32C17)

Then add the following example to the documention:

[config:ntfy]
targets  = {
    'test':  {
        'url': 'http://localhost:5555/testdrive',
        'authorization': 'Bearer tk_1234567890'
    }
@amotl
Copy link
Member

amotl commented May 19, 2023

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,
Andreas.

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?

@codebude
Copy link
Author

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.)

@amotl amotl self-assigned this May 25, 2023
@amotl
Copy link
Member

amotl commented Jan 4, 2024

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:

  • Solution A: Add the snippet you blueprinted to the documentation.
  • Solution B: Just add authorization to NTFY_FIELD_NAMES, and along the lines, also add the corresponding snippet to the documentation.

That sounds easy. Please correct me if I'm wrong or missed an important detail.

With kind regards,
Andreas.

@codebude
Copy link
Author

codebude commented Jan 5, 2024

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.

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

No branches or pull requests

2 participants