mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-21 22:31:46 +00:00
fix(naver-import): address PR #495 review issues
- SSRF: validate user-supplied URLs with checkSsrf() before fetch in both importNaverList and importGoogleList; upgrade naver.me substring check to exact hostname comparison to prevent bypass - i18n: add missing places.importNaverList key to de.ts and es.ts - migration: switch Naver addon seed to INSERT OR IGNORE to preserve admin customizations on re-runs; restore budget_category_order CREATE TABLE to its original formatting - route: remove redundant cast after type-narrowing guard in naver-list handler - component: hoist provider ternary above try/catch in handleListImport - tests: add four new Naver import cases (502, empty list, no-coords, canonical URL skipping redirect fetch)
This commit is contained in:
@@ -86,8 +86,8 @@ const PlacesSidebar = React.memo(function PlacesSidebar({
|
|||||||
const handleListImport = async () => {
|
const handleListImport = async () => {
|
||||||
if (!listImportUrl.trim()) return
|
if (!listImportUrl.trim()) return
|
||||||
setListImportLoading(true)
|
setListImportLoading(true)
|
||||||
|
const provider = listImportProvider === 'naver' && isNaverListImportEnabled ? 'naver' : 'google'
|
||||||
try {
|
try {
|
||||||
const provider = listImportProvider === 'naver' && isNaverListImportEnabled ? 'naver' : 'google'
|
|
||||||
const result = provider === 'google'
|
const result = provider === 'google'
|
||||||
? await placesApi.importGoogleList(tripId, listImportUrl.trim())
|
? await placesApi.importGoogleList(tripId, listImportUrl.trim())
|
||||||
: await placesApi.importNaverList(tripId, listImportUrl.trim())
|
: await placesApi.importNaverList(tripId, listImportUrl.trim())
|
||||||
@@ -105,7 +105,6 @@ const PlacesSidebar = React.memo(function PlacesSidebar({
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
const provider = listImportProvider === 'naver' && isNaverListImportEnabled ? 'naver' : 'google'
|
|
||||||
toast.error(err?.response?.data?.error || t(provider === 'google' ? 'places.googleListError' : 'places.naverListError'))
|
toast.error(err?.response?.data?.error || t(provider === 'google' ? 'places.googleListError' : 'places.naverListError'))
|
||||||
} finally {
|
} finally {
|
||||||
setListImportLoading(false)
|
setListImportLoading(false)
|
||||||
|
|||||||
@@ -899,6 +899,7 @@ const de: Record<string, string | { name: string; category: string }[]> = {
|
|||||||
'places.gpxError': 'GPX-Import fehlgeschlagen',
|
'places.gpxError': 'GPX-Import fehlgeschlagen',
|
||||||
'places.importList': 'Listenimport',
|
'places.importList': 'Listenimport',
|
||||||
'places.importGoogleList': 'Google Liste',
|
'places.importGoogleList': 'Google Liste',
|
||||||
|
'places.importNaverList': 'Naver Liste',
|
||||||
'places.googleListHint': 'Geteilten Google Maps Listen-Link einfügen, um alle Orte zu importieren.',
|
'places.googleListHint': 'Geteilten Google Maps Listen-Link einfügen, um alle Orte zu importieren.',
|
||||||
'places.googleListImported': '{count} Orte aus "{list}" importiert',
|
'places.googleListImported': '{count} Orte aus "{list}" importiert',
|
||||||
'places.googleListError': 'Google Maps Liste konnte nicht importiert werden',
|
'places.googleListError': 'Google Maps Liste konnte nicht importiert werden',
|
||||||
|
|||||||
@@ -870,6 +870,7 @@ const es: Record<string, string> = {
|
|||||||
'places.gpxError': 'Error al importar GPX',
|
'places.gpxError': 'Error al importar GPX',
|
||||||
'places.importList': 'Importar lista',
|
'places.importList': 'Importar lista',
|
||||||
'places.importGoogleList': 'Lista Google',
|
'places.importGoogleList': 'Lista Google',
|
||||||
|
'places.importNaverList': 'Lista Naver',
|
||||||
'places.googleListHint': 'Pega un enlace compartido de una lista de Google Maps para importar todos los lugares.',
|
'places.googleListHint': 'Pega un enlace compartido de una lista de Google Maps para importar todos los lugares.',
|
||||||
'places.googleListImported': '{count} lugares importados de "{list}"',
|
'places.googleListImported': '{count} lugares importados de "{list}"',
|
||||||
'places.googleListError': 'Error al importar la lista de Google Maps',
|
'places.googleListError': 'Error al importar la lista de Google Maps',
|
||||||
|
|||||||
@@ -868,39 +868,20 @@ function runMigrations(db: Database.Database): void {
|
|||||||
// Migration: Budget category ordering
|
// Migration: Budget category ordering
|
||||||
() => {
|
() => {
|
||||||
db.exec(`
|
db.exec(`
|
||||||
CREATE TABLE IF NOT EXISTS budget_category_order
|
CREATE TABLE IF NOT EXISTS budget_category_order (
|
||||||
(
|
trip_id INTEGER NOT NULL REFERENCES trips(id) ON DELETE CASCADE,
|
||||||
trip_id
|
|
||||||
INTEGER
|
|
||||||
NOT
|
|
||||||
NULL
|
|
||||||
REFERENCES
|
|
||||||
trips
|
|
||||||
(
|
|
||||||
id
|
|
||||||
) ON DELETE CASCADE,
|
|
||||||
category TEXT NOT NULL,
|
category TEXT NOT NULL,
|
||||||
sort_order INTEGER NOT NULL DEFAULT 0,
|
sort_order INTEGER NOT NULL DEFAULT 0,
|
||||||
PRIMARY KEY
|
PRIMARY KEY (trip_id, category)
|
||||||
(
|
);
|
||||||
trip_id,
|
|
||||||
category
|
|
||||||
)
|
|
||||||
);
|
|
||||||
`);
|
`);
|
||||||
// Seed existing categories with alphabetical order
|
// Seed existing categories with alphabetical order
|
||||||
const rows = db.prepare('SELECT DISTINCT trip_id, category FROM budget_items ORDER BY trip_id, category').all() as {
|
const rows = db.prepare('SELECT DISTINCT trip_id, category FROM budget_items ORDER BY trip_id, category').all() as { trip_id: number; category: string }[];
|
||||||
trip_id: number;
|
|
||||||
category: string
|
|
||||||
}[];
|
|
||||||
const ins = db.prepare('INSERT OR IGNORE INTO budget_category_order (trip_id, category, sort_order) VALUES (?, ?, ?)');
|
const ins = db.prepare('INSERT OR IGNORE INTO budget_category_order (trip_id, category, sort_order) VALUES (?, ?, ?)');
|
||||||
let lastTripId = -1;
|
let lastTripId = -1;
|
||||||
let idx = 0;
|
let idx = 0;
|
||||||
for (const r of rows) {
|
for (const r of rows) {
|
||||||
if (r.trip_id !== lastTripId) {
|
if (r.trip_id !== lastTripId) { lastTripId = r.trip_id; idx = 0; }
|
||||||
lastTripId = r.trip_id;
|
|
||||||
idx = 0;
|
|
||||||
}
|
|
||||||
ins.run(r.trip_id, r.category, idx++);
|
ins.run(r.trip_id, r.category, idx++);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -908,23 +889,9 @@ function runMigrations(db: Database.Database): void {
|
|||||||
() => {
|
() => {
|
||||||
try {
|
try {
|
||||||
db.prepare(`
|
db.prepare(`
|
||||||
INSERT INTO addons (id, name, description, type, icon, enabled, sort_order)
|
INSERT OR IGNORE INTO addons (id, name, description, type, icon, enabled, sort_order)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||||
ON CONFLICT(id) DO UPDATE SET
|
`).run('naver_list_import', 'Naver List Import', 'Import places from shared Naver Maps lists', 'trip', 'Link2', 0, 13);
|
||||||
name = excluded.name,
|
|
||||||
description = excluded.description,
|
|
||||||
type = excluded.type,
|
|
||||||
icon = excluded.icon,
|
|
||||||
sort_order = excluded.sort_order
|
|
||||||
`).run(
|
|
||||||
'naver_list_import',
|
|
||||||
'Naver List Import',
|
|
||||||
'Import places from shared Naver Maps lists',
|
|
||||||
'trip',
|
|
||||||
'Link2',
|
|
||||||
0,
|
|
||||||
13,
|
|
||||||
);
|
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
console.warn('[migrations] Non-fatal migration step failed:', err);
|
console.warn('[migrations] Non-fatal migration step failed:', err);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -123,10 +123,8 @@ router.post('/import/naver-list', authenticate, requireTripAccess, async (req: R
|
|||||||
return res.status(result.status).json({ error: result.error });
|
return res.status(result.status).json({ error: result.error });
|
||||||
}
|
}
|
||||||
|
|
||||||
const successResult = result as { places: any[]; listName: string };
|
res.status(201).json({ places: result.places, count: result.places.length, listName: result.listName });
|
||||||
|
for (const place of result.places) {
|
||||||
res.status(201).json({ places: successResult.places, count: successResult.places.length, listName: successResult.listName });
|
|
||||||
for (const place of successResult.places) {
|
|
||||||
broadcast(tripId, 'place:created', { place }, req.headers['x-socket-id'] as string);
|
broadcast(tripId, 'place:created', { place }, req.headers['x-socket-id'] as string);
|
||||||
}
|
}
|
||||||
} catch (err: unknown) {
|
} catch (err: unknown) {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { XMLParser } from 'fast-xml-parser';
|
import { XMLParser } from 'fast-xml-parser';
|
||||||
import { db, getPlaceWithTags } from '../db/database';
|
import { db, getPlaceWithTags } from '../db/database';
|
||||||
import { loadTagsByPlaceIds } from './queryHelpers';
|
import { loadTagsByPlaceIds } from './queryHelpers';
|
||||||
|
import { checkSsrf } from '../utils/ssrfGuard';
|
||||||
import { Place } from '../types';
|
import { Place } from '../types';
|
||||||
|
|
||||||
interface PlaceWithCategory extends Place {
|
interface PlaceWithCategory extends Place {
|
||||||
@@ -309,6 +310,10 @@ export async function importGoogleList(tripId: string, url: string) {
|
|||||||
let listId: string | null = null;
|
let listId: string | null = null;
|
||||||
let resolvedUrl = url;
|
let resolvedUrl = url;
|
||||||
|
|
||||||
|
// SSRF guard: validate user-supplied URL before fetching
|
||||||
|
const ssrf = await checkSsrf(url);
|
||||||
|
if (!ssrf.allowed) return { error: 'URL is not allowed', status: 400 };
|
||||||
|
|
||||||
// Follow redirects for short URLs (maps.app.goo.gl, goo.gl)
|
// Follow redirects for short URLs (maps.app.goo.gl, goo.gl)
|
||||||
if (url.includes('goo.gl') || url.includes('maps.app')) {
|
if (url.includes('goo.gl') || url.includes('maps.app')) {
|
||||||
const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
|
const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
|
||||||
@@ -416,8 +421,14 @@ export async function importNaverList(
|
|||||||
let resolvedUrl = url;
|
let resolvedUrl = url;
|
||||||
const limit = 20;
|
const limit = 20;
|
||||||
|
|
||||||
|
// SSRF guard: validate user-supplied URL before fetching
|
||||||
|
const ssrf = await checkSsrf(url);
|
||||||
|
if (!ssrf.allowed) return { error: 'URL is not allowed', status: 400 };
|
||||||
|
|
||||||
// Resolve naver.me short links to the canonical map.naver.com folder URL.
|
// Resolve naver.me short links to the canonical map.naver.com folder URL.
|
||||||
if (url.includes('naver.me')) {
|
let parsedUrl: URL;
|
||||||
|
try { parsedUrl = new URL(url); } catch { return { error: 'Invalid URL', status: 400 }; }
|
||||||
|
if (parsedUrl.hostname === 'naver.me') {
|
||||||
const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
|
const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
|
||||||
resolvedUrl = redirectRes.url;
|
resolvedUrl = redirectRes.url;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -603,6 +603,106 @@ describe('Naver list import', () => {
|
|||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
expect(res.body.error).toContain('Could not extract folder ID');
|
expect(res.body.error).toContain('Could not extract folder ID');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('POST /import/naver-list returns 502 when Naver API is unavailable', async () => {
|
||||||
|
const { user } = createUser(testDb);
|
||||||
|
const trip = createTrip(testDb, user.id);
|
||||||
|
const folderId = 'abc123';
|
||||||
|
|
||||||
|
testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'naver_list_import'").run();
|
||||||
|
|
||||||
|
const fetchMock = vi.fn()
|
||||||
|
.mockResolvedValueOnce({ ok: false });
|
||||||
|
|
||||||
|
vi.stubGlobal('fetch', fetchMock);
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/trips/${trip.id}/places/import/naver-list`)
|
||||||
|
.set('Cookie', authCookie(user.id))
|
||||||
|
.send({ url: `https://map.naver.com/v5/favorite/myPlace/folder/${folderId}` });
|
||||||
|
|
||||||
|
expect(res.status).toBe(502);
|
||||||
|
expect(res.body.error).toContain('Failed to fetch list from Naver Maps');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('POST /import/naver-list returns 400 when list is empty', async () => {
|
||||||
|
const { user } = createUser(testDb);
|
||||||
|
const trip = createTrip(testDb, user.id);
|
||||||
|
const folderId = 'abc123';
|
||||||
|
|
||||||
|
testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'naver_list_import'").run();
|
||||||
|
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ folder: { name: 'Empty List', bookmarkCount: 0 }, bookmarkList: [] }),
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.stubGlobal('fetch', fetchMock);
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/trips/${trip.id}/places/import/naver-list`)
|
||||||
|
.set('Cookie', authCookie(user.id))
|
||||||
|
.send({ url: `https://map.naver.com/v5/favorite/myPlace/folder/${folderId}` });
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain('List is empty or could not be read');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('POST /import/naver-list returns 400 when all items lack valid coordinates', async () => {
|
||||||
|
const { user } = createUser(testDb);
|
||||||
|
const trip = createTrip(testDb, user.id);
|
||||||
|
const folderId = 'abc123';
|
||||||
|
|
||||||
|
testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'naver_list_import'").run();
|
||||||
|
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({
|
||||||
|
folder: { name: 'No Coords', bookmarkCount: 2 },
|
||||||
|
bookmarkList: [
|
||||||
|
{ name: 'Place A', px: undefined, py: undefined },
|
||||||
|
{ name: 'Place B', px: 'not-a-number', py: 'not-a-number' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.stubGlobal('fetch', fetchMock);
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/trips/${trip.id}/places/import/naver-list`)
|
||||||
|
.set('Cookie', authCookie(user.id))
|
||||||
|
.send({ url: `https://map.naver.com/v5/favorite/myPlace/folder/${folderId}` });
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain('No places with coordinates found in list');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('POST /import/naver-list accepts canonical map.naver.com URL without redirect fetch', async () => {
|
||||||
|
const { user } = createUser(testDb);
|
||||||
|
const trip = createTrip(testDb, user.id);
|
||||||
|
const folderId = 'abc123';
|
||||||
|
|
||||||
|
testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'naver_list_import'").run();
|
||||||
|
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({
|
||||||
|
folder: { name: 'Seoul', bookmarkCount: 1 },
|
||||||
|
bookmarkList: [{ name: 'Gyeongbokgung', px: 126.9770, py: 37.5796, memo: null, address: 'Sejongno Seoul' }],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.stubGlobal('fetch', fetchMock);
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/trips/${trip.id}/places/import/naver-list`)
|
||||||
|
.set('Cookie', authCookie(user.id))
|
||||||
|
.send({ url: `https://map.naver.com/v5/favorite/myPlace/folder/${folderId}` });
|
||||||
|
|
||||||
|
expect(res.status).toBe(201);
|
||||||
|
expect(res.body.count).toBe(1);
|
||||||
|
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// ─────────────────────────────────────────────────────────────────────────────
|
// ─────────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user