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

Mercure publishes to topic JWT token should not be allowed to #579

Closed
mariuseliassen opened this issue Nov 2, 2021 · 8 comments · Fixed by #620 or #671
Closed

Mercure publishes to topic JWT token should not be allowed to #579

mariuseliassen opened this issue Nov 2, 2021 · 8 comments · Fixed by #620 or #671
Labels
pinned The issue must not be marked as stale spec Spec-related issues

Comments

@mariuseliassen
Copy link

Mercure version: 0.13

Without the "private" flag on messages are being sent to subscriber even if topic is not in subscriber or publish allowed whitelist.

image

Token:
eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJtZXJjdXJlIjp7InB1Ymxpc2giOlsiL3dvcmtzcGFjZXMvMTIzIl0sInN1YnNjcmliZSI6WyIvd29ya3NwYWNlcy8xMjMiXX19.bxyELLWxI60AUT1xC3OR0cU_62vRUV0WVyuj00EDWjs

Unauthorized is only thrown if "Private" is ticked.
This does not work well for client-to-client mercure applications as this can be overwritten by any client.

@dunglas
Copy link
Owner

dunglas commented Nov 2, 2021

This is the intended behavior (as described in the spec). It's why there is the private flag: to distinguish between public and private updates.

@mariuseliassen
Copy link
Author

@dunglas Ok thanks for your quick reply.

Can you suggest how to implement private publishing topics for client-to-client behavior or if this has to be done through a backend proxy service?

@dunglas
Copy link
Owner

dunglas commented Nov 2, 2021

I'm not sure to follow. Clients' code can set the publish flag to true too.

@mariuseliassen
Copy link
Author

mariuseliassen commented Nov 2, 2021

@dunglas Yes but we have a system where we can not trust the client. User can just create his own request to mercure with the same JWT token he got from our system and not set private and bypass the entire check.

Is it possible to request that we can control this private behavior with configuration?

https://github.com/dunglas/mercure/blob/main/publish.go#L49

example like

private := len(r.PostForm["private"]) != 0 || configOverridePrivateFlag

We would like to enforce this value to true always for our usecase, so that client-to-client can only publish to the topics that are encoded in the publish array. Is it possible to achieve?

@dunglas
Copy link
Owner

dunglas commented Nov 2, 2021

Good idea to add a configuration option to only allow private updates. +1 on my side. This would also prevent mistake from programmers forgetting to set the flag.

Still, I fail to understand the use case. Nothing can prevent a client to share the data it owns with another user (using another channel than Mercure if necessary). And in could any harm himself by doing so, as it already has the data.

@mariuseliassen
Copy link
Author

mariuseliassen commented Nov 2, 2021

Our use case is pretty simple.

We have a mercure we use as a ping service to show online state. A logged in account can send pings to other accounts on the same workspace through javascript (frontend). JWT's for mercure are produced by backend on page load. As such we would produce a jwt for the account A as follows:

{
  "mercure": {
    "publish": [
      "/workspaces/1"
    ],
    "subscribe": [
      "/workspaces/1"
    ]
  }
}

Now assume that a we have another account B with the following JWT

{
  "mercure": {
    "publish": [
      "/workspaces/2"
    ],
    "subscribe": [
      "/workspaces/2"
    ]
  }
}

By design Account B should not be able to send any pings to /workspace/1, but without this private flag set (by the client) he actually is able to send a message to topic /workspaces/1.

Now in OUR frontend code we can set private: true - this is not a problem. The problem comes in when a rogue account just takes the JWT token, takes the mercure url and uses a tool like Postman or curl to construct a ping to a workspace he/she really should not be allowed to post to by just omitting the private flag.

@stale
Copy link

stale bot commented Jan 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jan 2, 2022
@dunglas dunglas added pinned The issue must not be marked as stale and removed wontfix This will not be worked on labels Jan 2, 2022
@dunglas dunglas added the spec Spec-related issues label Feb 21, 2022
@dunglas
Copy link
Owner

dunglas commented Feb 22, 2022

@mariuseliassen sorry for the long delay, but could you check if #620 fixes your issue, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned The issue must not be marked as stale spec Spec-related issues
Projects
None yet
2 participants