Instead of handling just the default channel, make LogTenancyBootstrapper handle all the channels that should be affected (= $storagePathChannels, channels from $channelOverrides and the default channel in case 'stack' is the default)
This change was prompted by a phpstan failure after a recent update.
While making this change, I noticed we don't need the macro anymore
as useAssetOrigin() was added to the UrlGenerator earlier this year,
simplifying our implementation.
With the previous implementation, many users would use the default
config that enables scope_sessions. They would then deploy the app
to production and get the exception there since they use the
`database` session driver which is scoped by a different mechanism.
The idea behind throwing the exception only in prod was to make it
easy to use different setups locally without getting annoying
exceptions, while notifying users that a security feature they enabled
isn't running in production.
However, a better way of doing this is to just throw the exception
consistently in all setups and use a sane default for enabling the
scope_sessions setting based on the SESSION_DRIVER env var.
Users are always encouraged to read the session scoping docs to make
sure their session scoping configuration makes sense for their specific
setup, but this is a good balance for providing solid security out of
the box for most setups without requiring users to configure things
manually.
This is because the CreateTenantStorage listener only runs when
a tenant is created, but in multi-server setups the directory may
need to be created each time a tenant is *used*, not just created.
Also changed the listeners to use TenantEvent instead of specific
events, to make it possible to use them with other events, such as
TenancyBootstrapped.
Also update permission bits in a few mkdir() calls to better scope
data to the current OS user.
Also fix a typo in CacheTenancyBootstrapper (exception message).
By purging stores, we "detach" existing cache stores from the
CacheManager, making them impossible to adjust in the future.
We also unnecessarily recreate them on every tenancy bootstrap/revert.
A simpler case where this causes problems is defining a RateLimiter in
a service provider. That injects a single cache store into the
rate limiter singleton, which then becomes a completely independent
object after tenancy is initialized due to the purge. This in turn
means the central and tenant contexts share the rate limiter cache
instead of using separate caches as one would expect.
Before, when using UrlGeneratorBootstrapper, and your app had a
`https://` url, in tenant context, the url would have the `http://`
scheme.
Now, the bootstrapper makes sure that the TenancyUrlGenerator inherits
the original UrlGenerator's scheme. So if your app has e.g. url
"https://some-url.test", `route('home')` in tenant context will return
"http**s**://some-url.test/home" (originally, you'd get
"http://some-url.test/home" - the original scheme - https - wouldn't be
respected in the tenant context).
This PR addresses the issue reported on Discord
(https://discord.com/channels/976506366502006874/976506736120823909/1399012794514411621).
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
The change in SQLiteDatabaseManager wasn't properly saving the
updated internal value.
The check in CacheTenancyBootstrapper wasn't handling that local tests
have a 'testing' environment, not local. However fixing only the
condition would've still added the store to $names which would throw
an exception down the line. We make sure to only throw the exception
in prod, but also make sure to only add the store to $names if it is
supported.
Notable changes:
- CreateUserWithRLSPolicies: Clarify why we're creating a custom
DatabaseConfing instance
- HasDatabase: Clarify why we're ignoring tenancy_db_connection
- DatabaseConfig: General refactor, clarify the role of the host conn
- SQLiteDatabaseManager: Handle trailing DIRECTORY_SEPARATOR
in static::$path
- DisallowSqliteAttach: Don't throw any exceptions, just silently fail
since the class isn't 100% portable
- Clean up todos that are no longer relevant
- Clean up dead code or comments in some database managers
The feature was pretty much a soft-bootstrapper -- it listened
to both Bootstrapped and Reverted. Bootstrappers have a few more
protections in terms of error handling and safe reverting, so there's
no point in (badly) re-implementing bootstrapper functionality within
TenantConfig just so it could be a Feature.
Going forward, all Features should be things that are mostly agnostic
of the tenant state, and especially they should not use bootstrapped/
reverted events. Bootstrappers are simply more appropriate and safe.
* Initial implementation (lukinovec)
* Make sure DatabaseCacheBootstrapper runs after DatabaseTenancyBootstrapper, misc wip changes
* Fix withTenantDatabases()
* Add failing test (GlobalCacheTest)
* Configure globalCache's DB stores to use central connection instead of default connection every time it's reinstantiated
* Make GlobalCache facade not cached. Even though it wasn't causing issues
in our existing tests, it likely was flaky, and making it not $cached
makes it now consistent with global_cache() - always getting a new
CacheManager from the globalCache container binding
* Add database connection assertions in GlobalCacheTest
* Run all cached resolver/global cache tests with DatabaseCacheBootstrapper
* Reset adjustCacheManagerUsing in revert() and TestCase
* Reset static $stores property
* Finalize GlobalCache-related changes
* tests: remove pointless cache TTLs
* Refactor DatabaseCacheBootstrapper
* Refactor tests
Co-authored-by: lukinovec <lukinovec@gmail.com>
* Test encrypted cookie identification
* Add Fortify bootstrapper custom query param passing test
* Correct Fortify route bootstrapper test (todo refactor, convoluted)
* Clarify Fortify bootstrapper test
* Fix encrypted cookie identification test
* Move encrypted cookie assertion to "cookie identification works"
* Cover configured tenant model columns in cached resolver tests
* Refactor testing resolver with default vs custom tenant model name config
* Delete resolved todo
* Make code more concise
* Keep initial formatting (minimize diff noise)
* Make dataset/helper method parameter clearer
* Clarify fortify test
* Clarify assertions, improve comments
* Delete excessive comments, make existing comments consistent and clearer
* Make cached resolver test file clearer, update outdated comments
* Use the tenant model column term consistently
* FIx inconsistencies
* Provide more info in comment
* make comment more clear
* static property reset
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* UrlGenerator: set defaults based on config; request data: move config to config file+resolver
* Claude code adjustments
* improve request data tests, simplify complex test in UrlGeneratorBootstrapperTest
* url generator test: test changing tenant parameter name
* request data identification: add tenant_model_column configuration
* defaultParameterNames -> passQueryParameter
* move comment
* minor refactor in PathIdentificationTest, expand CLAUDE.md to include early identification section
* Fix COLOR_FLAG
* improve test name
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* TenancyUrlGenerator: add a check for queryParameterName being null
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Fix code style (php-cs-fixer)
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* cleanup, resolve todos, add immediate todos
* Improve path_identification_middleware docblock
* rename leave() method in tests
* wip fix hardcoded values making assumptions about the parameters used in routing
* defaultParameterNames
* fix CreatesDatabaseUsers return values
* $tenant -> tenant()
* resolve more todos
* make comment block a complete block
* Correct useTenantRoutesInFortify(), delete unused import
* test fixes
* remove todos
* remove JobPipeline todo
* simplify comment example
* remove todo
* fix VERSION_PREFIX in queue.yml
---------
Co-authored-by: lukinovec <lukinovec@gmail.com>
* Make RootUrlBootstrapper run ONLY in CLI by default (add $rootUrlOverrideInTests), work with resolved UrlGenerator
* Make resolving 'url' return a pre-created generator instance instead of creating it on every app('url') call
* Take care of doubling tenant keys in TenancyUrlGenerator, add regression test for using UrlGenerator and RootUrl bootstrappers together
* Fix code style (php-cs-fixer)
* refactor RootUrlBootstrapper
* add docblock
* clarify docblock
* simplify test: use concrete values instead of overly dynamic code
* Fix bootstrapper order in test, add url('/') assertion
* Use $this->app instead of app()
* Improve TenancyUrlGenerator and RootUrlBootstrapperTest clarity
* Revert attempt to maintain compatibility between the two bootstrappers
* Delete bootstrapper combining test
* Fix code style (php-cs-fixer)
---------
Co-authored-by: lukinovec <lukinovec@gmail.com>
Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>
* Declare sensitive parameters as sensitive
... just so that they don't show up in logs
* Remove unnecessary null-coalescing
* Simplify return
* Merge isset() calls
* Inline return
* Use nullsafe operator
* Simplify if-else branches
* Use direct empty string comparison instead of strlen()
* Add missing type
* Change interface as events expect a TenantWithDatabase not just a Tenant
* Narrow typehint
* Remove redundant type casts
* Fix style with php-cs-fixer
* Fix typos
* Revert unwanted if-else simplification
* fix phpstan errors
* narrow type
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
