logout user in the case that the underlying connection gets disconnected#383
logout user in the case that the underlying connection gets disconnected#383JacobBarthelmeh wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR ensures that an authenticated user session is properly logged out when the underlying communication channel is abruptly disconnected (not just on a clean COMM_CLOSE). It also defensively clears any stale auth session state during wh_Server_Init, in case the externally-owned auth context carries a leftover session from a prior connection.
Changes:
- In
wh_Server_SetConnected, invokewh_Auth_Logouton the transition toWH_COMM_DISCONNECTEDwhen a user is currently active. - In
wh_Server_Init, after binding the externally-owned auth context, log out any stale active user; fall back to zeroing the user struct if logout fails. - Add
_whTest_Auth_AbruptDisconnecttest that uses a wrappingtest_Logoutcallback to verify the disconnect path triggers exactly one logout and that repeated disconnects are no-ops.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/wh_server.c | Adds logout-on-disconnect in wh_Server_SetConnected and stale-session cleanup in wh_Server_Init. |
| test/wh_test_auth.c | Adds a logout-counting callback and a memory-transport test verifying logout fires on abrupt disconnect and is idempotent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JacobBarthelmeh
force-pushed
the
auth_connection
branch
from
d2a889c to
5985499
Compare
JacobBarthelmeh
force-pushed
the
auth_connection
branch
from
5985499 to
1109abb
Compare
bigbrett
left a comment
bigbrett
left a comment
There was a problem hiding this comment.
LGTM. A few questions and a nit but nothing that would block merge
| if (server->auth != NULL && | ||
| server->auth->user.user_id != WH_USER_ID_INVALID) { | ||
| whUserId stale_id = server->auth->user.user_id; | ||
| int logout_rc = wh_Auth_Logout(server->auth, stale_id); |
There was a problem hiding this comment.
would we ever want to propagate this failure back to the caller?
| server->auth != NULL && | ||
| server->auth->user.user_id != WH_USER_ID_INVALID) { | ||
| whUserId user_id = server->auth->user.user_id; | ||
| int logout_rc = wh_Auth_Logout(server->auth, user_id); |
There was a problem hiding this comment.
would we ever want to propagate this failure back to the caller? Perhaps not, just trying to think it through.
|
|
||
| #if !defined(WOLFHSM_CFG_TEST_CLIENT_ONLY_TCP) && \ | ||
| defined(WOLFHSM_CFG_ENABLE_SERVER) | ||
| static int _whTest_Auth_AbruptDisconnect(whClientContext* client_ctx) |
There was a problem hiding this comment.
nit: no need for whTest prefix if its a static function
Follow up to item 4 from #270