refactor: only delete ttk created aad (#12058)

* refactor: only delete aad created aad * refactor: clear aad set after debugging * refactor: fix pr comments
OfficeDev · Jul 26, 2024 · f173633 · f173633
1 parent 9f8fb0b
commit f173633
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 30 deletions.
diff --git a/packages/fx-core/src/common/globalVars.ts b/packages/fx-core/src/common/globalVars.ts
@@ -84,3 +84,5 @@ export function createContext(): Context {
   };
   return context;
 }
+
+export const AadSet: Set<string> = new Set<string>();
diff --git a/packages/fx-core/src/component/driver/aad/create.ts b/packages/fx-core/src/component/driver/aad/create.ts
@@ -31,6 +31,7 @@ import {
   logMessageKeys,
   telemetryKeys,
 } from "./utility/constants";
+import { AadSet } from "../../../common/globalVars";
 
 const actionName = "aadApp/create"; // DO NOT MODIFY the name
 const helpLink = "https://aka.ms/teamsfx-actions/aadapp-create";
@@ -89,6 +90,7 @@ export class CreateAadAppDriver implements StepDriver {
         );
         aadAppState.clientId = aadApp.appId!;
         aadAppState.objectId = aadApp.id!;
+        AadSet.add(aadApp.appId!);
         await this.setAadEndpointInfo(context.m365TokenProvider, aadAppState);
         outputs = mapStateToEnv(aadAppState, outputEnvVarNames, [OutputKeys.clientSecret]);
 

diff --git a/packages/fx-core/src/component/driver/botAadApp/create.ts b/packages/fx-core/src/component/driver/botAadApp/create.ts
@@ -27,6 +27,7 @@ import { CreateBotAadAppArgs } from "./interface/createBotAadAppArgs";
 import { CreateBotAadAppOutput } from "./interface/createBotAadAppOutput";
 import { logMessageKeys, progressBarKeys } from "./utility/constants";
 import { GraphScopes } from "../../../common/constants";
+import { AadSet } from "../../../common/globalVars";
 
 const actionName = "botAadApp/create"; // DO NOT MODIFY the name
 const helpLink = "https://aka.ms/teamsfx-actions/botaadapp-create";
@@ -105,6 +106,7 @@ export class CreateBotAadAppDriver implements StepDriver {
           SignInAudience.AzureADMultipleOrgs
         );
         botAadAppState.botId = aadApp.appId!;
+        AadSet.add(aadApp.appId!);
         botAadAppState.botPassword = await aadAppClient.generateClientSecret(aadApp.id!);
         context.logProvider?.info(getLocalizedString(logMessageKeys.successCreateBotAadApp));
       } else {

diff --git a/packages/fx-core/src/index.ts b/packages/fx-core/src/index.ts
@@ -105,3 +105,4 @@ export * from "./question/inputs";
 export * from "./question/options";
 export * from "./component/middleware/actionExecutionMW";
 export { TemplateInfo } from "./component/generator/templates/templateInfo";
+export { AadSet } from "./common/globalVars";
diff --git a/packages/vscode-extension/src/debug/deleteAadHelper.ts b/packages/vscode-extension/src/debug/deleteAadHelper.ts
@@ -6,7 +6,7 @@ import M365TokenInstance from "../commonlib/m365Login";
 import * as globalVariables from "../globalVariables";
 import * as fs from "fs-extra";
 import * as path from "path";
-import { GraphScopes } from "@microsoft/teamsfx-core";
+import { AadSet, GraphScopes } from "@microsoft/teamsfx-core";
 import axios from "axios";
 import { ConvertTokenToJson } from "../commonlib/codeFlowLogin";
 import VsCodeLogInstance from "../commonlib/log";
@@ -17,33 +17,33 @@ import { TelemetryEvent } from "../telemetry/extTelemetryEvents";
 import { FxError } from "@microsoft/teamsfx-api";
 
 const defaultNotificationLocalFile = ".notification.localstore.json";
-export async function deleteAad() {
+export async function deleteAad(): Promise<boolean> {
   try {
     if (globalVariables.deleteAadInProgress) {
-      return;
+      return true;
     }
     globalVariables.setDeleteAadInProgress(true);
     const projectPath = globalVariables.workspaceUri!.fsPath;
     const envFile = path.resolve(projectPath, "env", ".env.local");
     const userFile = path.resolve(projectPath, "env", ".env.local.user");
     if (!fs.existsSync(envFile) || !fs.existsSync(userFile)) {
-      return;
+      return true;
     }
     const envData = dotenvUtil.deserialize(fs.readFileSync(envFile, "utf-8"));
     const userEnvData = dotenvUtil.deserialize(fs.readFileSync(userFile, "utf-8"));
     if (!envData.obj["BOT_ID"] && !envData.obj["AAD_APP_CLIENT_ID"]) {
-      return;
+      return true;
     }
     const accountInfo = M365TokenInstance.getCachedAccountInfo();
     if (accountInfo !== undefined) {
       const tokenRes = await M365TokenInstance.getAccessToken({ scopes: GraphScopes });
       if (tokenRes.isErr()) {
-        return;
+        return true;
       }
       const accountJson = ConvertTokenToJson(tokenRes.value);
       const uniqueName = (accountJson as Record<string, string>)["unique_name"];
       if (!uniqueName || !uniqueName.includes("@microsoft.com")) {
-        return;
+        return true;
       }
       VsCodeLogInstance.info(localize("teamstoolkit.localDebug.startDeletingAadProcess"));
       ExtTelemetry.sendTelemetryEvent(TelemetryEvent.StartDeleteAadAfterDebug);
@@ -56,12 +56,18 @@ export async function deleteAad() {
         return config;
       });
       const list: string[] = [];
-      if (envData.obj["BOT_ID"] != undefined) {
+      if (envData.obj["BOT_ID"] != undefined && AadSet.has(envData.obj["BOT_ID"])) {
+        AadSet.delete(envData.obj["BOT_ID"]);
         list.push(envData.obj["BOT_ID"]);
         envData.obj["BOT_ID"] = "";
+        envData.obj["BOT_OBJECT_ID"] = "";
         userEnvData.obj["SECRET_BOT_PASSWORD"] = "";
       }
-      if (envData.obj["AAD_APP_CLIENT_ID"] != undefined) {
+      if (
+        envData.obj["AAD_APP_CLIENT_ID"] != undefined &&
+        AadSet.has(envData.obj["AAD_APP_CLIENT_ID"])
+      ) {
+        AadSet.delete(envData.obj["AAD_APP_CLIENT_ID"]);
         list.push(envData.obj["AAD_APP_CLIENT_ID"]);
         envData.obj["AAD_APP_CLIENT_ID"] = "";
         envData.obj["AAD_APP_OBJECT_ID"] = "";
@@ -106,6 +112,7 @@ export async function deleteAad() {
       VsCodeLogInstance.info(localize("teamstoolkit.localDebug.successDeleteAadProcess"));
       ExtTelemetry.sendTelemetryEvent(TelemetryEvent.SuccessDeleteAadAfterDebug);
     }
+    return true;
   } catch (error) {
     VsCodeLogInstance.warning(
       util.format(
@@ -114,6 +121,7 @@ export async function deleteAad() {
       )
     );
     ExtTelemetry.sendTelemetryErrorEvent(TelemetryEvent.FailDeleteAadAfterDebug, error as FxError);
+    return false;
   } finally {
     globalVariables.setDeleteAadInProgress(false);
   }

diff --git a/packages/vscode-extension/test/localdebug/deleteAADHelper.test.ts b/packages/vscode-extension/test/localdebug/deleteAADHelper.test.ts
@@ -10,6 +10,7 @@ import M365TokenInstance from "../../src/commonlib/m365Login";
 import { ok } from "@microsoft/teamsfx-api";
 import { ExtTelemetry } from "../../src/telemetry/extTelemetry";
 import axios from "axios";
+import * as chai from "chai";
 
 describe("delete aad helper", () => {
   const sandbox = sinon.createSandbox();
@@ -21,14 +22,17 @@ describe("delete aad helper", () => {
   describe("delete aad", () => {
     it("file does not exist", async () => {
       sandbox.stub(globalVariables, "workspaceUri").value(vscode.Uri.file("path"));
-      await deleteAad();
+      sandbox.stub(fs, "existsSync").returns(false);
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("no aad id", async () => {
       sandbox.stub(globalVariables, "workspaceUri").value(vscode.Uri.file("path"));
       sandbox.stub(fs, "existsSync").returns(true);
       sandbox.stub(fs, "readFileSync").returns("{}");
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("normal test account", async () => {
@@ -49,24 +53,26 @@ describe("delete aad helper", () => {
             "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidW5pcXVlX25hbWUiOiJKb2huIERvZSIsImlhdCI6MTUxNjIzOTAyMn0.Y7_rghuQEaTILkMN_421Cut4myfHIhk3hpvHVbpOvnQ"
           )
         );
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("no telemetry handler", async () => {
-      try {
-        sandbox.stub(globalVariables, "workspaceUri").value(vscode.Uri.file("path"));
-        sandbox.stub(fs, "existsSync").returns(true);
-        sandbox.stub(fs, "readFileSync").returns("BOT_ID=botId\n");
-        sandbox.stub(M365TokenInstance, "getCachedAccountInfo").resolves({ upn: "test.email.com" });
-        sandbox
-          .stub(M365TokenInstance, "getAccessToken")
-          .resolves(
-            ok(
-              "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidW5pcXVlX25hbWUiOiJ0ZXN0QG1pY3Jvc29mdC5jb20iLCJpYXQiOjE1MTYyMzkwMjJ9.Rejz-cPndtObAYVa3k3Q7BaltQGXY8KRDxRYKyUoHDw"
-            )
-          );
-        await deleteAad();
-      } catch (error) {}
+      sandbox.stub(globalVariables, "workspaceUri").value(vscode.Uri.file("path"));
+      sandbox.stub(fs, "existsSync").returns(true);
+      sandbox.stub(fs, "readFileSync").returns("BOT_ID=botId\n");
+      sandbox.stub(M365TokenInstance, "getCachedAccountInfo").resolves({ upn: "test.email.com" });
+      sandbox
+        .stub(M365TokenInstance, "getAccessToken")
+        .resolves(
+          ok(
+            "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidW5pcXVlX25hbWUiOiJ0ZXN0QG1pY3Jvc29mdC5jb20iLCJpYXQiOjE1MTYyMzkwMjJ9.Rejz-cPndtObAYVa3k3Q7BaltQGXY8KRDxRYKyUoHDw"
+          )
+        );
+      sandbox.stub(ExtTelemetry, "sendTelemetryEvent").throws(new Error("test error"));
+      sandbox.stub(ExtTelemetry, "sendTelemetryErrorEvent");
+      const res = await deleteAad();
+      chai.assert.isFalse(res);
     });
 
     it("happy path for bot id", async () => {
@@ -87,7 +93,8 @@ describe("delete aad helper", () => {
       const fakeAxiosInstance = axios.create();
       sandbox.stub(axios, "create").returns(fakeAxiosInstance);
       sandbox.stub(fakeAxiosInstance, "delete").resolves({ data: { status: 204 } });
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("happy path for sso id", async () => {
@@ -108,7 +115,8 @@ describe("delete aad helper", () => {
       const fakeAxiosInstance = axios.create();
       sandbox.stub(axios, "create").returns(fakeAxiosInstance);
       sandbox.stub(fakeAxiosInstance, "delete").resolves({ data: { status: 204 } });
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("happy path for bot id and sso id", async () => {
@@ -129,7 +137,8 @@ describe("delete aad helper", () => {
       const fakeAxiosInstance = axios.create();
       sandbox.stub(axios, "create").returns(fakeAxiosInstance);
       sandbox.stub(fakeAxiosInstance, "delete").resolves({ data: { status: 204 } });
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
 
     it("axios handler error", async () => {
@@ -150,7 +159,8 @@ describe("delete aad helper", () => {
       const fakeAxiosInstance = axios.create();
       sandbox.stub(axios, "create").returns(fakeAxiosInstance);
       sandbox.stub(fakeAxiosInstance, "delete").rejects(new Error("error"));
-      await deleteAad();
+      const res = await deleteAad();
+      chai.assert.isTrue(res);
     });
   });
 });