From 99d30d38b7d25bd90813cdc60afd0cc7b0a487c3 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Thu, 23 Jul 2026 09:23:07 +0900 Subject: [PATCH] Fix stale session permissions after wh_Auth_UserSetPermissions --- docs/src/5-Features.md | 11 + src/wh_auth.c | 20 +- test-refactor/README.md | 2 +- test-refactor/client-server/wh_test_auth.c | 346 +++++++++++++++++++++ test-refactor/wh_test_list.c | 2 + wolfhsm/wh_auth.h | 4 + 6 files changed, 382 insertions(+), 3 deletions(-) diff --git a/docs/src/5-Features.md b/docs/src/5-Features.md index 34e3aabc0..d3582d079 100644 --- a/docs/src/5-Features.md +++ b/docs/src/5-Features.md @@ -1201,6 +1201,17 @@ Credential updates through `wh_Auth_UserSetCredentials` add a further check on t `wh_Auth_UserDelete` and `wh_Auth_UserSetPermissions` remain admin-only operations in the base backend. +Independently of the backend, the core refuses any `wh_Auth_UserSetPermissions` call from a non-admin session whose supplied permissions **carry** the admin flag. The check is absolute rather than a promotion check: the core cannot see whether the target already held the flag, so it refuses even a request that would merely preserve an existing admin. That is the only permission policy the core enforces when changing an existing user. Subset limits on the remaining group, action, and key-id bits — and admin **revocation**, including demotion of the last remaining admin — are delegated to the backend. The core is handed only the target's `whUserId` and has no way to read that user's current permissions (`UserGet` is keyed by username), so it can enforce absolute rules but not rules relative to the target's existing state. A backend that allows non-admin permission edits is responsible for its own escalation and lockout policy. + +### Permission Changes and Live Sessions + +A permission change made through `wh_Auth_UserSetPermissions` takes effect on the calling session immediately: when the target `user_id` is the user logged in on that context, the cached session permissions are refreshed with the supplied values, so a revoked group or action bit is enforced on the very next request rather than at the next login. The cache mirrors what the caller supplied, so a backend that stores something other than the permissions it was handed leaves the session cache diverged from its own record. + +Two consequences are worth planning for: + +- **Self-demotion is immediate and can be self-locking.** A session that clears its own group or action bits loses those capabilities at once. If it clears the `USER_SET_PERMISSIONS` action, it can no longer restore itself and must log out and back in as a sufficiently privileged user. +- **Other sessions are not affected.** Permissions are cached per `whAuthContext`, and there is no cross-context notification. A user logged in on another connection keeps its existing permissions until it logs in again. + ### Pluggable Backend The authentication manager does not own the user database itself. All operations that read or modify user state — login, user add/delete, permission updates, credential updates — are dispatched through a `whAuthCb` callback table that the application supplies at server initialization. The storage backend is therefore a port-time decision: an in-memory table for development, an NVM-backed store for production, or a connector to an external identity service. diff --git a/src/wh_auth.c b/src/wh_auth.c index 95db35106..89f49b0cf 100644 --- a/src/wh_auth.c +++ b/src/wh_auth.c @@ -472,8 +472,24 @@ int wh_Auth_UserSetPermissions(whAuthContext* context, whUserId user_id, return rc; } - rc = context->cb->UserSetPermissions( - context->context, context->user.user_id, user_id, permissions); + /* A non-admin session can not set the admin flag, no matter what the + * backend policy allows. Absolute check; the target's current flag is + * not visible here. */ + if (!WH_AUTH_IS_ADMIN(context->user.permissions) && + WH_AUTH_IS_ADMIN(permissions)) { + rc = WH_AUTH_PERMISSION_ERROR; + } + else { + rc = context->cb->UserSetPermissions( + context->context, context->user.user_id, user_id, permissions); + + /* Keep the live session cache in sync when a logged-in user changes + * its own permissions; authorization reads only this cache. */ + if ((rc == WH_ERROR_OK) && (user_id == context->user.user_id) && + (context->user.user_id != WH_USER_ID_INVALID)) { + context->user.permissions = permissions; + } + } (void)WH_AUTH_UNLOCK(context); return rc; diff --git a/test-refactor/README.md b/test-refactor/README.md index 406f9a617..fbcf38327 100644 --- a/test-refactor/README.md +++ b/test-refactor/README.md @@ -99,7 +99,7 @@ Translated tests: | `wh_test_posix_threadsafe_stress.c::whTest_ThreadSafeStress` | called directly from `posix/wh_test_posix_main.c` | POSIX port-specific (direct call) | | | `wh_test_check_struct_padding.c` | `misc/wh_test_check_struct_padding.c` | Build-time (compile-only) | Wire-format `-Wpadded` audit; the POSIX Makefile compiles it with `-Wpadded -DWH_PADDING_CHECK`. Not a runtime test, so not registered in `wh_test_list.c` | | `wh_test_crypto_affinity.c::whTest_CryptoAffinity` | `misc/wh_test_crypto_affinity.c::whTest_CryptoAffinity` | Misc | Self-contained sequential client+server pair so the subtests can set the server devId. The WithCb subtest configures a HW devId plus a registered cryptoCb to cover the server's HW affinity-to-devId routing; the NoCb subtest uses INVALID_DEVID for the SW fallback. Under WOLFHSM_CFG_DMA both subtests also issue an AES CBC DMA request so the DMA crypto dispatch's affinity routing is covered with the same HW/SW branch matrix as the non-DMA path. Kept out of the client group because the shared port server is fixed at INVALID_DEVID and cannot exercise the HW routing branch. Build with `make CRYPTO_AFFINITY=1` | -| `wh_test_auth.c` (`whTest_AuthMEM` / `whTest_AuthTest` sub-tests) | `client-server/wh_test_auth.c::{whTest_AuthBadArgs, whTest_AuthLogin, whTest_AuthLogout, whTest_AuthAddUser, whTest_AuthDeleteUser, whTest_AuthSetPermissions, whTest_AuthSetCredentials, whTest_AuthRequestAuthorization}` | Client | Under `WOLFHSM_CFG_ENABLE_AUTHENTICATION` the POSIX server installs an auth context + admin user and the client logs in as admin at connect, so the ordinary client tests run authorized; each auth test brackets its own session (logout to start clean, restore admin on exit). Uses the blocking client API; the legacy own-server setup and single-thread manual-pump are dropped. Build with `make AUTH=1`. The TCP/client-only variant (`whTest_AuthTCP`) is not ported | +| `wh_test_auth.c` (`whTest_AuthMEM` / `whTest_AuthTest` sub-tests) | `client-server/wh_test_auth.c::{whTest_AuthBadArgs, whTest_AuthLogin, whTest_AuthLogout, whTest_AuthAddUser, whTest_AuthDeleteUser, whTest_AuthSetPermissions, whTest_AuthSetPermsCacheSync, whTest_AuthSetCredentials, whTest_AuthRequestAuthorization}` | Client | Under `WOLFHSM_CFG_ENABLE_AUTHENTICATION` the POSIX server installs an auth context + admin user and the client logs in as admin at connect, so the ordinary client tests run authorized; each auth test brackets its own session (logout to start clean, restore admin on exit). Uses the blocking client API; the legacy own-server setup and single-thread manual-pump are dropped. Build with `make AUTH=1`. The TCP/client-only variant (`whTest_AuthTCP`) is not ported. `whTest_AuthSetPermsCacheSync` is new-only and core-only: it drives `wh_Auth_*` against a mock `whAuthCb` and ignores its `whClientContext*`, so it never touches the running server | | `wh_test_she.c` (`whTest_SheMasterEcuKeyFallback`, `whTest_SheReqSizeChecking`) | `server/wh_test_she_server.c::{whTest_SheMasterEcuKeyFallback, whTest_SheReqSizeChecking}` | Server | server-internal checks reworked to use the shared server context; the POSIX server config gains a `whServerSheContext` under `WOLFHSM_CFG_SHE_EXTENSION` | | `wh_test_she.c::whTest_She` (client flows) | `client-server/wh_test_she.c::whTest_She` | Client | SHE UID/secure-boot state is one-shot per server lifetime, so the three legacy client flows are folded into one test that does `SetUid` plus a single comm-boundary-sized secure boot, then the load-key vectors, UID handling, RND, ECB/CBC/MAC, and write-protect rejection -- all of which only need UID set and secure boot complete. Build with `make SHE=1` | | `wh_test_server_img_mgr.c::whTest_ServerImgMgr` | `server/wh_test_server_img_mgr.c::whTest_ServerImgMgr` | Server | Per-method subtests (ECC P256, AES-CMAC, RSA2048, wolfBoot RSA4096, and -- under `WOLFHSM_CFG_CERTIFICATE_MANAGER` -- wolfBoot cert chain) run against the shared server context instead of each building its own server/NVM/transport. Each subtest scrubs the NVM objects and key-cache entries it creates so the group's shared NVM stays clean. Legacy ran FLASH and FLASH_LOG backends; the port runs the plain flash backend only -- FLASH_LOG re-run pending (see Known coverage gaps) | diff --git a/test-refactor/client-server/wh_test_auth.c b/test-refactor/client-server/wh_test_auth.c index 34eb55dac..86f10ed0a 100644 --- a/test-refactor/client-server/wh_test_auth.c +++ b/test-refactor/client-server/wh_test_auth.c @@ -568,8 +568,12 @@ static int _whTest_AuthSetPermissions_impl(whClientContext* client) whUserId user_id; whAuthPermissions perms, new_perms; whAuthPermissions fetched_perms; + whAuthPermissions full_perms, demoted_perms; whUserId fetched_user_id = WH_USER_ID_INVALID; + whUserId probe_id = WH_USER_ID_INVALID; int32_t get_rc = 0; + int32_t probe_server_rc = 0; + int probe_rc = 0; /* Login as admin first */ whAuthPermissions admin_perms; @@ -643,6 +647,83 @@ static int _whTest_AuthSetPermissions_impl(whClientContext* client) WH_TEST_ASSERT_RETURN(permissions_match); } + /* Test 2c: self-demote must bind the live session. Admin clears its own + * USER_ADD bit (keeping admin + SET_PERMISSIONS so it can restore). */ + WH_TEST_PRINT(" Test: Self-demote binds live session\n"); + memset(&full_perms, 0, sizeof(full_perms)); + fetched_user_id = WH_USER_ID_INVALID; + get_rc = 0; + WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(client, TEST_ADMIN_USERNAME, + &get_rc, &fetched_user_id, + &full_perms)); + WH_TEST_ASSERT_RETURN(get_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(fetched_user_id == admin_id); + + demoted_perms = full_perms; + WH_AUTH_CLEAR_ALLOWED_ACTION(demoted_perms, WH_MESSAGE_GROUP_AUTH, + WH_MESSAGE_AUTH_ACTION_USER_ADD); + server_rc = 0; + WH_TEST_RETURN_ON_FAIL( + _whTest_Auth_UserSetPermsOp(client, admin_id, demoted_perms, + &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + /* The revoked bit must now deny the admin's own UserAdd. Capture the + * probe result, restore, and only then assert -- a failing assert here + * must not leave the stored admin record demoted for later tests. */ + memset(&perms, 0, sizeof(perms)); + probe_server_rc = 0; + probe_id = WH_USER_ID_INVALID; + probe_rc = _whTest_Auth_UserAddOp(client, "selfdemote_probe", perms, + WH_AUTH_METHOD_PIN, "pass", 4, + &probe_server_rc, &probe_id); + + /* Restore full perms before asserting the probe outcome. */ + server_rc = 0; + WH_TEST_RETURN_ON_FAIL( + _whTest_Auth_UserSetPermsOp(client, admin_id, full_perms, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + /* If the probe wrongly succeeded it consumed a user slot, so drop it + * before asserting to keep the 5-slot table clean for later tests. */ + _whTest_Auth_DeleteUserByName(client, "selfdemote_probe"); + + /* A denial is a well-formed response, so the client call itself is OK */ + WH_TEST_ASSERT_RETURN(probe_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(probe_server_rc != WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(probe_id == WH_USER_ID_INVALID); + + /* The same session can add again after the restore. */ + memset(&perms, 0, sizeof(perms)); + server_rc = 0; + probe_id = WH_USER_ID_INVALID; + WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "selfdemote_probe", + perms, WH_AUTH_METHOD_PIN, + "pass", 4, &server_rc, + &probe_id)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(probe_id != WH_USER_ID_INVALID); + _whTest_Auth_DeleteUserByName(client, "selfdemote_probe"); + + /* Changing a different user's permissions leaves the caller's own + * session untouched: admin can still add after demoting testuser3. */ + memset(&new_perms, 0, sizeof(new_perms)); + server_rc = 0; + WH_TEST_RETURN_ON_FAIL( + _whTest_Auth_UserSetPermsOp(client, user_id, new_perms, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + memset(&perms, 0, sizeof(perms)); + server_rc = 0; + probe_id = WH_USER_ID_INVALID; + WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "selfdemote_probe2", + perms, WH_AUTH_METHOD_PIN, + "pass", 4, &server_rc, + &probe_id)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(probe_id != WH_USER_ID_INVALID); + _whTest_Auth_DeleteUserByName(client, "selfdemote_probe2"); + /* Test 3: Set user permissions for non-existent user */ WH_TEST_PRINT(" Test: Set user permissions for non-existent user\n"); memset(&new_perms, 0, sizeof(new_perms)); @@ -926,6 +1007,263 @@ static int _whTest_AuthRequestAuthorization_impl(whClientContext* client) } +/* ============================================================================ + * Session-cache sync unit test (mock backend; no live server needed) + * ============================================================================ + */ + +/* Mock backend driving wh_Auth_UserSetPermissions with a controllable return + * code, so the session-cache sync can be checked in isolation. Mock state is + * carried through the auth context's opaque context pointer. */ +typedef struct { + whAuthPermissions login_perms; /* permissions handed back by Login */ + whAuthPermissions stored_perms; /* permissions the backend last received */ + whUserId login_id; /* user id handed back by Login */ + int forced_rc; /* rc returned by UserSetPermissions */ + int set_calls; /* UserSetPermissions invocation count */ +} whTestAuthMock; + +static int _whTest_Auth_MockCleanup(void* context) +{ + (void)context; + return WH_ERROR_OK; +} + +static int _whTest_Auth_MockLogin(void* context, uint8_t client_id, + whAuthMethod method, const char* username, + const void* auth_data, uint16_t auth_data_len, + whUserId* out_user_id, + whAuthPermissions* out_permissions, + int* loggedIn) +{ + whTestAuthMock* mock = (whTestAuthMock*)context; + + (void)client_id; + (void)method; + (void)username; + (void)auth_data; + (void)auth_data_len; + + if ((mock == NULL) || (out_user_id == NULL) || (out_permissions == NULL) || + (loggedIn == NULL)) { + return WH_ERROR_BADARGS; + } + + *out_user_id = mock->login_id; + *out_permissions = mock->login_perms; + *loggedIn = 1; + return WH_ERROR_OK; +} + +static int _whTest_Auth_MockUserSetPermissions(void* ctx, uint16_t caller_id, + uint16_t user_id, + whAuthPermissions permissions) +{ + whTestAuthMock* mock = (whTestAuthMock*)ctx; + + (void)caller_id; + (void)user_id; + + if (mock == NULL) { + return WH_ERROR_BADARGS; + } + + /* Record what the frontend forwarded so the test can check fidelity */ + mock->stored_perms = permissions; + mock->set_calls++; + return mock->forced_rc; +} + +/* Field-wise compare so the result does not depend on struct padding. + * Returns 1 when the two permission sets match. */ +static int _whTest_Auth_PermsEqual(const whAuthPermissions* a, + const whAuthPermissions* b) +{ + int g; + int w; + int k; + + for (g = 0; g < WH_NUMBER_OF_GROUPS + 1; g++) { + if (a->groupPermissions[g] != b->groupPermissions[g]) { + return 0; + } + } + for (g = 0; g < WH_NUMBER_OF_GROUPS; g++) { + for (w = 0; w < (int)WH_AUTH_ACTION_WORDS; w++) { + if (a->actionPermissions[g][w] != b->actionPermissions[g][w]) { + return 0; + } + } + } + if (a->keyIdCount != b->keyIdCount) { + return 0; + } + for (k = 0; k < WH_AUTH_MAX_KEY_IDS; k++) { + if (a->keyIds[k] != b->keyIds[k]) { + return 0; + } + } + return 1; +} + +/* Covers the session-cache refresh: a self-targeted change is cached verbatim, + * while a backend failure, a different target, a logged-out session, and a + * non-admin admin grant leave it untouched. Also covers the admin branch. */ +static int _whTest_Auth_SetPermsCacheSync(void) +{ + int rc; + int loggedIn = 0; + whTestAuthMock mock; + whAuthCb cb; + whAuthConfig config; + whAuthContext ctx; + whAuthPermissions seed_perms; + whAuthPermissions new_perms; + whAuthPermissions over_perms; + whAuthPermissions admin_seed; + whAuthPermissions admin_perms; + whAuthPermissions zero_perms; + whUserId self_id = 1; + whUserId other_id = 2; + + WH_TEST_PRINT(" Test: Set permissions session-cache sync\n"); + + memset(&mock, 0, sizeof(mock)); + memset(&cb, 0, sizeof(cb)); + memset(&config, 0, sizeof(config)); + memset(&ctx, 0, sizeof(ctx)); + + /* Session starts with only the AUTH group allowed. */ + memset(&seed_perms, 0, sizeof(seed_perms)); + WH_AUTH_SET_ALLOWED_GROUP(seed_perms, WH_MESSAGE_GROUP_AUTH); + + mock.login_id = self_id; + mock.login_perms = seed_perms; + cb.Cleanup = _whTest_Auth_MockCleanup; + cb.Login = _whTest_Auth_MockLogin; + cb.UserSetPermissions = _whTest_Auth_MockUserSetPermissions; + config.cb = &cb; + config.context = &mock; + + WH_TEST_RETURN_ON_FAIL(wh_Auth_Init(&ctx, &config)); + + /* Establish the live session through the normal login path. */ + WH_TEST_RETURN_ON_FAIL(wh_Auth_Login(&ctx, 0, WH_AUTH_METHOD_PIN, "user", + "pin", 3, &loggedIn)); + WH_TEST_ASSERT_RETURN(loggedIn == 1); + WH_TEST_ASSERT_RETURN(ctx.user.user_id == self_id); + WH_TEST_ASSERT_RETURN(_whTest_Auth_PermsEqual(&ctx.user.permissions, + &seed_perms)); + + /* New permission set that differs from the seed. */ + memset(&new_perms, 0, sizeof(new_perms)); + WH_AUTH_SET_ALLOWED_ACTION(new_perms, WH_MESSAGE_GROUP_AUTH, + WH_MESSAGE_AUTH_ACTION_USER_ADD); + + /* Case 1: backend OK + self target -> cache updated. */ + mock.forced_rc = WH_ERROR_OK; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, new_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &new_perms)); + /* The backend must have been handed exactly what the caller supplied. */ + WH_TEST_ASSERT_RETURN(mock.set_calls == 1); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&mock.stored_perms, &new_perms)); + /* The rest of the session record must survive the refresh. */ + WH_TEST_ASSERT_RETURN(ctx.user.user_id == self_id); + WH_TEST_ASSERT_RETURN(ctx.user.is_active); + + /* Case 2: backend failure + self target -> cache unchanged. */ + ctx.user.permissions = seed_perms; + mock.forced_rc = WH_ERROR_ACCESS; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, new_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_ACCESS); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &seed_perms)); + WH_TEST_ASSERT_RETURN(ctx.user.user_id == self_id); + WH_TEST_ASSERT_RETURN(ctx.user.is_active); + + /* Case 3: backend OK + different target -> caller cache unchanged. */ + ctx.user.permissions = seed_perms; + mock.forced_rc = WH_ERROR_OK; + rc = wh_Auth_UserSetPermissions(&ctx, other_id, new_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &seed_perms)); + WH_TEST_ASSERT_RETURN(ctx.user.user_id == self_id); + WH_TEST_ASSERT_RETURN(ctx.user.is_active); + + /* Case 4: the cache mirrors the supplied values verbatim, including a + * keyIdCount the backend would clamp when storing. */ + ctx.user.permissions = seed_perms; + over_perms = new_perms; + over_perms.keyIdCount = WH_AUTH_MAX_KEY_IDS + 1; + mock.forced_rc = WH_ERROR_OK; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, over_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(ctx.user.permissions.keyIdCount == + WH_AUTH_MAX_KEY_IDS + 1); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &over_perms)); + + /* Case 5: a non-admin session can not grant the admin flag; the request + * is refused in the core, so the backend is never consulted. */ + ctx.user.permissions = seed_perms; + admin_perms = new_perms; + WH_AUTH_SET_IS_ADMIN(admin_perms, 1); + mock.forced_rc = WH_ERROR_OK; + mock.set_calls = 0; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, admin_perms); + WH_TEST_ASSERT_RETURN(rc == WH_AUTH_PERMISSION_ERROR); + WH_TEST_ASSERT_RETURN(mock.set_calls == 0); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &seed_perms)); + + /* The refusal is target-independent: promoting another user is refused + * on the same terms as self-promotion. */ + rc = wh_Auth_UserSetPermissions(&ctx, other_id, admin_perms); + WH_TEST_ASSERT_RETURN(rc == WH_AUTH_PERMISSION_ERROR); + WH_TEST_ASSERT_RETURN(mock.set_calls == 0); + + /* Case 6: an admin session is allowed to grant the admin flag. */ + admin_seed = seed_perms; + WH_AUTH_SET_IS_ADMIN(admin_seed, 1); + ctx.user.permissions = admin_seed; + mock.forced_rc = WH_ERROR_OK; + mock.set_calls = 0; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, admin_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(mock.set_calls == 1); + WH_TEST_ASSERT_RETURN(WH_AUTH_IS_ADMIN(ctx.user.permissions)); + + /* Case 7: an admin that clears its own admin bit loses it on the spot and + * can no longer promote itself back -- the self-locking hazard. */ + ctx.user.permissions = admin_seed; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, new_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(!WH_AUTH_IS_ADMIN(ctx.user.permissions)); + + mock.set_calls = 0; + rc = wh_Auth_UserSetPermissions(&ctx, self_id, admin_perms); + WH_TEST_ASSERT_RETURN(rc == WH_AUTH_PERMISSION_ERROR); + WH_TEST_ASSERT_RETURN(mock.set_calls == 0); + + /* Case 8: a logged-out session must never be refreshed, even though its + * zeroed user_id equals WH_USER_ID_INVALID passed as the target. */ + WH_TEST_RETURN_ON_FAIL(wh_Auth_Reset(&ctx)); + memset(&zero_perms, 0, sizeof(zero_perms)); + mock.forced_rc = WH_ERROR_OK; + rc = wh_Auth_UserSetPermissions(&ctx, WH_USER_ID_INVALID, new_perms); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN( + _whTest_Auth_PermsEqual(&ctx.user.permissions, &zero_perms)); + + WH_TEST_RETURN_ON_FAIL(wh_Auth_Cleanup(&ctx)); + return WH_TEST_SUCCESS; +} + + /* ============================================================================ * Bad-args tests (no live session needed; do not disturb the baseline) * ============================================================================ @@ -1106,6 +1444,14 @@ int whTest_AuthSetPermissions(whClientContext* client) return _whTest_Auth_RunBracketed(client, _whTest_AuthSetPermissions_impl); } +/* Core-only coverage; drives wh_Auth_* against a mock backend and does not + * touch the client or the running server. */ +int whTest_AuthSetPermsCacheSync(whClientContext* client) +{ + (void)client; + return _whTest_Auth_SetPermsCacheSync(); +} + int whTest_AuthSetCredentials(whClientContext* client) { return _whTest_Auth_RunBracketed(client, _whTest_AuthSetCredentials_impl); diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index d08245918..da50e54eb 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -97,6 +97,7 @@ WH_TEST_DECL(whTest_AuthLogout); WH_TEST_DECL(whTest_AuthAddUser); WH_TEST_DECL(whTest_AuthDeleteUser); WH_TEST_DECL(whTest_AuthSetPermissions); +WH_TEST_DECL(whTest_AuthSetPermsCacheSync); WH_TEST_DECL(whTest_AuthSetCredentials); WH_TEST_DECL(whTest_AuthRequestAuthorization); @@ -166,6 +167,7 @@ const whTestCase whTestsClient[] = { {"whTest_AuthAddUser", whTest_AuthAddUser}, {"whTest_AuthDeleteUser", whTest_AuthDeleteUser}, {"whTest_AuthSetPermissions", whTest_AuthSetPermissions}, + {"whTest_AuthSetPermsCacheSync", whTest_AuthSetPermsCacheSync}, {"whTest_AuthSetCredentials", whTest_AuthSetCredentials}, {"whTest_AuthRequestAuthorization", whTest_AuthRequestAuthorization}, }; diff --git a/wolfhsm/wh_auth.h b/wolfhsm/wh_auth.h index 26834d0a9..42a17c3df 100644 --- a/wolfhsm/wh_auth.h +++ b/wolfhsm/wh_auth.h @@ -338,6 +338,10 @@ int wh_Auth_UserDelete(whAuthContext* context, whUserId user_id); /** * @brief Set user permissions. * + * On success, a change targeting this context's logged-in user also refreshes + * its cached session permissions, so it binds immediately. A non-admin session + * supplying the admin flag is refused with WH_AUTH_PERMISSION_ERROR. + * * @param[in] context Pointer to the auth context. * @param[in] user_id The user ID to set permissions for. * @param[in] permissions The new permissions to set.