UNOMI-875: PR4 security and tenant hardening (941/942/938)#809
Merged
Conversation
…tion Derive @RequiresTenant checks from the authenticated subject instead of X-Unomi-Tenant header. Make the system Subject immutable via AtomicReference and clear thread-local subjects with remove() after executeAsSystem.
Add per-tenant getAllItemsCount(itemType, tenantId) to persistence and wire TenantQuotaService to use tenant-scoped profile/event counts instead of cluster-wide totals.
Store PBKDF2 key hashes and masked keys instead of plaintext in persistence. Return plaintext only once via ApiKeyCreationResult; suppress keyHash in REST via ApiKeyRestMixIn. Add legacy key read support and migrate-3.1.0-20 rehash script. TenantService null guards, create/delete error contracts, and IT updates.
…legacy keys Multi-tenancy was never released, so remove transitional plaintext "key" field support and the hash-only migration script. Introduce a reusable SecretHashService API with ApiKey helpers, implement PBKDF2 hashing in the services bundle, and wire TenantService to the local bean instead of services-common.
TestTenantService defaulted to real PBKDF2, slowing every test that calls setupCommonTestData by ~4.5s per method. Add TestSecretHashService for the default in-memory double and keep SecretHashServiceImpl only in dedicated tenant/hash unit tests.
API keys are machine-generated with high entropy, so slow password hashing is unnecessary on every HTTP request. Store and verify keyHash with fast SHA-256; keep PBKDF2 on hash()/verify() for future low-entropy secrets. Remove the redundant lookupHash field.
Drop unused PBKDF2 and the hashHighEntropySecret split. API keys use a single hash()/verify() pair with SHA-256 hex and constant-time compare.
…pers Keep the new security API minimal (two methods). Move key generation and masking onto ApiKey static helpers so the domain model does not depend on SecretHashService for non-cryptographic operations.
…dling Add getRequestSubject/clearRequestSubject so executeAsSystem snapshots only the request-subject slot and does not leak a privileged subject on restore. SecurityFilter checks the path tenantId for @RequiresTenant. Harden quota load and per-tenant usage refresh logging. Clarify Tenant masked-key javadoc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plain-language summary
Multi-tenant security in Unomi 3.1 had gaps that could weaken tenant isolation or expose secrets. API keys could be stored in plain text, quota checks could count the wrong data, and background system work could leave the wrong security identity on a thread. This PR fixes those issues so tenants stay separated and keys are stored safely.
What changed
SecretHashServicewithhash()andverify()only. Key generation and masking live onApiKeyhelpers. Migration tenant init hashes keys the same way.executeAsSystemso it saves and restores only the request subject slot and does not copy a privileged subject onto the thread.@RequiresTenantis used, check the{tenantId}path parameter against the caller’s access, not the caller’s own tenant id (which always matched before). No endpoints use this annotation yet; the filter is ready for future tenant-scoped routes.Test plan
mvn -pl api,services-common,services,rest -am test(unit tests including hash, tenant, quota, and execution context)References
UNOMI-941, UNOMI-942, UNOMI-938, UNOMI-875