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

Consider Including a Matrix Gateway endpoint as part of ntfy #319

Closed
MayeulC opened this issue Jun 11, 2022 · 20 comments
Closed

Consider Including a Matrix Gateway endpoint as part of ntfy #319

MayeulC opened this issue Jun 11, 2022 · 20 comments
Labels
enhancement New feature or request server Relates to the main binary (server or client) unified-push UnifiedPush feature or bug

Comments

@MayeulC
Copy link
Contributor

MayeulC commented Jun 11, 2022

Hello, this is kind of awkward to ask as the use-case is quite specific, but at the same time it should be fairly self-contained.

Rationale

The core issue is using UnifiedPush in conjunction with a Matrix Client and Server. The Matrix Client will pass a push URL to the Matrix server. However, the Matrix spec mandates some endpoints for push servers: https://spec.matrix.org/v1.2/push-gateway-api/

Basically, the spec mandates the use of the /_matrix/push/v1/notify path.

Therefore, one has to use an intermediate gateway before forwarding requests to ntfy. When users do not use their own, clients use default ones, which led to rate-limiting: #144

Benefit to users

Currently, users can host a gateway themselves through the common-proxies project, or some other rewrite proxy.

That's one more step for users, who might feel discouraged from doing so.
Including that gateway would increase privacy, for users of the public ntfy instance, but even more so for users who host their own homeserver and ntfy instance, as push messages do not need to travel through third-party gateways.

Needed changes

The set of features to support is relatively simple:

  • Answer
     GET /_matrix/push/v1/notify
    With
    HTTP/1.1 200 OK
    
    {"unifiedpush":{"gateway":"matrix"}}
  • Handle Pushes on that endpoint. From the spec:
    POST /_matrix/push/v1/notify
    
    {
    "notification": {
      "content": {
        "body": "I'm floating in a most peculiar way.",
        "msgtype": "m.text"
      },
      "counts": {
        "missed_calls": 1,
        "unread": 2
      },
      "devices": [
        {
          "app_id": "org.matrix.matrixConsole.ios",
          "data": {},
          "pushkey": "https://ntfy.sh/exampleTopicID?up=1",
          "pushkey_ts": 12345678,
          "tweaks": {
            "sound": "bing"
          }
        }
      ],
      "event_id": "$3957tyerfgewrf384",
      "prio": "high",
      "room_alias": "#exampleroom:matrix.org",
      "room_id": "!slw48wfj34rtnrf:example.com",
      "room_name": "Mission Control",
      "sender": "@exampleuser:matrix.org",
      "sender_display_name": "Major Tom",
      "type": "m.room.message"
    }
    }
    That should be forwarded to the topic in json_body["notification"]["devices"][0]["pushkey"]. In this case, exampleTopicID?up=1.
  • Answer that push with:
    HTTP/1.1 200 OK
    
    {"rejected":[]}
    Technically one could reject events for some topics that have never been listened to, or were deleted from the app, but that sounds a bit alien to the way ntfy works.

That's all. This should only be necessary until the Matrix Spec Change 2970 lands to delete that requirement and allow arbitrary paths, so that's your call.

Impact assessment

One more thing to expect: on the public ntfy instance, FluffyChat's and schildichat's public gateways wouldn't be used anymore, so individual Matrix servers would be subjected to rate-limiting instead, probably Matrix.org, kde.org and mozilla.org, as they are the biggest I can think of right now.

On the other hand, it makes it easier to single out abusive Matrix homeservers without locking users out. Moreover, as more clients appear (Element will soon support UP), more gateways will appear.

It might also make it easier to rate-limit matrix separately while the MSC isn't merged.

@binwiederhier binwiederhier added enhancement New feature or request server Relates to the main binary (server or client) unified-push UnifiedPush feature or bug labels Jun 11, 2022
@p1gp1g
Copy link

p1gp1g commented Jun 12, 2022

For information, that's the functions that does it for NextPush :
https://github.com/UP-NextPush/server-app/blob/main/lib/Controller/UnifiedPushProviderController.php#L374-L420

@binwiederhier
Copy link
Owner

Sooo... I can see why this is awkward.

You're asking me to put more load on the ntfy.sh server to support an unrelated feature, and introduce a potential security issue by letting the ntfy server make HTTP requests to arbitrary servers (see #93).

I am sympathetic to the cause, yet you have to understand my hesitation. ntfy is a pubsub system primarily for push notifications from scripts and such. It's not a Matrix chat relay/gateway or anything Matrix specific.

@p1gp1g
Copy link

p1gp1g commented Jun 12, 2022

I understand your hesitation since it is not directly related to the core of the project :)

Just to make it clear:
We are not asking for a public gateway (ntfy won't do any additional request [1][2]), just an endpoint (/_matrix/push/v1/notify) where the subject is read at body["notification"]["devices"][0]["pushkey"] [3] instead of the path [4]. So notification will do [matrix-server] -> [ntfy] instead of [matrix-server] -> [gateway] -> [ntfy]

For instance, the following request would do the same thing than posting it to https://ntfy.sh/exampleTopicID?up=1

POST /_matrix/push/v1/notify

{
"notification": {
  "content": {
    "body": "I'm floating in a most peculiar way.",
    "msgtype": "m.text"
  },
  "counts": {
    "missed_calls": 1,
    "unread": 2
  },
  "devices": [
    {
      "app_id": "org.matrix.matrixConsole.ios",
      "data": {},
      "pushkey": "https://ntfy.sh/exampleTopicID?up=1",
      "pushkey_ts": 12345678,
      "tweaks": {
        "sound": "bing"
      }
    }
  ],
  "event_id": "$3957tyerfgewrf384",
  "prio": "high",
  "room_alias": "#exampleroom:matrix.org",
  "room_id": "!slw48wfj34rtnrf:example.com",
  "room_name": "Mission Control",
  "sender": "@exampleuser:matrix.org",
  "sender_display_name": "Major Tom",
  "type": "m.room.message"
}
}

Once again, I understand if you do not want to implement it.

[1] Which would be totally unrelated to this project ^^
[2] So this won't add any more load nor introduce potential SSRF :)
[3] To be exact, ["pushkey"] for all device in body["notification"]["devices"]
[4] See https://github.com/UP-NextPush/server-app/blob/main/lib/Controller/UnifiedPushProviderController.php#L374-L420

@binwiederhier
Copy link
Owner

Ohhh I see. Yeah I can totally do that.

@MayeulC
Copy link
Contributor Author

MayeulC commented Jun 13, 2022

Thanks @p1gp1g for clarifying.

Also, while it would be nice to have it on the public ntfy.sh instance, I think it brings more value for self-hosted instances, as users don't have to fiddle with setting up a gateway or rewrite proxy themselves, which they can get wrong.

@MayeulC MayeulC changed the title Consider Including a Matrix Gateway as part of ntfy Consider Including a Matrix Gateway endpoint as part of ntfy Jun 13, 2022
@binwiederhier
Copy link
Owner

I thought this was a fun 30min thing to do and here's the result: #326

This implementation is incomplete and has issues:

  • It doesn't return the correct result as per spec (https://spec.matrix.org/v1.2/push-gateway-api/). It just returns the ntfy response. This is solvable but annoying.
  • It does not handle all devices in the device array, but only the first one. This is technically solvable, but would mean having to translate 1 request to N requests. This is something that I have avoided at all cost so far, because it mucks with rate limiting and such. I am not sure if I want to allow this. Is is realistic that there are more than 1 device in this array?

@p1gp1g
Copy link

p1gp1g commented Jun 14, 2022

I thought this was a fun 30min thing to do and here's the result: #326

I thought it too, but it might take more time depending on the internal architecture :/

Without the response {"rejected": []} (or {"rejected": ["pushkey_exmpl"]} which seems impossible with ntfy, but that's not very important), the matrix homeserver will probably spam the endpoint.

Also, without the discovery response (bellow), clients won't know there is a gateway and won't use it.

GET /_matrix/push/v1/notify

{"unifiedpush":{"gateway":"matrix"}}
  • It does not handle all devices in the device array, but only the first one. This is technically solvable, but would mean having to translate 1 request to N requests. This is something that I have avoided at all cost so far, because it mucks with rate limiting and such. I am not sure if I want to allow this. Is is realistic that there are more than 1 device in this array?

It seems all homeservers send only one device at this moment. I don't know if any plan to change this behavior

@MayeulC
Copy link
Contributor Author

MayeulC commented Jun 14, 2022

Hey, I was not expecting you to have a look at it so soon. Thanks a lot!

Here are my two cents (mostly the same as @p1gp1g's message):

  • Indeed, a GET should return that {"unifiedpush":{"gateway":"matrix"}} response. It's less complex to add this to the reverse proxy than de-serializing the json, but it should be fairly easy to implement inside ntfy.
  • For now, allowing only 1 device should be fine, I don't foresee this changing before MSC2970 lands.
  • I think it's better to return {"rejected":[]} in every case than returning the normal ntfy response. Ideally, that shouldn't be returned when rate-limitting happens, but I am not sure what should be returned in that case.
    Edit: I now see why it's more complex than anticipated, the response is only generated for an error. I can imagine cheating by returning an error code "200" and a body, but that requires adjusting the handle function in any case.

@binwiederhier
Copy link
Owner

I think I can do the response bit, as long as I don't have to implement 1-to-N I'm okay with the hacks. Just thought I'd ask if that's okay before proceeding.

@p1gp1g
Copy link

p1gp1g commented Jun 14, 2022

Right now, I'd say the discovery response + the {"rejected":[]} response works perfectly fine. But we have to keep in mind that homeservers might change their behavior in the future :/

@binwiederhier
Copy link
Owner

I forgot the GET but that'll be easy. It's still ugly but here's the behavior:

Entirely invalid request

$ curl -s -XPOST -d 'this is invalid' localhost:2586/_matrix/push/v1/notify
{"code":40019,"http":400,"error":"invalid request: Matrix JSON invalid","link":"https://ntfy.sh/docs/publish/#matrix-gateway"}

Log:

2022/06/14 20:42:29 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/14 20:42:29 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Connection closed with HTTP 400 (ntfy error 40019): invalid request: Matrix JSON invalid

Valid request:

$ curl -s -XPOST -T request.json localhost:2586/_matrix/push/v1/notify     
{"rejected":[]}

Log:

2022/06/14 20:42:46 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/14 20:42:46 DEBUG 127.0.0.1/exampleTopicID/s3Zr6WymX1P7 Received message: event=message, body=698 byte(s), delayed=false, firebase=false, cache=true, up=true, email=
2022/06/14 20:42:46 TRACE 127.0.0.1/exampleTopicID/s3Zr6WymX1P7 Message body: {
  "id": "s3Zr6WymX1P7",
  "time": 1655253766,
  "event": "message",
  "topic": "exampleTopicID",
  "message": "{\n\"notification\": {\n  \"content\": {\n    \"body\": \"I'm floating in a most peculiar way.\",\n    \"msgtype\": \"m.text\"\n  },\n  \"counts\": {\n    \"missed_calls\": 1,\n    \"unread\": 2\n  },\n  \"devices\": [\n    {\n      \"app_id\": \"org.matrix.matrixConsole.ios\",\n      \"data\": {},\n      \"pushkey\": \"http://127.0.0.1:2586/exampleTopicID?up=1\",\n      \"pushkey_ts\": 12345678,\n      \"tweaks\": {\n        \"sound\": \"bing\"\n      }\n    }\n  ],\n  \"event_id\": \"$3957tyerfgewrf384\",\n  \"prio\": \"high\",\n  \"room_alias\": \"#exampleroom:matrix.org\",\n  \"room_id\": \"!slw48wfj34rtnrf:example.com\",\n  \"room_name\": \"Mission Control\",\n  \"sender\": \"@exampleuser:matrix.org\",\n  \"sender_display_name\": \"Major Tom\",\n  \"type\": \"m.room.message\"\n}\n}\n"
}
2022/06/14 20:42:46 TRACE 127.0.0.1/exampleTopicID/s3Zr6WymX1P7 No stream or WebSocket subscribers, not forwarding

Valid Matrix message, but with invalid push key

$ cat request.json | jq '.notification.devices[0].pushkey'
"http://not-the-base-url-127.0.0.1:2586/exampleTopicID?up=1"

$ curl -s -XPOST -T a localhost:2586/_matrix/push/v1/notify
{"rejected":["http://not-the-base-url-127.0.0.1:2586/exampleTopicID?up=1"]}

Log:

2022/06/14 20:43:01 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/14 20:43:01 DEBUG Matrix message with push key http://not-the-base-url-127.0.0.1:2586/exampleTopicID?up=1 rejected: invalid request: Matrix JSON invalid

@MayeulC
Copy link
Contributor Author

MayeulC commented Jun 15, 2022

Wow, the PR grew more than I expected.

It looks like it touches quite a bit of core code, as in: it is not self-contained. I hope you're okay with this, I wouldn't want to increase the maintenance burden, especially for a feature that might get deprecated in 1-2 years.

Otherwise, the API and feature set looks perfect to me! I'll test it ASAP, thanks a lot!

@p1gp1g
Copy link

p1gp1g commented Jun 15, 2022

especially for a feature that might get deprecated in 1-2 years.

I don't think it will be deprecated in 1-2 years. The MSC is probably far for being accepted, and IIRC, this MSC alone is not enough to get rid of matrix gateway :)

Actually, I think we probably have to close this MSC :/ [edit: I don't know, let's see how it goes]

@binwiederhier
Copy link
Owner

Valid

 curl -v -s -XPOST -T valid localhost:2586/_matrix/push/v1/notify
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 2586 (#0)
> POST /_matrix/push/v1/notify HTTP/1.1
> Host: localhost:2586
> User-Agent: curl/7.58.0
> Accept: */*
> Content-Length: 691
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue
} [691 bytes data]
* We are completely uploaded and fine
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Thu, 16 Jun 2022 00:33:43 GMT
< Content-Length: 16
< 
{ [16 bytes data]
{"rejected":[]}
* Connection #0 to host localhost left intact

Log:

2022/06/15 20:33:04 INFO Listening on :2586[http] :2587[https] :1025[smtp], ntfy dev, log level is TRACE
2022/06/15 20:33:09 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/15 20:33:09 DEBUG 127.0.0.1/mytopic/ZK6h8Fe0wVJB Received message: event=message, body=691 byte(s), delayed=false, firebase=false, cache=true, up=true, email=
2022/06/15 20:33:09 TRACE 127.0.0.1/mytopic/ZK6h8Fe0wVJB Message body: {
  "id": "ZK6h8Fe0wVJB",
  "time": 1655339589,
  "event": "message",
  "topic": "mytopic",
  "message": "{\n\"notification\": {\n  \"content\": {\n    \"body\": \"I'm floating in a most peculiar way.\",\n    \"msgtype\": \"m.text\"\n  },\n  \"counts\": {\n    \"missed_calls\": 1,\n    \"unread\": 2\n  },\n  \"devices\": [\n    {\n      \"app_id\": \"org.matrix.matrixConsole.ios\",\n      \"data\": {},\n      \"pushkey\": \"http://127.0.0.1:2586/mytopic?up=1\",\n      \"pushkey_ts\": 12345678,\n      \"tweaks\": {\n        \"sound\": \"bing\"\n      }\n    }\n  ],\n  \"event_id\": \"$3957tyerfgewrf384\",\n  \"prio\": \"high\",\n  \"room_alias\": \"#exampleroom:matrix.org\",\n  \"room_id\": \"!slw48wfj34rtnrf:example.com\",\n  \"room_name\": \"Mission Control\",\n  \"sender\": \"@exampleuser:matrix.org\",\n  \"sender_display_name\": \"Major Tom\",\n  \"type\": \"m.room.message\"\n}\n}\n"
}
2022/06/15 20:33:09 TRACE 127.0.0.1/mytopic/ZK6h8Fe0wVJB No stream or WebSocket subscribers, not forwarding

Invalid

curl -v -s -XPOST -T invalid localhost:2586/_matrix/push/v1/notify
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 2586 (#0)
> POST /_matrix/push/v1/notify HTTP/1.1
> Host: localhost:2586
> User-Agent: curl/7.58.0
> Accept: */*
> Content-Length: 692
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue
} [692 bytes data]
* We are completely uploaded and fine
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Thu, 16 Jun 2022 00:34:14 GMT
< Content-Length: 53
< 
{ [53 bytes data]
{"rejected":["http://127.0.0.1:12456/mytopic?up=1"]}
* Connection #0 to host localhost left intact

Log:

2022/06/15 20:34:14 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/15 20:34:14 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Matrix gateway error: message with push key http://127.0.0.1:12456/mytopic?up=1 rejected: invalid request: push key must be prefixed with base URL

Total garbage

 curl -v -s -XPOST -d "omg what is this" localhost:2586/_matrix/push/v1/notify
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 2586 (#0)
> POST /_matrix/push/v1/notify HTTP/1.1
> Host: localhost:2586
> User-Agent: curl/7.58.0
> Accept: */*
> Content-Length: 16
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 16 out of 16 bytes
< HTTP/1.1 400 Bad Request
< Access-Control-Allow-Origin: *
< Content-Type: application/json
< Date: Thu, 16 Jun 2022 00:34:47 GMT
< Content-Length: 127
< 
{"code":40019,"http":400,"error":"invalid request: Matrix JSON invalid","link":"https://ntfy.sh/docs/publish/#matrix-gateway"}
* Connection #0 to host localhost left intact

Log:

2022/06/15 20:34:47 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Dispatching request
2022/06/15 20:34:47 DEBUG 127.0.0.1 HTTP POST /_matrix/push/v1/notify Connection closed with HTTP 400 (ntfy error 40019): invalid request: Matrix JSON invalid

@binwiederhier
Copy link
Owner

Feel free to review #326. I'm alright with it, though I'll add some more docs.

@binwiederhier
Copy link
Owner

This is live https://ntfy.sh/docs/releases/#ntfy-server-v1260

@MayeulC
Copy link
Contributor Author

MayeulC commented Jun 16, 2022

I upgraded my own server, it works perfectly 🎉

Thank you again!

@fauust
Copy link

fauust commented Jan 15, 2024

Hi!
Can someone point me to an example of JSON that I could use to test the notification flow with curl? I am having issues to make this work (self-hosted ntfy instance with haproxy and docker-compose + android ntfy client).

@p1gp1g
Copy link

p1gp1g commented Jan 15, 2024

You have one here : https://spec.matrix.org/v1.9/push-gateway-api/
Change the pushkey with your endpoint (eg. https://ntfy.sh/upabcd?up1)

@fauust
Copy link

fauust commented Jan 16, 2024

Thanks @p1gp1g!

So, the JSON seems to be accepted, but then the notification somehow does not get to my phone.
Something that surprises me in the ntfy logs, is message_unifiedpush=false and message_firebase=true see:

ntfy    | 2024/01/16 12:41:55 DEBUG Received message (http_method=POST, http_path=/up123456, message_body_size=729, message_call=, message_delayed=false, message_email=, message_event=message, message_firebase=true, message_id=zjXnrADqSNCX, message_sender=ipv6, message_time=1705408915, message_unifiedpush=false, tag=publish, topic=up123456, topic_last_access=2024-01-16T12:41:52.391Z, topic_subscribers=1, visitor_auth_limiter_limit=0.016666666666666666, visitor_auth_limiter_tokens=30, visitor_id=ip:ipv6, visitor_ip=ipv6, visitor_messages=15, visitor_messages_limit=17280, visitor_messages_remaining=17265, visitor_request_limiter_limit=0.2, visitor_request_limiter_tokens=58.6226667566, visitor_seen=2024-01-16T12:41:55.501Z)

As far as I understand, this should be the contrary? I'll recheck documentation and my setup (pretty sure I did everything right). I was worried that my haproxy configuration was not correct but apparently this is something else.

Anyway, thanks again for your help and sorry to pollute an old issue that has probably nothing to do with my problem (I'll reach on matrix ntfy or unifiedpush channel).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server Relates to the main binary (server or client) unified-push UnifiedPush feature or bug
Projects
None yet
Development

No branches or pull requests

4 participants