Merge pull request #902 from InseeFrLab/feat-disable-policy

feat: disable policy feature when 403 or error when fetching and parsing bucket policy

web/eslint.config.js

@@ -42,7 +42,8 @@ export default tseslint.config(
             "no-empty": "off",
             "@typescript-eslint/no-unused-vars": "off",
             "@typescript-eslint/ban-ts-comment": "off",
-            "@typescript-eslint/ban-types": "off"
+            "@typescript-eslint/ban-types": "off",
+            "@typescript-eslint/no-unused-expressions": "off"
         }
     },
     ...storybook.configs["flat/recommended"],

web/src/core/adapters/s3Client/s3Client.ts

@@ -10,7 +10,7 @@ import { bucketNameAndObjectNameFromS3Path } from "./utils/bucketNameAndObjectNa
 import { exclude } from "tsafe/exclude";
 import { fnv1aHashToHex } from "core/tools/fnv1aHashToHex";
 import { checkIfS3KeyIsPublic } from "core/tools/checkIfS3KeyIsPublic";
-import { s3BucketPolicySchema } from "./utils/policySchema";
+import { zS3BucketPolicy } from "./utils/policySchema";
 import {
     addObjectNameToListBucketCondition,
     addResourceArnInGetObjectStatement,
@@ -331,60 +331,107 @@ export function createS3Client(
 
             const { awsS3Client } = await getAwsS3Client();
 
-            const bucketPolicyAndAllowedPrefix = await (async () => {
-                const { GetBucketPolicyCommand } = await import("@aws-sdk/client-s3");
+            const resolveBucketPolicy = async () => {
+                const { GetBucketPolicyCommand, S3ServiceException } = await import(
+                    "@aws-sdk/client-s3"
+                );
 
                 let sendResp: import("@aws-sdk/client-s3").GetBucketPolicyCommandOutput;
-
                 try {
                     sendResp = await awsS3Client.send(
-                        new GetBucketPolicyCommand({
-                            Bucket: bucketName
-                        })
+                        new GetBucketPolicyCommand({ Bucket: bucketName })
                     );
-                } catch {
-                    console.log("The error is ok, there is no bucket policy");
-                    return undefined;
+                } catch (error) {
+                    if (!(error instanceof S3ServiceException)) {
+                        console.error(
+                            "An unknown error occurred when fetching bucket policy",
+                            error
+                        );
+                        return {
+                            isBucketPolicyAvailable: false,
+                            bucketPolicy: undefined,
+                            allowedPrefix: []
+                        };
+                    }
+
+                    switch (error.$metadata?.httpStatusCode) {
+                        case 404:
+                            console.info("Bucket policy does not exist (404), it's ok.");
+                            return {
+                                isBucketPolicyAvailable: true,
+                                bucketPolicy: undefined,
+                                allowedPrefix: []
+                            };
+                        case 403:
+                            console.info("Access denied to bucket policy (403).");
+                            break;
+                        default:
+                            console.error("An S3 error occurred:", error.message);
+                            break;
+                    }
+                    return {
+                        isBucketPolicyAvailable: false,
+                        bucketPolicy: undefined,
+                        allowedPrefix: []
+                    };
                 }
 
-                if (sendResp.Policy === undefined) {
-                    return undefined;
+                if (!sendResp.Policy) {
+                    return {
+                        isBucketPolicyAvailable: true,
+                        bucketPolicy: undefined,
+                        allowedPrefix: []
+                    };
                 }
 
-                try {
-                    // Validate and parse the policy
-                    const parsedPolicy = s3BucketPolicySchema.parse(
-                        JSON.parse(sendResp.Policy)
-                    );
+                const s3BucketPolicy = (() => {
+                    const s3BucketPolicy = JSON.parse(sendResp.Policy);
 
-                    // Extract allowed prefixes based on the policy statements
-                    const allowedPrefix = parsedPolicy.Statement.filter(
-                        statement =>
-                            statement.Effect === "Allow" &&
-                            (statement.Action.includes("s3:GetObject") ||
-                                statement.Action.includes("s3:*"))
-                    )
-                        .flatMap(statement =>
-                            Array.isArray(statement.Resource)
-                                ? statement.Resource
-                                : [statement.Resource]
-                        )
-                        .map(resource =>
-                            resource.replace(`arn:aws:s3:::${bucketName}/`, "")
-                        );
+                    try {
+                        // Validate and parse the policy
+                        zS3BucketPolicy.parse(s3BucketPolicy);
+                    } catch (error) {
+                        console.error("Bucket policy isn't of the expected shape", error);
+                        return undefined;
+                    }
 
-                    return { bucketPolicy: parsedPolicy, allowedPrefix };
-                } catch (e) {
-                    console.warn("The best effort attempt failed to parse the policy", e);
-                    return undefined;
+                    assert(is<S3BucketPolicy>(s3BucketPolicy));
+
+                    return s3BucketPolicy;
+                })();
+
+                if (s3BucketPolicy === undefined) {
+                    return {
+                        isBucketPolicyAvailable: false,
+                        bucketPolicy: undefined,
+                        allowedPrefix: []
+                    };
                 }
-            })();
 
-            const { allowedPrefix, bucketPolicy } = bucketPolicyAndAllowedPrefix ?? {
-                allowedPrefix: [],
-                bucketPolicy: undefined
+                // Extract allowed prefixes based on the policy statements
+                const allowedPrefix = s3BucketPolicy.Statement.filter(
+                    statement =>
+                        statement.Effect === "Allow" &&
+                        (statement.Action.includes("s3:GetObject") ||
+                            statement.Action.includes("s3:*"))
+                )
+                    .flatMap(statement =>
+                        Array.isArray(statement.Resource)
+                            ? statement.Resource
+                            : [statement.Resource]
+                    )
+                    .map(resource => resource.replace(`arn:aws:s3:::${bucketName}/`, ""));
+
+                return {
+                    isBucketPolicyAvailable: true,
+                    s3BucketPolicy,
+                    allowedPrefix
+                };
             };
 
+            const { isBucketPolicyAvailable, allowedPrefix, bucketPolicy } =
+                await resolveBucketPolicy();
+
             const Contents: import("@aws-sdk/client-s3")._Object[] = [];
             const CommonPrefixes: import("@aws-sdk/client-s3").CommonPrefix[] = [];
 
@@ -439,7 +486,11 @@ export function createS3Client(
                 }
             );
 
-            return { objects: [...directories, ...files], bucketPolicy };
+            return {
+                objects: [...directories, ...files],
+                bucketPolicy,
+                isBucketPolicyAvailable
+            };
         },
         setPathAccessPolicy: async ({ currentBucketPolicy, policy, path }) => {
             const { getAwsS3Client } = await prApi;

web/src/core/adapters/s3Client/utils/policySchema.ts

@@ -1,22 +1,43 @@
-import { S3BucketPolicy } from "core/ports/S3Client";
+import type { S3BucketPolicy } from "core/ports/S3Client";
 import { assert, Equals } from "tsafe";
 import { z } from "zod";
+import { id } from "tsafe/id";
 
-const s3ActionSchema = z.custom<`s3:${string}`>(
+const zS3Action = z.custom<`s3:${string}`>(
     val => typeof val === "string" && val.startsWith("s3:")
 );
 
-const s3PolicyStatementSchema = z.object({
-    Effect: z.enum(["Allow", "Deny"]),
-    Principal: z.union([z.string(), z.object({ AWS: z.string().array() })]),
-    Action: z.union([s3ActionSchema, s3ActionSchema.array()]),
-    Resource: z.string().array(),
-    Condition: z.record(z.any()).optional()
-});
+type S3PolicyStatement = S3BucketPolicy["Statement"][number];
 
-export const s3BucketPolicySchema = z.object({
-    Version: z.literal("2012-10-17"),
-    Statement: z.array(s3PolicyStatementSchema)
-});
+const zS3PolicyStatement = (() => {
+    type TargetType = S3PolicyStatement;
 
-assert<Equals<z.infer<typeof s3BucketPolicySchema>, S3BucketPolicy>>(true);
+    const zTargetType = z.object({
+        Effect: z.enum(["Allow", "Deny"]),
+        Principal: z.union([z.string(), z.object({ AWS: z.string().array() })]),
+        Action: z.union([zS3Action, zS3Action.array()]),
+        Resource: z.string().array(),
+        Condition: z.record(z.any()).optional()
+    });
+
+    type InferredType = z.infer<typeof zTargetType>;
+
+    assert<Equals<TargetType, InferredType>>;
+
+    return id<z.ZodType<TargetType>>(zTargetType);
+})();
+
+export const zS3BucketPolicy = (() => {
+    type TargetType = S3BucketPolicy;
+
+    const zTargetType = z.object({
+        Version: z.literal("2012-10-17"),
+        Statement: z.array(zS3PolicyStatement)
+    });
+
+    type InferredType = z.infer<typeof zTargetType>;
+
+    assert<Equals<TargetType, InferredType>>;
+
+    return id<z.ZodType<TargetType>>(zTargetType);
+})();

web/src/core/ports/S3Client.ts

@@ -33,9 +33,11 @@ export type S3Client = {
     /**
      *  In charge of creating bucket if doesn't exist.
      */
-    listObjects: (params: {
-        path: string;
-    }) => Promise<{ objects: S3Object[]; bucketPolicy: S3BucketPolicy | undefined }>;
+    listObjects: (params: { path: string }) => Promise<{
+        objects: S3Object[];
+        bucketPolicy: S3BucketPolicy | undefined;
+        isBucketPolicyAvailable: boolean;
+    }>;
 
     setPathAccessPolicy: (params: {
         path: string;
@@ -69,11 +71,11 @@ type s3Action = `s3:${string}`;
 
 export type S3BucketPolicy = {
     Version: "2012-10-17";
-    Statement: Array<{
+    Statement: {
         Effect: "Allow" | "Deny";
         Principal: string | { AWS: string[] };
         Action: s3Action | s3Action[];
         Resource: string[];
         Condition?: Record<string, any>;
-    }>;
+    }[];
 };

web/src/core/usecases/fileExplorer/selectors.ts

@@ -55,6 +55,7 @@ const commandLogsEntries = createSelector(
 export type CurrentWorkingDirectoryView = {
     directoryPath: string;
     items: CurrentWorkingDirectoryView.Item[];
+    isBucketPolicyFeatureEnabled: boolean;
 };
 
 export namespace CurrentWorkingDirectoryView {
@@ -93,12 +94,13 @@ const currentWorkingDirectoryView = createSelector(
     createSelector(state, state => state.objects),
     createSelector(state, state => state.ongoingOperations),
     createSelector(state, state => state.s3FilesBeingUploaded),
-
+    createSelector(state, state => state.isBucketPolicyAvailable),
     (
         directoryPath,
         objects,
         ongoingOperations,
-        s3FilesBeingUploaded
+        s3FilesBeingUploaded,
+        isBucketPolicyAvailable
     ): CurrentWorkingDirectoryView | null => {
         if (directoryPath === undefined) {
             return null;
@@ -184,7 +186,8 @@ const currentWorkingDirectoryView = createSelector(
 
         return {
             directoryPath,
-            items
+            items,
+            isBucketPolicyFeatureEnabled: isBucketPolicyAvailable
         };
     }
 );

web/src/core/usecases/fileExplorer/state.ts

@@ -30,6 +30,7 @@ export type State = {
         resp: string | undefined;
     }[];
     bucketPolicy: S3BucketPolicy;
+    isBucketPolicyAvailable: boolean;
     share:
         | {
               fileBasename: string;
@@ -57,6 +58,7 @@ export const { reducer, actions } = createUsecaseActions({
             Version: "2012-10-17",
             Statement: []
         },
+        isBucketPolicyAvailable: true,
         share: undefined
     }),
     reducers: {
@@ -127,17 +129,20 @@ export const { reducer, actions } = createUsecaseActions({
                     directoryPath: string;
                     objects: S3Object[];
                     bucketPolicy: S3BucketPolicy | undefined;
+                    isBucketPolicyAvailable: boolean;
                 };
             }
         ) => {
-            const { directoryPath, objects, bucketPolicy } = payload;
+            const { directoryPath, objects, bucketPolicy, isBucketPolicyAvailable } =
+                payload;
 
             state.directoryPath = directoryPath;
             state.objects = objects;
             state.isNavigationOngoing = false;
             if (bucketPolicy) {
                 state.bucketPolicy = bucketPolicy;
             }
+            state.isBucketPolicyAvailable = isBucketPolicyAvailable;
             // Properly restore state when navigating back to
             // a directory with ongoing operations.
             state.ongoingOperations

web/src/core/usecases/fileExplorer/thunks.ts

@@ -147,9 +147,10 @@ const privateThunks = {
                 return r.s3Client;
             });
 
-            const { objects, bucketPolicy } = await s3Client.listObjects({
-                path: directoryPath
-            });
+            const { objects, bucketPolicy, isBucketPolicyAvailable } =
+                await s3Client.listObjects({
+                    path: directoryPath
+                });
 
             if (ctx.completionStatus !== undefined) {
                 dispatch(actions.commandLogCancelled({ cmdId }));
@@ -173,7 +174,8 @@ const privateThunks = {
                 actions.navigationCompleted({
                     directoryPath,
                     objects,
-                    bucketPolicy
+                    bucketPolicy,
+                    isBucketPolicyAvailable
                 })
             );
         }
@@ -279,7 +281,12 @@ export const thunks = {
 
             const state = getState()[name];
 
-            const { directoryPath, objects } = state;
+            const { directoryPath, objects, isBucketPolicyAvailable } = state;
+
+            if (!isBucketPolicyAvailable) {
+                console.info("Bucket policy is not available");
+                return;
+            }
 
             const object = objects.find(o => o.basename === basename && o.kind === kind);