From 2886ac161c9da270d64acb9dc467946834875e33 Mon Sep 17 00:00:00 2001
From: Denis Kirillov <denis@nspcc.ru>
Date: Wed, 9 Nov 2022 17:25:02 +0300
Subject: [PATCH] [#740] Fix forming policy by ast

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
---
 api/handler/acl.go      |  9 +++---
 api/handler/acl_test.go | 72 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/api/handler/acl.go b/api/handler/acl.go
index 0c644e3f0..8e74cd201 100644
--- a/api/handler/acl.go
+++ b/api/handler/acl.go
@@ -1001,6 +1001,10 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) {
 			trimmedResource := strings.TrimPrefix(resource, arnAwsPrefix)
 			r, ok := rr[trimmedResource]
 			if !ok {
+				if !strings.HasPrefix(trimmedResource, bktPolicy.Bucket) {
+					return nil, fmt.Errorf("resource '%s' must be in the same bucket '%s'", trimmedResource, bktPolicy.Bucket)
+				}
+
 				r = &astResource{
 					resourceInfo: resourceInfoFromName(trimmedResource, bktPolicy.Bucket),
 				}
@@ -1046,9 +1050,6 @@ func astToPolicy(ast *ast) *bucketPolicy {
 	bktPolicy := &bucketPolicy{}
 
 	for _, resource := range ast.Resources {
-		if len(resource.Version) == 0 {
-			continue
-		}
 		allowed, denied := triageOperations(resource.Operations)
 		handleResourceOperations(bktPolicy, allowed, eacl.ActionAllow, resource.Name())
 		handleResourceOperations(bktPolicy, denied, eacl.ActionDeny, resource.Name())
@@ -1080,7 +1081,7 @@ func handleResourceOperations(bktPolicy *bucketPolicy, list []*astOperation, eac
 		for action, ops := range actionToOpMap {
 			for _, op := range ops {
 				if !contains(userOps, op) {
-					break LOOP
+					continue LOOP
 				}
 			}
 			actions = append(actions, action)
diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go
index 40f17ba12..42e2e4966 100644
--- a/api/handler/acl_test.go
+++ b/api/handler/acl_test.go
@@ -1,11 +1,13 @@
 package handler
 
 import (
+	"bytes"
 	"context"
 	"crypto/ecdsa"
 	"crypto/rand"
 	"crypto/sha256"
 	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -1297,7 +1299,7 @@ func TestPutBucketACL(t *testing.T) {
 	tc := prepareHandlerContext(t)
 	bktName := "bucket-for-acl"
 
-	box := createAccessBox(t)
+	box, _ := createAccessBox(t)
 	bktInfo := createBucket(t, tc, bktName, box)
 
 	header := map[string]string{api.AmzACL: "public-read"}
@@ -1308,6 +1310,70 @@ func TestPutBucketACL(t *testing.T) {
 	checkLastRecords(t, tc, bktInfo, eacl.ActionDeny)
 }
 
+func TestBucketPolicy(t *testing.T) {
+	hc := prepareHandlerContext(t)
+	bktName := "bucket-for-policy"
+
+	box, key := createAccessBox(t)
+	createBucket(t, hc, bktName, box)
+
+	bktPolicy := getBucketPolicy(hc, bktName)
+	for _, st := range bktPolicy.Statement {
+		if st.Effect == "Allow" {
+			require.Equal(t, hex.EncodeToString(key.PublicKey().Bytes()), st.Principal.CanonicalUser)
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
+		} else {
+			require.Equal(t, allUsersWildcard, st.Principal.AWS)
+			require.Equal(t, "Deny", st.Effect)
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
+		}
+	}
+
+	newPolicy := &bucketPolicy{
+		Statement: []statement{{
+			Effect:    "Allow",
+			Principal: principal{AWS: allUsersWildcard},
+			Action:    []string{s3GetObject},
+			Resource:  []string{arnAwsPrefix + "dummy"},
+		}},
+	}
+
+	putBucketPolicy(hc, bktName, newPolicy, box, http.StatusInternalServerError)
+
+	newPolicy.Statement[0].Resource[0] = arnAwsPrefix + bktName
+	putBucketPolicy(hc, bktName, newPolicy, box, http.StatusOK)
+
+	bktPolicy = getBucketPolicy(hc, bktName)
+	for _, st := range bktPolicy.Statement {
+		if st.Effect == "Allow" && st.Principal.AWS == allUsersWildcard {
+			require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource)
+			require.ElementsMatch(t, []string{s3GetObject, s3ListBucket}, st.Action)
+		}
+	}
+}
+
+func getBucketPolicy(hc *handlerContext, bktName string) *bucketPolicy {
+	w, r := prepareTestRequest(hc, bktName, "", nil)
+	hc.Handler().GetBucketPolicyHandler(w, r)
+
+	assertStatus(hc.t, w, http.StatusOK)
+	policy := &bucketPolicy{}
+	err := json.NewDecoder(w.Result().Body).Decode(policy)
+	require.NoError(hc.t, err)
+	return policy
+}
+
+func putBucketPolicy(hc *handlerContext, bktName string, bktPolicy *bucketPolicy, box *accessbox.Box, status int) {
+	body, err := json.Marshal(bktPolicy)
+	require.NoError(hc.t, err)
+
+	w, r := prepareTestPayloadRequest(hc, bktName, "", bytes.NewReader(body))
+	ctx := context.WithValue(r.Context(), api.BoxData, box)
+	r = r.WithContext(ctx)
+	hc.Handler().PutBucketPolicyHandler(w, r)
+	assertStatus(hc.t, w, status)
+}
+
 func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo, action eacl.Action) {
 	bktACL, err := tc.Layer().GetBucketACL(tc.Context(), bktInfo)
 	require.NoError(t, err)
@@ -1325,7 +1391,7 @@ func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo
 	}
 }
 
-func createAccessBox(t *testing.T) *accessbox.Box {
+func createAccessBox(t *testing.T) (*accessbox.Box, *keys.PrivateKey) {
 	key, err := keys.NewPrivateKey()
 	require.NoError(t, err)
 
@@ -1345,7 +1411,7 @@ func createAccessBox(t *testing.T) *accessbox.Box {
 		},
 	}
 
-	return box
+	return box, key
 }
 
 func createBucket(t *testing.T, tc *handlerContext, bktName string, box *accessbox.Box) *data.BucketInfo {