diff --git a/packages/graph-explorer/src/connections/__fixtures__/connection-file-v1.5-iam-without-service-type.json b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v1.5-iam-without-service-type.json new file mode 100644 index 000000000..c6a80b66f --- /dev/null +++ b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v1.5-iam-without-service-type.json @@ -0,0 +1,35 @@ +{ + "id": "55555555-5555-4555-8555-555555555555", + "displayLabel": "Neptune with IAM (v1.5)", + "connection": { + "url": "https://proxy.example.com", + "queryEngine": "gremlin", + "proxyConnection": true, + "graphDbUrl": "https://neptune.example.com:8182", + "awsAuthEnabled": true, + "awsRegion": "us-west-2" + }, + "schema": { + "vertices": [ + { + "type": "airport", + "displayLabel": "airport", + "total": 3, + "attributes": [ + { "name": "code", "displayLabel": "code", "dataType": "String" } + ] + } + ], + "edges": [ + { + "type": "route", + "displayLabel": "route", + "total": 2, + "attributes": [ + { "name": "dist", "displayLabel": "dist", "dataType": "Number" } + ] + } + ], + "lastUpdate": "2024-01-01T12:30:00.000Z" + } +} diff --git a/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-iam-proxy.json b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-iam-proxy.json new file mode 100644 index 000000000..58df64f99 --- /dev/null +++ b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-iam-proxy.json @@ -0,0 +1,30 @@ +{ + "id": "66666666-6666-4666-8666-666666666666", + "displayLabel": "Neptune with IAM (v3.2.2)", + "connection": { + "url": "https://proxy.example.com", + "queryEngine": "gremlin", + "proxyConnection": true, + "awsAuthEnabled": true, + "serviceType": "neptune-db", + "awsRegion": "us-west-2", + "graphDbUrl": "https://neptune.example.com:8182" + }, + "schema": { + "vertices": [ + { + "type": "airport", + "attributes": [{ "name": "code", "dataType": "String" }], + "total": 3 + } + ], + "edges": [ + { + "type": "route", + "attributes": [{ "name": "dist", "dataType": "Number" }], + "total": 2 + } + ], + "lastUpdate": "2024-01-01T12:30:00.000Z" + } +} diff --git a/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-proxy-iam-off.json b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-proxy-iam-off.json new file mode 100644 index 000000000..0e81977c1 --- /dev/null +++ b/packages/graph-explorer/src/connections/__fixtures__/connection-file-v3.2.2-proxy-iam-off.json @@ -0,0 +1,17 @@ +{ + "id": "77777777-7777-4777-8777-777777777777", + "displayLabel": "Neptune without IAM (v3.2.2)", + "connection": { + "url": "https://proxy.example.com", + "queryEngine": "gremlin", + "proxyConnection": true, + "awsAuthEnabled": false, + "serviceType": "neptune-db", + "awsRegion": "", + "graphDbUrl": "https://neptune.example.com:8182" + }, + "schema": { + "vertices": [], + "edges": [] + } +} diff --git a/packages/graph-explorer/src/connections/activeConnection.ts b/packages/graph-explorer/src/connections/activeConnection.ts index f1ffbe755..fa4fabc82 100644 --- a/packages/graph-explorer/src/connections/activeConnection.ts +++ b/packages/graph-explorer/src/connections/activeConnection.ts @@ -29,11 +29,7 @@ export const activeConnectionAtom = atom(get => { }); export const queryEngineSelector = atom(get => - get( - selectAtom(activeConnectionAtom, c => - c && c.queryEngine ? c.queryEngine : "gremlin", - ), - ), + get(selectAtom(activeConnectionAtom, c => c?.queryEngine ?? "gremlin")), ); export function useQueryEngine() { diff --git a/packages/graph-explorer/src/connections/connectionFileGoldenFiles.test.ts b/packages/graph-explorer/src/connections/connectionFileGoldenFiles.test.ts index a589a9c5a..c3d86e64c 100644 --- a/packages/graph-explorer/src/connections/connectionFileGoldenFiles.test.ts +++ b/packages/graph-explorer/src/connections/connectionFileGoldenFiles.test.ts @@ -5,6 +5,7 @@ import type { ConfigurationContextProps } from "@/core/StateProvider/typeConfigT import type { IriNamespace, RdfPrefix } from "@/utils/rdf"; import { createEdgeType, createVertexType } from "@/core/entities"; +import { logger } from "@/utils"; import { stubDocumentUrl } from "@/utils/testing"; import { exportConnectionFileText } from "@/utils/testing/exportConnectionFileText"; @@ -15,6 +16,9 @@ import exportGoldenLegacyUrlProxy from "./__fixtures__/connection-file-export-go import exportGoldenWithoutUrl from "./__fixtures__/connection-file-export-golden.txt?raw"; import legacyUrlDirect from "./__fixtures__/connection-file-legacy-url-direct.json?raw"; import legacyUrlProxy from "./__fixtures__/connection-file-legacy-url-proxy.json?raw"; +import v1_5IamWithoutServiceType from "./__fixtures__/connection-file-v1.5-iam-without-service-type.json?raw"; +import v3_2_2IamProxy from "./__fixtures__/connection-file-v3.2.2-iam-proxy.json?raw"; +import v3_2_2ProxyIamOff from "./__fixtures__/connection-file-v3.2.2-proxy-iam-off.json?raw"; import { parseConnectionFile } from "./parseConnectionFile"; /** @@ -28,6 +32,10 @@ import { parseConnectionFile } from "./parseConnectionFile"; * including the pre-unified-proxy `url`/`proxyConnection` form and legacy * pass-through keys (`__inferred`, `dataType`), and the file `main` wrote * between #1773 and #2315 with no `url` (`connection-file-export-golden.txt`). + * The `connection-file-v*` fixtures pin the IAM fields as tagged releases + * wrote them: v1.0.0 to v1.5.x wrote `awsAuthEnabled` and `awsRegion` with no + * `serviceType`, and v3.2.2 wrote every proxy connection with `serviceType` + * and `awsRegion`, even with IAM off. * The export cases compare the writer's output byte-for-byte against the * `connection-file-export-golden-legacy-url-*.txt` fixtures, so any change to * the wire format — values, field order, or whitespace — is caught here rather @@ -87,6 +95,62 @@ describe("golden Exported Connection Files import on the current build", () => { expect(connection.proxyConnection).toBe(true); }); + test("v1.5 IAM connection without serviceType keeps IAM on without a warning", () => { + const parsed = parseConnectionFile(JSON.parse(v1_5IamWithoutServiceType)); + + expect(parsed?.connection).toStrictEqual({ + url: "https://proxy.example.com", + queryEngine: "gremlin", + proxyConnection: true, + graphDbUrl: "https://neptune.example.com:8182", + awsAuthEnabled: true, + awsRegion: "us-west-2", + }); + expect(logger.warn).not.toHaveBeenCalled(); + + // v1.5 schema sync wrote displayLabel and total on each type. + expect(parsed?.schema.vertices).toStrictEqual([ + { + type: createVertexType("airport"), + displayLabel: "airport", + total: 3, + attributes: [ + { name: "code", displayLabel: "code", dataType: "String" }, + ], + }, + ]); + }); + + test("v3.2.2 IAM proxy connection keeps IAM on without a warning", () => { + const parsed = parseConnectionFile(JSON.parse(v3_2_2IamProxy)); + + expect(parsed?.connection).toStrictEqual({ + url: "https://proxy.example.com", + queryEngine: "gremlin", + proxyConnection: true, + awsAuthEnabled: true, + serviceType: "neptune-db", + awsRegion: "us-west-2", + graphDbUrl: "https://neptune.example.com:8182", + }); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + test("v3.2.2 proxy connection with IAM off keeps its empty awsRegion and serviceType", () => { + const parsed = parseConnectionFile(JSON.parse(v3_2_2ProxyIamOff)); + + expect(parsed?.connection).toStrictEqual({ + url: "https://proxy.example.com", + queryEngine: "gremlin", + proxyConnection: true, + awsAuthEnabled: false, + serviceType: "neptune-db", + awsRegion: "", + graphDbUrl: "https://neptune.example.com:8182", + }); + expect(logger.warn).not.toHaveBeenCalled(); + }); + test("legacy direct connection with only url and no graphDbUrl", () => { const parsed = parseConnectionFile(JSON.parse(legacyUrlDirect)); diff --git a/packages/graph-explorer/src/connections/defaultConnection.ts b/packages/graph-explorer/src/connections/defaultConnection.ts index 7e343ac39..d2507d351 100644 --- a/packages/graph-explorer/src/connections/defaultConnection.ts +++ b/packages/graph-explorer/src/connections/defaultConnection.ts @@ -1,3 +1,5 @@ +import type { ConnectionConfig } from "@shared/types"; + import { neptuneServiceTypeOptions, queryEngineOptions } from "@shared/types"; import { z } from "zod"; @@ -36,7 +38,7 @@ export type DefaultConnectionData = z.infer; * on failure. Throws `ReverseProxyMisconfiguredError` when the API URL can't * be resolved from the page's path. */ -export async function fetchDefaultConnection() { +export async function fetchDefaultConnection(): Promise { const url = apiUrl("defaultConnection"); try { const defaultConnection = await fetchDefaultConnectionFor(url); @@ -47,7 +49,7 @@ export async function fetchDefaultConnection() { const config = mapToConnection(defaultConnection); - if (config.connection?.queryEngine) { + if (config.connection.queryEngine) { return [config]; } @@ -59,7 +61,7 @@ export async function fetchDefaultConnection() { ...config.connection, queryEngine: queryEngine, }, - } as SavedConnection; + }; }); return configs; @@ -105,7 +107,9 @@ export async function fetchDefaultConnectionFor( } } -export function mapToConnection(data: DefaultConnectionData): SavedConnection { +export function mapToConnection( + data: DefaultConnectionData, +): SavedConnection & { connection: ConnectionConfig } { return { id: "Default Connection" as ConnectionId, displayLabel: "Default Connection", diff --git a/packages/graph-explorer/src/connections/parseConnectionFile.test.ts b/packages/graph-explorer/src/connections/parseConnectionFile.test.ts index 7470445c7..31456a1bb 100644 --- a/packages/graph-explorer/src/connections/parseConnectionFile.test.ts +++ b/packages/graph-explorer/src/connections/parseConnectionFile.test.ts @@ -1,6 +1,8 @@ import { createRandomName, createRandomUrlString } from "@shared/utils/testing"; import { describe, expect, test } from "vitest"; +import { logger } from "@/utils"; + import { parseConnectionFile } from "./parseConnectionFile"; import { createConnectionId } from "./types"; @@ -410,13 +412,15 @@ describe("parseConnectionFile", () => { expect(result?.connection.awsAuthEnabled).toBeUndefined(); }); - test("degrades an invalid awsRegion to absent while parsing the rest of the file", () => { + test("drops an invalid awsRegion, turns IAM off, and warns while parsing the rest of the file", () => { const connection = { id: createConnectionId(), connection: { - url: createRandomUrlString(), + graphDbUrl: createRandomUrlString(), queryEngine: "gremlin" as const, + awsAuthEnabled: true, awsRegion: 12345, + serviceType: "neptune-db" as const, }, schema: { vertices: [], edges: [] }, }; @@ -425,14 +429,19 @@ describe("parseConnectionFile", () => { expect(result).not.toBeNull(); expect(result?.connection.awsRegion).toBeUndefined(); + expect(result?.connection.serviceType).toBe("neptune-db"); + expect(result?.connection.awsAuthEnabled).toBe(false); + expect(logger.warn).toHaveBeenCalledOnce(); }); - test("degrades an invalid serviceType to absent while parsing the rest of the file", () => { + test("drops an invalid serviceType, turns IAM off, and warns while parsing the rest of the file", () => { const connection = { id: createConnectionId(), connection: { - url: createRandomUrlString(), + graphDbUrl: createRandomUrlString(), queryEngine: "gremlin" as const, + awsAuthEnabled: true, + awsRegion: "us-west-2", serviceType: "not-a-real-service-type", }, schema: { vertices: [], edges: [] }, @@ -442,6 +451,66 @@ describe("parseConnectionFile", () => { expect(result).not.toBeNull(); expect(result?.connection.serviceType).toBeUndefined(); + expect(result?.connection.awsRegion).toBe("us-west-2"); + expect(result?.connection.awsAuthEnabled).toBe(false); + expect(logger.warn).toHaveBeenCalledOnce(); + }); + + test("drops an invalid serviceType without a warning when IAM was not on", () => { + const connection = { + id: createConnectionId(), + connection: { + graphDbUrl: createRandomUrlString(), + queryEngine: "gremlin" as const, + serviceType: "not-a-real-service-type", + }, + schema: { vertices: [], edges: [] }, + }; + + const result = parseConnectionFile(connection); + + expect(result).not.toBeNull(); + expect(result?.connection.serviceType).toBeUndefined(); + expect(result?.connection.awsAuthEnabled).toBeUndefined(); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + test("keeps IAM on without a warning when awsRegion and serviceType are valid", () => { + const connection = { + id: createConnectionId(), + connection: { + graphDbUrl: createRandomUrlString(), + queryEngine: "gremlin" as const, + awsAuthEnabled: true, + awsRegion: "us-west-2", + serviceType: "neptune-graph" as const, + }, + schema: { vertices: [], edges: [] }, + }; + + const result = parseConnectionFile(connection); + + expect(result?.connection.awsAuthEnabled).toBe(true); + expect(result?.connection.awsRegion).toBe("us-west-2"); + expect(result?.connection.serviceType).toBe("neptune-graph"); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + test("keeps IAM on without a warning when awsRegion and serviceType are absent", () => { + const connection = { + id: createConnectionId(), + connection: { + graphDbUrl: createRandomUrlString(), + queryEngine: "gremlin" as const, + awsAuthEnabled: true, + }, + schema: { vertices: [], edges: [] }, + }; + + const result = parseConnectionFile(connection); + + expect(result?.connection.awsAuthEnabled).toBe(true); + expect(logger.warn).not.toHaveBeenCalled(); }); }); @@ -457,7 +526,8 @@ describe("parseConnectionFile", () => { * accept a file with only `url`, still needs to accept a file with only the * canonical `graphDbUrl`, and must pass legacy/unknown keys through * untouched rather than stripping or rejecting them, since downstream - * migration (`transformLegacyConnection`) depends on seeing them. + * migration (`transformLegacyConnection`) depends on seeing them. An invalid + * `awsRegion` or `serviceType` on these legacy shapes must still turn IAM off. * * DO NOT delete or weaken these tests without confirming that no exported * file in the wild can still be missing `graphDbUrl` or carrying these @@ -541,4 +611,95 @@ describe("backward compatibility: legacy url/proxyConnection shape in exported f "http://www.w3.org/1999/02/22-rdf-syntax-ns#type", ]); }); + + const legacyProxyShapes = [ + { + shape: "url only", + endpoints: () => ({ url: "https://neptune.example.com:8182" }), + }, + { + shape: "url and graphDbUrl", + endpoints: () => ({ + url: "https://proxy.example.com", + graphDbUrl: "https://neptune.example.com:8182", + }), + }, + ]; + + const invalidSigningTargets = [ + { + field: "awsRegion", + signingTarget: { awsRegion: 12345, serviceType: "neptune-db" }, + kept: { serviceType: "neptune-db" }, + }, + { + field: "serviceType", + signingTarget: { + awsRegion: "us-west-2", + serviceType: "not-a-real-service-type", + }, + kept: { awsRegion: "us-west-2" }, + }, + ]; + + describe.each(legacyProxyShapes)( + "legacy proxy file with $shape", + ({ endpoints }) => { + test.each(invalidSigningTargets)( + "drops an invalid $field, turns IAM off, and warns", + ({ signingTarget, kept }) => { + const connection = { + id: createConnectionId(), + connection: { + ...endpoints(), + proxyConnection: true, + queryEngine: "gremlin" as const, + awsAuthEnabled: true, + ...signingTarget, + }, + schema: { vertices: [], edges: [] }, + }; + + const result = parseConnectionFile(connection); + + expect(result?.connection).toStrictEqual({ + ...endpoints(), + proxyConnection: true, + queryEngine: "gremlin", + awsAuthEnabled: false, + ...kept, + }); + expect(logger.warn).toHaveBeenCalledOnce(); + }, + ); + + test.each(invalidSigningTargets)( + "drops an invalid $field without a warning when IAM was off", + ({ signingTarget, kept }) => { + const connection = { + id: createConnectionId(), + connection: { + ...endpoints(), + proxyConnection: true, + queryEngine: "gremlin" as const, + awsAuthEnabled: false, + ...signingTarget, + }, + schema: { vertices: [], edges: [] }, + }; + + const result = parseConnectionFile(connection); + + expect(result?.connection).toStrictEqual({ + ...endpoints(), + proxyConnection: true, + queryEngine: "gremlin", + awsAuthEnabled: false, + ...kept, + }); + expect(logger.warn).not.toHaveBeenCalled(); + }, + ); + }, + ); }); diff --git a/packages/graph-explorer/src/connections/parseConnectionFile.ts b/packages/graph-explorer/src/connections/parseConnectionFile.ts index a7f1db964..2688ea31b 100644 --- a/packages/graph-explorer/src/connections/parseConnectionFile.ts +++ b/packages/graph-explorer/src/connections/parseConnectionFile.ts @@ -1,12 +1,68 @@ -import { neptuneServiceTypeOptions, queryEngineOptions } from "@shared/types"; +import { + type NeptuneServiceType, + neptuneServiceTypeOptions, + queryEngineOptions, +} from "@shared/types"; import { z } from "zod"; import type { IriNamespace, RdfPrefix } from "@/utils/rdf"; import { createEdgeType, createVertexType } from "@/core/entities"; +import { logger } from "@/utils"; import type { ConnectionId } from "./types"; +const awsRegionSchema = z.string().optional(); +const serviceTypeSchema = z.enum(neptuneServiceTypeOptions).optional(); + +type WithSigningTarget = Omit & { + awsRegion?: string; + serviceType?: NeptuneServiceType; +}; + +/** + * Parses `awsRegion` and `serviceType`, dropping an invalid value. A dropped + * value also turns IAM off if it was on, since signing would otherwise fall + * back to a region or service the file never named. + */ +function disableIamForInvalidSigningTarget< + T extends { + awsAuthEnabled?: boolean; + awsRegion?: unknown; + serviceType?: unknown; + }, +>(connection: T): WithSigningTarget { + const { + awsRegion: rawAwsRegion, + serviceType: rawServiceType, + ...rest + } = connection; + const awsRegion = awsRegionSchema.safeParse(rawAwsRegion); + const serviceType = serviceTypeSchema.safeParse(rawServiceType); + + // Assigned only when defined so a missing field stays missing. + const resolved: WithSigningTarget = rest; + if (awsRegion.data !== undefined) { + resolved.awsRegion = awsRegion.data; + } + if (serviceType.data !== undefined) { + resolved.serviceType = serviceType.data; + } + + if ( + (awsRegion.success && serviceType.success) || + resolved.awsAuthEnabled !== true + ) { + return resolved; + } + + logger.warn( + "[connection-import] Unrecognized awsRegion or serviceType; importing with IAM off", + { awsRegion: rawAwsRegion, serviceType: rawServiceType }, + ); + return { ...resolved, awsAuthEnabled: false }; +} + const attributesSchema = z .array(z.looseObject({ name: z.string().min(1) })) .optional() @@ -58,11 +114,10 @@ const exportedConnectionFileSchema = z.looseObject({ // connection, and a stray truthy value would make the Proxy Server // sign outbound requests with its own IAM credentials. awsAuthEnabled: z.boolean().optional().catch(undefined), - awsRegion: z.string().optional().catch(undefined), - serviceType: z - .enum(neptuneServiceTypeOptions) - .optional() - .catch(undefined), + // Resolved by `disableIamForInvalidSigningTarget`, which must see + // whether a value was dropped. + awsRegion: z.unknown().optional(), + serviceType: z.unknown().optional(), }) // Requires at least one of the canonical or legacy endpoint fields to be // present. This does not guarantee a non-empty `graphDbUrl` after @@ -73,7 +128,8 @@ const exportedConnectionFileSchema = z.looseObject({ { error: "connection must have a graphDbUrl or url", }, - ), + ) + .transform(disableIamForInvalidSigningTarget), schema: z.looseObject({ vertices: z.array( z.looseObject({ diff --git a/packages/graph-explorer/src/core/StateProvider/activeConnectionStorage.test.ts b/packages/graph-explorer/src/core/StateProvider/activeConnectionStorage.test.ts index e696e2eda..8b9084ca6 100644 --- a/packages/graph-explorer/src/core/StateProvider/activeConnectionStorage.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/activeConnectionStorage.test.ts @@ -2,6 +2,8 @@ import { createStore } from "jotai"; import localForage from "localforage"; import { beforeEach, describe, expect, test } from "vitest"; +import type { ConnectionId } from "@/connections"; + import { createConnectionId } from "@/connections"; import { readPersistedValue } from "@/utils/testing"; @@ -32,7 +34,7 @@ async function openTab() { */ subscribe: () => store.sub(atom, () => {}), /** Activates a connection; resolves once the breadcrumb has landed. */ - activate: (id: ReturnType | null) => { + activate: (id: ConnectionId | null) => { store.set(atom, id); return persistenceStatusStore.waitForIdle(); },