Multitenancy with tenant as subdomain - cookie issue

[qwenger]

Hi,

We would like to use multitenancy, but prefer to have the tenant name as a subdomain instead of as the head of the path. So scheme://tenant.some.domain/some/service instead of scheme://some.domain/tenant/some/service.

The doc is not very clear whether that setup is fundamentally unsupported or just not commonly used.

We found settings that almost work, as follows:

• In the general env variables in qwc-docker/docker-compose.yml, set TENANT_HEADER: Tenant and TENANT_PATH_PREFIX: ' @service_prefix@'
• In qwc-docker/api-gateway/nginx.conf, set up the proxy headers (Tenant, Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Proto) as per the multitenancy doc, but don't use rewrite rules for the locations; the Tenant header variable comes from server_name in regex form instead of a location block
• In the tenantConfig.json/tenantConfig.template.json's, only use the tenant name or $tenant$ for dirs (e.g. /data/$tenant$/scan), not for URLs (e.g. http://qwc-qgis-server/ows/)
• Of course set up the DNS as needed.

While basic requests then go through and maps are shown, not everything works. In particular, logging to qwc_admin gets stuck in a loop.

We diagnosed it as an issue in https://github.com/qwc-services/qwc-services-core/blob/v1.3.28/qwc_services_core/tenant_handler.py#L219: if we set neither a tenant name (TENANT_PATH_PREFIX without @tenant@) nor a service prefix (QWC_SERVICE_PREFIX not set), tenant_path_prefix() returns an empty string, so cookies end up sent with an empty Path= attribute and matching fails (if I understand the RFC correctly, this is because the attribute is then set client-side to the full path of the resource, so typically /auth/login because the POST happens there, hence /qwc_admin doesn't match).

After fixing that (https://github.com/qwenger/qwc-services-core/commit/04564cee96f121f66e3cdf53655baad87e422a42), it seems that everything works correctly so far.

So, a few questions:

• Is multitenancy via subdomains meant to be supported to some extent or not at all?
• Would you consider integrating the fix above? Even if you don't officially support multitenancy via subdomains, it should never be harmful in any other scenario because then tenant_path_prefix() shouldn't return an empty string. And we don't see a case where the empty string is what's needed, see the discussion above.
• Independently of the above, isn't there a possible bug or inconsistency in https://github.com/qwc-services/qwc-services-core/blob/v1.3.28/qwc_services_core/tenant_handler.py#L213? It seems strange to return a path with a trailing slash while the conditional branch with multi doesn't have it (no slash in the tenant name). We haven't researched much further but it looks like some other area of the code currently has to accommodate for both cases.
• Some of the config variables / constants are very succinctly documented, or not at all. Typically TENANT_PATH_PREFIX, TENANT_URL_RE, QWC_SERVICE_PREFIX, QWC_TENANT... Would it be possible to document them properly? It's currently very hard to know what they do, when/where/how they should be set, and with what syntax, in particular regarding slashes (e.g. QWC_SERVICE_PREFIX should start with a slash and can optionally end with one and can also be empty, QWC_TENANT however shouldn't contain slashes).

Many thanks in advance!

[manisandro]

The subdomain approach is indeed seldom used, and hence hasn't received the same focus as the path approach. But it would be great to polish its support and add a example to the Documentation.

Regarding your fix: there is indeed an inconsistency here, as a matter of fact consumers of tenant_path_prefix are using

app.session_interface.tenant_path_prefix().rstrip("/") + "/"

So I believe the correct fix is to also append the trailing slash to the multi-tenant path, which would make the above "workarounds" unnecessary.

[qwenger]

This way of fixing it looks good to me. Would you consider integrating those changes? Or should I create a pull request myself?

[manisandro]

Please do create a PR