From a51188882b95bfa688813db0f2ed2579c8595118 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 12 Dec 2025 22:54:43 -0500 Subject: [PATCH] fix: isUrl requireHttps option is never hit (#10885) --- server/utils/oauth/OAuthInterface.test.ts | 13 +++++++ shared/utils/urls.test.ts | 45 +++++++++++++++++++++++ shared/utils/urls.ts | 6 +-- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/server/utils/oauth/OAuthInterface.test.ts b/server/utils/oauth/OAuthInterface.test.ts index c03a514257..bc4adf4b71 100644 --- a/server/utils/oauth/OAuthInterface.test.ts +++ b/server/utils/oauth/OAuthInterface.test.ts @@ -115,6 +115,19 @@ describe("OAuthInterface", () => { ); expect(result).toBe(false); }); + + it("should return false for HTTP redirect URI (security requirement)", async () => { + const httpClient = { + ...client, + redirectUris: ["http://example.com/callback"], + }; + const redirectUri = "http://example.com/callback"; + const result = await OAuthInterface.validateRedirectUri( + redirectUri, + httpClient + ); + expect(result).toBe(false); + }); }); describe("#validateScope", () => { diff --git a/shared/utils/urls.test.ts b/shared/utils/urls.test.ts index 0920117099..739f6327c6 100644 --- a/shared/utils/urls.test.ts +++ b/shared/utils/urls.test.ts @@ -22,6 +22,51 @@ describe("isUrl", () => { true ); }); + + describe("requireHttps option", () => { + it("should reject HTTP URLs when requireHttps is true", () => { + expect( + urlsUtils.isUrl("http://example.com", { requireHttps: true }) + ).toBe(false); + expect( + urlsUtils.isUrl("http://example.com/callback", { requireHttps: true }) + ).toBe(false); + expect( + urlsUtils.isUrl("http://localhost:3000/auth", { requireHttps: true }) + ).toBe(false); + }); + + it("should accept HTTPS URLs when requireHttps is true", () => { + expect( + urlsUtils.isUrl("https://example.com", { requireHttps: true }) + ).toBe(true); + expect( + urlsUtils.isUrl("https://example.com/callback", { requireHttps: true }) + ).toBe(true); + expect( + urlsUtils.isUrl("https://localhost:3000/auth", { requireHttps: true }) + ).toBe(true); + }); + + it("should accept HTTP URLs when requireHttps is false or not specified", () => { + expect(urlsUtils.isUrl("http://example.com")).toBe(true); + expect( + urlsUtils.isUrl("http://example.com", { requireHttps: false }) + ).toBe(true); + }); + + it("should allow custom protocols when requireHttps is true", () => { + expect( + urlsUtils.isUrl("seafile://openfile", { requireHttps: true }) + ).toBe(true); + expect(urlsUtils.isUrl("figma://launch", { requireHttps: true })).toBe( + true + ); + expect(urlsUtils.isUrl("myapp://callback", { requireHttps: true })).toBe( + true + ); + }); + }); }); describe("isBase64Url", () => { diff --git a/shared/utils/urls.ts b/shared/utils/urls.ts index a822db396a..6fea255ba4 100644 --- a/shared/utils/urls.ts +++ b/shared/utils/urls.ts @@ -131,12 +131,12 @@ export function isUrl( if (blockedProtocols.includes(url.protocol)) { return false; } - if (url.hostname) { - return true; - } if (requireHttps && url.protocol === "http:") { return false; } + if (url.hostname) { + return true; + } return ( url.protocol !== "" &&