search

nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

ACL over websocket #240

Closed pagesailor closed 9 months ago

pagesailor commented 9 months ago

@nicolasff Hi! It looks like ACL is not working for me over websocket. Is it not implemented, or can it be a configuration problem?

nicolasff commented 9 months ago

Hi @pagesailor, thanks for this bug report.

I will look into it shortly and report what I find here.

pagesailor commented 9 months ago

@nicolasff Hi, sorry for harassing you about this, I know it's free software, and provided as is, but I really have to know if it's a problem with webdis, or is something wrong with our configuration. If you don't have the time to get into this, please let me know if it's a feature which should work (and if there will be a fix sometime), or if ACL was never implemented for websocket, and it won't work in the near future. If it's a bug, and will be fixed, we can work around it for now, but if not, we have to choose another solution. (Unfortunately there isn't any quite like webdis.) Thank you for your time and work!

nicolasff commented 9 months ago

Hey @pagesailor sorry about the delay. As you can see mentioned on this page there's a commit implementing it already, I'm reviewing it and expect to merge it today.

edit: I sent this comment too early. First, thanks for reporting this issue, you didn't configure it wrong or make a mistake, it was just not implemented. Obviously the logic for validating whether a command is allowed exists, it's just that the code path leading to a command being executed is slightly different depending on whether this command comes from. For HTTP requests we did have the correct sequence of (1) build a command object from the HTTP request, (2) validate that it's allowed, and (3) send it to Redis. For WebSocket step 1 is different since we build it from the WS frame, and step 2 was just missing.

The contributed commit comes with a test so I expect this will help demonstrate the issue when ran against a non-patched Webdis instance as well as show that it is addressed by the server-side fix.

pagesailor commented 9 months ago

@nicolasff @jessie-murray thank you, you are both awesome! I didn't expect such a fast response. Wish you all the best!

nicolasff commented 8 months ago

@pagesailor you're welcome! And thanks @jessie-murray once again for WebSocket improvements.

Note that there is a limitation with ACLs and WebSocket clients, since the WS implementation doesn't support HTTP basic auth (is that even a thing?). This is documented in the ACL section of the README, the main point being:

These rules apply to WebSocket connections as well, although without support for HTTP Basic Auth filtering. IP filtering is supported.

If you want to see an example of what the response looks like for rejected commands, run Webdis locally and open tests/websocket.html. There you can create two clients, one JSON-based and one "raw" using the "RESP" Redis protocol – less commonly used I'm sure. Once you've connected you can click on DEBUG OBJECT foo and if you're running with the default webdis.json or webdis-prod.json you'll see the error message and its format:

image

As noted in the README:

The http_status code is an indicator of how Webdis would have responded if the client had used HTTP instead of a WebSocket connection, since WebSocket messages do not inherently have a status code.