fix(project_roles): fixed bad permission check in command layer for project roles add/update/delete (#10531)

# Which Problems Are Solved

Project Admins would get permission errors when trying to add project
roles

# How the Problems Are Solved

Fixed wrong parameters were being passed into the permission check

- Closes https://github.com/zitadel/zitadel/issues/10505

(cherry picked from commit 24a7d3ceb1)
This commit is contained in:
Iraq
2025-08-22 08:08:53 +02:00
committed by Livio Spring
parent ec3d79a37b
commit 388582d348
2 changed files with 171 additions and 52 deletions

View File

@@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/zitadel/zitadel/internal/integration"
"github.com/zitadel/zitadel/pkg/grpc/management"
project "github.com/zitadel/zitadel/pkg/grpc/project/v2beta"
)
@@ -29,7 +30,7 @@ func TestServer_AddProjectRole(t *testing.T) {
tests := []struct {
name string
ctx context.Context
prepare func(request *project.AddProjectRoleRequest)
prepare func(t *testing.T, request *project.AddProjectRoleRequest)
req *project.AddProjectRoleRequest
want
wantErr bool
@@ -37,7 +38,7 @@ func TestServer_AddProjectRole(t *testing.T) {
{
name: "empty key",
ctx: iamOwnerCtx,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -50,7 +51,7 @@ func TestServer_AddProjectRole(t *testing.T) {
{
name: "empty displayname",
ctx: iamOwnerCtx,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -63,7 +64,7 @@ func TestServer_AddProjectRole(t *testing.T) {
{
name: "already existing, error",
ctx: iamOwnerCtx,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
request.ProjectId = alreadyExistingProject.GetId()
},
req: &project.AddProjectRoleRequest{
@@ -75,7 +76,7 @@ func TestServer_AddProjectRole(t *testing.T) {
{
name: "empty, ok",
ctx: iamOwnerCtx,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -91,7 +92,7 @@ func TestServer_AddProjectRole(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.prepare != nil {
tt.prepare(tt.req)
tt.prepare(t, tt.req)
}
creationDate := time.Now().UTC()
@@ -118,18 +119,20 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
type want struct {
creationDate bool
}
tests := []struct {
type test struct {
name string
ctx context.Context
prepare func(request *project.AddProjectRoleRequest)
prepare func(t *testing.T, request *project.AddProjectRoleRequest)
req *project.AddProjectRoleRequest
want
wantErr bool
}{
}
tests := []*test{
{
name: "unauthenticated",
ctx: CTX,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -142,7 +145,7 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
{
name: "no permission",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission),
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -155,7 +158,7 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
{
name: "organization owner, other org",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner),
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -168,7 +171,7 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
{
name: "organization owner, ok",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner),
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.GetId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -183,7 +186,7 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
{
name: "instance owner, ok",
ctx: iamOwnerCtx,
prepare: func(request *project.AddProjectRoleRequest) {
prepare: func(t *testing.T, request *project.AddProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
request.ProjectId = projectResp.GetId()
},
@@ -195,11 +198,47 @@ func TestServer_AddProjectRole_Permission(t *testing.T) {
creationDate: true,
},
},
func() *test {
out := test{
name: "add project role as a added project admin, ok",
req: &project.AddProjectRoleRequest{
RoleKey: integration.RoleKey(),
DisplayName: integration.RoleDisplayName(),
},
want: want{
creationDate: true,
},
}
out.prepare = func(t *testing.T, request *project.AddProjectRoleRequest) {
// create project
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.Id, integration.ProjectName(), false, false)
// create user
userID := instance.CreateHumanUser(iamOwnerCtx).GetUserId()
loginCTX := instance.WithAuthorization(iamOwnerCtx, integration.UserTypeLogin)
instance.RegisterUserPasskey(iamOwnerCtx, userID)
_, token, _, _ := instance.CreateVerifiedWebAuthNSession(t, loginCTX, userID)
// assign user as project admin
_, err := instance.Client.Mgmt.AddProjectMember(iamOwnerCtx, &management.AddProjectMemberRequest{
ProjectId: projectResp.GetId(),
UserId: userID,
Roles: []string{"PROJECT_OWNER_GLOBAL"},
})
assert.NoError(t, err)
// set context
out.ctx = integration.WithAuthorizationToken(context.Background(), token)
request.ProjectId = projectResp.GetId()
}
return &out
}(),
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.prepare != nil {
tt.prepare(tt.req)
tt.prepare(t, tt.req)
}
creationDate := time.Now().UTC()
@@ -241,14 +280,14 @@ func TestServer_UpdateProjectRole(t *testing.T) {
}
tests := []struct {
name string
prepare func(request *project.UpdateProjectRoleRequest)
prepare func(t *testing.T, request *project.UpdateProjectRoleRequest)
args args
want want
wantErr bool
}{
{
name: "missing permission",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -265,7 +304,7 @@ func TestServer_UpdateProjectRole(t *testing.T) {
},
{
name: "not existing",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
request.RoleKey = "notexisting"
return
},
@@ -279,7 +318,7 @@ func TestServer_UpdateProjectRole(t *testing.T) {
},
{
name: "no change, ok",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -298,7 +337,7 @@ func TestServer_UpdateProjectRole(t *testing.T) {
},
{
name: "change display name, ok",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -318,7 +357,7 @@ func TestServer_UpdateProjectRole(t *testing.T) {
},
{
name: "change full, ok",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -341,7 +380,7 @@ func TestServer_UpdateProjectRole(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
creationDate := time.Now().UTC()
tt.prepare(tt.args.req)
tt.prepare(t, tt.args.req)
got, err := instance.Client.Projectv2Beta.UpdateProjectRole(tt.args.ctx, tt.args.req)
if tt.wantErr {
@@ -370,16 +409,17 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
change bool
changeDate bool
}
tests := []struct {
type test struct {
name string
prepare func(request *project.UpdateProjectRoleRequest)
prepare func(t *testing.T, request *project.UpdateProjectRoleRequest)
args args
want want
wantErr bool
}{
}
tests := []*test{
{
name: "unauthenicated",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -396,7 +436,7 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
},
{
name: "no permission",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -413,7 +453,7 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
},
{
name: "organization owner, other org",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -430,7 +470,7 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
},
{
name: "organization owner, ok",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.GetId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -450,7 +490,7 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
},
{
name: "instance owner, ok",
prepare: func(request *project.UpdateProjectRoleRequest) {
prepare: func(t *testing.T, request *project.UpdateProjectRoleRequest) {
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
@@ -468,11 +508,53 @@ func TestServer_UpdateProjectRole_Permission(t *testing.T) {
changeDate: true,
},
},
func() *test {
out := test{
name: "change project role as a added project admin, ok",
args: args{
req: &project.UpdateProjectRoleRequest{
DisplayName: gu.Ptr(integration.RoleKey()),
Group: gu.Ptr(integration.RoleKey()),
},
},
want: want{
change: true,
changeDate: true,
},
}
out.prepare = func(t *testing.T, request *project.UpdateProjectRoleRequest) {
// create project
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.Id, integration.ProjectName(), false, false)
// create user
userID := instance.CreateHumanUser(iamOwnerCtx).GetUserId()
loginCTX := instance.WithAuthorization(iamOwnerCtx, integration.UserTypeLogin)
instance.RegisterUserPasskey(iamOwnerCtx, userID)
_, token, _, _ := instance.CreateVerifiedWebAuthNSession(t, loginCTX, userID)
// assign user as project admin
_, err := instance.Client.Mgmt.AddProjectMember(iamOwnerCtx, &management.AddProjectMemberRequest{
ProjectId: projectResp.GetId(),
UserId: userID,
Roles: []string{"PROJECT_OWNER_GLOBAL"},
})
assert.NoError(t, err)
// set context
out.args.ctx = integration.WithAuthorizationToken(context.Background(), token)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
request.ProjectId = projectResp.GetId()
request.RoleKey = roleName
}
return &out
}(),
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
creationDate := time.Now().UTC()
tt.prepare(tt.args.req)
tt.prepare(t, tt.args.req)
got, err := instance.Client.Projectv2Beta.UpdateProjectRole(tt.args.ctx, tt.args.req)
if tt.wantErr {
@@ -508,7 +590,7 @@ func TestServer_DeleteProjectRole(t *testing.T) {
tests := []struct {
name string
ctx context.Context
prepare func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time)
prepare func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time)
req *project.RemoveProjectRoleRequest
wantDeletionDate bool
wantErr bool
@@ -534,7 +616,7 @@ func TestServer_DeleteProjectRole(t *testing.T) {
{
name: "delete",
ctx: iamOwnerCtx,
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -549,7 +631,7 @@ func TestServer_DeleteProjectRole(t *testing.T) {
{
name: "delete, already removed",
ctx: iamOwnerCtx,
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -567,7 +649,7 @@ func TestServer_DeleteProjectRole(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var creationDate, deletionDate time.Time
if tt.prepare != nil {
creationDate, deletionDate = tt.prepare(tt.req)
creationDate, deletionDate = tt.prepare(t, tt.req)
}
got, err := instance.Client.Projectv2Beta.RemoveProjectRole(tt.ctx, tt.req)
if tt.wantErr {
@@ -584,18 +666,19 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
iamOwnerCtx := instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner)
orgResp := instance.CreateOrganization(iamOwnerCtx, integration.OrganizationName(), gofakeit.Email())
tests := []struct {
type test struct {
name string
ctx context.Context
prepare func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time)
prepare func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time)
req *project.RemoveProjectRoleRequest
wantDeletionDate bool
wantErr bool
}{
}
tests := []*test{
{
name: "unauthenticated",
ctx: CTX,
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -610,7 +693,7 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
{
name: "no permission",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission),
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -625,7 +708,7 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
{
name: "organization owner, other org",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner),
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -640,7 +723,7 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
{
name: "organization owner, ok",
ctx: instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner),
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.GetId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -655,7 +738,7 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
{
name: "instance owner, ok",
ctx: iamOwnerCtx,
prepare: func(request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
prepare: func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, orgResp.GetOrganizationId(), integration.ProjectName(), false, false)
roleName := integration.RoleKey()
@@ -667,12 +750,48 @@ func TestServer_DeleteProjectRole_Permission(t *testing.T) {
req: &project.RemoveProjectRoleRequest{},
wantDeletionDate: true,
},
func() *test {
out := test{
name: "delete project role as a added project admin, ok",
req: &project.RemoveProjectRoleRequest{},
wantDeletionDate: true,
}
out.prepare = func(t *testing.T, request *project.RemoveProjectRoleRequest) (time.Time, time.Time) {
// create project
creationDate := time.Now().UTC()
projectResp := instance.CreateProject(iamOwnerCtx, t, instance.DefaultOrg.Id, integration.ProjectName(), false, false)
// create user
userID := instance.CreateHumanUser(iamOwnerCtx).GetUserId()
loginCTX := instance.WithAuthorization(iamOwnerCtx, integration.UserTypeLogin)
instance.RegisterUserPasskey(iamOwnerCtx, userID)
_, token, _, _ := instance.CreateVerifiedWebAuthNSession(t, loginCTX, userID)
// assign user as project admin
_, err := instance.Client.Mgmt.AddProjectMember(iamOwnerCtx, &management.AddProjectMemberRequest{
ProjectId: projectResp.GetId(),
UserId: userID,
Roles: []string{"PROJECT_OWNER_GLOBAL"},
})
assert.NoError(t, err)
// set context
out.ctx = integration.WithAuthorizationToken(context.Background(), token)
roleName := integration.RoleKey()
instance.AddProjectRole(iamOwnerCtx, t, projectResp.GetId(), roleName, roleName, "")
request.ProjectId = projectResp.GetId()
request.RoleKey = roleName
return creationDate, time.Time{}
}
return &out
}(),
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var creationDate, deletionDate time.Time
if tt.prepare != nil {
creationDate, deletionDate = tt.prepare(tt.req)
creationDate, deletionDate = tt.prepare(t, tt.req)
}
got, err := instance.Client.Projectv2Beta.RemoveProjectRole(tt.ctx, tt.req)
if tt.wantErr {