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

MQTT5 request-response implementation #893

Open
4 tasks
hylkevds opened this issue Jan 10, 2025 · 3 comments
Open
4 tasks

MQTT5 request-response implementation #893

hylkevds opened this issue Jan 10, 2025 · 3 comments

Comments

@hylkevds
Copy link
Collaborator

I've had time to look at the request-response implementation and I'm using this issue to gather some thing I've noticed.

  • The response-topics are currently hard-coded as "/reqresp/response/" + clientId. I think that should not start with a /, should it?
  • The response-topics are currently hard-coded, they probably should be configurable.
  • The hard-coded allow rule only allows the exact response topic "/reqresp/response/" + clientId, but I think the general idea is that the user can freely choose response topics in the full sub-tree below the response topic. Of course a custom IAuthorizatorPolicy can manually allow this, but shouldn't the default hard-coded one do this already?
  • A subscription on /reqresp/# currently returns all responses. I think users should never be allowed to see the responses to requests of other users when using the broker-provided response tree... This can be fixed with a custom IAuthorizatorPolicy of course.
@andsel
Copy link
Collaborator

andsel commented Jan 12, 2025

@hylkevds I think all your points are fair and good. The last one is properly a bug IMHO.

@hylkevds
Copy link
Collaborator Author

Thanks for confirming.
None of it is really breaking, or a major security issue. I'll probably make a PR while making a prototype for the draft SensorThings API v2.0 MQTT bindings.

@andsel
Copy link
Collaborator

andsel commented Jan 13, 2025

Great! Eventually we could backport and create a patch version 0.18.1

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

No branches or pull requests

2 participants