search

shazow / ssh-chat

Chat over SSH.
https://shazow.net/posts/ssh-how-does-it-even/
MIT License
5.59k stars 408 forks source link

/op USER remove only works until USER reconnects #397

Closed mikitsu closed 3 years ago

mikitsu commented 3 years ago

Describe the bug Using the /op command to remove a user from being op only works until they reconnect again. Un-opping is done by adding an entry with 1ns expiry time, which is rejected by the Set (https://github.com/shazow/ssh-chat/blob/7413539965622ba07e8cd3719fc9d545fb52c5fd/set/set.go#L130 and https://github.com/shazow/ssh-chat/blob/7413539965622ba07e8cd3719fc9d545fb52c5fd/set/set.go#L111) as expired (unless you're really, really fast, which never happened to me). Upon reconnection, the public key is still in the op set, so a removed op regains op status.

Versions

To Reproduce Steps to reproduce the behavior:

  1. Start a chat server and connect as op
  2. run: /op (your name) remove
  3. you are no longer op (as seen with /whois and "must be op" for op commands)
  4. disconnect and reconnect
  5. you are op again (as seen with /whois and "must be op" for op commands)

This also works with two ops and one up-opping the other (probably a more typical case), but the above seems to be the simplest.

Expected behavior op2 should not be op after being removed.

Additional context I found this while writing a /whitelist for #270, so I'll just incorporate a fix for this there, if that's ok (doesn't seem extremely urgent to me).

The two possible solutions I can think of are:

In any case, I'd probably also add some error logging or passing errors up where auth deals with Set.

shazow commented 3 years ago

I thiiiiink the first solution should be fine. Might be easier to review and merge if it's in its own PR, if possible. :)