[BUG-FIX] validate project and org names (#442)

* [BUG-FIX] validate project and org names

* add validation for project create pipeline command

Author: dennisseah

src/commands/hld/pipeline.test.ts

@@ -73,6 +73,54 @@ describe("test emptyStringIfUndefined function", () => {
   });
 });
 
+const orgNameTest = (hasVal: boolean): void => {
+  const data = {
+    buildScriptUrl: "",
+    devopsProject: "project",
+    hldName: "",
+    hldUrl: "https://dev.azure.com/mocked/fabrikam/_git/hld",
+    manifestUrl: "https://dev.azure.com/mocked/fabrikam/_git/materialized",
+    orgName: hasVal ? "org Name" : "",
+    personalAccessToken: "",
+    pipelineName: "",
+    yamlFileBranch: "",
+  };
+
+  if (hasVal) {
+    expect(() => populateValues(data)).toThrow(
+      "Organization names must start with a letter or number, followed by letters, numbers or hyphens, and must end with a letter or number."
+    );
+  } else {
+    expect(() => populateValues(data)).toThrow(
+      "value for -o, --org-name <organization-name> is missing"
+    );
+  }
+};
+
+const projectNameTest = (hasVal: boolean): void => {
+  const data = {
+    buildScriptUrl: "",
+    devopsProject: hasVal ? "project\\abc" : "",
+    hldName: "",
+    hldUrl: "https://dev.azure.com/mocked/fabrikam/_git/hld",
+    manifestUrl: "https://dev.azure.com/mocked/fabrikam/_git/materialized",
+    orgName: "orgName",
+    personalAccessToken: "",
+    pipelineName: "",
+    yamlFileBranch: "",
+  };
+
+  if (hasVal) {
+    expect(() => populateValues(data)).toThrow(
+      "Project name can't contain special characters, such as / : \\ ~ & % ; @ ' \" ? < > | # $ * } { , + = [ ]"
+    );
+  } else {
+    expect(() => populateValues(data)).toThrow(
+      "value for -d, --devops-project <devops-project> is missing"
+    );
+  }
+};
+
 describe("test populateValues function", () => {
   it("with all values in command opts", () => {
     jest.spyOn(config, "Config").mockImplementationOnce(
@@ -125,31 +173,24 @@ describe("test populateValues function", () => {
     expect(() =>
       populateValues({
         buildScriptUrl: "",
-        devopsProject: "",
+        devopsProject: "project",
         hldName: "",
         hldUrl: "https://github.com/fabrikam/hld",
         manifestUrl: "https://github.com/fabrikam/materialized",
-        orgName: "",
+        orgName: "orgName",
         personalAccessToken: "",
         pipelineName: "",
         yamlFileBranch: "",
       })
     ).toThrow(`GitHub repos are not supported`);
   });
-  it("negative tests: github repos not supported", () => {
-    expect(() =>
-      populateValues({
-        buildScriptUrl: "",
-        devopsProject: "",
-        hldName: "",
-        hldUrl: "https://github.com/fabrikam/hld",
-        manifestUrl: "https://github.com/fabrikam/materialized",
-        orgName: "",
-        personalAccessToken: "",
-        pipelineName: "",
-        yamlFileBranch: "",
-      })
-    ).toThrow(`GitHub repos are not supported`);
+  it("negative tests: missing and invalid org name", () => {
+    orgNameTest(false);
+    orgNameTest(true);
+  });
+  it("negative tests: missing and invalid project name", () => {
+    projectNameTest(false);
+    projectNameTest(true);
   });
 });
 

src/commands/hld/pipeline.ts

@@ -7,7 +7,11 @@ import {
 import commander from "commander";
 import { Config } from "../../config";
 import { validateRepository } from "../../lib/azdoClient";
-import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder";
+import {
+  build as buildCmd,
+  exit as exitCmd,
+  getOption as getCmdOption,
+} from "../../lib/commandBuilder";
 import {
   BUILD_SCRIPT_URL,
   RENDER_HLD_PIPELINE_FILENAME,
@@ -23,6 +27,11 @@ import {
 } from "../../lib/pipelines/pipelines";
 import { logger } from "../../logger";
 import decorator from "./pipeline.decorator.json";
+import {
+  hasValue,
+  validateOrgNameThrowable,
+  validateProjectNameThrowable,
+} from "../../lib/validator";
 
 export interface CommandOptions {
   pipelineName: string;
@@ -65,13 +74,31 @@ export const populateValues = (opts: CommandOptions): CommandOptions => {
 
   opts.orgName = opts.orgName || emptyStringIfUndefined(azure_devops?.org);
 
+  if (hasValue(opts.orgName)) {
+    validateOrgNameThrowable(opts.orgName);
+  } else {
+    throw Error(
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      `value for ${getCmdOption(decorator, "org-name")!.arg} is missing`
+    );
+  }
+
   opts.personalAccessToken =
     opts.personalAccessToken ||
     emptyStringIfUndefined(azure_devops?.access_token);
 
   opts.devopsProject =
     opts.devopsProject || emptyStringIfUndefined(azure_devops?.project);
 
+  if (hasValue(opts.devopsProject)) {
+    validateProjectNameThrowable(opts.devopsProject);
+  } else {
+    throw Error(
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      `value for ${getCmdOption(decorator, "devops-project")!.arg} is missing`
+    );
+  }
+
   opts.pipelineName =
     opts.hldName + "-to-" + getRepositoryName(opts.manifestUrl);
 

src/commands/project/create-variable-group.test.ts

@@ -5,11 +5,13 @@ import mockFs from "mock-fs";
 import path from "path";
 import uuid from "uuid/v4";
 import { readYaml, write } from "../../config";
+import * as config from "../../config";
 import {
   create as createBedrockYaml,
   isExists as isBedrockFileExists,
   read as readBedrockFile,
 } from "../../lib/bedrockYaml";
+import * as commandBuilder from "../../lib/commandBuilder";
 import {
   PROJECT_PIPELINE_FILENAME,
   VERSION_MESSAGE,
@@ -32,7 +34,9 @@ import {
   execute,
   setVariableGroupInBedrockFile,
   updateLifeCyclePipeline,
+  validateValues,
 } from "./create-variable-group";
+import * as createVariableGrp from "./create-variable-group";
 import * as fileutils from "../../lib/fileutils";
 
 beforeAll(() => {
@@ -57,11 +61,45 @@ const hldRepoUrl = uuid();
 const servicePrincipalId = uuid();
 const servicePrincipalPassword: string = uuid();
 const tenant = uuid();
-const orgName = uuid();
-const devopsProject = uuid();
+const orgName = "orgName";
+const devopsProject = "projectName";
 const personalAccessToken = uuid();
 
 describe("test execute function", () => {
+  it("positive test", async () => {
+    const exitFn = jest.fn();
+    jest.spyOn(createVariableGrp, "checkDependencies").mockReturnValueOnce();
+    jest.spyOn(config, "Config").mockReturnValueOnce({});
+    jest
+      .spyOn(commandBuilder, "validateForRequiredValues")
+      .mockReturnValueOnce([]);
+    jest.spyOn(createVariableGrp, "create").mockResolvedValueOnce({
+      name: "groupName",
+    });
+    jest
+      .spyOn(createVariableGrp, "setVariableGroupInBedrockFile")
+      .mockReturnValueOnce();
+    jest
+      .spyOn(createVariableGrp, "updateLifeCyclePipeline")
+      .mockReturnValueOnce();
+
+    await execute(
+      "groupName",
+      {
+        devopsProject,
+        hldRepoUrl,
+        orgName,
+        personalAccessToken,
+        registryName,
+        servicePrincipalId,
+        servicePrincipalPassword,
+        tenant,
+      },
+      exitFn
+    );
+    expect(exitFn).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[0]]);
+  });
   it("missing variable name", async () => {
     const exitFn = jest.fn();
     await execute(
@@ -79,6 +117,7 @@ describe("test execute function", () => {
       exitFn
     );
     expect(exitFn).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[1]]);
   });
   it("missing registry name", async () => {
     const exitFn = jest.fn();
@@ -97,25 +136,18 @@ describe("test execute function", () => {
       exitFn
     );
     expect(exitFn).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[1]]);
   });
 });
 
 describe("create", () => {
-  test("Should fail with empty variable group arguments", async () => {
+  it("Should fail with empty variable group arguments", async () => {
     const accessOpts: AzureDevOpsOpts = {
       orgName,
       personalAccessToken,
       project: devopsProject,
     };
-
-    let invalidDataError: Error | undefined;
-    try {
-      logger.info("calling create");
-      await create("", "", "", "", "", "", accessOpts);
-    } catch (err) {
-      invalidDataError = err;
-    }
-    expect(invalidDataError).toBeDefined();
+    await expect(create("", "", "", "", "", "", accessOpts)).rejects.toThrow();
   });
 
   test("Should pass with variable group arguments", async () => {
@@ -336,7 +368,7 @@ describe("updateLifeCyclePipeline", () => {
 
     fs.writeFileSync(hldFilePath, asYaml);
 
-    await updateLifeCyclePipeline(randomTmpDir);
+    updateLifeCyclePipeline(randomTmpDir);
 
     const hldLifeCycleYaml = readYaml<AzurePipelinesYaml>(hldFilePath);
     logger.info(`filejson: ${JSON.stringify(hldLifeCycleYaml)}`);
@@ -345,3 +377,19 @@ describe("updateLifeCyclePipeline", () => {
     });
   });
 });
+
+describe("test validateValues function", () => {
+  it("valid org and project name", () => {
+    validateValues(devopsProject, orgName);
+  });
+  it("invalid project name", () => {
+    expect(() => {
+      validateValues("project\\abc", orgName);
+    }).toThrow();
+  });
+  it("invalid org name", () => {
+    expect(() => {
+      validateValues(devopsProject, "org name");
+    }).toThrow();
+  });
+});

src/commands/project/create-variable-group.ts

@@ -17,7 +17,11 @@ import {
 } from "../../lib/constants";
 import { AzureDevOpsOpts } from "../../lib/git";
 import { addVariableGroup } from "../../lib/pipelines/variableGroup";
-import { hasValue } from "../../lib/validator";
+import {
+  hasValue,
+  validateProjectNameThrowable,
+  validateOrgNameThrowable,
+} from "../../lib/validator";
 import { logger } from "../../logger";
 import {
   AzurePipelinesYaml,
@@ -46,6 +50,11 @@ export const checkDependencies = (projectPath: string): void => {
   }
 };
 
+export const validateValues = (projectName: string, orgName: string): void => {
+  validateProjectNameThrowable(projectName);
+  validateOrgNameThrowable(orgName);
+};
+
 /**
  * Executes the command.
  *
@@ -105,6 +114,11 @@ export const execute = async (
       return;
     }
 
+    // validateForRequiredValues assure that devopsProject
+    // and orgName are not empty string
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    validateValues(devopsProject!, orgName!);
+
     const variableGroup = await create(
       variableGroupName,
       registryName,
@@ -176,6 +190,7 @@ export const create = (
   logger.info(
     `Creating Variable Group from group definition '${variableGroupName}'`
   );
+
   const vars: VariableGroupDataVariable = {
     ACR_NAME: {
       value: registryName,