Skip to content

fix(rfb): report is_owner in session users and reset on disconnect - #369

Open
MahdiBaghbani wants to merge 3 commits into
kasmtech:masterfrom
MahdiBaghbani:mahdi/fix/session-info-users
Open

fix(rfb): report is_owner in session users and reset on disconnect#369
MahdiBaghbani wants to merge 3 commits into
kasmtech:masterfrom
MahdiBaghbani:mahdi/fix/session-info-users

Conversation

@MahdiBaghbani

@MahdiBaghbani MahdiBaghbani commented Aug 16, 2026

Copy link
Copy Markdown

I was developing an in K8S cluster VMs for my university and I was stumbled upon a blocker which seems to be originated from the Kasm.

This fix resolved my issue, and I thought maybe you guys would find it useful as well,

The session-users list previously:

  • could return garbage for the owner flag due to the uninitialized read
  • never exposed which client owns the session
  • never signaled an empty session, clients could not detect a disconnect.

Signed-off-by: Mahdi Baghbani <mahdi-baghbani@azadehafzar.io>
Signed-off-by: Mahdi Baghbani <mahdi-baghbani@azadehafzar.io>
Signed-off-by: Mahdi Baghbani <mahdi-baghbani@azadehafzar.io>
@mmcclaskey

Copy link
Copy Markdown
Collaborator

This pull request has three different changes.

  1. Real bug: stale /api/get_sessions after the final disconnect.

KasmVNC updates the cached session list only when at least one authenticated client remains. When the last client disconnects, the previous user remains in the cached JSON. The PR correctly resets it to {"users":[]}. This does not mean KasmVNC failed to notice the disconnect; specifically, API consumers could not observe the session becoming empty.

  1. Real correctness bug: partially uninitialized permission outputs.

getPerms() does not populate every output on every path. In particular, owner is uninitialized with Basic Auth disabled, while the constructor can inspect uninitialized read/write after lookup failure.

  1. Adding is_owner to every connected-user record.

This is not necessary to fix disconnect detection. This appears to be a feature request.


This is a read-only mirror, for security purposes the project is maintained internally and mirrored to Github. That being said, changes will be reviewed individually and made internally.

For issue number 1, this is a real bug and you will likely see it merged into master in the next few weeks, it needs to go through testing and validation. You won't see your original commit merged in directly, since this is a read-only mirror. I will try to comment on this pull request once it has been merged to master.

For issue number 2, we intentionally disable the API if basic auth is disabled.

For issue number 3, this is a feature request. A ticket has been created for this FR internally and it will need to go to the product team for prioritization and get assigned to a future release. Unfortunately, we cannot mix features in with bug fixes, and bug fixes I can address immediately while feature requests require more rigor.

@mmcclaskey mmcclaskey self-assigned this Aug 16, 2026
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

Successfully merging this pull request may close these issues.

2 participants