Validate tenant id for OAuth flow

elliotdickison commented 1 year ago

Validate tenant id for OAuth flow

Problem

We allow different tenants to access our FusionAuth instance's oauth flow via different domain names (e.g. tenant A's users can sign in at tenanta.com/oauth/authorize and tenant B's users can sign in via tenantb.com/oauth/authorize). Each tenant also has their own theme for a full whitelabel experience.

This worked well, but we wanted to isolate the different domains so that they could only be used for the intended tenant. To do this we setup a reverse proxy with Caddy that adds the X-FusionAuth-TenantId header to every request based on the domain name used. If the request comes from tenanta.com then it's forwarded to our fusionauth instance with X-FusionAuth-TenantId set to tenant a's uuid.

We expected this to result in an error if someone attempted to sign into one of tenant b's clients from tenant a's domain because there would be a mismatch between the client and tenant provided. This is not the case though - the sign in page for tenant b's client can be loaded under tenanta.com even though the client id does not belong to the specified tenant id. We tried using the tenantId query parameter instead of the X-FusionAuth-TenantId header but got the same result.

Solution

It would be great if the oauth flow validated that the tenant specified via X-FusionAuth-TenantId actually owns the client.

Alternatives/workarounds

I haven't found a foolproof one. I experimented with updating our theme like this:

[#macro html]
<!DOCTYPE html>
<html lang="en">
  [#-- If the X-FusionAuth-TenantId header is set then make sure it matches the
       current tenant. --]
  [#local queryString = request.getQueryString()!""]
  [#if !queryString?contains("tenantId=") || queryString?contains("tenantId=${tenant.id}")]
    [#nested/]
  [#else]
    <body>
      Bad Request
    </body>
  [/#if]
</html>
[/#macro]

This isn't ideal for a couple reasons:

we either have to require the tenantId query param always (tricky) or else anyone can just remove the tenantId query string to get back to the undesired state of loading one tenant's sign in page under another's URL
the client gets a 200 with a lame unstyled error message

Additional context

Just to clarify this is about reducing potential confusion and being "correct", not about security.

Making sure Authorized request origin URLs is configured correctly for each tenant's application seems to prevent sign in attempts from the wrong domain from working, but it doesn't affect the ability to load the sign in page for one tenant's client under another tenant's domain name.

Community guidelines

All issues filed in this repository must abide by the FusionAuth community guidelines.

How to vote

Please give us a thumbs up or thumbs down as a reaction to help us prioritize this feature. Feel free to comment if you have a particular need or comment on how this feature should work.

robotdan commented 1 year ago

This header X-FusionAuth-TenantId is only for APIs, it is not documented as applicable for the OAuth2 endpoints as far as I know.

Instead can you just append tenantId={tenantId} to the query string if you're already going through Caddy?

elliotdickison commented 1 year ago

Makes sense. The tenantId parameter doesn’t have to have any affect if the client ID is provided, it’s just ignored.

On July 9, 2023, David Daniel @.***> wrote:

This header X-FusionAuth-TenantId is only for APIs, it is not documented as applicable for the OAuth2 endpoints as far as I know.

Instead can you just append tenantId={tenantId} to the query string if you're already going through Caddy?

— Reply to this email directly, view it on GitHub https://github.com/FusionAuth/fusionauth- issues/issues/2347#issuecomment-1627981624, or unsubscribe https://github.com/notifications/unsubscribe- auth/AATIEHUABKBGIQYBE773WX3XPNSJLANCNFSM6AAAAAAZXUE7XU. You are receiving this because you authored the thread.Message ID: @.***>

-- Sent with HEY https://hey.com/sent

FusionAuth / fusionauth-issues