auth-backend: remove unused isCimdUrl function (#33882)
The function was a thin wrapper around validateCimdUrl that caught errors and returned a boolean. It was never imported by any production code — only by test files. Tests now use validateCimdUrl directly. Made-with: Cursor Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
@@ -17,7 +17,7 @@
|
||||
import { rest } from 'msw';
|
||||
import { setupServer } from 'msw/node';
|
||||
import { registerMswTestHooks } from '@backstage/backend-test-utils';
|
||||
import { isCimdUrl, validateCimdUrl, fetchCimdMetadata } from './CimdClient';
|
||||
import { validateCimdUrl, fetchCimdMetadata } from './CimdClient';
|
||||
import * as dns from 'node:dns/promises';
|
||||
|
||||
jest.mock('dns/promises');
|
||||
@@ -45,65 +45,6 @@ describe('CimdClient', () => {
|
||||
originalNodeEnv;
|
||||
});
|
||||
|
||||
describe('isCimdUrl', () => {
|
||||
it('should return true for valid CIMD URLs', () => {
|
||||
expect(isCimdUrl('https://example.com/oauth-metadata.json')).toBe(true);
|
||||
expect(isCimdUrl('https://example.com/path/to/metadata')).toBe(true);
|
||||
expect(
|
||||
isCimdUrl('https://sub.example.com/.well-known/oauth-client'),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for URLs without path', () => {
|
||||
expect(isCimdUrl('https://example.com')).toBe(false);
|
||||
expect(isCimdUrl('https://example.com/')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for non-HTTPS URLs on public hosts', () => {
|
||||
expect(isCimdUrl('http://example.com/metadata')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for HTTP localhost URLs (development)', () => {
|
||||
expect(
|
||||
isCimdUrl(
|
||||
'http://localhost:7007/api/auth/.well-known/oauth-client/cli',
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isCimdUrl(
|
||||
'http://127.0.0.1:7007/api/auth/.well-known/oauth-client/cli',
|
||||
),
|
||||
).toBe(true);
|
||||
expect(isCimdUrl('http://localhost/path')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for HTTP localhost URLs in production', () => {
|
||||
(process.env as Record<string, string | undefined>).NODE_ENV =
|
||||
'production';
|
||||
expect(isCimdUrl('http://localhost:7007/path')).toBe(false);
|
||||
expect(isCimdUrl('http://127.0.0.1:7007/path')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for non-URL strings', () => {
|
||||
expect(isCimdUrl('not-a-url')).toBe(false);
|
||||
expect(isCimdUrl('uuid-like-client-id')).toBe(false);
|
||||
expect(isCimdUrl('')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for URLs with query strings', () => {
|
||||
expect(isCimdUrl('https://example.com/metadata?foo=bar')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for URLs with dot path segments', () => {
|
||||
expect(isCimdUrl('https://example.com/./metadata')).toBe(false);
|
||||
expect(isCimdUrl('https://example.com/../metadata')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for URLs with fragments', () => {
|
||||
expect(isCimdUrl('https://example.com/metadata#section')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateCimdUrl', () => {
|
||||
it('should return URL for valid CIMD URLs', () => {
|
||||
const url = validateCimdUrl('https://example.com/metadata.json');
|
||||
|
||||
@@ -110,19 +110,6 @@ export function validateCimdUrl(clientId: string): URL {
|
||||
return url;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a client_id is a valid CIMD URL.
|
||||
* Requires HTTPS for production, but allows HTTP for localhost (development).
|
||||
*/
|
||||
export function isCimdUrl(clientId: string): boolean {
|
||||
try {
|
||||
validateCimdUrl(clientId);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* SSRF (Server-Side Request Forgery) Protection
|
||||
*
|
||||
|
||||
@@ -35,7 +35,7 @@ import { AuthDatabase } from '../database/AuthDatabase';
|
||||
import { OidcService } from '../service/OidcService';
|
||||
import { TokenIssuer } from '../identity/types';
|
||||
import { OfflineAccessService } from './OfflineAccessService';
|
||||
import { CimdClientInfo, isCimdUrl } from './CimdClient';
|
||||
import { CimdClientInfo, validateCimdUrl } from './CimdClient';
|
||||
|
||||
jest.mock('./CimdClient', () => {
|
||||
const actual = jest.requireActual('./CimdClient');
|
||||
@@ -1253,8 +1253,7 @@ describe('OidcRouter', () => {
|
||||
};
|
||||
mockFetchCimdMetadata.mockResolvedValue(cimdMetadata);
|
||||
|
||||
// Verify isCimdUrl works correctly
|
||||
expect(isCimdUrl(cimdClientId)).toBe(true);
|
||||
expect(() => validateCimdUrl(cimdClientId)).not.toThrow();
|
||||
|
||||
const knex = await databases.init(databaseId);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user