[#553] Check group grantee based on stored list of users

Signed-off-by: Alex Vanin <alexey@nspcc.ru>
This commit is contained in:
Alex Vanin 2022-07-06 16:28:28 +03:00 committed by Alex Vanin
parent c7de7d2928
commit d6065c64c4
2 changed files with 114 additions and 123 deletions

View file

@ -123,11 +123,14 @@ func (r *resourceInfo) IsBucket() bool {
type astOperation struct { type astOperation struct {
Users []string Users []string
IsGroupGrantee bool
Op eacl.Operation Op eacl.Operation
Action eacl.Action Action eacl.Action
} }
func (a astOperation) IsGroupGrantee() bool {
return len(a.Users) == 0
}
func (h *handler) GetBucketACLHandler(w http.ResponseWriter, r *http.Request) { func (h *handler) GetBucketACLHandler(w http.ResponseWriter, r *http.Request) {
reqInfo := api.GetReqInfo(r.Context()) reqInfo := api.GetReqInfo(r.Context())
@ -618,14 +621,14 @@ func mergeAst(parent, child *ast) (*ast, bool) {
switch len(ops) { switch len(ops) {
case 2: case 2:
// potential inconsistency // potential inconsistency
if astOp.IsGroupGrantee { if groupGrantee := astOp.IsGroupGrantee(); groupGrantee {
// it is not likely (such state must be detected early) // it is not likely (such state must be detected early)
// inconsistency // inconsistency
action := eacl.ActionAllow action := eacl.ActionAllow
if astOp.Action == eacl.ActionAllow { if astOp.Action == eacl.ActionAllow {
action = eacl.ActionDeny action = eacl.ActionDeny
} }
removeAstOp(parentResource, astOp.IsGroupGrantee, astOp.Op, action) removeAstOp(parentResource, groupGrantee, astOp.Op, action)
updated = true updated = true
continue continue
} }
@ -641,15 +644,12 @@ func mergeAst(parent, child *ast) (*ast, bool) {
if handleRemoveOperations(parentResource, astOp, opToDelete) { if handleRemoveOperations(parentResource, astOp, opToDelete) {
updated = true updated = true
} }
if !opToDelete.IsGroupGrantee && len(opToDelete.Users) == 0 {
removeAstOp(parentResource, opToDelete.IsGroupGrantee, opToDelete.Op, opToDelete.Action)
}
case 1: case 1:
if astOp.Action != ops[0].Action { if astOp.Action != ops[0].Action {
// potential inconsistency // potential inconsistency
if astOp.IsGroupGrantee { if groupGrantee := astOp.IsGroupGrantee(); groupGrantee {
// inconsistency // inconsistency
removeAstOp(parentResource, astOp.IsGroupGrantee, astOp.Op, ops[0].Action) removeAstOp(parentResource, groupGrantee, astOp.Op, ops[0].Action)
parentResource.Operations = append(parentResource.Operations, astOp) parentResource.Operations = append(parentResource.Operations, astOp)
updated = true updated = true
continue continue
@ -658,9 +658,6 @@ func mergeAst(parent, child *ast) (*ast, bool) {
if handleRemoveOperations(parentResource, astOp, ops[0]) { if handleRemoveOperations(parentResource, astOp, ops[0]) {
updated = true updated = true
} }
if !ops[0].IsGroupGrantee && len(ops[0].Users) == 0 {
removeAstOp(parentResource, ops[0].IsGroupGrantee, ops[0].Op, ops[0].Action)
}
parentResource.Operations = append(parentResource.Operations, astOp) parentResource.Operations = append(parentResource.Operations, astOp)
continue continue
} }
@ -723,7 +720,7 @@ func containsStr(list []string, element string) bool {
func getAstOps(resource *astResource, childOp *astOperation) []*astOperation { func getAstOps(resource *astResource, childOp *astOperation) []*astOperation {
var res []*astOperation var res []*astOperation
for _, astOp := range resource.Operations { for _, astOp := range resource.Operations {
if astOp.IsGroupGrantee == childOp.IsGroupGrantee && astOp.Op == childOp.Op { if astOp.IsGroupGrantee() == childOp.IsGroupGrantee() && astOp.Op == childOp.Op {
res = append(res, astOp) res = append(res, astOp)
} }
} }
@ -732,7 +729,7 @@ func getAstOps(resource *astResource, childOp *astOperation) []*astOperation {
func removeAstOp(resource *astResource, group bool, op eacl.Operation, action eacl.Action) { func removeAstOp(resource *astResource, group bool, op eacl.Operation, action eacl.Action) {
for i, astOp := range resource.Operations { for i, astOp := range resource.Operations {
if astOp.IsGroupGrantee == group && astOp.Op == op && astOp.Action == action { if astOp.IsGroupGrantee() == group && astOp.Op == op && astOp.Action == action {
resource.Operations = append(resource.Operations[:i], resource.Operations[i+1:]...) resource.Operations = append(resource.Operations[:i], resource.Operations[i+1:]...)
return return
} }
@ -741,7 +738,7 @@ func removeAstOp(resource *astResource, group bool, op eacl.Operation, action ea
func addUsers(resource *astResource, astO *astOperation, users []string) { func addUsers(resource *astResource, astO *astOperation, users []string) {
for _, astOp := range resource.Operations { for _, astOp := range resource.Operations {
if astOp.IsGroupGrantee == astO.IsGroupGrantee && astOp.Op == astO.Op && astOp.Action == astO.Action { if astOp.IsGroupGrantee() == astO.IsGroupGrantee() && astOp.Op == astO.Op && astOp.Action == astO.Action {
astOp.Users = append(astO.Users, users...) astOp.Users = append(astO.Users, users...)
return return
} }
@ -749,15 +746,19 @@ func addUsers(resource *astResource, astO *astOperation, users []string) {
} }
func removeUsers(resource *astResource, astOperation *astOperation, users []string) { func removeUsers(resource *astResource, astOperation *astOperation, users []string) {
for _, astOp := range resource.Operations { for ind, astOp := range resource.Operations {
if astOp.IsGroupGrantee == astOperation.IsGroupGrantee && astOp.Op == astOperation.Op && astOp.Action == astOperation.Action { if !astOp.IsGroupGrantee() && astOp.Op == astOperation.Op && astOp.Action == astOperation.Action {
filteredUsers := astOp.Users[:0] // new slice without allocation filteredUsers := astOp.Users[:0] // new slice without allocation
for _, user := range astOp.Users { for _, user := range astOp.Users {
if !containsStr(users, user) { if !containsStr(users, user) {
filteredUsers = append(filteredUsers, user) filteredUsers = append(filteredUsers, user)
} }
} }
if len(filteredUsers) == 0 { // remove ast resource
resource.Operations = append(resource.Operations[:ind], resource.Operations[ind+1:]...)
} else {
astOp.Users = filteredUsers astOp.Users = filteredUsers
}
return return
} }
} }
@ -796,7 +797,7 @@ func formRecords(operations []*astOperation, resource *astResource) ([]*eacl.Rec
record := eacl.NewRecord() record := eacl.NewRecord()
record.SetOperation(astOp.Op) record.SetOperation(astOp.Op)
record.SetAction(astOp.Action) record.SetAction(astOp.Action)
if astOp.IsGroupGrantee { if astOp.IsGroupGrantee() {
eacl.AddFormedTarget(record, eacl.RoleOthers) eacl.AddFormedTarget(record, eacl.RoleOthers)
} else { } else {
// TODO(av): optimize target // TODO(av): optimize target
@ -832,7 +833,7 @@ func addToList(operations []*astOperation, rec eacl.Record, target eacl.Target)
) )
for _, astOp := range operations { for _, astOp := range operations {
if astOp.Op == rec.Operation() && astOp.IsGroupGrantee == groupTarget { if astOp.Op == rec.Operation() && astOp.IsGroupGrantee() == groupTarget {
found = astOp found = astOp
} }
} }
@ -845,7 +846,6 @@ func addToList(operations []*astOperation, rec eacl.Record, target eacl.Target)
} }
} else { } else {
astOperation := &astOperation{ astOperation := &astOperation{
IsGroupGrantee: groupTarget,
Op: rec.Operation(), Op: rec.Operation(),
Action: rec.Action(), Action: rec.Action(),
} }
@ -922,7 +922,7 @@ func handleResourceOperations(bktPolicy *bucketPolicy, list []*astOperation, eac
userOpsMap := make(map[string][]eacl.Operation) userOpsMap := make(map[string][]eacl.Operation)
for _, op := range list { for _, op := range list {
if !op.IsGroupGrantee { if !op.IsGroupGrantee() {
for _, user := range op.Users { for _, user := range op.Users {
userOps := userOpsMap[user] userOps := userOpsMap[user]
userOps = append(userOps, op.Op) userOps = append(userOps, op.Op)
@ -976,7 +976,7 @@ func triageOperations(operations []*astOperation) ([]*astOperation, []*astOperat
func addTo(list []*astOperation, userID string, op eacl.Operation, groupGrantee bool, action eacl.Action) []*astOperation { func addTo(list []*astOperation, userID string, op eacl.Operation, groupGrantee bool, action eacl.Action) []*astOperation {
var found *astOperation var found *astOperation
for _, astop := range list { for _, astop := range list {
if astop.Op == op && astop.IsGroupGrantee == groupGrantee { if astop.Op == op && astop.IsGroupGrantee() == groupGrantee {
found = astop found = astop
} }
} }
@ -987,7 +987,6 @@ func addTo(list []*astOperation, userID string, op eacl.Operation, groupGrantee
} }
} else { } else {
astoperation := &astOperation{ astoperation := &astOperation{
IsGroupGrantee: groupGrantee,
Op: op, Op: op,
Action: action, Action: action,
} }

View file

@ -48,7 +48,6 @@ func TestTableToAst(t *testing.T) {
{ {
resourceInfo: resourceInfo{Bucket: "bucketName"}, resourceInfo: resourceInfo{Bucket: "bucketName"},
Operations: []*astOperation{{ Operations: []*astOperation{{
IsGroupGrantee: true,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}}, }}},
@ -63,7 +62,6 @@ func TestTableToAst(t *testing.T) {
hex.EncodeToString(key.PublicKey().Bytes()), hex.EncodeToString(key.PublicKey().Bytes()),
hex.EncodeToString(key2.PublicKey().Bytes()), hex.EncodeToString(key2.PublicKey().Bytes()),
}, },
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}}, }}},
@ -111,7 +109,6 @@ func TestPolicyToAst(t *testing.T) {
Bucket: "bucketName", Bucket: "bucketName",
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
IsGroupGrantee: true,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}, }},
@ -139,12 +136,17 @@ func TestPolicyToAst(t *testing.T) {
} }
func getReadOps(key *keys.PrivateKey, groupGrantee bool, action eacl.Action) []*astOperation { func getReadOps(key *keys.PrivateKey, groupGrantee bool, action eacl.Action) []*astOperation {
var result []*astOperation var (
result []*astOperation
users []string
)
if !groupGrantee {
users = append(users, hex.EncodeToString(key.PublicKey().Bytes()))
}
for _, op := range readOps { for _, op := range readOps {
result = append(result, &astOperation{ result = append(result, &astOperation{
Users: []string{hex.EncodeToString(key.PublicKey().Bytes())}, Users: users,
IsGroupGrantee: groupGrantee,
Op: op, Op: op,
Action: action, Action: action,
}) })
@ -166,7 +168,6 @@ func TestMergeAstUnModified(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{hex.EncodeToString(key.PublicKey().Bytes())}, Users: []string{hex.EncodeToString(key.PublicKey().Bytes())},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}, }},
@ -181,7 +182,6 @@ func TestMergeAstUnModified(t *testing.T) {
Bucket: "bucket", Bucket: "bucket",
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
IsGroupGrantee: true,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}, }},
@ -204,12 +204,10 @@ func TestMergeAstModified(t *testing.T) {
Object: "objectName", Object: "objectName",
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
IsGroupGrantee: true,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}, { }, {
Users: []string{"user2"}, Users: []string{"user2"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}, }},
@ -226,7 +224,6 @@ func TestMergeAstModified(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{"user1"}, Users: []string{"user1"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}, }},
@ -245,7 +242,6 @@ func TestMergeAstModified(t *testing.T) {
child.Resources[0].Operations[0], child.Resources[0].Operations[0],
{ {
Users: []string{"user1", "user2"}, Users: []string{"user1", "user2"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}, },
@ -269,12 +265,10 @@ func TestMergeAstModifiedConflict(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{"user1"}, Users: []string{"user1"},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}, { }, {
Users: []string{"user3"}, Users: []string{"user3"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}, }},
@ -291,17 +285,14 @@ func TestMergeAstModifiedConflict(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{"user1"}, Users: []string{"user1"},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}, { }, {
Users: []string{"user2"}, Users: []string{"user2"},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}, { }, {
Users: []string{"user3"}, Users: []string{"user3"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}, }},
@ -319,12 +310,10 @@ func TestMergeAstModifiedConflict(t *testing.T) {
Operations: []*astOperation{ Operations: []*astOperation{
{ {
Users: []string{"user2", "user1"}, Users: []string{"user2", "user1"},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}, { }, {
Users: []string{"user3"}, Users: []string{"user3"},
IsGroupGrantee: false,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}, },
@ -350,7 +339,6 @@ func TestAstToTable(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{hex.EncodeToString(key.PublicKey().Bytes())}, Users: []string{hex.EncodeToString(key.PublicKey().Bytes())},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}, }},
@ -361,7 +349,6 @@ func TestAstToTable(t *testing.T) {
Object: "objectName", Object: "objectName",
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
IsGroupGrantee: true,
Op: eacl.OperationGet, Op: eacl.OperationGet,
Action: eacl.ActionDeny, Action: eacl.ActionDeny,
}}, }},
@ -394,19 +381,28 @@ func TestRemoveUsers(t *testing.T) {
}, },
Operations: []*astOperation{{ Operations: []*astOperation{{
Users: []string{"user1", "user3", "user4"}, Users: []string{"user1", "user3", "user4"},
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
}}, },
{
Users: []string{"user5"},
Op: eacl.OperationGet,
Action: eacl.ActionDeny,
},
},
} }
op := &astOperation{ op1 := &astOperation{
IsGroupGrantee: false,
Op: eacl.OperationPut, Op: eacl.OperationPut,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
} }
op2 := &astOperation{
Op: eacl.OperationGet,
Action: eacl.ActionDeny,
}
removeUsers(resource, op, []string{"user1", "user2", "user4"}) removeUsers(resource, op1, []string{"user1", "user2", "user4"}) // modify astOperation
removeUsers(resource, op2, []string{"user5"}) // remove astOperation
require.Equal(t, len(resource.Operations), 1) require.Equal(t, len(resource.Operations), 1)
require.Equal(t, []string{"user3"}, resource.Operations[0].Users) require.Equal(t, []string{"user3"}, resource.Operations[0].Users)
@ -782,7 +778,6 @@ func TestObjectAclToAst(t *testing.T) {
hex.EncodeToString(key.PublicKey().Bytes()), hex.EncodeToString(key.PublicKey().Bytes()),
hex.EncodeToString(key2.PublicKey().Bytes()), hex.EncodeToString(key2.PublicKey().Bytes()),
}, },
IsGroupGrantee: false,
Op: op, Op: op,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
} }
@ -845,7 +840,6 @@ func TestBucketAclToAst(t *testing.T) {
astOp := &astOperation{Users: []string{ astOp := &astOperation{Users: []string{
hex.EncodeToString(key.PublicKey().Bytes()), hex.EncodeToString(key.PublicKey().Bytes()),
}, },
IsGroupGrantee: false,
Op: op, Op: op,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
} }
@ -856,7 +850,6 @@ func TestBucketAclToAst(t *testing.T) {
hex.EncodeToString(key.PublicKey().Bytes()), hex.EncodeToString(key.PublicKey().Bytes()),
hex.EncodeToString(key2.PublicKey().Bytes()), hex.EncodeToString(key2.PublicKey().Bytes()),
}, },
IsGroupGrantee: false,
Op: op, Op: op,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
} }
@ -864,7 +857,6 @@ func TestBucketAclToAst(t *testing.T) {
} }
for _, op := range readOps { for _, op := range readOps {
astOp := &astOperation{ astOp := &astOperation{
IsGroupGrantee: true,
Op: op, Op: op,
Action: eacl.ActionAllow, Action: eacl.ActionAllow,
} }