Skip to content

Commit dcf0d87

Browse files
dschmidtdschmidt
committed
fix(decomposedfs): merge service account permissions with node grants
Service accounts short-circuited to ServiceAccountPermissions and ignored grants on the node (e.g. the SpaceManager grant a creator gets on project space creation), so the initial creator share was denied and the space rolled back with CODE_INTERNAL. Read the grants first, then merge the service account defaults on top.
1 parent ee70fc3 commit dcf0d87

4 files changed

Lines changed: 63 additions & 8 deletions

File tree

‎pkg/storage/pkg/decomposedfs/node/node_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,32 @@ var _ = Describe("Node", func() {
290290
expected := ocsconv.NewManagerRole().CS3ResourcePermissions()
291291
Expect(grants.PermissionsEqual(perms, expected)).To(BeTrue())
292292
})
293+
It("Merges grants with the service account permissions for service accounts", func() {
294+
pSpace, err := env.CreateTestStorageSpace("project", &provider.Quota{QuotaMaxBytes: 2000})
295+
Expect(err).ToNot(HaveOccurred())
296+
sa := &userpb.User{Id: &userpb.UserId{OpaqueId: "service-account", Type: userpb.UserType_USER_TYPE_SERVICE}}
297+
env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
298+
AddGrant: true,
299+
Stat: true,
300+
}, nil).Times(1)
301+
err = env.Fs.AddGrant(env.Ctx, &provider.Reference{
302+
ResourceId: &provider.ResourceId{SpaceId: pSpace.SpaceId, OpaqueId: pSpace.OpaqueId},
303+
}, &provider.Grant{
304+
Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: sa.Id}},
305+
Permissions: ocsconv.NewEditorRole().CS3ResourcePermissions(),
306+
})
307+
Expect(err).ToNot(HaveOccurred())
308+
// resolve as the service account: grant must be honored (not shadowed by an
309+
// early return) and merged with, not replaced by, the service account defaults.
310+
spaceRoot, err := env.Lookup.NodeFromSpaceID(env.Ctx, pSpace.SpaceId)
311+
Expect(err).ToNot(HaveOccurred())
312+
saCtx := ctxpkg.ContextSetUser(env.Ctx, sa)
313+
perms, err := node.NewPermissions(env.Lookup).AssemblePermissions(saCtx, spaceRoot)
314+
Expect(err).ToNot(HaveOccurred())
315+
expected := ocsconv.NewEditorRole().CS3ResourcePermissions()
316+
node.AddPermissions(expected, node.ServiceAccountPermissions())
317+
Expect(grants.PermissionsEqual(perms, expected)).To(BeTrue())
318+
})
293319
It("Checks the Editor permissions on a project space and a denial", func() {
294320
storageSpace, err := env.CreateTestStorageSpace("project", &provider.Quota{QuotaMaxBytes: 2000})
295321
Expect(err).ToNot(HaveOccurred())

‎pkg/storage/pkg/decomposedfs/node/permissions.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,6 @@ func (p *Permissions) assemblePermissions(ctx context.Context, n *Node, failOnTr
134134
return NoPermissions(), nil
135135
}
136136

137-
if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE {
138-
return ServiceAccountPermissions(), nil
139-
}
140-
141137
// are we reading a revision?
142138
if strings.Contains(n.ID, RevisionIDDelimiter) {
143139
// verify revision key format
@@ -204,6 +200,11 @@ func (p *Permissions) assemblePermissions(ctx context.Context, n *Node, failOnTr
204200
return OwnerPermissions(), nil
205201
}
206202

203+
// merge in the general service account permissions; grants can only add to them
204+
if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE {
205+
AddPermissions(ap, ServiceAccountPermissions())
206+
}
207+
207208
appctx.GetLogger(ctx).Debug().Interface("permissions", ap).Str("spaceid", n.SpaceID).Str("nodeid", n.ID).Interface("user", u).Msg("returning agregated permissions")
208209
return ap, nil
209210
}

‎pkg/storage/utils/decomposedfs/node/node_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"encoding/json"
2323
"time"
2424

25+
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
2526
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
2627
. "github.com/onsi/ginkgo/v2"
2728
. "github.com/onsi/gomega"
@@ -266,6 +267,32 @@ var _ = Describe("Node", func() {
266267
expected := ocsconv.NewManagerRole().CS3ResourcePermissions()
267268
Expect(grants.PermissionsEqual(perms, expected)).To(BeTrue())
268269
})
270+
It("Merges grants with the service account permissions for service accounts", func() {
271+
pSpace, err := env.CreateTestStorageSpace("project", &provider.Quota{QuotaMaxBytes: 2000})
272+
Expect(err).ToNot(HaveOccurred())
273+
sa := &userpb.User{Id: &userpb.UserId{OpaqueId: "service-account", Type: userpb.UserType_USER_TYPE_SERVICE}}
274+
env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
275+
AddGrant: true,
276+
Stat: true,
277+
}, nil).Times(1)
278+
err = env.Fs.AddGrant(env.Ctx, &provider.Reference{
279+
ResourceId: &provider.ResourceId{SpaceId: pSpace.SpaceId, OpaqueId: pSpace.OpaqueId},
280+
}, &provider.Grant{
281+
Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: sa.Id}},
282+
Permissions: ocsconv.NewEditorRole().CS3ResourcePermissions(),
283+
})
284+
Expect(err).ToNot(HaveOccurred())
285+
// resolve as the service account: grant must be honored (not shadowed by an
286+
// early return) and merged with, not replaced by, the service account defaults.
287+
spaceRoot, err := env.Lookup.NodeFromSpaceID(env.Ctx, pSpace.SpaceId)
288+
Expect(err).ToNot(HaveOccurred())
289+
saCtx := ctxpkg.ContextSetUser(env.Ctx, sa)
290+
perms, err := node.NewPermissions(env.Lookup).AssemblePermissions(saCtx, spaceRoot)
291+
Expect(err).ToNot(HaveOccurred())
292+
expected := ocsconv.NewEditorRole().CS3ResourcePermissions()
293+
node.AddPermissions(expected, node.ServiceAccountPermissions())
294+
Expect(grants.PermissionsEqual(perms, expected)).To(BeTrue())
295+
})
269296
It("Checks the Editor permissions on a project space and a denial", func() {
270297
storageSpace, err := env.CreateTestStorageSpace("project", &provider.Quota{QuotaMaxBytes: 2000})
271298
Expect(err).ToNot(HaveOccurred())

‎pkg/storage/utils/decomposedfs/node/permissions.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,6 @@ func (p *Permissions) assemblePermissions(ctx context.Context, n *Node, failOnTr
133133
return NoPermissions(), nil
134134
}
135135

136-
if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE {
137-
return ServiceAccountPermissions(), nil
138-
}
139-
140136
// are we reading a revision?
141137
if strings.Contains(n.ID, RevisionIDDelimiter) {
142138
// verify revision key format
@@ -203,6 +199,11 @@ func (p *Permissions) assemblePermissions(ctx context.Context, n *Node, failOnTr
203199
return OwnerPermissions(), nil
204200
}
205201

202+
// merge in the general service account permissions; grants can only add to them
203+
if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE {
204+
AddPermissions(ap, ServiceAccountPermissions())
205+
}
206+
206207
appctx.GetLogger(ctx).Debug().Interface("permissions", ap).Str("spaceid", n.SpaceID).Str("nodeid", n.ID).Interface("user", u).Msg("returning agregated permissions")
207208
return ap, nil
208209
}

0 commit comments

Comments
 (0)