From 3ccea37c4ab6e9576b55e40d005d6c4018e24b06 Mon Sep 17 00:00:00 2001 From: Arun Date: Mon, 20 Nov 2023 15:46:58 -0800 Subject: [PATCH 1/4] Strip protocol before feeding just the outgoing domain to the manifest API Signed-off-by: Arun --- src/manifest/mod.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/manifest/mod.ts b/src/manifest/mod.ts index 5677e7f7..ad94589b 100644 --- a/src/manifest/mod.ts +++ b/src/manifest/mod.ts @@ -132,7 +132,13 @@ export class SlackManifest { ); } - manifest.outgoing_domains = def.outgoingDomains || []; + manifest.outgoing_domains = (def.outgoingDomains || []).map((domain) => { + try { + return new URL(domain).hostname; + } catch (e) { + throw new Error(`Invalid outgoing domain: ${domain}, error ${e}`); + } + }); // Assign remote hosted app properties if (manifest.settings.function_runtime === "slack") { From 2cff6cacd34793aed913e64704ed338270fb257c Mon Sep 17 00:00:00 2001 From: Arun Date: Mon, 20 Nov 2023 20:22:47 -0800 Subject: [PATCH 2/4] Refactor into a validateOutgoingDomains helper function and create tests to parse valid and invalid domains Signed-off-by: Arun --- src/manifest/manifest_test.ts | 66 ++++++++++++++++++++++++++++++++++- src/manifest/mod.ts | 22 ++++++++---- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/manifest/manifest_test.ts b/src/manifest/manifest_test.ts index 6dbbfa62..aa3db938 100644 --- a/src/manifest/manifest_test.ts +++ b/src/manifest/manifest_test.ts @@ -3,7 +3,7 @@ import { ISlackManifestRunOnSlack, SlackManifestType, } from "./types.ts"; -import { Manifest, SlackManifest } from "./mod.ts"; +import { Manifest, SlackManifest, validateOutgoingDomains } from "./mod.ts"; import { DefineDatastore, DefineEvent, @@ -26,6 +26,7 @@ import { import { DefineConnector } from "../functions/mod.ts"; import { InternalSlackTypes } from "../schema/slack/types/custom/mod.ts"; import { DuplicateCallbackIdError, DuplicateNameError } from "./errors.ts"; +import { assertThrows } from "https://deno.land/std@0.152.0/testing/asserts.ts"; Deno.test("SlackManifestType correctly resolves to a Hosted App when runOnSlack = true", () => { const definition: SlackManifestType = { @@ -1172,3 +1173,66 @@ Deno.test("Manifest throws error when CustomEvents with duplicate name are added assertStringIncludes(error.message, "CustomEvent"); } }); + +Deno.test("Manifest correctly parses valid domains", () => { + const def = { + outgoingDomains: [ + "https://slack.com", + "http://salesforce.com", + "https://api.slack.com", + ], + }; + const expected = ["slack.com", "salesforce.com", "api.slack.com"]; + + const definition: SlackManifestType = { + runOnSlack: false, + name: "test", + description: "description", + backgroundColor: "#FFF", + longDescription: + "The book is a roman à clef, rooted in autobiographical incidents. The story follows its protagonist, Raoul Duke, and his attorney, Dr. Gonzo, as they descend on Las Vegas to chase the American Dream...", + displayName: "displayName", + icon: "icon.png", + botScopes: ["channels:history", "chat:write", "commands"], + }; + const manifest = Manifest(definition); + + manifest.outgoing_domains = validateOutgoingDomains( + def.outgoingDomains || [], + ); + + assertEquals(manifest.outgoing_domains, expected); +}); + +Deno.test("Manifest throws error for invalid domains", () => { + const def = { + outgoingDomains: [ + "slack", + "htt*ps://api.slack.com", + "https://api slack.com", + ], + }; + + assertThrows( + () => { + const definition: SlackManifestType = { + runOnSlack: false, + name: "test", + description: "description", + backgroundColor: "#FFF", + longDescription: + "The book is a roman à clef, rooted in autobiographical incidents. The story follows its protagonist, Raoul Duke, and his attorney, Dr. Gonzo, as they descend on Las Vegas to chase the American Dream...", + displayName: "displayName", + icon: "icon.png", + botScopes: ["channels:history", "chat:write", "commands"], + }; + const manifest = Manifest(definition); + + manifest.outgoing_domains = validateOutgoingDomains( + def.outgoingDomains || [], + ); + }, + Error, + "Invalid outgoing domain", + ); +}); diff --git a/src/manifest/mod.ts b/src/manifest/mod.ts index ad94589b..522763ed 100644 --- a/src/manifest/mod.ts +++ b/src/manifest/mod.ts @@ -32,6 +32,18 @@ export const Manifest = ( return manifest.export(); }; +export const validateOutgoingDomains = ( + domains: string[], +) => { + return domains.map((domain) => { + try { + return new URL(domain).hostname; + } catch (e) { + throw new Error(`Invalid outgoing domain: ${domain}, error ${e}`); + } + }); +}; + export class SlackManifest { constructor(private definition: SlackManifestType) { this.registerFeatures(); @@ -132,13 +144,9 @@ export class SlackManifest { ); } - manifest.outgoing_domains = (def.outgoingDomains || []).map((domain) => { - try { - return new URL(domain).hostname; - } catch (e) { - throw new Error(`Invalid outgoing domain: ${domain}, error ${e}`); - } - }); + manifest.outgoing_domains = validateOutgoingDomains( + def.outgoingDomains || [], + ); // Assign remote hosted app properties if (manifest.settings.function_runtime === "slack") { From 8c9ade25d136466c8c160f9427c9c54b92cbf193 Mon Sep 17 00:00:00 2001 From: Arun Sathiya Date: Tue, 21 Nov 2023 13:54:29 -0800 Subject: [PATCH 3/4] Return domain as is if the input doesn't have a protocol Co-authored-by: William Bergamin --- src/manifest/mod.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manifest/mod.ts b/src/manifest/mod.ts index 522763ed..07dea718 100644 --- a/src/manifest/mod.ts +++ b/src/manifest/mod.ts @@ -39,7 +39,7 @@ export const validateOutgoingDomains = ( try { return new URL(domain).hostname; } catch (e) { - throw new Error(`Invalid outgoing domain: ${domain}, error ${e}`); + return domain; } }); }; From b584112331dcfa2a2b1dd4b42f0b192741024740 Mon Sep 17 00:00:00 2001 From: Arun Date: Tue, 21 Nov 2023 14:02:01 -0800 Subject: [PATCH 4/4] Assume http protocol for inputs that have just the host name, and implement an additional defensive check that returns the input as is or throw an error Signed-off-by: Arun --- src/manifest/mod.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/manifest/mod.ts b/src/manifest/mod.ts index 07dea718..abc69029 100644 --- a/src/manifest/mod.ts +++ b/src/manifest/mod.ts @@ -37,13 +37,25 @@ export const validateOutgoingDomains = ( ) => { return domains.map((domain) => { try { - return new URL(domain).hostname; + const url = domain.includes("://") ? domain : `http://${domain}`; + return new URL(url).hostname; } catch (e) { - return domain; + if (isValidDomain(domain)) { + return domain; + } else { + throw new Error( + `Invalid outgoing domain: ${domain}, error ${e}`, + ); + } } }); }; +const isValidDomain = (domain: string) => { + const domainPattern = /^[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)*\.[a-zA-Z]{2,}$/; + return domainPattern.test(domain); +}; + export class SlackManifest { constructor(private definition: SlackManifestType) { this.registerFeatures();