Always resolve a tenant into scope for authenticated users - #179
Merged
Merged
Conversation
pierredup
force-pushed
the
tenant-scope-resolution
branch
from
August 29, 2026 09:25
8b8a447 to
1d56432
Compare
Coverage Report for CI Build 35006617565Warning No base build found for commit Coverage: 35.495%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
pierredup
force-pushed
the
tenant-scope-resolution
branch
from
September 15, 2026 08:17
8f30b0e to
468765c
Compare
Multi-tenancy resolved a tenant when it could and did nothing when it could not: a request with no resolvable tenant proceeded with the Doctrine filter disabled, so tenant-aware queries returned rows across every tenant. This closes that gap and defines a path into scope from every starting state. A scope guard on kernel.controller — late enough to read the new authenticated user without a tenant somewhere they can get one: their only tenant is entered automatically, several send them to selection, none to onboarding, and none-with-onboarding-disabled to a "no workspace" page. Anonymous requests are ignored outright, which covers login, 2FA, password reset and registration with no route allowlist to maintain. XHR and JSON requests get a 403 with a machine-readable body rather than a redirect to an HTML page. Auto-selection runs on every tenant-less request, not only at login, so a session that loses its tenant repairs itself. It goes through TenantManager, so the membership check still applies. Custom domains now lock: DomainTenantResolver is a LockingTenantResolverInterface, so winning the chain engages TenantLock and switchTo()/clear() throw, /tenant/select returns 403 and the switcher hides. runAs() and runWithoutFilter() stay exempt — they are bounded, self-restoring scopes for deliberate cross-tenant work. Membership is also checked at login time on a custom domain, inside the firewall, so a user without access never gets a session instead of authenticating and meeting a 403 on the next request. Onboarding is customisable at three levels: listen to TenantOnboardingFormEvent to add fields, replace the type via onboarding.form_type, or decorate the TenantOnboarder service to seed a new workspace. Also fixes two existing defects: SelectTenant redirected to itself after a successful POST, and UserTenantRepository::findTenantsForUser() was annotated list<TenantInterface> while returning partial-select arrays — it now returns a TenantChoice DTO.
Three errors surfaced by CI that a local install could not see: this checkout's vendor/ had been installed with --no-scripts, leaving phpstan/extension-installer's GeneratedConfig empty, so the Symfony form stubs and the deprecation rules never loaded. With the extensions active, FormTypeInterface and FormBuilderInterface are generic and the real types show up. - OnboardTenant::$formType is a class-string<FormTypeInterface<TenantInterface>>: a replacement onboarding form must still produce a tenant. - TenantOnboardingFormEvent takes a FormBuilderInterface<TenantInterface|null>, matching what buildForm() actually receives. TData is invariant, so the previous <mixed> could not accept it. Null is accurate: the tenant model needs a name in its constructor and so cannot exist before submission. - Use expectExceptionMessageIsOrContains() in the login listener test, which is what the rest of the suite already uses; expectExceptionMessage() is deprecated in PHPUnit 13.
pierredup
force-pushed
the
tenant-scope-resolution
branch
from
September 15, 2026 11:37
cc1b922 to
db7f771
Compare
Onboarding was a first-workspace-only screen: a user who already had one was bounced to the selection page, so there was no way to make another. It now creates any workspace, and the switcher is where you reach it. - The switcher becomes the workspace menu in the navigation bar rather than a section of the user dropdown. It names the workspace you are in (a single workspace now renders, since naming it is the point), lists the others, and offers "Create new workspace". It moved to the `workspace_switcher` block, rendered by _navbar.html.twig and, on small screens, by _sidebar.html.twig. - OnboardTenant drops the redirect-to-select bounce and uses the condensed layout, so a user creating a second workspace keeps the navigation bar and gets a cancel link back to the application. TenantRedirector::defaultPath() backs that link without consuming the remembered target, which is not a completed selection. - TenantCreationGate is the one answer to "may this user create a workspace?". The switcher, the selection page, the onboarding controller and TenantScopeResolver all ask it, so a refusal cannot be routed around by going straight to the URL, and a user refused before they have any workspace sees the "no workspace" page rather than an onboarding page that would turn them away. It refuses on its own when onboarding is disabled or the tenant is domain-locked, and otherwise dispatches TenantCreationCheckEvent. - TenantCreationCheckEvent lets an application cap workspaces, gate them behind a plan, or keep them invite-only. Listeners can only refuse, and must give a reason: it is what the user is shown. The first refusal wins. - The selection page gained the same creation link; with no workspaces it was a dead end.
The data collector filtered the user's tenants against collector.tenant.id, which is null on every page that runs without a tenant: login, workspace select, onboarding. The toolbar died with a Twig RuntimeError and a 500 on exactly the pages this branch added. Guard the filter. With no tenant in scope every workspace the user has is an "additional" one, which is what the list already shows.
TenantCreationGate and TenantCreationCheckEvent were a second extension mechanism sitting next to TenantVoter, which already answers the sibling question of whether a user may enter a tenant. Both halves are now ordinary Symfony authorization, so an application extends them the way it extends anything else. - TenantCreationVoter answers TENANT_CREATE, refusing with reasons when onboarding is disabled, the tenant is domain-locked, or nobody is signed in. Symfony 7.3 vote reasons reach the 403 on their own, so #[IsGranted] needs no message of its own. - Call sites became is_granted() in the switcher and selection page, #[IsGranted] on OnboardTenant, and isGrantedForUser() in TenantScopeResolver, which asks about the user it was given rather than the session's. - An application limits creation with its own voter for the same attribute. That only works if a refusal counts, which under Symfony's default affirmative strategy it does not: the first granting voter wins and the rest are never asked. PerAttributeAccessDecisionManager therefore decides configured attributes with their own strategy, TENANT_CREATE unanimously, while every other permission keeps the application's strategy. It is installed by rewriting the definition of security.access.decision_manager rather than pointing that id elsewhere: AddSecurityVotersPass gives up on an alias, taking the collected voters and their profiler tracing with it. Rewriting the definition leaves Symfony's wiring intact, so argument 0 is still the voters and argument 1 still the application's default strategy. Only argument 2 is ours, which makes the pass order-independent with regard to Symfony's own. platform.security.access_decision.strategies opens this to any attribute. The platform's own entries are merged over whatever an application configures, so overriding the map cannot quietly drop the guarantee, and an application that replaces the whole manager through security.access_decision_manager.service gets a compile-time error instead of decisions that silently stop being unanimous. The feature is useful well beyond tenancy, so it is documented on its own at docs/security/access-decision.md.
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.
The problem
Multi-tenancy resolved a tenant when it could, and did nothing when it could not. A request where every resolver returned
nullproceeded with no tenant in scope and the Doctrine filter disabled — so a tenant-aware query returned rows across every tenant. Nothing forced a tenant to be in scope, auto-selected one, onboarded a user without one, or pinned a custom domain.This adds the enforcement layer on top of the existing resolver chain, filter and write guard, and defines a path into scope from every starting state.
The scope guard
TenantScopeGuardListenerruns onkernel.controllerrather thankernel.request. That is deliberate: by then the controller is resolved, soControllerEvent::getAttributes()exposes the new#[WithoutTenant]opt-out for free — the same mechanism#[IsGranted]uses, and no route allowlist to maintain.It stands down for sub-requests, for anyone not
IS_AUTHENTICATED_FULLY, for controllers carrying#[WithoutTenant], and when a tenant is already in scope. The authentication condition alone covers login, the 2FA challenge, password reset and registration without naming any of them.Decision logic lives in
TenantScopeResolver, which is HTTP-free and returns one of five outcomes:/tenant/select/tenant/onboardingAuto-selection runs on every tenant-less request, not only at login, so a session that loses its tenant repairs itself. It goes through
TenantManager, so the membership check still applies — auto-select is a convenience, never a way around validation.Non-navigational requests are never redirected. An XHR or a request negotiating JSON gets a 403 with a machine-readable body, because a 302 to an HTML page tells a
fetch()caller nothing.Custom domains
DomainTenantResolveris now aLockingTenantResolverInterface, so winning the chain engagesTenantLockfor the request:TenantManager::switchTo()/clear()throwTenantLockedException,/tenant/selectreturns 403, and the switcher renders nothing.runAs()andrunWithoutFilter()stay exempt. They are bounded, self-restoring scopes for deliberate cross-tenant work, and a report or batch job must still run on a request that happened to arrive via a custom domain. The lock exists to stop a user changing workspace.The lock is engaged only after the context commits, so a switch the access-validation listener vetoes cannot leave a lock behind.
Membership is also verified at login time on a custom domain.
TenantDomainLoginListenerruns inside the firewall onCheckPassportEvent, after credentials are checked and before tenant resolution, so a user without access stays on the login form and no session is created — rather than authenticating successfully and meeting a 403 on the next request.Onboarding
/tenant/onboardinglets a user with no workspace create their first. Customisable at three levels, cheapest first:TenantOnboardingFormEvent, which carries theFormBuilderInterface. The form is bound to the configured tenant class, so mapped fields land on a custom entity directly.onboarding.form_type, validated at config-compile time to implementFormTypeInterface.TenantOnboarderservice. The default persists the tenant, records the creator, creates the membership, enters the tenant and dispatchesTenantCreatedEvent(with the tenant already in scope, so seeded tenant-aware entities are attributed automatically).Tenant switcher
A standard (not live) Twig component, so it is reusable anywhere:
<twig:Platform:Tenant:Switcher />It decides for itself whether it has anything to show — nothing when the user has one workspace or the tenant is domain-locked — so callers need no surrounding condition. Added to the user menu behind a
user_menu_workspacesblock.Configuration
require_tenantdefaults to true. This is a behavioural change for existing multi-tenancy users — intended, since no application depends on the current permissive behaviour yet, and a security feature that has to be switched on is one that ships switched off. The UI bundle gainstemplates.tenant.{select,onboarding,no_access}, mirroringtemplates.login.platform-schema.jsonis regenerated (additions only).Incidental fixes
SelectTenantredirected tosolidworx_platform_tenant_selectafter a successful POST — it redirected to itself. Selection and onboarding now share aTenantRedirector: the interrupted path (recorded as a path only, GET only, so it cannot become an open redirect), thendefault_route, then/.UserTenantRepository::findTenantsForUser()was annotatedlist<TenantInterface>but ran a partial select and returned arrays; only an inline@varkept PHPStan quiet. It now returns aTenantChoiceDTO, and gainedcountTenantsForUser().Testing
43 new tests covering the five scope outcomes, every guard skip condition,
#[WithoutTenant]on class and method, content negotiation, target-path recording, the lock (engaged only for locking resolvers, released on veto,runAs()still working under it), the login listener, the onboarding form and its event, the onboarder, the switcher and the redirector.Full suite: 464 tests, 1371 assertions, all passing. ECS clean; Rector clean on all files touched here.
PHPStan reports 6 errors, all pre-existing and unrelated — verified by running it against
LoginType.php, a file this branch does not touch, which fails identically. They aregenerics.notGenericcomplaints aboutAbstractType/Kernelarising from the localsymfony/formversion; the new form type follows the same annotation style as the three existing ones.Notes for review
TenantManager, notTenantContext::setTenant(). Guarding the context would also vetopush()/pop()and breakrunAs(). The trade-off is that code reaching past the manager can still switch under a lock — documented alongside the existing "preferTenantManager" guidance.#[WithoutTenant]is a documentation burden: an app page that legitimately runs outside a tenant will redirect-loop until it is added. The failure is loud and immediate rather than silent.SelectTenantis removed from the container when multi-tenancy is disabled, but its route is still discovered from the#[Route]attribute, so/tenant/selectbecomes a service-resolution failure rather than a 404. It cannot simply stay registered, since its dependencies are removed too — it needs the route-loader fix the existing@TODOabout 2FA routes already anticipates. Onboarding avoids the same trap by keeping its services registered and letting the controller return 404 from its ownenabledcheck.