Skip to content

Commit 74db582

Browse files
committed
Warn about a few login scenarios that might not work
https://harperdb.atlassian.net/browse/STUDIO-242
1 parent db866e6 commit 74db582

File tree

3 files changed

+118
-1
lines changed

3 files changed

+118
-1
lines changed

src/features/auth/ClusterInstanceSignIn.tsx

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { getClusterInfoQueryOptions } from '@/features/cluster/queries/getCluste
1313
import { useInstanceLoginMutation } from '@/features/instance/operations/mutations/useInstanceLoginMutation';
1414
import { getInstanceUserInfo } from '@/features/instance/operations/queries/getInstanceUserInfo';
1515
import { authStore, OverallAppSignIn } from '@/lib/authStore';
16+
import { CrossLocalhostIssueType, detectCrossLocalhostUrls } from '@/lib/urls/detectCrossLocalhostUrls';
1617
import { getOperationsUrlForCluster } from '@/lib/urls/getOperationsUrlForCluster';
1718
import { getOperationsUrlForInstance } from '@/lib/urls/getOperationsUrlForInstance';
1819
import { queryKeys } from '@/react-query/constants';
@@ -42,6 +43,7 @@ export function ClusterInstanceSignIn() {
4243
const navigate = useNavigate();
4344
const router = useRouter();
4445
const queryClient = useQueryClient();
46+
const { redirect } = useSearch({ strict: false });
4547
const { clusterId, instanceId }: { instanceId?: string; clusterId?: string; } = useParams({ strict: false });
4648
const { data: cluster } = useQuery(
4749
getClusterInfoQueryOptions(clusterId, true),
@@ -61,8 +63,12 @@ export function ClusterInstanceSignIn() {
6163
}
6264
return null;
6365
}, [cluster, instance]);
66+
6467
const instanceClient = useInstanceClient(operationsUrl);
65-
const { redirect } = useSearch({ strict: false });
68+
const warnAboutLoginCookieIssues = useMemo(
69+
() => detectCrossLocalhostUrls(navigator.userAgent, location.hostname, operationsUrl),
70+
[operationsUrl],
71+
);
6672

6773
const form = useForm<z.infer<typeof SignInSchema>>({
6874
resolver: zodResolver(SignInSchema),
@@ -158,6 +164,19 @@ export function ClusterInstanceSignIn() {
158164
<Button disabled={isPending} type="submit" variant="submit" className="w-full my-2 rounded-full">
159165
Sign In
160166
</Button>
167+
{warnAboutLoginCookieIssues === CrossLocalhostIssueType.MixedLoopback && (
168+
<div className="p-4 mt-4 text-sm text-yellow-800 rounded-lg bg-yellow-50 dark:bg-gray-800 dark:text-yellow-300" role="alert">
169+
<span className="font-medium">Warning!</span> Your login might not work because
170+
you're mixing 127.0.0.1 and localhost. Pick one or the other.
171+
</div>
172+
)}
173+
{warnAboutLoginCookieIssues === CrossLocalhostIssueType.InsecureCookieOutsideChromeAndFirefox && (
174+
<div className="p-4 mt-4 text-sm text-yellow-800 rounded-lg bg-yellow-50 dark:bg-gray-800 dark:text-yellow-300" role="alert">
175+
<span className="font-medium">Warning!</span> Your login might not work because your
176+
browser doesn't consider localhost to be secure, so it doesn't pass the cookies
177+
along. Firefox or Chromium based browsers should pass the cookies properly.
178+
</div>
179+
)}
161180
</form>
162181
</Form>
163182
</div>
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { CrossLocalhostIssueType, detectCrossLocalhostUrls } from './detectCrossLocalhostUrls';
3+
4+
describe('detectCrossLocalhostUrls', () => {
5+
const chrome = 'Chrome';
6+
const firefox = 'Firefox';
7+
const safari = 'Safari';
8+
9+
it('returns null when operationsUrl is null', () => {
10+
expect(detectCrossLocalhostUrls(chrome, 'localhost', null)).toBe(null);
11+
});
12+
13+
it('returns null when operationsUrl is undefined', () => {
14+
expect(detectCrossLocalhostUrls(chrome, 'localhost', undefined)).toBe(null);
15+
});
16+
17+
it('returns null when operationsUrl is empty string', () => {
18+
expect(detectCrossLocalhostUrls(chrome, 'localhost', '')).toBe(null);
19+
});
20+
21+
it('returns null when browserHostname is missing', () => {
22+
expect(detectCrossLocalhostUrls(chrome, undefined, 'http://localhost:3000')).toBe(null);
23+
});
24+
25+
it('returns null when browserHostname is not localhost or 127.0.0.1', () => {
26+
expect(detectCrossLocalhostUrls(chrome, 'example.com', 'http://localhost:3000')).toBe(null);
27+
expect(detectCrossLocalhostUrls(chrome, 'my.dev', 'http://127.0.0.1:3000')).toBe(null);
28+
});
29+
30+
it('returns true when browser is localhost and operationsUrl is 127.0.0.1', () => {
31+
expect(detectCrossLocalhostUrls(chrome, 'localhost', 'http://127.0.0.1:3000')).toBe(CrossLocalhostIssueType.MixedLoopback);
32+
});
33+
34+
it('returns true when browser is 127.0.0.1 and operationsUrl is localhost', () => {
35+
expect(detectCrossLocalhostUrls(chrome, '127.0.0.1', 'http://localhost:8080')).toBe(CrossLocalhostIssueType.MixedLoopback);
36+
});
37+
38+
it('returns null when both hostnames match (localhost)', () => {
39+
expect(detectCrossLocalhostUrls(chrome, 'localhost', 'http://localhost:8080')).toBe(null);
40+
});
41+
42+
it('returns null when both hostnames match (127.0.0.1)', () => {
43+
expect(detectCrossLocalhostUrls(chrome, '127.0.0.1', 'http://127.0.0.1:8080')).toBe(null);
44+
});
45+
46+
it('ignores port differences and only compares hostname', () => {
47+
// Hostnames differ -> true
48+
expect(detectCrossLocalhostUrls(chrome, 'localhost', 'http://127.0.0.1:1234')).toBe(CrossLocalhostIssueType.MixedLoopback);
49+
// Hostnames same -> null even if ports different
50+
expect(detectCrossLocalhostUrls(chrome, 'localhost', 'http://localhost:1234')).toBe(null);
51+
});
52+
53+
it('returns true when connecting from the cloud to local outside Chrome and Firefox', () => {
54+
expect(detectCrossLocalhostUrls(chrome, 'prod.com', 'http://127.0.0.1:8080')).toBe(null);
55+
expect(detectCrossLocalhostUrls(firefox, 'prod.com', 'http://127.0.0.1:8080')).toBe(null);
56+
expect(detectCrossLocalhostUrls(safari, 'prod.com', 'http://127.0.0.1:8080')).toBe(CrossLocalhostIssueType.InsecureCookieOutsideChromeAndFirefox);
57+
});
58+
});
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
const localHostnames = [
2+
'localhost',
3+
'127.0.0.1',
4+
];
5+
6+
export const enum CrossLocalhostIssueType {
7+
MixedLoopback = 'MixedLoopback',
8+
InsecureCookieOutsideChromeAndFirefox = 'InsecureCookieOutsideChromeAndFirefox',
9+
}
10+
11+
export function detectCrossLocalhostUrls(
12+
userAgent: string | undefined,
13+
browserHostname: string | undefined,
14+
operationsUrl: string | undefined | null,
15+
): CrossLocalhostIssueType | null {
16+
if (!operationsUrl || !browserHostname || !userAgent) {
17+
return null;
18+
}
19+
const serverHostname = new URL(operationsUrl).hostname;
20+
const browserIsLocal = localHostnames.includes(browserHostname);
21+
const serverIsLocal = localHostnames.includes(serverHostname);
22+
23+
if (browserIsLocal && serverIsLocal && serverHostname !== browserHostname) {
24+
// When running Studio locally and self-managing locally, warn when we mix localhost and 127.0.0.1.
25+
return CrossLocalhostIssueType.MixedLoopback;
26+
}
27+
28+
if (!browserIsLocal && serverIsLocal) {
29+
const isChromium = userAgent.indexOf('Chrome') >= 0;
30+
const isFirefox = userAgent.indexOf('Firefox') >= 0;
31+
if (!isChromium && !isFirefox) {
32+
// When running Studio in the cloud, and signing in to a local instance, Chrome and Firefox allow the
33+
// cookies to cross to the insecure localhost because they consider it to be secure enough.
34+
// https://github.com/httpwg/http-extensions/issues/2605
35+
return CrossLocalhostIssueType.InsecureCookieOutsideChromeAndFirefox;
36+
}
37+
}
38+
39+
return null;
40+
}

0 commit comments

Comments
 (0)