feat: Add read-only mode for terminal sessions #104
Conversation
- Add support for read-only viewer access via --enable-readers flag - Implement write permissions check for terminal operations - New users default to read-only when write password not provided - Add visual indicator for read-only mode in UI - Disable terminal editing and controls for read-only users
|
Woohoo! The video you sent looks great. Let me check the code now, there's also a couple minor changes we could make to the interface. |
There was a problem hiding this comment.
This is fantastic! Thanks for implementing this. I'm super excited to have this feature out. I think we could add tests + figure out how to unify the same Encrypt struct for the two kinds of passwords (auth key and write key) and we'd be set.
As for the display on the frontend, I like what you did with stopping the events from going off. The server error handling also looks robust. In this display though, I think we might want to make the "Read Only" badge smaller, maybe move it to the right of the status line. And in the NameList component, move the read-only users below the read-write users, and gray out their names with lower opacity? What do you think?
Something like this:
- Current (if not
--enable-readers): https://play.tailwindcss.com/2sBMyEFomE
- New (if having
--enable-readers): https://play.tailwindcss.com/X66EJc5hdD
Having read-only users not be able to move terminals might be a point of contention, but I think it makes sense personally! This way you can share the link more widely with a group without having to worry about people potentially resizing or moving stuff. And in practice it's probably not too bad, since the editor will usually position the terminal how they would like.
Happy holidays! :)
| @@ -113,7 +113,8 @@ impl ClientSocket { | |||
|
|
|||
| async fn authenticate(&mut self) { | |||
| let encrypted_zeros = self.encrypt.zeros().into(); | |||
| self.send(WsClient::Authenticate(encrypted_zeros)).await; | |||
| self.send(WsClient::Authenticate(encrypted_zeros, None)) | |||
There was a problem hiding this comment.
I do think we should write at least one test for this read/write functionality so we're sure that it's working according to spec and continues working as such, is that something you'd be able to take on?
|
Thanks for the feedback! I’ll add the tests and unify the Encrypt struct. I agree with your suggestions for the frontend changes, and I’ll make those adjustments I’ll fix everything and update the PR. Happy holidays! |
- Update read-only badge styles and poistion - Use subtle instead of constant_time_eq dependency - Add constant-time comparison for encrypted_zeros - Improve write password entropy - Optimize user scope updates to avoid double broadcast - Sort cargo dependencies - format cli display properly
|
fixed most of the issues. tests and the Encrypt struct updates are remaining - working on those now. now we are now creating the write password in the controller itself and only saving the write password encryption zeros in metadata and snapshot. |
|
fixed most of the issues. Let me know if I missed something. |
|
Test looks great 👍 — just what I was looking for! |
|
Thank you for this! I'm super excited for this feature, I suspect this will push sshx to be the default for terminal sharing at $WORK. |
Fixes #72
Demo:
https://github.com/user-attachments/assets/95434d88-5895-4db2-ad2e-577bbb061616