Fixed an issue with subpaths in resources policies (#2856)

Signed-off-by: Benjamin Perez <benjamin@bexsoft.net>

portal-ui/src/screens/Console/Buckets/BucketDetails/BrowserHandler.tsx

@@ -214,7 +214,7 @@ const BrowserHandler = () => {
           }
 
           const permitItems = permissionItems(
-            bucketName,
+            response.bucketName || bucketName,
             pathPrefix,
             allowResources || []
           );

portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/types.tsx

@@ -46,6 +46,7 @@ export interface WebsocketResponse {
   request_end?: boolean;
   data?: ObjectResponse[];
   prefix?: string;
+  bucketName?: string;
 }
 
 export interface ObjectResponse {

portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/utils.ts

@@ -296,7 +296,7 @@ export const permissionItems = (
     return null;
   }
 
-  const returnElements: BucketObjectItem[] = [];
+  let returnElements: BucketObjectItem[] = [];
 
   // We split current path
   const splitCurrentPath = currentPath.split("/");
@@ -354,6 +354,14 @@ export const permissionItems = (
 
           let pathToRouteElements: string[] = [];
 
+          // We verify if currentPath is contained in the path begin, if is not contained the  user has no access to this subpath
+          const cleanCurrPath = currentPath.replace(/\/$/, "");
+
+          if (!prefixItem.startsWith(cleanCurrPath) && currentPath !== "") {
+            return;
+          }
+
+          // For every split element we iterate and check if we can construct a URL
           splitItems.every((splitElement, index) => {
             if (!splitElement.includes("*") && splitElement !== "") {
               if (splitElement !== splitCurrentPath[index]) {
@@ -380,5 +388,20 @@ export const permissionItems = (
     }
   });
 
+  // We clean duplicated name entries
+  if (returnElements.length > 0) {
+    let clElements: BucketObjectItem[] = [];
+    let keys: string[] = [];
+
+    returnElements.forEach((itm) => {
+      if (!keys.includes(itm.name)) {
+        clElements.push(itm);
+        keys.push(itm.name);
+      }
+    });
+
+    returnElements = clElements;
+  }
+
   return returnElements;
 };

portal-ui/tests/permissions-7/resourceTesting.ts

@@ -25,10 +25,14 @@ import {
 fixture("Test resources policy").page("http://localhost:9090/");
 
 const bucket1 = "testcondition";
+const bucket3 = "my-company";
 const test1BucketBrowseButton = namedTestBucketBrowseButtonFor(bucket1);
+const test3BucketBrowseButton = namedTestBucketBrowseButtonFor(bucket3);
 export const file = Selector(".ReactVirtualized__Table__rowColumn").withText(
   "test.txt"
 );
+export const deniedError = Selector(".message-text").withText("Access Denied.");
+
 test
   .before(async (t) => {
     await functions.setUpNamedBucket(t, bucket1);
@@ -142,3 +146,71 @@ test
   .after(async (t) => {
     await functions.cleanUpNamedBucketAndUploads(t, bucket1);
   });
+
+test
+  .before(async (t) => {
+    await functions.setUpNamedBucket(t, bucket3);
+    await functions.uploadNamedObjectToBucket(
+      t,
+      bucket3,
+      "test.txt",
+      "portal-ui/tests/uploads/test.txt"
+    );
+    await functions.uploadNamedObjectToBucket(
+      t,
+      bucket3,
+      "home/UserY/test.txt",
+      "portal-ui/tests/uploads/test.txt"
+    );
+    await functions.uploadNamedObjectToBucket(
+      t,
+      bucket3,
+      "home/UserX/test.txt",
+      "portal-ui/tests/uploads/test.txt"
+    );
+    await functions.uploadNamedObjectToBucket(
+      t,
+      bucket3,
+      "home/User/test.txt",
+      "portal-ui/tests/uploads/test.txt"
+    );
+    await functions.uploadNamedObjectToBucket(
+      t,
+      bucket3,
+      "home/User/secondlevel/thirdlevel/test.txt",
+      "portal-ui/tests/uploads/test.txt"
+    );
+  })("User can browse from sub levels as policy has wildcard", async (t) => {
+    await t
+      .useRole(roles.conditions3)
+      .navigateTo(`http://localhost:9090/browser`)
+      .click(test3BucketBrowseButton)
+      .wait(1500)
+      .click(Selector(".ReactVirtualized__Table__rowColumn").withText("home"))
+      .wait(1500)
+      .click(Selector(".ReactVirtualized__Table__rowColumn").withText("User"))
+      .wait(1500)
+      .expect(file.exists)
+      .ok()
+      .click(
+        Selector(".ReactVirtualized__Table__rowColumn").withText("secondlevel")
+      )
+      .wait(1500)
+      .click(
+        Selector(".ReactVirtualized__Table__rowColumn").withText("thirdlevel")
+      )
+      .wait(1500)
+      .expect(file.exists)
+      .ok()
+      .navigateTo(`http://localhost:9090/browser`)
+      .click(test3BucketBrowseButton)
+      .wait(1500)
+      .click(Selector(".ReactVirtualized__Table__rowColumn").withText("home"))
+      .wait(1500)
+      .click(Selector(".ReactVirtualized__Table__rowColumn").withText("UserX"))
+      .expect(deniedError.exists)
+      .ok();
+  })
+  .after(async (t) => {
+    await functions.cleanUpNamedBucketAndUploads(t, bucket3);
+  });

portal-ui/tests/policies/conditionsPolicy3.json

@@ -0,0 +1,36 @@
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "AllowUserToSeeBucketListInTheConsole",
+      "Action": ["s3:ListAllMyBuckets", "s3:GetBucketLocation"],
+      "Effect": "Allow",
+      "Resource": ["arn:aws:s3:::*"]
+    },
+    {
+      "Sid": "AllowRootAndHomeListingOfCompanyBucket",
+      "Action": ["s3:ListBucket"],
+      "Effect": "Allow",
+      "Resource": ["arn:aws:s3:::my-company"],
+      "Condition": {
+        "StringEquals": {
+          "s3:prefix": ["", "home/", "home/User"],
+          "s3:delimiter": ["/"]
+        }
+      }
+    },
+    {
+      "Sid": "AllowListingOfUserFolder",
+      "Action": ["s3:ListBucket"],
+      "Effect": "Allow",
+      "Resource": ["arn:aws:s3:::my-company"],
+      "Condition": { "StringLike": { "s3:prefix": ["home/User/*"] } }
+    },
+    {
+      "Sid": "AllowAllS3ActionsInUserFolder",
+      "Effect": "Allow",
+      "Action": ["s3:*"],
+      "Resource": ["arn:aws:s3:::my-company/home/User/*"]
+    }
+  ]
+}

portal-ui/tests/scripts/cleanup-env.sh

@@ -30,6 +30,7 @@ remove_users() {
   mc admin user remove minio prefix-policy-ui-crash-$TIMESTAMP
   mc admin user remove minio conditions-$TIMESTAMP
   mc admin user remove minio conditions-2-$TIMESTAMP
+  mc admin user remove minio conditions-3-$TIMESTAMP
 }
 
 remove_policies() {
@@ -54,6 +55,7 @@ remove_policies() {
   mc admin policy remove minio fix-prefix-policy-ui-crash-$TIMESTAMP
   mc admin policy remove minio conditions-policy-$TIMESTAMP
   mc admin policy remove minio conditions-policy-2-$TIMESTAMP
+  mc admin policy remove minio conditions-policy-3-$TIMESTAMP
 }
 
 __init__() {

portal-ui/tests/scripts/common.sh

@@ -48,6 +48,7 @@ create_policies() {
   mc admin policy create minio delete-object-with-prefix-$TIMESTAMP portal-ui/tests/policies/deleteObjectWithPrefix.json
   mc admin policy create minio conditions-policy-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy.json
   mc admin policy create minio conditions-policy-2-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy2.json
+  mc admin policy create minio conditions-policy-3-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy3.json
 }
 
 create_users() {
@@ -77,6 +78,7 @@ create_users() {
   mc admin user add minio delete-object-with-prefix-$TIMESTAMP deleteobjectwithprefix1234
   mc admin user add minio conditions-$TIMESTAMP conditions1234
   mc admin user add minio conditions-2-$TIMESTAMP conditions1234
+  mc admin user add minio conditions-3-$TIMESTAMP conditions1234
 }
 
 create_buckets() {
@@ -111,4 +113,5 @@ assign_policies() {
   mc admin policy attach minio delete-object-with-prefix-$TIMESTAMP --user delete-object-with-prefix-$TIMESTAMP
   mc admin policy attach minio conditions-policy-$TIMESTAMP --user conditions-$TIMESTAMP
   mc admin policy attach minio conditions-policy-2-$TIMESTAMP --user conditions-2-$TIMESTAMP
+  mc admin policy attach minio conditions-policy-3-$TIMESTAMP --user conditions-3-$TIMESTAMP
 }

portal-ui/tests/scripts/permissions.sh

@@ -38,6 +38,7 @@ remove_users() {
   mc admin user remove minio delete-object-with-prefix-"$TIMESTAMP"
   mc admin user remove minio conditions-"$TIMESTAMP"
   mc admin user remove minio conditions-2-"$TIMESTAMP"
+  mc admin user remove minio conditions-3-"$TIMESTAMP"
 }
 
 remove_policies() {
@@ -63,6 +64,7 @@ remove_policies() {
   mc admin policy remove minio delete-object-with-prefix-"$TIMESTAMP"
   mc admin policy remove conditions-policy-"$TIMESTAMP"
   mc admin policy remove conditions-policy-2-"$TIMESTAMP"
+  mc admin policy remove conditions-policy-3-"$TIMESTAMP"
 }
 
 remove_buckets() {

portal-ui/tests/utils/roles.ts

@@ -272,3 +272,14 @@ export const conditions2 = Role(
   },
   { preserveUrl: true }
 );
+
+export const conditions3 = Role(
+  loginUrl,
+  async (t) => {
+    await t
+      .typeText("#accessKey", "conditions-3-" + unixTimestamp)
+      .typeText("#secretKey", "conditions1234")
+      .click(submitButton);
+  },
+  { preserveUrl: true }
+);

restapi/admin_objects.go

@@ -44,6 +44,7 @@ type WSResponse struct {
 	Error      string           `json:"error,omitempty"`
 	RequestEnd bool             `json:"request_end,omitempty"`
 	Prefix     string           `json:"prefix,omitempty"`
+	BucketName string           `json:"bucketName,omitempty"`
 	Data       []ObjectResponse `json:"data,omitempty"`
 }
 

restapi/ws_objects.go

@@ -104,9 +104,10 @@ func (wsc *wsMinioClient) objectManager(session *models.Principal) {
 							}
 							if lsObj.Err != nil {
 								writeChannel <- WSResponse{
-									RequestID: messageRequest.RequestID,
-									Error:     lsObj.Err.Error(),
-									Prefix:    messageRequest.Prefix,
+									RequestID:  messageRequest.RequestID,
+									Error:      lsObj.Err.Error(),
+									Prefix:     messageRequest.Prefix,
+									BucketName: messageRequest.BucketName,
 								}
 
 								continue
@@ -177,9 +178,10 @@ func (wsc *wsMinioClient) objectManager(session *models.Principal) {
 						for lsObj := range startRewindListing(ctx, mcS3C, objectRqConfigs) {
 							if lsObj.Err != nil {
 								writeChannel <- WSResponse{
-									RequestID: messageRequest.RequestID,
-									Error:     lsObj.Err.String(),
-									Prefix:    messageRequest.Prefix,
+									RequestID:  messageRequest.RequestID,
+									Error:      lsObj.Err.String(),
+									Prefix:     messageRequest.Prefix,
+									BucketName: messageRequest.BucketName,
 								}
 
 								continue