perf: fix unknown error when update aad (#11211)

Co-authored-by: turenlong <[email protected]>
OfficeDev · Mar 28, 2024 · 09c7965 · 09c7965
1 parent 26eb7e1
commit 09c7965
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 0 deletions.
diff --git a/packages/fx-core/resource/package.nls.json b/packages/fx-core/resource/package.nls.json
@@ -540,6 +540,8 @@
   "error.aad.manifest.PreAuthorizedApplicationsIsMissing": "preAuthorizedApplications is missing\n",
   "error.aad.manifest.ResourceAppIdIsMissing": "Some item(s) in requiredResourceAccess misses resourceAppId property.",
   "error.aad.manifest.ResourceAccessIdIsMissing": "Some item(s) in resourceAccess misses id property.",
+  "error.aad.manifest.ResourceAccessShouldBeArray": "resourceAccess should be an array.",
+  "error.aad.manifest.RequiredResourceAccessShouldBeArray": "requiredResourceAccess should be an array.",
   "error.aad.manifest.AccessTokenAcceptedVersionIs1": "accessTokenAcceptedVersion is 1\n",
   "error.aad.manifest.OptionalClaimsIsMissing": "optionalClaims is missing\n",
   "error.aad.manifest.OptionalClaimsMissingIdtypClaim": "optionalClaims access token doesn't contain idtyp claim\n",

diff --git a/packages/fx-core/src/component/driver/aad/error/aadManifestError.ts b/packages/fx-core/src/component/driver/aad/error/aadManifestError.ts
@@ -66,6 +66,30 @@ export class MissingResourceAccessIdUserError extends UserError {
   }
 }
 
+export class ResourceAccessShouldBeArrayUserError extends UserError {
+  constructor(actionName: string) {
+    super({
+      source: actionName,
+      name: "ResourceAccessShouldBeArray",
+      message: getDefaultString("error.aad.manifest.ResourceAccessShouldBeArray"),
+      displayMessage: getLocalizedString("error.aad.manifest.ResourceAccessShouldBeArray"),
+      helpLink: "https://aka.ms/teamsfx-aad-manifest",
+    });
+  }
+}
+
+export class RequiredResourceAccessShouldBeArrayUserError extends UserError {
+  constructor(actionName: string) {
+    super({
+      source: actionName,
+      name: "RequiredResourceAccessShouldBeArray",
+      message: getDefaultString("error.aad.manifest.RequiredResourceAccessShouldBeArray"),
+      displayMessage: getLocalizedString("error.aad.manifest.RequiredResourceAccessShouldBeArray"),
+      helpLink: "https://aka.ms/teamsfx-aad-manifest",
+    });
+  }
+}
+
 export class UnknownResourceAccessIdUserError extends UserError {
   constructor(actionName: string, unknownId: string) {
     super({

diff --git a/packages/fx-core/src/component/driver/aad/utility/aadManifestHelper.ts b/packages/fx-core/src/component/driver/aad/utility/aadManifestHelper.ts
@@ -9,6 +9,8 @@ import {
   AadManifestErrorMessage,
   MissingResourceAccessIdUserError,
   MissingResourceAppIdUserError,
+  RequiredResourceAccessShouldBeArrayUserError,
+  ResourceAccessShouldBeArrayUserError,
   UnknownResourceAccessIdUserError,
   UnknownResourceAccessTypeUserError,
   UnknownResourceAppIdUserError,
@@ -216,6 +218,11 @@ export class AadManifestHelper {
 
   public static processRequiredResourceAccessInManifest(manifest: AADManifest): void {
     const map = getPermissionMap();
+
+    if (manifest.requiredResourceAccess && !Array.isArray(manifest.requiredResourceAccess)) {
+      throw new RequiredResourceAccessShouldBeArrayUserError(componentName);
+    }
+
     manifest.requiredResourceAccess?.forEach((requiredResourceAccessItem) => {
       const resourceIdOrName = requiredResourceAccessItem.resourceAppId;
       let resourceId = resourceIdOrName;
@@ -230,6 +237,13 @@ export class AadManifestHelper {
         requiredResourceAccessItem.resourceAppId = resourceId;
       }
 
+      if (
+        requiredResourceAccessItem.resourceAccess &&
+        !Array.isArray(requiredResourceAccessItem.resourceAccess)
+      ) {
+        throw new ResourceAccessShouldBeArrayUserError(componentName);
+      }
+
       requiredResourceAccessItem.resourceAccess?.forEach((resourceAccessItem) => {
         const resourceAccessIdOrName = resourceAccessItem.id;
         if (!resourceAccessIdOrName) {

diff --git a/packages/fx-core/tests/component/driver/aad/aadManifestHelper.test.ts b/packages/fx-core/tests/component/driver/aad/aadManifestHelper.test.ts
@@ -267,6 +267,44 @@ describe("Microsoft Entra manifest helper Test", () => {
         "Unknown resourceAccess id: Sites.Read.All, if you're using permission as resourceAccess id, please try to use permission id instead."
       );
   });
+
+  it("processRequiredResourceAccessInManifest with non-array required resource access/resource access", async () => {
+    let manifest: any = {
+      requiredResourceAccess: {
+        resourceAppId: "Microsoft Graph",
+        resourceAccess: [
+          {
+            id: "User.Read",
+            type: "Scope",
+          },
+        ],
+      },
+    };
+
+    chai
+      .expect(() => {
+        AadManifestHelper.processRequiredResourceAccessInManifest(manifest);
+      })
+      .to.throw("requiredResourceAccess should be an array.");
+
+    manifest = {
+      requiredResourceAccess: [
+        {
+          resourceAppId: "Microsoft Graph",
+          resourceAccess: {
+            id: "Sites.Read.All",
+            type: "Role",
+          },
+        },
+      ],
+    };
+
+    chai
+      .expect(() => {
+        AadManifestHelper.processRequiredResourceAccessInManifest(manifest);
+      })
+      .to.throw("resourceAccess should be an array.");
+  });
 });
 
 const invalidAadManifest: AADManifest = {