Skip to content

Commit 838cac5

Browse files
shikharbsarshikharbsar
authored
add special handling for my clusters (#56)
* add special handling for my clusters * update oauth-handler.js as well
1 parent 766b972 commit 838cac5

File tree

3 files changed

+142
-2
lines changed

3 files changed

+142
-2
lines changed

‎src/handlers.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class Handler {
6666
// TODO: Remove this once we have a proper way to handle this
6767
// This is a temporary fix to handle the case where the instance URL is a free trial instance URL
6868
// Since, free trial does not support IAMv2, we will assume that the user is logged in.
69-
if (instanceUrl.match(/https:\/\/team\d+\.thoughtspot\.cloud/)) {
69+
if (instanceUrl.match(/^https:\/\/(?:team|my)\d+\.thoughtspot\.cloud$/)) {
7070
const callbackUrl = new URL("/callback", origin);
7171
callbackUrl.searchParams.set("instanceUrl", instanceUrl);
7272
callbackUrl.searchParams.set(

‎static/oauth-callback.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
// TODO: Remove this once we have a proper way to handle this
3636
// This is a temporary fix to handle the case where the instance URL is a free trial instance URL
3737
// Since, free trial does not support IAMv2, the user needs to login to the free trial instance in a separate tab manually.
38-
if (base.toString().match(/https:\/\/team\d+\.thoughtspot\.cloud/)) {
38+
if (base.toString().match(/^https:\/\/(?:team|my)\d+\.thoughtspot\.cloud$/)) {
3939
freeTrialErrorSection.style.display = 'flex';
4040
const errorTextEl = document.getElementById('free-trial-error-text');
4141
errorTextEl.textContent = 'Click ';

‎test/handlers.spec.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,146 @@ describe("Handlers", () => {
446446
// Consume the response body to prevent storage cleanup issues
447447
await result.text();
448448
});
449+
450+
describe("Instance URL regex pattern matching", () => {
451+
it("should redirect to callback for team URLs with numbers", async () => {
452+
const testCases = [
453+
'https://team1.thoughtspot.cloud',
454+
'https://team2.thoughtspot.cloud',
455+
'https://team3.thoughtspot.cloud'
456+
];
457+
458+
for (const instanceUrl of testCases) {
459+
const formData = new FormData();
460+
formData.append('state', btoa(JSON.stringify({ oauthReqInfo: { clientId: 'test' } })));
461+
formData.append('instanceUrl', instanceUrl);
462+
463+
const result = await app.fetch(new Request("https://example.com/authorize", {
464+
method: 'POST',
465+
body: formData
466+
}), mockEnv);
467+
468+
expect(result.status).toBe(302);
469+
expect(result.headers.get('location')).toContain('https://example.com/callback');
470+
expect(result.headers.get('location')).toContain(`instanceUrl=${encodeURIComponent(instanceUrl)}`);
471+
}
472+
});
473+
474+
it("should redirect to callback for my URLs with numbers", async () => {
475+
const testCases = [
476+
'https://my1.thoughtspot.cloud',
477+
'https://my2.thoughtspot.cloud',
478+
'https://my3.thoughtspot.cloud',
479+
];
480+
481+
for (const instanceUrl of testCases) {
482+
const formData = new FormData();
483+
formData.append('state', btoa(JSON.stringify({ oauthReqInfo: { clientId: 'test' } })));
484+
formData.append('instanceUrl', instanceUrl);
485+
486+
const result = await app.fetch(new Request("https://example.com/authorize", {
487+
method: 'POST',
488+
body: formData
489+
}), mockEnv);
490+
491+
expect(result.status).toBe(302);
492+
expect(result.headers.get('location')).toContain('https://example.com/callback');
493+
expect(result.headers.get('location')).toContain(`instanceUrl=${encodeURIComponent(instanceUrl)}`);
494+
}
495+
});
496+
497+
it("should NOT redirect to callback for URLs that don't match the pattern", async () => {
498+
const testCases = [
499+
'https://company.thoughtspot.cloud', // no team/my prefix
500+
'https://team.thoughtspot.cloud', // no number after team
501+
'https://my.thoughtspot.cloud', // no number after my
502+
'https://teamabc.thoughtspot.cloud', // non-numeric after team
503+
'https://myabc.thoughtspot.cloud', // non-numeric after my
504+
'https://team1test.thoughtspot.cloud', // extra characters after number
505+
'https://my1test.thoughtspot.cloud', // extra characters after number
506+
'https://test-team1.thoughtspot.cloud', // prefix before team
507+
'https://test-my1.thoughtspot.cloud', // prefix before my
508+
'https://team1.test.cloud', // different domain
509+
'https://my1.test.cloud', // different domain
510+
'https://team123.thoughtspot.com', // wrong TLD
511+
'https://my123.thoughtspot.com', // wrong TLD
512+
'http://team1.thoughtspot.cloud', // http instead of https
513+
'http://my1.thoughtspot.cloud', // http instead of https
514+
];
515+
516+
for (const instanceUrl of testCases) {
517+
const formData = new FormData();
518+
formData.append('state', btoa(JSON.stringify({ oauthReqInfo: { clientId: 'test' } })));
519+
formData.append('instanceUrl', instanceUrl);
520+
521+
const result = await app.fetch(new Request("https://example.com/authorize", {
522+
method: 'POST',
523+
body: formData
524+
}), mockEnv);
525+
526+
// These should not redirect to callback (should go through SAML redirect)
527+
expect(result.status).not.toBe(400);
528+
if (result.status === 302) {
529+
const location = result.headers.get('location');
530+
// Should redirect to SAML login, not directly to callback
531+
// SAML redirects contain '/callosum/v1/saml/login'
532+
// Direct callback redirects start with 'https://example.com/callback'
533+
expect(location).not.toMatch(/^https:\/\/example\.com\/callback/);
534+
expect(location).toContain('/callosum/v1/saml/login');
535+
}
536+
}
537+
});
538+
539+
it("should verify the exact regex pattern behavior", () => {
540+
// Test the actual regex pattern used in the code
541+
const regex = /^https:\/\/(?:team|my)\d+\.thoughtspot\.cloud$/;
542+
543+
// URLs that should match
544+
const matchingUrls = [
545+
'https://team1.thoughtspot.cloud',
546+
'https://my1.thoughtspot.cloud',
547+
'https://team123.thoughtspot.cloud',
548+
'https://my456.thoughtspot.cloud',
549+
'https://team999999.thoughtspot.cloud',
550+
'https://my999999.thoughtspot.cloud',
551+
'https://team01.thoughtspot.cloud', // leading zeros match \d+
552+
'https://my01.thoughtspot.cloud',
553+
'https://team001.thoughtspot.cloud',
554+
'https://my001.thoughtspot.cloud'
555+
];
556+
557+
// URLs that should not match
558+
const nonMatchingUrls = [
559+
'https://company.thoughtspot.cloud',
560+
'https://team.thoughtspot.cloud',
561+
'https://my.thoughtspot.cloud',
562+
'https://teamabc.thoughtspot.cloud',
563+
'https://myabc.thoughtspot.cloud',
564+
'https://team1test.thoughtspot.cloud',
565+
'https://my1test.thoughtspot.cloud',
566+
'https://test-team1.thoughtspot.cloud',
567+
'https://test-my1.thoughtspot.cloud',
568+
'https://team1.test.cloud',
569+
'https://my1.test.cloud',
570+
'https://team123.thoughtspot.com',
571+
'https://my123.thoughtspot.com',
572+
'http://team1.thoughtspot.cloud',
573+
'http://my1.thoughtspot.cloud',
574+
'https://TEAM1.thoughtspot.cloud', // case sensitive
575+
'https://MY1.thoughtspot.cloud' // case sensitive
576+
];
577+
578+
// Test matching URLs
579+
for (const url of matchingUrls) {
580+
expect(regex.test(url)).toBe(true);
581+
}
582+
583+
// Test non-matching URLs
584+
for (const url of nonMatchingUrls) {
585+
expect(regex.test(url)).toBe(false);
586+
}
587+
});
588+
});
449589
});
450590

451591
describe("GET /callback", () => {

0 commit comments

Comments
 (0)