From af36c92cab6d49779fbefbe82b3708e79125426b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 12 Aug 2019 10:27:38 -0700 Subject: [PATCH] With ListBuckets() access-list only buckets the user has access (#8037) This is a behavior change from AWS S3, but it is done with better judgment on our end to allow the listing of buckets only which user has access to. The advantage is this declutters the UI for users and only lists bucket which they have access to. Precursor for this feature to be applicable is a policy must have the following actions ``` s3:ListAllMyBuckets ``` and ``` s3:ListBucket ``` enabled in the policy. --- cmd/auth-handler.go | 39 ++++++++++++++++++++++++++------------- cmd/bucket-handlers.go | 26 ++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 2e16f668b..9ede51e38 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -259,14 +259,24 @@ func checkClaimsFromToken(r *http.Request, cred auth.Credentials) (map[string]in // for authenticated requests validates IAM policies. // returns APIErrorCode if any to be replied to the client. func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Action, bucketName, objectName string) (s3Err APIErrorCode) { + _, _, s3Err = checkRequestAuthTypeToAccessKey(ctx, r, action, bucketName, objectName) + return s3Err +} + +// Check request auth type verifies the incoming http request +// - validates the request signature +// - validates the policy action if anonymous tests bucket policies if any, +// for authenticated requests validates IAM policies. +// returns APIErrorCode if any to be replied to the client. +// Additionally returns the accessKey used in the request, and if this request is by an admin. +func checkRequestAuthTypeToAccessKey(ctx context.Context, r *http.Request, action policy.Action, bucketName, objectName string) (accessKey string, owner bool, s3Err APIErrorCode) { var cred auth.Credentials - var owner bool switch getRequestAuthType(r) { case authTypeUnknown, authTypeStreamingSigned: - return ErrAccessDenied + return accessKey, owner, ErrAccessDenied case authTypePresignedV2, authTypeSignedV2: if s3Err = isReqAuthenticatedV2(r); s3Err != ErrNone { - return s3Err + return accessKey, owner, s3Err } cred, owner, s3Err = getReqAccessKeyV2(r) case authTypeSigned, authTypePresigned: @@ -276,17 +286,18 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac region = "" } if s3Err = isReqAuthenticated(ctx, r, region, serviceS3); s3Err != ErrNone { - return s3Err + return accessKey, owner, s3Err } cred, owner, s3Err = getReqAccessKeyV4(r, region, serviceS3) } if s3Err != ErrNone { - return s3Err + return accessKey, owner, s3Err } - claims, s3Err := checkClaimsFromToken(r, cred) + var claims map[string]interface{} + claims, s3Err = checkClaimsFromToken(r, cred) if s3Err != ErrNone { - return s3Err + return accessKey, owner, s3Err } // LocationConstraint is valid only for CreateBucketAction. @@ -296,7 +307,7 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac payload, err := ioutil.ReadAll(io.LimitReader(r.Body, maxLocationConstraintSize)) if err != nil { logger.LogIf(ctx, err) - return ErrMalformedXML + return accessKey, owner, ErrMalformedXML } // Populate payload to extract location constraint. @@ -305,7 +316,7 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac var s3Error APIErrorCode locationConstraint, s3Error = parseLocationConstraint(r) if s3Error != ErrNone { - return s3Error + return accessKey, owner, s3Error } // Populate payload again to handle it in HTTP handler. @@ -321,9 +332,10 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac IsOwner: false, ObjectName: objectName, }) { - return ErrNone + // Request is allowed return the appropriate access key. + return cred.AccessKey, owner, ErrNone } - return ErrAccessDenied + return accessKey, owner, ErrAccessDenied } if globalIAMSys.IsAllowed(iampolicy.Args{ @@ -335,9 +347,10 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac IsOwner: owner, Claims: claims, }) { - return ErrNone + // Request is allowed return the appropriate access key. + return cred.AccessKey, owner, ErrNone } - return ErrAccessDenied + return accessKey, owner, ErrAccessDenied } // Verify if request has valid AWS Signature Version '2'. diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index ead2eaf74..72f82526a 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -38,6 +38,7 @@ import ( "github.com/minio/minio/pkg/event" "github.com/minio/minio/pkg/handlers" "github.com/minio/minio/pkg/hash" + iampolicy "github.com/minio/minio/pkg/iam/policy" "github.com/minio/minio/pkg/policy" "github.com/minio/minio/pkg/sync/errgroup" ) @@ -202,7 +203,8 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R listBuckets := objectAPI.ListBuckets - if s3Error := checkRequestAuthType(ctx, r, policy.ListAllMyBucketsAction, "", ""); s3Error != ErrNone { + accessKey, owner, s3Error := checkRequestAuthTypeToAccessKey(ctx, r, policy.ListAllMyBucketsAction, "", "") + if s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) return } @@ -236,8 +238,28 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R } } + // Set prefix value for "s3:prefix" policy conditionals. + r.Header.Set("prefix", "") + + // Set delimiter value for "s3:delimiter" policy conditionals. + r.Header.Set("delimiter", SlashSeparator) + + var newBucketsInfo []BucketInfo + for _, bucketInfo := range bucketsInfo { + if globalIAMSys.IsAllowed(iampolicy.Args{ + AccountName: accessKey, + Action: iampolicy.ListBucketAction, + BucketName: bucketInfo.Name, + ConditionValues: getConditionValues(r, "", accessKey), + IsOwner: owner, + ObjectName: "", + }) { + newBucketsInfo = append(newBucketsInfo, bucketInfo) + } + } + // Generate response. - response := generateListBucketsResponse(bucketsInfo) + response := generateListBucketsResponse(newBucketsInfo) encodedSuccessResponse := encodeResponse(response) // Write response.