fix(Catalog): Add List PolarisStorageAction for all metadata read operations

[fivetran-ashokborra]

Fixes #1380

Issue:

Currently, loadTable returns a 403 error when the metadata file is not found, which can be misleading. Since users are expected to have configured the appropriate ListBucket permissions in their IAM policy, a 404 error would be more accurate in this scenario.

Access Denied or Forbidden error: User: arn:aws:sts::{account}:assumed-role/{role}/PolarisAwsCredentialsStorageIntegration is not authorized to perform: s3:ListBucket on resource: \"arn:aws:s3:::ashok-test-local\" because no session policy allows the s3:ListBucket action (Service: S3, Status Code: 403, Request ID: W6Q2D563ETEKR6XZ, Extended Request ID: Izq3QS7eZmGjhjfyoxJWMHeCgrFvUlpZjj73JYMO8i/qnKw6CjOaPVgOWVLFr/JsToTeTxO0YaM=)

As per S3 GetObject documentation, the error message returned depens on ListBucket permission

If the object that you request doesn’t exist, the error that Amazon S3 returns depends on whether you also have the s3:ListBucket permission.

If you have the s3:ListBucket permission on the bucket, Amazon S3 returns an HTTP status code 404 Not Found error.

If you don’t have the s3:ListBucket permission, Amazon S3 returns an HTTP status code 403 Access Denied error.

We are adding ListBucket statement to the session policy only when PolarisStorageAction.List is passed

[singhpk234]

IMHO, we should in general go with a prinicipal of least priviledges, how hard is this to catch this / parse this and re:throw 404 ?

That being set I do see catalog such as Unity does this already code, mean while can you please add an UT for this ?

[singhpk234]

cc @flyrain @dennishuo

[collado-mike]

IMHO, we should in general go with a prinicipal of least priviledges, how hard is this to catch this / parse this and re:throw 404 ?

Generally, I'd agree about the least privilege concern, but I'm opposed to reinterpreting AWS exceptions. I think we ought to get AWS to return the correct error code so we don't have to guess at runtime

[fivetran-ashokborra]

@singhpk234

mean while can you please add an UT for this ?

This is already covered here and here. Do we still need 1 more UT?

[singhpk234]

@fivetran-ashokborra i was mostly coming from the fact that this was not catched by a test case, so to be it future proof, if some one removes or adds new permission this would fail.