Potential security fixes

This commit is contained in:
Arunavo Ray
2025-07-17 13:41:17 +05:30
parent bde1f7b5d6
commit f83711ecd6
4 changed files with 187 additions and 5 deletions

View File

@@ -0,0 +1,85 @@
import { describe, test, expect } from "bun:test";
import { isValidRedirectUri, parseRedirectUris } from "./oauth-validation";
describe("OAuth Validation", () => {
describe("parseRedirectUris", () => {
test("parses comma-separated URIs", () => {
const result = parseRedirectUris("https://app1.com,https://app2.com, https://app3.com ");
expect(result).toEqual([
"https://app1.com",
"https://app2.com",
"https://app3.com"
]);
});
test("handles empty string", () => {
expect(parseRedirectUris("")).toEqual([]);
});
test("filters out empty values", () => {
const result = parseRedirectUris("https://app1.com,,https://app2.com,");
expect(result).toEqual(["https://app1.com", "https://app2.com"]);
});
});
describe("isValidRedirectUri", () => {
test("validates exact match", () => {
const authorizedUris = ["https://app.example.com/callback"];
expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(true);
expect(isValidRedirectUri("https://app.example.com/other", authorizedUris)).toBe(false);
});
test("validates wildcard paths", () => {
const authorizedUris = ["https://app.example.com/*"];
expect(isValidRedirectUri("https://app.example.com/", authorizedUris)).toBe(true);
expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(true);
expect(isValidRedirectUri("https://app.example.com/deep/path", authorizedUris)).toBe(true);
// Different domain should fail
expect(isValidRedirectUri("https://evil.com/callback", authorizedUris)).toBe(false);
});
test("validates protocol", () => {
const authorizedUris = ["https://app.example.com/callback"];
// HTTP instead of HTTPS should fail
expect(isValidRedirectUri("http://app.example.com/callback", authorizedUris)).toBe(false);
});
test("validates host and port", () => {
const authorizedUris = ["https://app.example.com:3000/callback"];
// Different port should fail
expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(false);
expect(isValidRedirectUri("https://app.example.com:3000/callback", authorizedUris)).toBe(true);
expect(isValidRedirectUri("https://app.example.com:4000/callback", authorizedUris)).toBe(false);
});
test("handles invalid URIs", () => {
const authorizedUris = ["not-a-valid-uri", "https://valid.com"];
// Invalid redirect URI
expect(isValidRedirectUri("not-a-valid-uri", authorizedUris)).toBe(false);
// Valid redirect URI with invalid authorized URI should still work if it matches valid one
expect(isValidRedirectUri("https://valid.com", authorizedUris)).toBe(true);
});
test("handles empty inputs", () => {
expect(isValidRedirectUri("", ["https://app.com"])).toBe(false);
expect(isValidRedirectUri("https://app.com", [])).toBe(false);
});
test("prevents open redirect attacks", () => {
const authorizedUris = ["https://app.example.com/callback"];
// Various attack vectors
expect(isValidRedirectUri("https://app.example.com.evil.com/callback", authorizedUris)).toBe(false);
expect(isValidRedirectUri("https://app.example.com@evil.com/callback", authorizedUris)).toBe(false);
expect(isValidRedirectUri("//evil.com/callback", authorizedUris)).toBe(false);
expect(isValidRedirectUri("https:evil.com/callback", authorizedUris)).toBe(false);
});
});
});

View File

@@ -0,0 +1,59 @@
/**
* Validates a redirect URI against a list of authorized URIs
* @param redirectUri The redirect URI to validate
* @param authorizedUris List of authorized redirect URIs
* @returns true if the redirect URI is authorized, false otherwise
*/
export function isValidRedirectUri(redirectUri: string, authorizedUris: string[]): boolean {
if (!redirectUri || authorizedUris.length === 0) {
return false;
}
try {
// Parse the redirect URI to ensure it's valid
const redirectUrl = new URL(redirectUri);
return authorizedUris.some(authorizedUri => {
try {
// Handle wildcard paths (e.g., https://example.com/*)
if (authorizedUri.endsWith('/*')) {
const baseUri = authorizedUri.slice(0, -2);
const baseUrl = new URL(baseUri);
// Check protocol, host, and port match
return redirectUrl.protocol === baseUrl.protocol &&
redirectUrl.host === baseUrl.host &&
redirectUrl.pathname.startsWith(baseUrl.pathname);
}
// Handle exact match
const authorizedUrl = new URL(authorizedUri);
// For exact match, everything must match including path and query params
return redirectUrl.href === authorizedUrl.href;
} catch {
// If authorized URI is not a valid URL, treat as invalid
return false;
}
});
} catch {
// If redirect URI is not a valid URL, it's invalid
return false;
}
}
/**
* Parses a comma-separated list of redirect URIs and trims whitespace
* @param redirectUrls Comma-separated list of redirect URIs
* @returns Array of trimmed redirect URIs
*/
export function parseRedirectUris(redirectUrls: string): string[] {
if (!redirectUrls) {
return [];
}
return redirectUrls
.split(',')
.map(uri => uri.trim())
.filter(uri => uri.length > 0);
}