cal.com icon indicating copy to clipboard operation
cal.com copied to clipboard

fix: allow multiple levels of subdomains in organization urls

Open victormattosvm opened this issue 1 year ago • 9 comments

What does this PR do?

It was not possible to use multiple levels of subdomains in organization URLs.

Examples: Works: org.domain.com Does not work: org.calendar.domain.com

This PR fixes this by replacing the old code snippet that checked for more than one subdomain level, returning null. Now, it doesn't have this verification and returns the correct org slug.

Mandatory Tasks (DO NOT REMOVE)

  • [x] I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • [x] N/A
  • [x] I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Create an organization with more than one subdomain level, such as org.calendar, and try accessing an event type URL. It will not be accessible. This PR resolves that issue.

victormattosvm avatar Sep 20 '24 02:09 victormattosvm

@victormattosvm is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Sep 20 '24 02:09 vercel[bot]

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 20 '24 02:09 CLAassistant

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (09/20/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (09/20/24)

1 label was added to this PR based on Keith Williams's automation.

graphite-app[bot] avatar Sep 20 '24 02:09 graphite-app[bot]

Although this PR fixes the issue with multiple subdomains, there is still a use case that it doesn't solve. In our structure, we use:

WEBAPP_URL = calendar.domain.com ORG 1 URL = client1.calendar.domain.com ORG 2 URL = client2.calendar.domain.com

The getOrgSlug function won't recognize the slug correctly if "calendar.domain.com" is added first in ALLOWED_HOSTNAMES.

In this scenario, I need (1) "domain.com" to be added as the hostname, not "calendar.domain.com", and (2) a modification in this code to return null for the slug if the requested hostname is the WEBAPP_URL (I have already done this and can submit it in this PR).

const url = new URL(WEBAPP_URL); if (hostname === url.hostname) { return null; }

It's working well in production here.

Steps to reproduce the issue:

  1. Set the WEBAPP_URL as "app.cal.local" and add "app.cal.local" to ALLOWED_HOSTNAMES
  2. Create an organization with the slug "client.app" - final url: client.app.cal.local
  3. It won't be possible to access event types links.

victormattosvm avatar Sep 20 '24 12:09 victormattosvm

This wasn’t in the original specs of the orgs feature as we wanted to avoid longer URLs.

Will pull in @PeerRich and @ciaranha to see if we want this.

keithwillcode avatar Sep 20 '24 19:09 keithwillcode

i’m not quiet sure what benefits / negative side effects this could cause

PeerRich avatar Sep 20 '24 20:09 PeerRich

I wouldn't say it's a must-do, but there are a few points to consider:

  1. The absence of this limitation is already a benefit.
  2. There is currently no validation on the slug input when creating an organization, allowing someone to add more than one subdomain level and encounter an error during setup.
  3. I don't believe that the use of multiple levels is very common, only in more specific cases like mine, where the client subscribing to the enterprise plan wants to place the application under calendar.domain.com and all their organizations under clientA.calendar.domain.com. It would be even better if clientA could map their main domain, such as calendar.clientA.com.

If opting to allow multiple levels, the differentiation of the subdomain would occur as follows:

  1. If the WEBAPP_URL exists in the hostname, everything before it is the organization's slug.
  2. If the WEBAPP_URL doesn't exist in the hostname, it takes the base domain and everything before it is the organization's slug.

This way, the user can choose how they prefer to create, whether with multiple subdomains or not.

Example:
calendar.domain.com = WEBAPP_URL
clientA.calendar.domain.com = slug would be clientA

calendar.domain.com = WEBAPP_URL
clientA-calendar.domain.com = slug would be clientA-calendar

Both methods are possible within the same application. So, at the application level, I can't see major changes to allow that. We are running it in production.

victormattosvm avatar Sep 23 '24 11:09 victormattosvm

E2E results are ready!

github-actions[bot] avatar Oct 03 '24 21:10 github-actions[bot]

This PR is being marked as stale due to inactivity.

github-actions[bot] avatar Oct 18 '24 00:10 github-actions[bot]

Hello? Any update on this topic?

victormattosvm avatar Oct 22 '24 00:10 victormattosvm