fix(api-security): use AsyncLocalStorage in withoutAuthorization

.adiorc.js

@@ -1,4 +1,4 @@
-const get = require("lodash.get");
+const get = require("lodash/get");
 const getWorkspaces = require("get-yarn-workspaces");
 const path = require("path");
 
@@ -22,23 +22,24 @@ module.exports = {
     },
     ignore: {
         src: [
+            "~tests",
+            "~",
+            "async_hooks",
+            "aws-sdk",
+            "buffer",
+            "child_process",
+            "crypto",
+            "events",
+            "follow-redirects",
+            "fs",
             "http",
-            "path",
             "https",
-            "follow-redirects",
-            "child_process",
             "os",
-            "buffer",
-            "fs",
+            "path",
             "readline",
             "util",
-            "events",
-            "crypto",
-            "aws-sdk",
             "url",
-            "worker_threads",
-            "~tests",
-            "~"
+            "worker_threads"
         ],
         dependencies: [
             "@babel/runtime",

packages/api-security/__tests__/graphql/parallelQueries.ts

@@ -0,0 +1,49 @@
+import { GraphQLSchemaPlugin } from "@webiny/handler-graphql";
+import { SecurityContext } from "~/types";
+
+export const PARALLEL_QUERY = /* GraphQL */ `
+    query ParallelQueries {
+        withoutAuthorization
+        withAuthorization
+        security {
+            listApiKeys {
+                data {
+                    id
+                    token
+                }
+                error {
+                    code
+                }
+            }
+        }
+    }
+`;
+
+export const withoutAuthorizationPlugin = new GraphQLSchemaPlugin<SecurityContext>({
+    typeDefs: /* GraphQL*/ `
+        extend type Query {
+            withoutAuthorization: String!
+            withAuthorization: String!
+        }
+    `,
+    resolvers: {
+        Query: {
+            withoutAuthorization(_, args, context) {
+                return context.security.withoutAuthorization(async () => {
+                    const permissions = await context.security.getPermissions("security.apiKey");
+                    if (!permissions.length) {
+                        return "NOT_AUTHORIZED";
+                    }
+                    return "YOUR DATA!";
+                });
+            },
+            async withAuthorization(_, args, context) {
+                const permissions = await context.security.getPermissions("security.apiKey");
+                if (!permissions.length) {
+                    return "NOT_AUTHORIZED";
+                }
+                return "AUTHORIZED";
+            }
+        }
+    }
+});

packages/api-security/__tests__/parallelQueries.test.ts

@@ -0,0 +1,31 @@
+import useGqlHandler from "./useGqlHandler";
+import { PARALLEL_QUERY, withoutAuthorizationPlugin } from "./graphql/parallelQueries";
+
+describe("Security Parallel Queries", () => {
+    const { install, invoke } = useGqlHandler({ plugins: [withoutAuthorizationPlugin] });
+
+    beforeEach(async () => {
+        await install.install();
+    });
+
+    test("should not disable authorization in parallel queries", async () => {
+        const [response] = await invoke({
+            body: { query: PARALLEL_QUERY },
+            // We want to simulate an anonymous user.
+            headers: { authorization: "anonymous" }
+        });
+
+        expect(response.data).toEqual({
+            withoutAuthorization: "YOUR DATA!",
+            withAuthorization: "NOT_AUTHORIZED",
+            security: {
+                listApiKeys: {
+                    data: null,
+                    error: {
+                        code: "SECURITY_NOT_AUTHORIZED"
+                    }
+                }
+            }
+        });
+    });
+});

packages/api-security/__tests__/withoutAuthorization.test.ts

@@ -11,7 +11,9 @@ describe("without authorization", function () {
         advancedAccessControlLayer: {
             enabled: true,
             options: {
-                teams: false
+                teams: false,
+                folderLevelPermissions: false,
+                privateFiles: false
             }
         },
         storageOperations: {} as any,
@@ -24,7 +26,7 @@ describe("without authorization", function () {
         security = await createSecurity(config);
     });
 
-    it("should disable authorization inside the withoutAuthorization method", async () => {
+    it(`should disable authorization inside "withoutAuthorization" execution scope`, async () => {
         /**
          * Should not return permission as user does not have it (not defined in this case)
          */
@@ -47,74 +49,29 @@ describe("without authorization", function () {
         expect(noPermissionCheckAfterWithoutAuthorization).toEqual(null);
     });
 
-    it("should not enable authorization if it was disabled before the withoutAuthorization method", async () => {
-        security.disableAuthorization();
-        /**
-         * Should have full permission as we are disabling authorization.
-         */
-        const noPermissionCheck = await security.getPermission("some-unknown-permission");
-        expect(noPermissionCheck).toEqual(fullPermissions);
-        /**
-         * Should return full permission as we are disabling authorization.
-         */
-        const result = await security.withoutAuthorization(async () => {
-            return security.getPermission("some-unknown-permission");
-        });
-
-        expect(result).toEqual(fullPermissions);
-        /**
-         * Should have full permission as the authorization is not still enabled.
-         */
-        const hasPermissionsCheckAfterWithoutAuthorization = await security.getPermission(
-            "some-unknown-permission"
-        );
-        expect(hasPermissionsCheckAfterWithoutAuthorization).toEqual(fullPermissions);
-        security.enableAuthorization();
-        /**
-         * Should not have permission again.
-         */
-        const noPermissionCheckAfterEnabling = await security.getPermission(
-            "some-unknown-permission"
-        );
-        expect(noPermissionCheckAfterEnabling).toEqual(null);
-    });
-
-    it("should enable authorization if there is an exception in the function - previously enabled authorization", async () => {
+    it("should re-enable authorization if callback throws an error", async () => {
         let error: Error | null = null;
         let result: any = null;
+        let authorizationWithinCallback = null;
+
         const noPermissionCheck = await security.getPermission("some-unknown-permission");
         expect(noPermissionCheck).toEqual(null);
+
         try {
             result = await security.withoutAuthorization(async () => {
+                authorizationWithinCallback = security.isAuthorizationEnabled();
                 throw new Error("Some error");
             });
         } catch (ex) {
             error = ex;
         }
+
         expect(result).toBeNull();
         expect(error?.message).toEqual("Some error");
+        expect(authorizationWithinCallback).toBe(false);
+        expect(security.isAuthorizationEnabled()).toBe(true);
 
         const stillNoPermissionCheck = await security.getPermission("some-unknown-permission");
         expect(stillNoPermissionCheck).toEqual(null);
     });
-
-    it("should not enable authorization if there is an exception in the function - previously disabled authorization", async () => {
-        let error: Error | null = null;
-        let result: any = null;
-        security.disableAuthorization();
-        const hasPermissionCheck = await security.getPermission("some-unknown-permission");
-        expect(hasPermissionCheck).toEqual(fullPermissions);
-        try {
-            result = await security.withoutAuthorization(async () => {
-                throw new Error("Some error");
-            });
-        } catch (ex) {
-            error = ex;
-        }
-        expect(result).toBeNull();
-        expect(error?.message).toEqual("Some error");
-
-        const stillHasPermissionCheck = await security.getPermission("some-unknown-permission");
-        expect(stillHasPermissionCheck).toEqual(fullPermissions);
-    });
 });

packages/api-security/src/createSecurity.ts

@@ -1,3 +1,4 @@
+import { AsyncLocalStorage } from "async_hooks";
 import minimatch from "minimatch";
 import { createAuthentication } from "@webiny/api-authentication/createAuthentication";
 import { Authorizer, Security, SecurityPermission, SecurityConfig } from "./types";
@@ -14,15 +15,16 @@ export interface GetTenant {
     (): string | undefined;
 }
 
+const asyncLocalStorage = new AsyncLocalStorage<boolean>();
+
 export const createSecurity = async (config: SecurityConfig): Promise<Security> => {
     const authentication = createAuthentication();
     const authorizers: Authorizer[] = [];
-    let performAuthorization = true;
 
     let permissions: SecurityPermission[];
     let permissionsLoader: Promise<SecurityPermission[]>;
 
-    const loadPermissions = async (security: Security): Promise<SecurityPermission[]> => {
+    const loadPermissions = async (): Promise<SecurityPermission[]> => {
         if (permissions) {
             return permissions;
         }
@@ -31,13 +33,11 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
             return permissionsLoader;
         }
 
-        const shouldEnableAuthorization = performAuthorization;
         permissionsLoader = new Promise<SecurityPermission[]>(async resolve => {
             // Authorizers often need to query business-related data, and since the identity is not yet
             // authorized, these operations can easily trigger a NOT_AUTHORIZED error.
             // To avoid this, we disable permission checks (assume `full-access` permissions) for
             // the duration of the authorization process.
-            security.disableAuthorization();
             for (const authorizer of authorizers) {
                 const result = await authorizer();
                 if (Array.isArray(result)) {
@@ -48,13 +48,6 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
             // Set an empty array since no permissions were found.
             permissions = [];
             resolve(permissions);
-        }).then(permissions => {
-            // Re-enable authorization.
-            if (shouldEnableAuthorization) {
-                security.enableAuthorization();
-            }
-
-            return permissions;
         });
 
         return permissionsLoader;
@@ -70,12 +63,6 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
         getStorageOperations() {
             return config.storageOperations;
         },
-        enableAuthorization() {
-            performAuthorization = true;
-        },
-        disableAuthorization() {
-            performAuthorization = false;
-        },
         addAuthorizer(authorizer: Authorizer) {
             authorizers.push(authorizer);
         },
@@ -87,24 +74,16 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
             this.onIdentity.publish({ identity });
         },
         isAuthorizationEnabled: () => {
-            return performAuthorization;
+            return asyncLocalStorage.getStore() ?? true;
         },
-        async withoutAuthorization<T = any>(cb: () => Promise<T>): Promise<T> {
-            const isAuthorizationEnabled = performAuthorization;
-            performAuthorization = false;
-            try {
-                return await cb();
-            } finally {
-                if (isAuthorizationEnabled) {
-                    performAuthorization = true;
-                }
-            }
+        async withoutAuthorization<T = any>(this: Security, cb: () => Promise<T>): Promise<T> {
+            return await asyncLocalStorage.run(false, cb);
         },
         async getPermission<TPermission extends SecurityPermission = SecurityPermission>(
             this: Security,
             permission: string
         ): Promise<TPermission | null> {
-            if (!performAuthorization) {
+            if (!this.isAuthorizationEnabled()) {
                 return { name: "*" } as TPermission;
             }
 
@@ -129,7 +108,7 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
             this: Security,
             permission: string
         ): Promise<TPermission[]> {
-            if (!performAuthorization) {
+            if (!this.isAuthorizationEnabled()) {
                 return [{ name: "*" }] as TPermission[];
             }
 
@@ -146,7 +125,7 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
         },
 
         async listPermissions(this: Security): Promise<SecurityPermission[]> {
-            const permissions = await loadPermissions(this);
+            const permissions = await this.withoutAuthorization(() => loadPermissions());
 
             // Now we start checking whether we want to return all permissions, or we
             // need to omit the custom ones because of the one of the following reasons.

packages/api-security/src/types.ts

@@ -90,22 +90,6 @@ export interface Security<TIdentity = SecurityIdentity> extends Authentication<T
 
     withoutAuthorization<T = any>(cb: () => Promise<T>): Promise<T>;
 
-    /**
-     * Replace in favor of withoutAuthorization.
-     *
-     * If really required, should be used carefully.
-     * @deprecated
-     */
-    enableAuthorization(): void;
-
-    /**
-     * Replace in favor of withoutAuthorization.
-     *
-     * If really required, should be used carefully.
-     * @deprecated
-     */
-    disableAuthorization(): void;
-
     addAuthorizer(authorizer: Authorizer): void;
 
     getAuthorizers(): Authorizer[];