Skip to content

Conversation

@binaryfire
Copy link
Contributor

@binaryfire binaryfire commented Dec 15, 2025

Summary

This PR adds small extensibility methods to two middleware classes, allowing multi-tenant applications to extend them. Tenant middleware must dynamically determine session cookie domains and Sanctum stateful domains, and these hooks allow multi tenant apps to reuse framework middleware logic, which avoids duplication and makes maintenance easier.

Changes

StartSession middleware

  • Added protected getSessionCookieDomain(array $config): string method
  • Subclasses can override to dynamically determine the session cookie domain per-request

EnsureFrontendRequestsAreStateful middleware

  • Added public static statefulDomains(): array method (uses late static binding)
  • Subclasses can override to dynamically determine stateful domains per-request

Motivation

In multi-tenant applications where each tenant has a different domain (e.g., tenant-a.example.com, tenant-b.example.com, or custom domains like customsite.com), the session cookie domain and Sanctum stateful domains need to be determined dynamically per-request.

Normally, you’d create fully custom middleware for this. With these changes, custom middleware can extend the framework classes and take advantage of the existing logic:

  class TenantAwareStartSession extends StartSession
  {
      protected function getSessionCookieDomain(array $config): string
      {
          return tenancy()->getDomain() ?? $config['domain'] ?? '';
      }
  }

  class TenantAwareStatefulMiddleware extends EnsureFrontendRequestsAreStateful
  {
      public static function statefulDomains(): array
      {
          $tenant = tenancy()->getTenant();
          return $tenant ? [$tenant->domain] : [];
      }
  }

Tests

Added tests for statefulDomains() method including override behavior verification.

@albertcht albertcht added the enhancement Improved feature or adjustments. label Dec 18, 2025
@albertcht
Copy link
Member

Hi @binaryfire, thank you for your contribution and for addressing a real pain point in multi-tenant applications!

However, I have some concerns about this approach:

Design Philosophy Concern

Framework components shouldn't need to know about the multi-tenancy concept. They should rely solely on their configuration as the single source of truth. The StartSession middleware doesn't need to understand why the domain might be dynamic—it just needs to read from config.

Consistency Issue

In StartSession.php, you've only extracted the domain config into a method. But what about path, secure, and http_only? Won't they have the same requirements in multi-tenant scenarios? If we add an extension point for one config value, we'd need to add them for all potentially dynamic values, which doesn't scale well.

Alternative Approach

I believe a better solution would be to implement a multi-tenancy-aware config instance at the configuration layer, rather than modifying individual framework components. This approach would:

  • Work universally - Any framework component that reads from config would automatically support dynamic values by different tenants
  • Maintain separation of concerns - Framework components remain agnostic to multi-tenancy
  • Be more maintainable - One centralized place to handle tenant-specific configuration

What do you think?

@binaryfire
Copy link
Contributor Author

binaryfire commented Dec 21, 2025

Hi @albertcht, thanks for the feedback.

I want to make sure I understand the distinction here, because this approach is similar to the getCors() pattern we just discussed for the CORS middleware. These are just simple hook methods where the base implementation reads from config, and subclasses can override these extension points to return dynamic values. The framework components themselves are completely tenant-agnostic.

Regarding the config-layer solution - could you help me understand how this would work in practice? In my case (and I imagine many multi-tenant apps), tenant configuration is stored in the database with hundreds or thousands of tenants. The domain for a given request is determined by looking up the current tenant at runtime. It's dynamic. Wouldn't a config-layer solution be problematic because configs are persistent / stateful with Swoole?

On the consistency point about path, secure, http_only - I'm happy to consolidate into a single getSessionCookieConfig(): array method if that's cleaner. In practice, the domain is the only value that almost always needs to vary per-tenant, but it would be cleaner to have it all under the one method.

@binaryfire
Copy link
Contributor Author

binaryfire commented Dec 21, 2025

Hi @albertcht. I realised I used multi tenant examples in the doc blocks and in the tests. The code itself is completely tenant agnostic, but the comments and test examples were confusing. I've removed all references to tenancy now. I also extracted the entire cookie config getSessionCookieConfig.

Just to clarify - the main thing I'm trying to solve in this PR is avoiding duplication and making maintenance easier. Currently the only way to create custom versions of these middleware classes is to duplicate all the logic:

StartSession: ~250 lines
EnsureFrontendRequestsAreStateful: ~100 lines

Having extension points means we can extend them using just a few lines. And more importantly, there's no risk of our child classes breaking if the framework versions change.

Replace getSessionCookieDomain with getSessionCookieConfig that returns
all cookie settings, providing a single extension point for customization.
@albertcht
Copy link
Member

Hi @binaryfire, thanks for the clarifications and updates!

My concern is more about the broader framework implications. If other components also face this scenario where different tenants need different config values, would we need to add similar overridable methods to all those components wherever they depend on config?

I'm wondering if we could solve this pattern more generally with a tenant-aware config implementation. For example, when calling config('foo.bar'), the config system could:

  • Check if the current request's tenant has a custom foo.bar value
  • If yes, return the tenant-specific value
  • If no, return the default foo.bar value

This way, framework components that depend on config would naturally get different values per tenant without any modifications. However, this approach has its own limitation: components couldn't cache config values - they'd need to always fetch from the config instance dynamically, which isn't a universal solution either.

So my main question is: Should framework components anticipate and accommodate multi-tenancy scenarios (or any dynamic config scenarios) by design?

Again, this specific PR is well-implemented. I'm just thinking about whether this sets a precedent where we'd need to consider extensibility for dynamic configs across many other framework components in the future.

What's your take on this?

@binaryfire
Copy link
Contributor Author

binaryfire commented Dec 21, 2025

@albertcht That's a fair question about precedent. In my experience with multi-tenancy in Laravel, only a handful of things need to vary per-tenant: session domain, CORS origins, sanctum stateful domains. It's not a widespread problem across all framework components. A tenant-aware config system is an interesting idea, but as you noted it has limitations around caching. And personally, I think overriding the app configs at runtime is a bad idea and should be avoided.

All the Laravel tenancy packages use custom middleware to handle dynamic tenant settings. These settings are stored independently of configs - eg. in the db (and cached on retrieval), set automatically per-request in Context etc. This PR just adds a way for tenancy packages to borrow the existing logic from a few framework middleware classes, instead of duplicating that code. Multi tenancy is actually pretty straightforward - it's usually just some middleware, and model traits with global scopes.

There's only one other piece of the puzzle that's missing - Laravel's useOrigin method: https://github.com/laravel/framework/blob/302cda21e290216a6b1aa63ac16ef9e6529028ce/src/Illuminate/Routing/UrlGenerator.php#L761. I'll make a separate PR for that.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improved feature or adjustments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants