Repository navigation
GH-4586: adopt DisabledTenantException, and actually refuse a disabled tenant - #4600
Merged
Merged
Conversation
…ly refuse one JasperFx 2.74.0 lifted DisabledTenantException (a subclass of UnknownTenantIdException) and Marten, Polecat and Fisher all throw it now. Wolverine's own two tenant sources did not move, so an operator who had just disabled a tenant read "Unknown tenant id" about a tenant that is still sitting in the table. The EF Core conjoined source and its stamping interceptor both track the disabled state explicitly, so those are a straight swap. MasterTenantSource was worse than a wrong exception type. TryFindTenantConnectionString does not filter on the disabled flag, so a disabled tenant resolved on any lookup that missed the cache -- which is every lookup after DisableTenantAsync, because that drops the cached entry. And the "no row at all" branch was dead: AddApplicationNameToConnectionString ran BEFORE the IsEmpty() check and returned a non-empty connection string for a null one, so an unregistered tenant got a connection string with no server in it rather than UnknownTenantIdException. The source now keeps the disabled set beside its connection strings, checks it on the cold path, and tests the stored value before decorating it. ProblemDetails keeps the 404 -- from outside, a tenant that was switched off is not addressable -- but no longer titles it "Unknown tenant". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
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
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.
Closes #4586. Takes JasperFx 2.74.0.
JasperFx 2.74.0 lifted
DisabledTenantException(a subclass ofUnknownTenantIdException), and Marten 9.39.0, Polecat 5.31.0 and Fisher 1.12.0 already throw it. Wolverine's own two tenant sources did not move, so an operator who had just run CritterWatch'sdisable_tenantread "Unknown tenant id" about a tenant that is still sitting in the table.Because the new type subclasses the old one, every existing
catch (UnknownTenantIdException)andOnException<UnknownTenantIdException>()policy keeps working unchanged.EF Core conjoined tenancy — a straight swap
ConjoinedTenantSource.FindAsyncandTenantStampingInterceptor.applyTenancyRulesboth consultConjoinedTenancy.IsTenantDisabled, so they knew the answer and threw the wrong exception. Two lines.MasterTenantSource — the exception type was the smaller half
Writing the test turned up two defects underneath it.
A disabled tenant was not refused at all.
TryFindTenantConnectionStringdoes not filter on the disabled flag — onlyLoadAllTenantConnectionStringsdoes. SoFindAsyncresolved a disabled tenant on any lookup that missed the in-memory cache, which is every lookup afterDisableTenantAsync, because that drops the cached entry. Disabling a tenant removed it fromAllActive()and then handed out its connection string on the next request.The "unknown tenant" branch was dead code. The old order was:
AddApplicationNameToConnectionStringbuilds a connection string from the null it was handed, so the result isApplication Name=...— non-empty. An unregistered tenant id got a connection string with no server in it rather than an exception, and failed later and less usefully. The new testa_tenant_with_no_row_at_all_is_still_unknown_rather_than_disabledfails onmainfor that reason.The source now keeps the disabled set beside its connection strings — refreshed in
RefreshAsync, maintained byDisableTenantAsync/EnableTenantAsync/AddTenantAsync(an upsert re-enables) /RemoveTenantAsync— checks it on the cold path, and tests the stored value before decorating it. The cold-path check costs one extra query on the first resolve of a tenant this source has not seen, not per call.RefreshAsyncloads the disabled ids afterLoadAllTenantConnectionStrings, deliberately: that call is what seeds and creates the tenants table, andRefreshAsyncruns during resource setup against a database that may not have one yet.ProblemDetails
DisabledTenantExceptionreachesUnknownTenantMiddlewaretoo. The status stays 404 — from outside, a tenant that was switched off is not addressable, and I would rather not leak "this tenant exists but is off" to an unauthenticated caller — but the title no longer says "Unknown tenant" for it. If a disabled tenant should answer 403 instead, that is a separate call and a behavior change worth its own issue.Tests
SqlServerTests/disabled_tenant_is_refused_4586.cs, five tests, including a secondMasterTenantSourceover the same tenants table standing in for every other node in the cluster — that is the one that takes the cold path. Onmain, three of the five fail.ConjoinedTenantRegistryComplianceis tightened fromUnknownTenantIdExceptionto the specific type.Green: CoreTests 3077/3079 (2 skipped), EfCoreTests.MultiTenancy 200/200, SqlServer + Postgres master-table tenancy,
Wolverine.Http.Testsunknown-tenant 5/5. Cleandotnet build wolverine.slnx -c Release -f net9.0on JasperFx 2.74.0.🤖 Generated with Claude Code
https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj