feat(group): add group permissions (#10853)

# Which Problems Are Solved
Ensuring the user group resource is managed with appropriate
permissions.

# How the Problems Are Solved
By configuring and checking for the relevant permissions needed to
create, read, update, and delete the user groups resource.

# Additional Changes
N/A

# Additional Context
- Related to #9702 
- Follow-up for PRs #10455,  #10758
This commit is contained in:
Gayathri Vijayan
2025-10-21 11:18:21 +02:00
committed by GitHub
parent ff869482b1
commit ebc6c503a3
13 changed files with 1065 additions and 145 deletions
+20 -10
View File
@@ -30,7 +30,13 @@ func (c *Commands) CreateGroup(ctx context.Context, group *CreateGroup) (details
ctx, span := tracing.NewSpan(ctx)
defer func() { span.EndWithError(err) }()
// todo: check permissions
// create a unique group ID if not provided
if group.AggregateID == "" {
group.AggregateID, err = c.idGenerator.Next()
if err != nil {
return nil, err
}
}
if err = group.IsValid(); err != nil {
return nil, err
@@ -38,20 +44,17 @@ func (c *Commands) CreateGroup(ctx context.Context, group *CreateGroup) (details
if group.ResourceOwner == "" {
return nil, zerrors.ThrowInvalidArgument(nil, "CMDGRP-msc0Tt", "Errors.Group.MissingOrganizationID")
}
if err = c.checkPermissionCreateGroup(ctx, group.ResourceOwner, group.AggregateID); err != nil {
return nil, err
}
// check whether the organization where the group should be created exists
err = c.checkOrgExists(ctx, group.ResourceOwner)
if err != nil {
return nil, zerrors.ThrowPreconditionFailed(nil, "CMDGRP-j1mH8l", "Errors.Org.NotFound")
}
// create a unique group ID if not provided
if group.AggregateID == "" {
group.AggregateID, err = c.idGenerator.Next()
if err != nil {
return nil, err
}
}
// check if a group with the same ID already exists
groupWriteModel, err := c.getGroupWriteModelByID(ctx, group.AggregateID, group.ResourceOwner)
if err != nil {
@@ -97,7 +100,6 @@ func (c *Commands) UpdateGroup(ctx context.Context, groupUpdate *UpdateGroup) (d
return nil, err
}
// todo: check permissions
existingGroup, err := c.getGroupWriteModelByID(ctx, groupUpdate.AggregateID, groupUpdate.ResourceOwner)
if err != nil {
return nil, err
@@ -106,6 +108,10 @@ func (c *Commands) UpdateGroup(ctx context.Context, groupUpdate *UpdateGroup) (d
return nil, zerrors.ThrowNotFound(nil, "CMDGRP-b33zly", "Errors.Group.NotFound")
}
if err = c.checkPermissionUpdateGroup(ctx, existingGroup.ResourceOwner, existingGroup.AggregateID); err != nil {
return nil, err
}
changedEvent := existingGroup.NewChangedEvent(
ctx,
GroupAggregateFromWriteModel(ctx, &existingGroup.WriteModel),
@@ -138,6 +144,10 @@ func (c *Commands) DeleteGroup(ctx context.Context, groupID string) (details *do
return writeModelToObjectDetails(&existingGroup.WriteModel), nil
}
if err = c.checkPermissionDeleteGroup(ctx, existingGroup.ResourceOwner, existingGroup.AggregateID); err != nil {
return nil, err
}
err = c.pushAppendAndReduce(ctx,
existingGroup,
repo.NewGroupRemovedEvent(ctx,
+114 -27
View File
@@ -26,8 +26,9 @@ func TestCommands_CreateGroup(t *testing.T) {
idGeneratorErr := errors.New("id generator error")
type fields struct {
eventstore func(t *testing.T) *eventstore.Eventstore
idGenerator id.Generator
eventstore func(t *testing.T) *eventstore.Eventstore
idGenerator id.Generator
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -51,6 +52,7 @@ func TestCommands_CreateGroup(t *testing.T) {
group: &CreateGroup{
ObjectRoot: models.ObjectRoot{
ResourceOwner: "org1",
AggregateID: "1234",
},
Name: " ",
Description: "example group",
@@ -66,6 +68,9 @@ func TestCommands_CreateGroup(t *testing.T) {
args: args{
ctx: context.Background(),
group: &CreateGroup{
ObjectRoot: models.ObjectRoot{
AggregateID: "1234",
},
Name: "example",
Description: "example group",
},
@@ -73,17 +78,38 @@ func TestCommands_CreateGroup(t *testing.T) {
wantErr: zerrors.IsErrorInvalidArgument,
},
{
name: "org not found, precondition error",
name: "missing permissions, error",
fields: fields{
eventstore: expectEventstore(
expectFilter(),
),
eventstore: expectEventstore(),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args: args{
ctx: context.Background(),
group: &CreateGroup{
ObjectRoot: models.ObjectRoot{
ResourceOwner: "org1",
AggregateID: "1234",
},
Name: "example",
Description: "example group",
},
},
wantErr: zerrors.IsPermissionDenied,
},
{
name: "org not found, precondition error",
fields: fields{
eventstore: expectEventstore(
expectFilter(),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
group: &CreateGroup{
ObjectRoot: models.ObjectRoot{
ResourceOwner: "org1",
AggregateID: "1234",
},
Name: "example",
Description: "example group",
@@ -94,16 +120,8 @@ func TestCommands_CreateGroup(t *testing.T) {
{
name: "failed to generate group id, error",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
"org1",
),
),
),
),
eventstore: expectEventstore(),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -144,6 +162,7 @@ func TestCommands_CreateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -164,12 +183,14 @@ func TestCommands_CreateGroup(t *testing.T) {
eventstore: expectEventstore(
expectFilterError(filterErr),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
group: &CreateGroup{
ObjectRoot: models.ObjectRoot{
ResourceOwner: "org1",
AggregateID: "1234",
},
Name: "example",
Description: "example group",
@@ -193,6 +214,7 @@ func TestCommands_CreateGroup(t *testing.T) {
),
expectFilterError(filterErr),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -233,6 +255,7 @@ func TestCommands_CreateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -272,6 +295,7 @@ func TestCommands_CreateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -312,6 +336,7 @@ func TestCommands_CreateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -347,8 +372,9 @@ func TestCommands_CreateGroup(t *testing.T) {
}
c := &Commands{
eventstore: tt.fields.eventstore(t),
idGenerator: tt.fields.idGenerator,
eventstore: tt.fields.eventstore(t),
idGenerator: tt.fields.idGenerator,
checkPermission: tt.fields.checkPermission,
}
got, err := c.CreateGroup(tt.args.ctx, tt.args.group)
@@ -370,7 +396,8 @@ func TestCommands_UpdateGroup(t *testing.T) {
pushErr := errors.New("push error")
type fields struct {
eventstore func(t *testing.T) *eventstore.Eventstore
eventstore func(t *testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -434,6 +461,33 @@ func TestCommands_UpdateGroup(t *testing.T) {
return errors.Is(err, filterErr)
},
},
{
name: "missing permission, error",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
group.NewGroupAddedEvent(context.Background(),
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
"group1 description",
),
),
)),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args: args{
ctx: context.Background(),
group: &UpdateGroup{
ObjectRoot: models.ObjectRoot{
AggregateID: "1234",
},
Name: gu.Ptr("updated name"),
Description: gu.Ptr("updated description"),
},
},
wantErr: zerrors.IsPermissionDenied,
},
{
name: "failed to push group changed event, error",
fields: fields{
@@ -459,6 +513,7 @@ func TestCommands_UpdateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -485,6 +540,7 @@ func TestCommands_UpdateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -524,6 +580,7 @@ func TestCommands_UpdateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -562,6 +619,7 @@ func TestCommands_UpdateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -601,6 +659,7 @@ func TestCommands_UpdateGroup(t *testing.T) {
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -623,7 +682,8 @@ func TestCommands_UpdateGroup(t *testing.T) {
t.Parallel()
c := &Commands{
eventstore: tt.fields.eventstore(t),
eventstore: tt.fields.eventstore(t),
checkPermission: tt.fields.checkPermission,
}
got, err := c.UpdateGroup(tt.args.ctx, tt.args.group)
if tt.wantErr == nil {
@@ -644,7 +704,8 @@ func TestCommands_DeleteGroup(t *testing.T) {
pushErr := errors.New("push error")
type fields struct {
eventstore func(t *testing.T) *eventstore.Eventstore
eventstore func(t *testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
}
type args struct {
ctx context.Context
@@ -687,6 +748,28 @@ func TestCommands_DeleteGroup(t *testing.T) {
ID: "1234",
},
},
{
name: "missing permission, error",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
group.NewGroupAddedEvent(context.Background(),
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
"group1 description",
),
),
),
),
checkPermission: newMockPermissionCheckNotAllowed(),
},
args: args{
ctx: context.Background(),
groupID: "1234",
},
wantErr: zerrors.IsPermissionDenied,
},
{
name: "failed to push group delete event, error",
fields: fields{
@@ -694,7 +777,7 @@ func TestCommands_DeleteGroup(t *testing.T) {
expectFilter(
eventFromEventPusher(
group.NewGroupAddedEvent(context.Background(),
&group.NewAggregate("1234", "").Aggregate,
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
"group1 description",
),
@@ -703,11 +786,12 @@ func TestCommands_DeleteGroup(t *testing.T) {
expectPushFailed(
pushErr,
group.NewGroupRemovedEvent(context.Background(),
&group.NewAggregate("1234", "").Aggregate,
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
@@ -724,7 +808,7 @@ func TestCommands_DeleteGroup(t *testing.T) {
expectFilter(
eventFromEventPusher(
group.NewGroupAddedEvent(context.Background(),
&group.NewAggregate("1234", "").Aggregate,
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
"group1 description",
),
@@ -733,19 +817,21 @@ func TestCommands_DeleteGroup(t *testing.T) {
expectPush(
eventFromEventPusher(
group.NewGroupRemovedEvent(context.Background(),
&group.NewAggregate("1234", "").Aggregate,
&group.NewAggregate("1234", "org1").Aggregate,
"group1",
),
),
),
),
checkPermission: newMockPermissionCheckAllowed(),
},
args: args{
ctx: context.Background(),
groupID: "1234",
},
want: &domain.ObjectDetails{
ID: "1234",
ID: "1234",
ResourceOwner: "org1",
},
},
}
@@ -755,7 +841,8 @@ func TestCommands_DeleteGroup(t *testing.T) {
t.Parallel()
c := &Commands{
eventstore: tt.fields.eventstore(t),
eventstore: tt.fields.eventstore(t),
checkPermission: tt.fields.checkPermission,
}
got, err := c.DeleteGroup(tt.args.ctx, tt.args.groupID)
if tt.wantErr == nil {
+13
View File
@@ -6,6 +6,7 @@ import (
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/repository/group"
"github.com/zitadel/zitadel/internal/repository/instance"
"github.com/zitadel/zitadel/internal/repository/org"
"github.com/zitadel/zitadel/internal/repository/project"
@@ -159,3 +160,15 @@ func (c *Commands) NewPermissionCheckUserGrantWrite(ctx context.Context) UserGra
func (c *Commands) NewPermissionCheckUserGrantDelete(ctx context.Context) UserGrantPermissionCheck {
return c.newUserGrantPermissionCheck(ctx, domain.PermissionUserGrantDelete)
}
func (c *Commands) checkPermissionCreateGroup(ctx context.Context, resourceOwner, groupID string) error {
return c.newPermissionCheck(ctx, domain.PermissionGroupCreate, group.AggregateType)(resourceOwner, groupID)
}
func (c *Commands) checkPermissionUpdateGroup(ctx context.Context, resourceOwner, groupID string) error {
return c.newPermissionCheck(ctx, domain.PermissionGroupWrite, group.AggregateType)(resourceOwner, groupID)
}
func (c *Commands) checkPermissionDeleteGroup(ctx context.Context, resourceOwner, groupID string) error {
return c.newPermissionCheck(ctx, domain.PermissionGroupDelete, group.AggregateType)(resourceOwner, groupID)
}