diff --git a/src/Common/Constants.ts b/src/Common/Constants.ts index c4d0e1cd5..5f52c1d05 100644 --- a/src/Common/Constants.ts +++ b/src/Common/Constants.ts @@ -1,14 +1,3 @@ -export class EndpointsRegex { - public static readonly cassandra = [ - "AccountEndpoint=(.*).cassandra.cosmosdb.azure.com", - "HostName=(.*).cassandra.cosmos.azure.com", - ]; - public static readonly mongo = "mongodb://.*:(.*)@(.*).documents.azure.com"; - public static readonly mongoCompute = "mongodb://.*:(.*)@(.*).mongo.cosmos.azure.com"; - public static readonly sql = "AccountEndpoint=https://(.*).documents.azure.com"; - public static readonly table = "TableEndpoint=https://(.*).table.cosmos(?:db)?.azure.com"; -} - export class ApiEndpoints { public static runtimeProxy: string = "/api/RuntimeProxy"; public static guestRuntimeProxy: string = "/api/guest/RuntimeProxy"; diff --git a/src/ConfigContext.ts b/src/ConfigContext.ts index 76574a117..6a9b8f03e 100644 --- a/src/ConfigContext.ts +++ b/src/ConfigContext.ts @@ -30,6 +30,13 @@ export interface ConfigContext { allowedCassandraProxyEndpoints: ReadonlyArray; allowedMongoProxyEndpoints: ReadonlyArray; allowedParentFrameOrigins: ReadonlyArray; + SQL_DNS_ZONES: ReadonlyArray; + MONGO_DNS_ZONES: ReadonlyArray; + MONGO_COMPUTE_DNS_ZONES: ReadonlyArray; + CASSANDRA_DNS_ZONES: ReadonlyArray; + TABLE_DNS_ZONES: ReadonlyArray; + DOCUMENT_ENDPOINT_ZONES: ReadonlyArray; + GREMLIN_ENDPOINT_ZONES: ReadonlyArray; gitSha?: string; proxyPath?: string; AAD_ENDPOINT: string; @@ -81,6 +88,33 @@ let configContext: Readonly = { `^https:\\/\\/explorer\\.cosmos\\.sovcloud-api\\.sg$`, `^https:\\/\\/portal\\.sovcloud-azure\\.sg$`, ], // Webpack injects this at build time + SQL_DNS_ZONES: [ + "documents.azure.com", + "sql.cosmosdb.azure.com", + "sql.cosmos.azure.com", + "sqlx.cosmosdb.azure.com", + "sqlx.cosmos.azure.com", + "documents-staging.windows-ppe.net", + "sql.cosmosdb.windows-ppe.net", + "sql.cosmos.windows-ppe.net", + "sqlx.cosmos.windows-ppe.net", + ], + MONGO_DNS_ZONES: ["documents.azure.com", "documents-staging.windows-ppe.net"], + MONGO_COMPUTE_DNS_ZONES: ["mongo.cosmos.azure.com", "mongo.cosmos.windows-ppe.net"], + CASSANDRA_DNS_ZONES: [ + "cassandra.cosmosdb.azure.com", + "cassandra.cosmos.azure.com", + "cassandra.cosmosdb.windows-ppe.net", + "cassandra.cosmos.windows-ppe.net", + ], + TABLE_DNS_ZONES: [ + "table.cosmosdb.azure.com", + "table.cosmos.azure.com", + "table.cosmosdb.windows-ppe.net", + "table.cosmos.windows-ppe.net", + ], + DOCUMENT_ENDPOINT_ZONES: ["documents.azure.com", "documents-staging.windows-ppe.net"], + GREMLIN_ENDPOINT_ZONES: ["gremlin.cosmos.azure.com", "gremlin.cosmos.windows-ppe.net"], gitSha: process.env.GIT_SHA, hostedExplorerURL: "https://cosmos.azure.com/", AAD_ENDPOINT: "https://login.microsoftonline.com/", diff --git a/src/HostedExplorer.tsx b/src/HostedExplorer.tsx index ca28376d6..642141002 100644 --- a/src/HostedExplorer.tsx +++ b/src/HostedExplorer.tsx @@ -1,6 +1,5 @@ import { initializeIcons } from "@fluentui/react"; import { useBoolean } from "@fluentui/react-hooks"; -import { getErrorMessage } from "Common/ErrorHandlingUtils"; import { AadAuthorizationFailure } from "Platform/Hosted/Components/AadAuthorizationFailure"; import * as React from "react"; import { render } from "react-dom"; @@ -83,9 +82,9 @@ const App: React.FunctionComponent = () => { setEncryptedToken(token); setAuthType(AuthType.ConnectionString); }) - .catch((error) => { + .catch(async (error) => { logError( - `Failed to connect with connection string: ${getErrorMessage(error)}`, + `Failed to connect with connection string: ${await (error as Response).text()}`, "HostedExplorer/connectWithConnectionString", ); }); diff --git a/src/Platform/Hosted/Components/ConnectExplorer.test.tsx b/src/Platform/Hosted/Components/ConnectExplorer.test.tsx index a98a89ccc..c37f6e841 100644 --- a/src/Platform/Hosted/Components/ConnectExplorer.test.tsx +++ b/src/Platform/Hosted/Components/ConnectExplorer.test.tsx @@ -1,11 +1,31 @@ jest.mock("../../../hooks/useDirectories"); +jest.mock("../../../Common/PortalBackendClient"); +// ScenarioMonitor subscribes to web-vitals on import, and onTTFB reads a navigation timing entry that +// jsdom never produces, throwing from a timer that lands on whichever test happens to be running. +jest.mock("web-vitals"); import "@testing-library/jest-dom"; import { fireEvent, render, screen } from "@testing-library/react"; import { extractFeatures } from "Platform/Hosted/extractFeatures"; import { updateUserContext, userContext } from "UserContext"; import React from "react"; +import { fetchEncryptedToken, isAccountRestrictedForConnectionStringLogin } from "../../../Common/PortalBackendClient"; import { ConnectExplorer } from "./ConnectExplorer"; +const mockFetchEncryptedToken = fetchEncryptedToken as jest.MockedFunction; +const mockIsAccountRestricted = isAccountRestrictedForConnectionStringLogin as jest.MockedFunction< + typeof isAccountRestrictedForConnectionStringLogin +>; + +// fetchEncryptedToken rejects with the raw Response. jsdom implements no part of the fetch API, so +// there is no Response constructor to build one with. +const rejectWithResponse = (status: number, body: string) => + mockFetchEncryptedToken.mockRejectedValue({ status, text: async () => body } as Response); + +beforeEach(() => { + jest.resetAllMocks(); + mockIsAccountRestricted.mockResolvedValue(false); +}); + it("shows the connect form", () => { const connectionString = "fakeConnectionString"; const login = jest.fn(); @@ -63,3 +83,102 @@ it("hides the connection string link when feature.disableConnectionStringLogin i updateUserContext({ features: oldFeatures }); }); + +it("shows the error when the Portal Backend rejects the connection string", async () => { + // Mongo and Cassandra are the APIs that still exchange the connection string for an encrypted token. + const mongoConnectionString = "mongodb://test:key@test.documents.azure.com:10255"; + rejectWithResponse(403, "Request originated from IP 1.2.3.4 through public internet."); + + render( + , + ); + fireEvent.click(screen.getByText("Connect to your account with connection string")); + fireEvent.click(screen.getByDisplayValue("Connect")); + + expect( + await screen.findByText( + "Couldn't authenticate with Cosmos DB: Request originated from IP 1.2.3.4 through public internet.", + ), + ).toBeInTheDocument(); +}); + +it("shows a generic error when the Portal Backend fails without a message", async () => { + const mongoConnectionString = "mongodb://test:key@test.documents.azure.com:10255"; + // A failure with an empty body leaves nothing worth rendering. + rejectWithResponse(500, ""); + + render( + , + ); + fireEvent.click(screen.getByText("Connect to your account with connection string")); + fireEvent.click(screen.getByDisplayValue("Connect")); + + expect( + await screen.findByText("Failed to connect to the account. Please check the connection string and try again."), + ).toBeInTheDocument(); +}); + +it("offers the firewall help link when the Portal Backend is blocked by the account firewall", async () => { + const mongoConnectionString = "mongodb://test:key@test.documents.azure.com:10255"; + rejectWithResponse(403, "Request originated from IP 1.2.3.4 through public internet."); + + render( + , + ); + fireEvent.click(screen.getByText("Connect to your account with connection string")); + fireEvent.click(screen.getByDisplayValue("Connect")); + + expect(await screen.findByText("Allow access from Azure Portal")).toBeInTheDocument(); +}); + +it("does not offer the firewall help link for a failure the firewall did not cause", async () => { + const mongoConnectionString = "mongodb://test:key@test.documents.azure.com:10255"; + rejectWithResponse(401, "The connection string is invalid."); + + render( + , + ); + fireEvent.click(screen.getByText("Connect to your account with connection string")); + fireEvent.click(screen.getByDisplayValue("Connect")); + + expect( + await screen.findByText("Couldn't authenticate with Cosmos DB: The connection string is invalid."), + ).toBeInTheDocument(); + expect(screen.queryByText("Allow access from Azure Portal")).toBeNull(); +}); diff --git a/src/Platform/Hosted/Components/ConnectExplorer.tsx b/src/Platform/Hosted/Components/ConnectExplorer.tsx index 99ce638cd..d83637419 100644 --- a/src/Platform/Hosted/Components/ConnectExplorer.tsx +++ b/src/Platform/Hosted/Components/ConnectExplorer.tsx @@ -1,10 +1,11 @@ +import { FluentProvider, Link, MessageBar, MessageBarBody, webLightTheme } from "@fluentui/react-components"; import { useBoolean } from "@fluentui/react-hooks"; import { getErrorMessage } from "Common/ErrorHandlingUtils"; import { userContext } from "UserContext"; import * as React from "react"; import ConnectImage from "../../../../images/HdeConnectCosmosDB.svg"; -import ErrorImage from "../../../../images/error.svg"; import { AuthType } from "../../../AuthType"; +import { HttpStatusCodes } from "../../../Common/Constants"; import { fetchEncryptedToken, isAccountRestrictedForConnectionStringLogin } from "../../../Common/PortalBackendClient"; import { AccessInputMetadata } from "../../../Contracts/DataModels"; import { parseConnectionString } from "../Helpers/ConnectionStringParser"; @@ -30,6 +31,7 @@ export const ConnectExplorer: React.FunctionComponent = ({ }: Props) => { const [isFormVisible, { setTrue: showForm }] = useBoolean(false); const [errorMessage, setErrorMessage] = React.useState(""); + const [isBlockedByFirewall, setIsBlockedByFirewall] = React.useState(false); const enableConnectionStringLogin = !userContext.features.disableConnectionStringLogin; return ( @@ -46,6 +48,7 @@ export const ConnectExplorer: React.FunctionComponent = ({ onSubmit={async (event) => { event.preventDefault(); setErrorMessage(""); + setIsBlockedByFirewall(false); try { if (await isAccountRestrictedForConnectionStringLogin(connectionString)) { @@ -73,9 +76,24 @@ export const ConnectExplorer: React.FunctionComponent = ({ return; } - const encryptedToken = await fetchEncryptedToken(connectionString); - setEncryptedToken(encryptedToken); - setAuthType(AuthType.ConnectionString); + // Mongo and Cassandra go through the Portal Backend + try { + const encryptedToken = await fetchEncryptedToken(connectionString); + setEncryptedToken(encryptedToken); + setAuthType(AuthType.ConnectionString); + } catch (error) { + const errorDetails = await (error as Response).text(); + + setErrorMessage( + errorDetails + ? `Couldn't authenticate with Cosmos DB: ${errorDetails}` + : "Failed to connect to the account. Please check the connection string and try again.", + ); + // A Forbidden usually means the account firewall dropped the request. The connection + // string is exchanged by the Portal Backend rather than the browser, so the account has + // to allowlist those services. + setIsBlockedByFirewall((error as Response).status === HttpStatusCodes.Forbidden); + } }} >

Connect to your account with connection string

@@ -90,13 +108,26 @@ export const ConnectExplorer: React.FunctionComponent = ({ setConnectionString(event.target.value); }} /> - {errorMessage.length > 0 && ( - - Error notification - {errorMessage} - - )}

+ {errorMessage.length > 0 && ( + + + + {errorMessage} + {isBlockedByFirewall && ( + + Allow access from Azure Portal + + )} + + + + )}

diff --git a/src/Platform/Hosted/ConnectScreen.less b/src/Platform/Hosted/ConnectScreen.less index d4123f23f..2be2cf128 100644 --- a/src/Platform/Hosted/ConnectScreen.less +++ b/src/Platform/Hosted/ConnectScreen.less @@ -55,41 +55,19 @@ .connectExplorerContainer .connectExplorer .connectExplorerContent .inputToken::placeholder { font-style: italic; } -.connectExplorerContainer .connectExplorer .connectExplorerContent .errorDetailsInfoTooltip { - position: relative; - display: inline-block; - padding-left: 4px; - vertical-align: top; +.connectExplorerContainer .connectExplorer .connectErrorMessageBar { + /* Matches the width of the connection string input so the form stays balanced. */ + width: 308px; + margin: 0px auto 8px auto; + text-align: left; } -.connectExplorerContainer .connectExplorer .connectExplorerContent .errorDetailsInfoTooltip:hover .errorDetails { - visibility: visible; +.connectExplorerContainer .connectExplorer .connectErrorMessageBar .errorDetails { + /* Service messages are diagnostics containing unbroken URIs and ids. */ + overflow-wrap: anywhere; } -.connectExplorerContainer .connectExplorer .connectExplorerContent .errorDetailsInfoTooltip .errorDetails { - bottom: 24px; - width: 165px; - visibility: hidden; - background-color: #393939; - color: #ffffff; - position: absolute; - z-index: 1; - left: -10px; - padding: 6px; -} -.connectExplorerContainer .connectExplorer .connectExplorerContent .errorDetailsInfoTooltip .errorDetails:after { - border-width: 10px 10px 0px 10px; - bottom: -8px; - content: ""; - position: absolute; - right: 100%; - border-style: solid; - left: 12px; - width: 0; - height: 0; - border-color: #3b3b3b transparent; -} -.connectExplorerContainer .connectExplorer .connectExplorerContent .errorDetailsInfoTooltip .errorImg { - height: 14px; - width: 14px; +.connectExplorerContainer .connectExplorer .connectErrorMessageBar .errorHelpLink { + display: block; + margin-top: 8px; } .filterbtnstyle { diff --git a/src/Platform/Hosted/Helpers/ConnectionStringParser.test.ts b/src/Platform/Hosted/Helpers/ConnectionStringParser.test.ts index 823ebb115..94410eea2 100644 --- a/src/Platform/Hosted/Helpers/ConnectionStringParser.test.ts +++ b/src/Platform/Hosted/Helpers/ConnectionStringParser.test.ts @@ -1,10 +1,72 @@ +import { configContext, updateConfigContext } from "../../../ConfigContext"; import * as DataModels from "../../../Contracts/DataModels"; -import { parseConnectionString } from "./ConnectionStringParser"; +import { + buildEndpointsRegex, + dnsZoneAlternation, + parseConnectionString, + selectEndpointZone, +} from "./ConnectionStringParser"; describe("ConnectionStringParser", () => { const mockAccountName = "Test"; const mockMasterKey = "some-key"; + // Keyed by ApiKind so adding an API to the enum fails to compile here rather than silently going + // untested. + const connectionStringsByApiKind: Record = { + [DataModels.ApiKind + .SQL]: `AccountEndpoint=https://${mockAccountName}.documents.azure.com:443/;AccountKey=${mockMasterKey};`, + [DataModels.ApiKind + .MongoDB]: `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.documents.azure.com:10255`, + [DataModels.ApiKind + .MongoDBCompute]: `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.mongo.cosmos.azure.com:10255`, + [DataModels.ApiKind + .Cassandra]: `AccountEndpoint=${mockAccountName}.cassandra.cosmosdb.azure.com;AccountKey=${mockMasterKey};`, + [DataModels.ApiKind + .Table]: `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmosdb.azure.com:443/;`, + [DataModels.ApiKind + .Graph]: `AccountEndpoint=https://${mockAccountName}.documents.azure.com:443/;AccountKey=${mockMasterKey};ApiKind=Gremlin;`, + }; + + it("should parse a connection string for every api kind", () => { + Object.entries(connectionStringsByApiKind).forEach(([apiKind, connectionString]) => { + const metadata = parseConnectionString(connectionString); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(Number(apiKind)); + }); + }); + + // The parameterized tests below iterate the zone lists, so removing a zone would silently shrink the + // suite rather than fail it. Pin the expected contents so that stays visible in review. + it("should support the expected dns zones", () => { + expect(configContext.SQL_DNS_ZONES).toEqual([ + "documents.azure.com", + "sql.cosmosdb.azure.com", + "sql.cosmos.azure.com", + "sqlx.cosmosdb.azure.com", + "sqlx.cosmos.azure.com", + "documents-staging.windows-ppe.net", + "sql.cosmosdb.windows-ppe.net", + "sql.cosmos.windows-ppe.net", + "sqlx.cosmos.windows-ppe.net", + ]); + expect(configContext.MONGO_DNS_ZONES).toEqual(["documents.azure.com", "documents-staging.windows-ppe.net"]); + expect(configContext.MONGO_COMPUTE_DNS_ZONES).toEqual(["mongo.cosmos.azure.com", "mongo.cosmos.windows-ppe.net"]); + expect(configContext.CASSANDRA_DNS_ZONES).toEqual([ + "cassandra.cosmosdb.azure.com", + "cassandra.cosmos.azure.com", + "cassandra.cosmosdb.windows-ppe.net", + "cassandra.cosmos.windows-ppe.net", + ]); + expect(configContext.TABLE_DNS_ZONES).toEqual([ + "table.cosmosdb.azure.com", + "table.cosmos.azure.com", + "table.cosmosdb.windows-ppe.net", + "table.cosmos.windows-ppe.net", + ]); + }); + it("should parse a valid sql account connection string", () => { const metadata = parseConnectionString( `AccountEndpoint=https://${mockAccountName}.documents.azure.com:443/;AccountKey=${mockMasterKey};`, @@ -26,6 +88,20 @@ describe("ConnectionStringParser", () => { expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents.azure.com/`); }); + it.each(configContext.SQL_DNS_ZONES)( + "should parse a sql account connection string using the %s zone", + (dnsZone: string) => { + const metadata = parseConnectionString( + `AccountEndpoint=https://${mockAccountName}.${dnsZone}:443/;AccountKey=${mockMasterKey};`, + ); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(DataModels.ApiKind.SQL); + expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.${dnsZone}:443/`); + expect(metadata.apiEndpoint).toBeUndefined(); + }, + ); + it("should parse a valid mongo account connection string", () => { const metadata = parseConnectionString( `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.documents.azure.com:10255`, @@ -35,15 +111,85 @@ describe("ConnectionStringParser", () => { expect(metadata.apiKind).toBe(DataModels.ApiKind.MongoDB); }); - it("should parse a valid compute mongo account connection string", () => { + it.each(configContext.MONGO_DNS_ZONES)( + "should parse a mongo account connection string using the %s zone", + (dnsZone: string) => { + const metadata = parseConnectionString( + `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.${dnsZone}:10255`, + ); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(DataModels.ApiKind.MongoDB); + }, + ); + + it.each(configContext.MONGO_COMPUTE_DNS_ZONES)( + "should parse a compute mongo account connection string using the %s zone", + (dnsZone: string) => { + const metadata = parseConnectionString( + `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.${dnsZone}:10255`, + ); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(DataModels.ApiKind.MongoDBCompute); + }, + ); + + it("should parse a valid cassandra account connection string", () => { const metadata = parseConnectionString( - `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.mongo.cosmos.azure.com:10255`, + `AccountEndpoint=${mockAccountName}.cassandra.cosmosdb.azure.com;AccountKey=${mockMasterKey};`, ); expect(metadata.accountName).toBe(mockAccountName); - expect(metadata.apiKind).toBe(DataModels.ApiKind.MongoDBCompute); + expect(metadata.apiKind).toBe(DataModels.ApiKind.Cassandra); }); + it.each( + ["AccountEndpoint", "HostName"].flatMap((key) => + configContext.CASSANDRA_DNS_ZONES.map((dnsZone) => [key, dnsZone]), + ), + )("should parse a cassandra account connection string using %s and the %s zone", (key: string, dnsZone: string) => { + const metadata = parseConnectionString(`${key}=${mockAccountName}.${dnsZone};AccountKey=${mockMasterKey};`); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(DataModels.ApiKind.Cassandra); + }); + + it.each(configContext.TABLE_DNS_ZONES)( + "should parse a table account connection string using the %s zone", + (dnsZone: string) => { + const metadata = parseConnectionString( + `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.${dnsZone}:443/;`, + ); + + expect(metadata.accountName).toBe(mockAccountName); + expect(metadata.apiKind).toBe(DataModels.ApiKind.Table); + expect(metadata.apiEndpoint).toBeUndefined(); + }, + ); + + it("should construct the document endpoint for a table account from the account name", () => { + const metadata = parseConnectionString( + `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmosdb.azure.com:443/;`, + ); + + // Table connection strings only carry the table endpoint, so the document endpoint that data plane + // operations go through is built from the account name. + expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents.azure.com:443/`); + }); + + it.each(["table.cosmosdb.windows-ppe.net", "table.cosmos.windows-ppe.net"])( + "should construct a PPE document endpoint for a table account in the %s zone", + (dnsZone: string) => { + const metadata = parseConnectionString( + `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.${dnsZone}:443/;`, + ); + + // The constructed endpoint has to match the kind of zone the table endpoint we matched came from. + expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents-staging.windows-ppe.net:443/`); + }, + ); + it("should parse a valid graph account connection string", () => { const metadata = parseConnectionString( `AccountEndpoint=https://${mockAccountName}.documents.azure.com:443/;AccountKey=${mockMasterKey};ApiKind=Gremlin;`, @@ -55,36 +201,45 @@ describe("ConnectionStringParser", () => { expect(metadata.apiEndpoint).toBe(`${mockAccountName}.gremlin.cosmos.azure.com:443`); }); - it("should parse a valid table account connection string", () => { + it("should construct a PPE gremlin endpoint for a PPE graph account", () => { const metadata = parseConnectionString( - `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmosdb.azure.com:443/;`, + `AccountEndpoint=https://${mockAccountName}.documents-staging.windows-ppe.net:443/;AccountKey=${mockMasterKey};ApiKind=Gremlin;`, ); expect(metadata.accountName).toBe(mockAccountName); - expect(metadata.apiKind).toBe(DataModels.ApiKind.Table); - // Table data operations go through the document endpoint, which is constructed from the account name. - expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents.azure.com:443/`); - expect(metadata.apiEndpoint).toBeUndefined(); + expect(metadata.apiKind).toBe(DataModels.ApiKind.Graph); + expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents-staging.windows-ppe.net:443/`); + // The constructed endpoint has to match the kind of zone the document endpoint we matched came from. + expect(metadata.apiEndpoint).toBe(`${mockAccountName}.gremlin.cosmos.windows-ppe.net:443`); }); - it("should parse a valid table account connection string using the cosmos.azure.com zone", () => { - const metadata = parseConnectionString( - `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmos.azure.com:443/;`, - ); + it("should reject a connection string when no DNS zone matches the account", () => { + const originalZones = configContext.DOCUMENT_ENDPOINT_ZONES; + updateConfigContext({ DOCUMENT_ENDPOINT_ZONES: ["documents.azure.com"] }); - expect(metadata.accountName).toBe(mockAccountName); - expect(metadata.apiKind).toBe(DataModels.ApiKind.Table); - expect(metadata.documentEndpoint).toBe(`https://${mockAccountName}.documents.azure.com:443/`); - expect(metadata.apiEndpoint).toBeUndefined(); + try { + const metadata = parseConnectionString( + `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmos.windows-ppe.net:443/;`, + ); + + // The account key travels to the constructed document endpoint, so a config carrying no PPE zone + // has to fail on a PPE account rather than fall back to a zone the account does not own. + expect(metadata).toBe(undefined); + } finally { + updateConfigContext({ DOCUMENT_ENDPOINT_ZONES: originalZones }); + } }); - it("should parse a valid cassandra account connection string", () => { - const metadata = parseConnectionString( - `AccountEndpoint=${mockAccountName}.cassandra.cosmosdb.azure.com;AccountKey=${mockMasterKey};`, - ); - - expect(metadata.accountName).toBe(mockAccountName); - expect(metadata.apiKind).toBe(DataModels.ApiKind.Cassandra); + it.each([ + `AccountEndpoint=https://${mockAccountName}.documents.azure.com.attacker.example:443/;AccountKey=${mockMasterKey};`, + `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.documents.azure.com.attacker.example:10255`, + `mongodb://${mockAccountName}:${mockMasterKey}@${mockAccountName}.mongo.cosmos.azure.com.attacker.example:10255`, + `AccountEndpoint=${mockAccountName}.cassandra.cosmosdb.azure.com.attacker.example;AccountKey=${mockMasterKey};`, + `DefaultEndpointsProtocol=https;AccountName=${mockAccountName};AccountKey=${mockMasterKey};TableEndpoint=https://${mockAccountName}.table.cosmosdb.azure.com.attacker.example:443/;`, + ])("should not accept a host that only begins with a known zone: %s", (connectionString: string) => { + // The zone list is what keeps the account key from being sent somewhere arbitrary, so a host that + // appends to an allowed zone must not pass as that zone. + expect(parseConnectionString(connectionString)).toBe(undefined); }); it("should fail to parse an invalid connection string", () => { @@ -98,4 +253,92 @@ describe("ConnectionStringParser", () => { expect(metadata).toBe(undefined); }); + + describe("dnsZoneAlternation", () => { + it("should escape the dots in a zone", () => { + expect(dnsZoneAlternation(["documents.azure.com"])).toBe("(documents\\.azure\\.com)(?=[:/\\s]|$)"); + }); + + it("should join multiple zones into a single alternation", () => { + expect(dnsZoneAlternation(["a.example", "b.test"])).toBe("(a\\.example|b\\.test)(?=[:/\\s]|$)"); + }); + + it("should not let the dots match arbitrary characters", () => { + // An unescaped dot would make the zone list match hosts that only resemble a real zone. + const regex = RegExp(dnsZoneAlternation(["documents.azure.com"])); + + expect(regex.test("documents.azure.com")).toBe(true); + expect(regex.test("documentsXazure.com")).toBe(false); + }); + + it("should capture the zone that matched", () => { + const regex = RegExp(dnsZoneAlternation(["a.example", "b.test"])); + + expect("account.b.test".match(regex)[1]).toBe("b.test"); + }); + + it("should require the zone to run to the end of the host", () => { + const regex = RegExp(dnsZoneAlternation(["documents.azure.com"])); + + expect(regex.test("account.documents.azure.com")).toBe(true); + expect(regex.test("account.documents.azure.com:443/")).toBe(true); + expect(regex.test("account.documents.azure.com/")).toBe(true); + // Without this the zone list stops being an allowlist, since anything can be appended to a zone. + expect(regex.test("account.documents.azure.com.attacker.example")).toBe(false); + }); + }); + + describe("buildEndpointsRegex", () => { + it("should build a pattern for every api matched by dns zone", () => { + expect(Object.keys(buildEndpointsRegex())).toEqual(["sql", "mongo", "mongoCompute", "cassandra", "table"]); + }); + + it("should build a cassandra pattern for each supported key", () => { + const { cassandra } = buildEndpointsRegex(); + + expect(cassandra).toHaveLength(2); + expect(cassandra[0]).toContain("AccountEndpoint="); + expect(cassandra[1]).toContain("HostName="); + }); + + it("should build each pattern from its own zone list", () => { + // The patterns are near identical, so a zone list wired to the wrong api would be easy to miss in + // review and would let an account of one api be parsed as another. + const { sql, mongo, mongoCompute, cassandra, table } = buildEndpointsRegex(); + + expect(sql).toContain(dnsZoneAlternation(configContext.SQL_DNS_ZONES)); + expect(mongo).toContain(dnsZoneAlternation(configContext.MONGO_DNS_ZONES)); + expect(mongoCompute).toContain(dnsZoneAlternation(configContext.MONGO_COMPUTE_DNS_ZONES)); + cassandra.forEach((pattern) => expect(pattern).toContain(dnsZoneAlternation(configContext.CASSANDRA_DNS_ZONES))); + expect(table).toContain(dnsZoneAlternation(configContext.TABLE_DNS_ZONES)); + }); + }); + + describe("selectEndpointZone", () => { + const nonPpeZone = "documents.azure.com"; + const ppeZone = "documents-staging.windows-ppe.net"; + + it("should pick the ppe zone for a ppe account", () => { + expect(selectEndpointZone([nonPpeZone, ppeZone], true)).toBe(ppeZone); + }); + + it("should pick the non ppe zone for a non ppe account", () => { + expect(selectEndpointZone([nonPpeZone, ppeZone], false)).toBe(nonPpeZone); + }); + + it("should not depend on the order of the zones", () => { + expect(selectEndpointZone([ppeZone, nonPpeZone], true)).toBe(ppeZone); + expect(selectEndpointZone([ppeZone, nonPpeZone], false)).toBe(nonPpeZone); + }); + + it("should return undefined when no zone matches the kind of account", () => { + // Sovereign configs carry no ppe zone, and a ppe only config carries no non ppe zone. + expect(selectEndpointZone([nonPpeZone], true)).toBeUndefined(); + expect(selectEndpointZone([ppeZone], false)).toBeUndefined(); + }); + + it("should return undefined for an empty zone list", () => { + expect(selectEndpointZone([], false)).toBeUndefined(); + }); + }); }); diff --git a/src/Platform/Hosted/Helpers/ConnectionStringParser.ts b/src/Platform/Hosted/Helpers/ConnectionStringParser.ts index 44be7e82a..0e3dd2bdf 100644 --- a/src/Platform/Hosted/Helpers/ConnectionStringParser.ts +++ b/src/Platform/Hosted/Helpers/ConnectionStringParser.ts @@ -1,44 +1,76 @@ -import * as Constants from "../../../Common/Constants"; +import { configContext } from "../../../ConfigContext"; import { AccessInputMetadata, ApiKind } from "../../../Contracts/DataModels"; -// Cosmos DB DNS zones used to construct endpoints client-side. These mirror what the Portal Backend's -// accessinputmetadata API constructs from the account name when the connection string does not already -// contain the endpoint. -const DocumentEndpointZone = "documents.azure.com"; -const GremlinEndpointZone = "gremlin.cosmos.azure.com"; +const PpeDnsSuffix = "windows-ppe.net"; const DnsPort = "443"; +const isPpeZone = (zone: string): boolean => zone.endsWith(PpeDnsSuffix); + +// Picks the DNS zone matching the kind of account the connection string came from, since a PPE +// account's endpoints sit under PPE zones and every other account's do not. Returns undefined when the +// config carries no zone of that kind, so the caller can reject the connection string rather than build +// an endpoint the account does not own and send the account key there. +export const selectEndpointZone = (zones: ReadonlyArray, isPpeAccount: boolean): string | undefined => + zones.find((zone) => isPpeZone(zone) === isPpeAccount); + +// Builds an alternation matching any of the given DNS zones, e.g. "(documents\.azure\.com|sql\.cosmos\.azure\.com)". +// The group captures so callers can tell which zone matched, and with it whether the account is a PPE account. +// The zone has to run to the end of the host, otherwise a host that merely starts with an allowed zone +// would pass as that zone and the account key would travel to whatever was appended to it. +export const dnsZoneAlternation = (zones: ReadonlyArray): string => + `(${zones.map((zone) => zone.replace(/\./g, "\\.")).join("|")})(?=[:/\\s]|$)`; + +// The zone lists live in ConfigContext, which is populated asynchronously by initializeConfiguration, +// so these are built per call rather than once at module load. +export const buildEndpointsRegex = () => ({ + sql: `AccountEndpoint=https://([^.]+)\\.${dnsZoneAlternation(configContext.SQL_DNS_ZONES)}`, + mongo: `mongodb://.*:(.*)@([^.]+)\\.${dnsZoneAlternation(configContext.MONGO_DNS_ZONES)}`, + mongoCompute: `mongodb://.*:(.*)@([^.]+)\\.${dnsZoneAlternation(configContext.MONGO_COMPUTE_DNS_ZONES)}`, + cassandra: ["AccountEndpoint", "HostName"].map( + (key) => `${key}=([^.]+)\\.${dnsZoneAlternation(configContext.CASSANDRA_DNS_ZONES)}`, + ), + table: `TableEndpoint=https://([^.]+)\\.${dnsZoneAlternation(configContext.TABLE_DNS_ZONES)}`, +}); + export function parseConnectionString(connectionString: string): AccessInputMetadata { if (connectionString) { try { const accessInput = {} as AccessInputMetadata; const connectionStringParts = connectionString.split(";"); + const endpointsRegex = buildEndpointsRegex(); + // Endpoints we build from the account name have to match the kind of zone the connection string + // actually matched, since PPE accounts and other accounts do not share zones. + let isPpeAccount = false; connectionStringParts.forEach((connectionStringPart: string) => { - if (RegExp(Constants.EndpointsRegex.sql).test(connectionStringPart)) { - accessInput.accountName = connectionStringPart.match(Constants.EndpointsRegex.sql)[1]; + if (RegExp(endpointsRegex.sql).test(connectionStringPart)) { + const matches: string[] = connectionStringPart.match(endpointsRegex.sql); + accessInput.accountName = matches[1]; accessInput.apiKind = ApiKind.SQL; // SQL and Gremlin connection strings carry the account's document endpoint, so take it as // given instead of rebuilding it from the account name. accessInput.documentEndpoint = connectionStringPart.substring(connectionStringPart.indexOf("=") + 1); - } else if (RegExp(Constants.EndpointsRegex.mongo).test(connectionStringPart)) { - const matches: string[] = connectionStringPart.match(Constants.EndpointsRegex.mongo); + isPpeAccount = isPpeZone(matches[2]); + } else if (RegExp(endpointsRegex.mongo).test(connectionStringPart)) { + const matches: string[] = connectionStringPart.match(endpointsRegex.mongo); accessInput.accountName = matches && matches.length > 1 && matches[2]; accessInput.apiKind = ApiKind.MongoDB; - } else if (RegExp(Constants.EndpointsRegex.mongoCompute).test(connectionStringPart)) { - const matches: string[] = connectionStringPart.match(Constants.EndpointsRegex.mongoCompute); + } else if (RegExp(endpointsRegex.mongoCompute).test(connectionStringPart)) { + const matches: string[] = connectionStringPart.match(endpointsRegex.mongoCompute); accessInput.accountName = matches && matches.length > 1 && matches[2]; accessInput.apiKind = ApiKind.MongoDBCompute; - } else if (Constants.EndpointsRegex.cassandra.some((regex) => RegExp(regex).test(connectionStringPart))) { - Constants.EndpointsRegex.cassandra.forEach((regex) => { + } else if (endpointsRegex.cassandra.some((regex) => RegExp(regex).test(connectionStringPart))) { + endpointsRegex.cassandra.forEach((regex) => { if (RegExp(regex).test(connectionStringPart)) { accessInput.accountName = connectionStringPart.match(regex)[1]; accessInput.apiKind = ApiKind.Cassandra; } }); - } else if (RegExp(Constants.EndpointsRegex.table).test(connectionStringPart)) { - accessInput.accountName = connectionStringPart.match(Constants.EndpointsRegex.table)[1]; + } else if (RegExp(endpointsRegex.table).test(connectionStringPart)) { + const matches: string[] = connectionStringPart.match(endpointsRegex.table); + accessInput.accountName = matches[1]; accessInput.apiKind = ApiKind.Table; + isPpeAccount = isPpeZone(matches[2]); } else if (connectionStringPart.indexOf("ApiKind=Gremlin") >= 0) { accessInput.apiKind = ApiKind.Graph; } @@ -53,9 +85,17 @@ export function parseConnectionString(connectionString: string): AccessInputMeta // need the Gremlin endpoint, which is never part of the connection string. if (accessInput.accountName) { if (accessInput.apiKind === ApiKind.Table) { - accessInput.documentEndpoint = `https://${accessInput.accountName}.${DocumentEndpointZone}:${DnsPort}/`; + const documentEndpointZone = selectEndpointZone(configContext.DOCUMENT_ENDPOINT_ZONES, isPpeAccount); + if (!documentEndpointZone) { + return undefined; + } + accessInput.documentEndpoint = `https://${accessInput.accountName}.${documentEndpointZone}:${DnsPort}/`; } else if (accessInput.apiKind === ApiKind.Graph) { - accessInput.apiEndpoint = `${accessInput.accountName}.${GremlinEndpointZone}:${DnsPort}`; + const gremlinEndpointZone = selectEndpointZone(configContext.GREMLIN_ENDPOINT_ZONES, isPpeAccount); + if (!gremlinEndpointZone) { + return undefined; + } + accessInput.apiEndpoint = `${accessInput.accountName}.${gremlinEndpointZone}:${DnsPort}`; } } diff --git a/test/fx.ts b/test/fx.ts index a3cad859a..f3fc2e679 100644 --- a/test/fx.ts +++ b/test/fx.ts @@ -49,6 +49,7 @@ export enum TestAccount { SQLReadOnly = "SQLReadOnly", SQLContainerCopyOnly = "SQLContainerCopyOnly", SQLConnectionString = "SQLConnectionString", + SQLConnectionStringPublicNetworkAccessDisabled = "SQLConnectionStringPublicNetworkAccessDisabled", TableConnectionString = "TableConnectionString", GremlinConnectionString = "GremlinConnectionString", } @@ -83,6 +84,8 @@ export function getDefaultAccountName(accountType: TestAccount): string { return `${accountNamePrefix}-de-test-sql-containercopy`; case TestAccount.SQLConnectionString: return `${accountNamePrefix}-de-test-sql-connstring-1`; + case TestAccount.SQLConnectionStringPublicNetworkAccessDisabled: + return `${accountNamePrefix}-de-test-sql-connstring-nopublic-1`; case TestAccount.TableConnectionString: return `${accountNamePrefix}-de-test-table-connstring-1`; case TestAccount.GremlinConnectionString: @@ -258,6 +261,7 @@ export async function getTestExplorerUrl(accountType: TestAccount, options?: Tes break; case TestAccount.SQLConnectionString: + case TestAccount.SQLConnectionStringPublicNetworkAccessDisabled: case TestAccount.TableConnectionString: case TestAccount.GremlinConnectionString: // Connection string (account key) login navigates directly to hostedExplorer.html and doesn't diff --git a/test/sql/connectionStringLogin.spec.ts b/test/sql/connectionStringLogin.spec.ts index 8359e6dee..88293bd64 100644 --- a/test/sql/connectionStringLogin.spec.ts +++ b/test/sql/connectionStringLogin.spec.ts @@ -115,4 +115,32 @@ test.describe("SQL account using connection string login", () => { await expect(page.locator("#connectExplorer")).toHaveCount(0); await expect(page.locator(".errorDetails")).toHaveCount(0); }); + + test("opens Data Explorer but loads no databases when the account rejects the client IP", async ({ page }) => { + // An account that refuses this client's IP. + const armClient = new CosmosDBManagementClient(getAzureCLICredentials(), subscriptionId); + const blockedAccountName = getAccountName(TestAccount.SQLConnectionStringPublicNetworkAccessDisabled); + const blockedAccount = await armClient.databaseAccounts.get(resourceGroupName, blockedAccountName); + const blockedKeys = await armClient.databaseAccounts.listKeys(resourceGroupName, blockedAccountName); + + await loginWithConnectionString( + page, + `AccountEndpoint=${blockedAccount.documentEndpoint!};AccountKey=${blockedKeys.primaryMasterKey};`, + ); + + const explorer = await DataExplorer.waitForExplorer(page); + + // Login is a client-side parse of the connection string, so nothing checks whether the account will + // accept requests from this IP before letting the user in. + await expect(page.locator("#connectExplorer")).toHaveCount(0); + await expect(page.locator(".errorDetails")).toHaveCount(0); + + // The rejection surfaces once the tree tries to read the data plane, and only in the console. + const consoleMessages = await explorer.getNotificationConsoleMessages(); + await expect(consoleMessages).toContainText("Error while refreshing databases", { timeout: ONE_MINUTE_MS }); + + // The tree is left with the static Home node and no database or container beneath it. + await expect(explorer.treeNode("Home").element).toBeAttached(); + await expect(explorer.frame.locator("[data-test^='TreeNode:']")).toHaveCount(1); + }); });