Skip to content

Commit

Permalink
fix: more explicit domain check (#18348)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Nov 2, 2023
1 parent 9db468b commit 7d2797f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@ import { cleanedCookieSubdomain } from 'scenes/authentication/redirectToLoggedIn
describe('redirectToLoggedInInstance', () => {
test.each([
['handles null', null, null],
['handles EU', 'https://eu.posthog.com', 'eu'],
['handles US', 'https://app.posthog.com', 'app'],
['handles leading quotes', '"https://eu.posthog.com', 'eu'],
['handles trailing quotes', 'https://eu.posthog.com"', 'eu'],
['handles wrapping quotes', '"https://eu.posthog.com"', 'eu'],
['handles the empty string', '', null],
['handles the sneaky string', ' ', null],
['handles not URLs', 'yo ho ho', null],
['handles EU', 'https://eu.posthog.com', 'EU'],
['handles US', 'https://app.posthog.com', 'US'],
['handles leading quotes', '"https://eu.posthog.com', 'EU'],
['handles trailing quotes', 'https://eu.posthog.com"', 'EU'],
['handles wrapping quotes', '"https://eu.posthog.com"', 'EU'],
['handles ports', 'https://app.posthog.com:8123', 'US'],
['handles longer urls', 'https://app.posthog.com:1234?query=parameter#hashParam', 'US'],
])('%s', (_name, cookie, expected) => {
expect(cleanedCookieSubdomain(cookie)).toEqual(expected)
})
Expand Down
50 changes: 22 additions & 28 deletions frontend/src/scenes/authentication/redirectToLoggedInInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,23 @@ const PH_CURRENT_INSTANCE = 'ph_current_instance'

const REDIRECT_TIMEOUT = 2500

const SUBDOMAIN_TO_NAME = {
eu: 'EU',
app: 'US',
} as const

export function cleanedCookieSubdomain(loggedInInstance: string | null): string | null {
export function cleanedCookieSubdomain(loggedInInstance: string | null): 'EU' | 'US' | null {
try {
// replace '"' as for some reason the cookie value is wrapped in quotes e.g. "https://eu.posthog.com"
const url = loggedInInstance?.replace(/"/g, '')
if (!url) {
return null
}

const parsedURL = new URL(url)
const host = parsedURL.host
return url ? host.split('.')[0] : null
// convert to URL, so that we can be sure we're dealing with a valid URL
const hostname = new URL(url).hostname
switch (hostname) {
case 'app.posthog.com':
return 'US'
case 'eu.posthog.com':
return 'EU'
default:
return null
}
} catch (e) {
// let's not allow errors in this code break the log-in page 🤞
captureException(e, { extra: { loggedInInstance } })
Expand All @@ -63,10 +64,6 @@ export function redirectIfLoggedInOtherInstance(): (() => void) | undefined {
return // not logged into another subdomain
}

if (!SUBDOMAIN_TO_NAME[loggedInSubdomain]) {
return // not logged into a valid subdomain
}

const loggedIntoOtherSubdomain = loggedInSubdomain !== currentSubdomain

if (loggedIntoOtherSubdomain) {
Expand All @@ -82,20 +79,17 @@ export function redirectIfLoggedInOtherInstance(): (() => void) | undefined {
window.location.assign(newUrl.href)
}

lemonToast.info(
`Redirecting to your logged-in account in the Cloud ${SUBDOMAIN_TO_NAME[loggedInSubdomain]} region`,
{
button: {
label: 'Cancel',
action: () => {
cancelClicked = true
},
lemonToast.info(`Redirecting to your logged-in account in the Cloud ${loggedInSubdomain} region`, {
button: {
label: 'Cancel',
action: () => {
cancelClicked = true
},
onClose: closeToastAction,
// we want to force the user to click the cancel button as otherwise the default close will still redirect
closeButton: false,
autoClose: REDIRECT_TIMEOUT,
}
)
},
onClose: closeToastAction,
// we want to force the user to click the cancel button as otherwise the default close will still redirect
closeButton: false,
autoClose: REDIRECT_TIMEOUT,
})
}
}

0 comments on commit 7d2797f

Please sign in to comment.