This PR adds the `toRoute()` method override to `TenancyUrlGenerator`.
`toRoute()` now attempts to find a tenant equivalent of the passed route
(= a route with the same name as the passed one, but with the tenant
prefix) and generates URL for the tenant route. This behavior can be
bypassed using the bypass parameter, like with the `route()` method
override `TenancyUrlGenerator` had until now.
The primary reason for adding this is that Livewire v4 no longer uses
the `route()` helper (which automatically prefixes the passed route name
because of the override in `TenancyUrlGenerator`) in
`Livewire::getUpdateUri()`. Now, it uses `toRoute()`
(544aa3dfb8 (diff-e7609f8b0a60bde5a85067803d4e2f08f235c7cee9225a51ea67a85ff9a1d694R52)),
which didn't automatically swap the route for its 'tenant.'-prefixed
equivalent in tenant context (until now). So for the Livewire
integration to work with path identification, we need to override
`toRoute()` as described.
The `temporarySignedRoute()` override got removed because
`temporarySignedRoute()` calls `route()` under the hood, there's no need
to specifically override `temporarySignedRoute()`.
> Note: Browsing old convos, it seems like the `temporarySignedRoute()`
override was needed to make Livewire file uploads work with path
identification, but it's not needed anymore. TenancyUrlGenerator had
some changes since then, and now, I can't see the _exact_ reason why we
needed the override (`temporarySignedRoute()` uses `route()` under the
hood, so the only thing that should really matter is overriding
`route()`/`toRoute()`). It was likely a leftover from some older
implementation.
The `route()` override got simplified. Since `route()` uses `toRoute()`
under the hood, the `route()` override only has to have the prefixing
logic. The rest is delegated to `toRoute()`.
> Note: Even though we override `toRoute()` now which `route()` uses for
generating the URLs, we still need to override `route()` for its
`$this->routes->getByName($name)` call to receive the prefixed name. For
example, if `route()` wasn't overridden, and we only had one route:
`tenant.foo` (no central `foo` route), and we'd call `route('foo')`,
we'd get an exception saying that route "foo" wasn't found, even if
automatic route name prefixing was enabled and `toRoute()` was
overridden. With the `route()` override, `route('foo')` acts as if we
passed 'tenant.foo' instead of 'foo'.
Comments in TenancyUrlGenerator and UrlGeneratorBootstrapper got updated
to be more accurate. All _intentionally_ affected methods are listed in
TenancyUrlGenerator's docblock.
---------
Co-authored-by: Samuel Stancl <samuel@archte.ch>
> Minor breaking change: Pending tenants would previously go through the
creation pipeline as *not* pending and would only be marked as pending
after full creation. Now, pending tenants go through the creation
process with pending_since set from the start.
Pending tenants aren't getting their `pending_since` set until they're
created completely (e.g. their DB was created, migrated and seeded --
first, the tenant is created fully, and only after that, the tenant is
updated to have `pending_since`). This is a problem if someone wants to
e.g. add a job to the `DatabaseCreated` job pipeline that would check
`$this->tenant->pending()`. Since at the point of `DatabaseCreated`, the
tenant's `pending_since` isn't set yet, `$this->tenant->pending()`
returns `false`, even for tenants created using `createPending()`. So
instead of letting the pending tenant get fully created, and only after
that, setting its `pending_since` (using `update()`), we now set
`pending_since` in `create()`. `CreatingPendingTenant` is now dispatched
from the `static::creating` hook, and `PendingTenantCreated` is
dispatched from `static::created` for consistency.
Setting `pending_since` right in `create()` made the `MigrateDatabase`
and `SeedDatabase` jobs exclude the pending tenants during their
creation if the `tenancy.pending.include_in_queries` config was set to
`false` -- in that case, these jobs would never migrate or seed the
databases of pending tenants. So these jobs now pass `--with-pending` to
their underlying commands, with the value set in their `$includePending`
static property (`true` by default). This overrides the
`tenancy.pending.include_in_queries` config -- unless the
`$includePending` properties are set to `false`, these jobs will always
include pending tenants.
The `--with-pending` tenant command option originally worked just to
opt-in for including pending tenants in the command. Now,
`--with-pending` can accept values (`true`/`1` or `false`/`0`), so e.g.
- `tenants:run foo` with
`--with-pending`/`--with-pending=true`/`--with-pending=1` includes
pending tenants
- `tenants:run foo` with `--with-pending=false`/`--with-pending=0`
**excludes** pending tenants (also `--with-pending=foobar` -- invalid
input, considered `false`)
Passing `--with-pending` makes the command bypass the
`tenancy.pending.include_in_queries` config (so e.g. if
`tenancy.pending.include_in_queries` is set to `true`, and
`--with-pending=false` is passed to a command, the command will exclude
pending tenants). When `--with-pending` is not passed, the command will
include or exclude pending tenants based on the
`tenancy.pending.include_in_queries` config.
---------
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
> Minor breaking change: `session('tenancy_impersonating')` doesn't work
anymore. Use `session('tenancy_impersonation_guard')` instead.
The 'tenancy_impersonating' session variable got replaced by
'tenancy_impersonation_guard'. `UserImpersonation::stopImpersonating()`
now calls `logout()` on the guard retrieved by
`session()->get('tenancy_impersonation_guard')` instead of calling
`logout()` on the _current_ auth guard. Now. if you create the
impersonation token with guard 'web', and call
`UserImpersonation::stopImpersonating()`, for example in a route that
has the `auth:sanctum` middleware (= the current guard in that route
would be `RequestGuard` which doesn't even have the `logout()` method --
not the guard for which the impersonation token was created), the method
will correctly log the user out of the 'web' guard using which he was
actually authenticated instead of the current guard of the visited route
(which doesn't have to be the same guard for which impersonation
started).
`UserImpersonation::stopImpersonating()` now also accepts the `$logout`
parameter, which is `true` by default. If `false` is passed, the method
just forgets `tenancy_impersonation_guard` from session without logging
out.
`UserImpersonation::stopImpersonating()` now throws an exception if
impersonation wasn't active at the point of calling the method.
---------
Co-authored-by: Samuel Stancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Added a failing test for determining if a host is a subdomain, then
fixed `DomainTenantResolver::isSubdomain()` (similar fix as in #1423)
and a related assertion.
Previously, while having `tenancy.identification.central_domains` set to
e.g. `['site.com']`, the `isSubdomain()` check consider `tenantsite.com`
a subdomain because it ends with `site.com`. Now, instead of the
`endsWith()` check, the method checks if the passed domain is in the
configured central domains. If it is, it returns `false`. Otherwise,
loop through all the central domains and check if the passed domain
matches any of the central domains prefixed with a dot (e.g.
`tenant.site.com` would be considered a subdomain, `tenant.site.com`
wouldn't).
Because in InitializeTenancyByDomainOrSubdomain, if tenancy fails to
initialize using a subdomain (before this PR's changes, e.g.
`tenantsite.com` would be considered a subdomain, and `tenantsite` would
be used for initializing tenancy), it'll catch the exception and use the
whole domain for identification instead, this error will likely never be
noticed in real-world usage. So this PR corrects the subdomain detection
logic, but the real-world impact of that is negligible.
> Note: The subdomain error catching logic in domainOrSubdomain ID MW
was added in v4. If we applied this change in v3, it'd fix a real issue
where domainOrSubdomain ID MW would just fail at the subdomain
initialization, without attempting domain initialization after the
failure.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
Database deletion is now skipped by default if the tenant has the
`create_database` internal attribute set to false, meaning it was likely
created without a database. This skip can be opted out of by changing a
static property.
It also adds an opt-in static property for ignoring any other failures
during database deletion, to allow continuing execution of the delete
pipeline.
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
The `CreateTenantStorage` and `DeleteTenantStorage` listeners were used
alongside JobPipelines. When the `TenantCreated` JobPipeline had
`shouldBeQueued(true)` and the `Listeners\CreateTenantStorage` was
uncommented, the listener would throw an exception
(`Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException
Database tenantX.sqlite does not exist.`) because at the time of
executing the listener, the tenant DB wasn't created yet.
The same issue could likely also occur in the `DeleteTenantStorage`
listener as it uses `tenancy()->run()` to resolve the tenant's storage
path which wouldn't work if the tenant's database (or other resources)
was already deleted, making initialization impossible.
This PR changes `DeleteTenantStorage` into a job and puts it (commented)
into the job pipeline, so that it can be queued with the rest of the
jobs. It also removes `CreateTenantStorage` because it should be
redundant with the FilesystemTenancyBootstrapper creating the same paths
automatically when storage path is suffixed.
The old classes are kept but deprecated for backwards compatibility.
We've also added some edge case hardening to `DeleteTenantStorage` to
make sure it never deletes the central storage path directory, which
previously could in theory occur due to a misconfiguration if a user
enabled this job/listener but disabled storage path suffixing.
Co-authored-by: Samuel Štancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
At the moment, `where()` cannot be used correctly while using
`withoutPending()`. For example, if we have a single non-pending tenant
in our DB (with ID 'foo'), queries like
`Tenant::withoutPending()->where('id', 'nonexistent')->first()`will
incorrectly return the non-pending tenant ('foo').
This is because `withoutPending()` does
`$builder->whereNull('data->pending_since')->orWhereNull('data')`. These
two aren't grouped, so `withoutPending()->where('id', 'nonexistent')`
basically translates to "WHERE data->pending_since IS NULL **OR (data IS
NULL AND id = 'nonexistent')**". So the query will include all tenants
whose `pending_since` is null (= all non-pending tenants).
Grouping `->whereNull('data->pending_since')->orWhereNull('data')` in a
closure passed to a separate `where()` fixes this issue.
Some bootstrappers read attributes of the tenant during bootstrap() but
don't respond to changes made to the tenant afterwards.
Therefore, when making changes to the tenant that'd affect the behavior
of a bootstrapper, it's necessary to reinitialize tenancy (if it matters
that changes are reflected immediately). This adds a convenience helper
for that purpose.
- Update ci.yml and composer.json
- Wrap single database tenancy trait scopes in whenBooted()
- Update SessionSeparationTest to use laravel-cache- prefix in L13
and laravel_cache_ in <=L12. Our own prefix remains tenant_%tenant%_
(as configured in tenancy.cache.prefix). We could update this to be
tenant-%tenant%- from now on for consistency with Laravel's prefixes
(changed in https://github.com/laravel/framework/pull/56172) but I'm
not sure yet. _ seems to read a bit better but perhaps consistency
is more important. We may change this later and it can be adjusted
in userland easily (since it's just a config option).
Using `URL::temporarySignedRoute()` in tenant context with
`UrlGeneratorBootstrapper` enabled doesn't work the same as `route()`.
The bypass parameter doesn't actually bypass the route name prefixing.
`route()` is called in the `parent::temporarySignedRoute()` call, and
because the bypass parameter is removed before calling
`parent::temporarySignedRoute()`, the underlying `route()` call doesn't
get the bypass parameter and it ends up attempting to generate URL for a
route with the name prefixed with 'tenant.'.
This PR adds the bypass parameter back after `prepareRouteInputs()`, so
that `parent::temporarySignedRoute()` receives it, and the underlying
`route()` call respects it. Also added basic tests for the
`URL::temporarySignedRoute()` behavior (the new bypass parameter test
works as a regression test).
The old names of the class and method were misleading. We don't
actually need any relation. And we don't even need a model instance
as we were returning previously -- the only use of that method was
in TriggerSyncingEvents which would immediately use ::class on the
returned value. Therefore, all we are asking for in this interface
is just the central resource class.
Also move pivot record deletion to that listener and improve tests
The 'tenant pivot records are deleted along with the tenants to which
they belong to' test is failing in this commit -- the listener
for deleting mappings when a *tenant* is deleted is only implemented
in the next commit. The only change done here is to re-add FKs
(necessary for passing *in this commit* in that specific dataset
variant) that were removed from the default test migration as we now
have the DeleteResourceMapping listener that's enabled by default.
Previously, tenant identification middleware was typically specified
for the cloned route by "inheriting" it from the central route, which
necessarily meant that the central route had to also be marked as
universal so it could continue working in the central context --
despite presumably not being usable in the tenant context, thus being
universal for no proper reason. In such cases, universal routes were
used mainly as a mechanism for specifying the tenant identification
middleware to use on the cloned tenant route.
Given that recent refactors of the cloning feature have made it more
customizable and a bit nicer to use "multiple times", i.e. run handle()
with a few different configurations of the action, letting the
developer specify the used tenant middleware using a method like this
only makes sense.
The feature also becomes more independently usable and not just a
"hack for universal routes with path identification".
In such a case, the cloned route will actually *override* the original
route, rather than being unused as the original docblock claimed.
Also adds a static make() function for convenience.
Previously, if a universal route was cloned without a
cloneRoutesWithMiddleware(['universal']) call, i.e. it had both
'clone' and 'universal' flags, with only the former triggering cloning,
the 'universal' flag would be included in the middleware of the cloned
route.
Now, we make sure to remove all context flags -- central, tenant,
universal -- in the first step of processing middleware, before adding
just 'tenant'.
`?` parameters are not supported in these statements, so we have to use
string interpolation like in other related code.
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
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).
This PR makes the expired/invalid tenant impersonation tokens get
deleted instead of just aborting with 403.
The PR also adds a command (ClearExpiredImpersonationTokens) used like
`php artisan tenants:purge-impersonation-tokens`. As the name suggests,
it clears all expired impersonation tokens (= tokens older than
`UserImpersonation::$ttl`).
Resolves#1348
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
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>
This method lets the user specify default values for custom
non-nullable columns. The primary use case is when the tenants table
has a column like 'slug' and createPending() is called with no
value for 'slug'. This would produce an exception due to the column
having no default value.
Here, getPendingAttributes() can set an initial dummy slug (like a
randomly generated string) before it's overwritten during a pull.
getPendingAttributes() accepts an $attributes array which corresponds
to the attributes passed to createPending(). The array returned from
getPendingAttributes() is ultimately merged with $attributes, so
the user doesn't need to use the $attributes value in
getPendingAttributes(), however it serves to provide more context when
the pending attributes might be dependent on $attributes and therefore
derived from the $attributes actually being used.
Also fixed the `finally` branch in createPending() as it was
potentially referencing the $tenant variable before it was initialized.
This commit adds support for building a docker image based on PHP 8.5
(RC). It also removes some unused code in tests that was triggering
deprecation warnings. For similar deprecation warnings coming from
testbench we have a temporary patch script until this is resolved
upstream.
This commit also adds logic to the DisallowSqliteAttach feature
leveraging the new native setAuthorizer() method, instead of loading
a compiled extension.
We also remove the unused `php` parameter from ci.yml
From the perspective of the master branch, this commit merges in a
few small breaking changes from the dev branch:
6b0066c5ef
- Make pullPendingFromPool() $firstOrCreate arg default to false
(pullPending() is now a direct alias for pullPendingFromPool() with
default $firstOrCreate=true)
- See full commit message for other changes. They shouldn't be breaking
though.
13a2209f11
- Remove $WAL static property. We instead just let Laravel use its
journal_mode config now
This merge also adds a deprecation:
b320f8f33d
- Deprecate TenantConfig feature in favor of TenantConfigBootstrapper
This commit also corrects an Event::fake() call in a separate test, as
general Event::fake() calls without specified events can lead to
incorrect (and difficult to debug) behavior in some cases, since
Tenancy depends on the event system being functional.
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.
- (BC BREAK) Remove $WAL static property. We instead just let
Laravel use its journal_mode config now
- Remove journal, wal, and shm files when deleting tenant DB
- Check that the system is 64-bit when using NoAttach (we don't
build 32 bit extensions)
- Use local static instead of a class static property for caching
loadExtensionSupported
Features are now *always* bootstrapped, even if Tenancy is not resolved
from the container.
Previous implementations include
https://github.com/tenancy-for-laravel/v4/pull/19https://github.com/archtechx/tenancy/pull/1021
Bug originally reported here
https://github.com/archtechx/tenancy/issues/949
This implementation is much simpler, we do not distinguish between
features that should be "always bootstrapped" and features that should
only be bootstrapped after Tenancy is resolved. All features should work
without issues if they're bootstrapped when TSP::boot() is called. We
also add a Tenancy::bootstrapFeatures() method that can be used to
bootstrap any features dynamically added at runtime that weren't
bootstrapped in TSP::boot(). The function keeps track of which features
were already bootstrapped so it doesn't bootstrap them again.
The only potentialy risky thing in this implementation is that we're now
resolving Tenancy in TSP::boot() (previously Tenancy was not being
resolved) but that shouldn't be causing any issues.
* Fix ViteBundler not affecting Vite static calls
Replace custom Vite class override with Vite::createAssetPathsUsing() to ensure ViteBundler works for both container and static usage when asset_helper_override is enabled.
Fixes#1388
* Remove redundant logic from tests
* Simplify test further
* Re-add file creation logic
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* Make the `--database` option passed to `tenants:migrate` use the passed connection as the tenant connection template
* Reset template connection regardless of process count
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
* 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>
* Add test for the new clone action addTenantParameter property
* Add $addTenantParameter properyt to the clone action
* Fix code style (php-cs-fixer)
* Update clone action annotation
* Make addTenantParameter(false) sound by adding domain() logic to route cloning
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>
