From 2ffeb7f3e873901791b6b6f9d864fa6760592850 Mon Sep 17 00:00:00 2001 From: Sacha Al Himdani Date: Fri, 3 Jul 2026 17:11:13 +0200 Subject: [PATCH] Require set-owner authorization to create OWNER membership An organization ADMIN could mint an OWNER membership via createUser, which only gated iam:membership-profile:create and bypassed the owner-only iam:membership-role:set-owner check that updateMembership already enforces. Gate the requested role in both createUser entry points (connect resolver and the MCP CreateUserTool) with an additional set-owner authorization when the role is OWNER, mirroring updateMembership. Add a regression test that locks the ADMIN/OWNER privilege boundary the fix relies on. Signed-off-by: Sacha Al Himdani --- e2e/console/user_test.go | 71 +++++++++++++++++++ .../api/connect/v1/profile_resolvers.go | 6 ++ pkg/server/api/mcp/v1/schema.resolvers.go | 6 ++ 3 files changed, 83 insertions(+) diff --git a/e2e/console/user_test.go b/e2e/console/user_test.go index 47c5cec72..fe30c6859 100644 --- a/e2e/console/user_test.go +++ b/e2e/console/user_test.go @@ -24,6 +24,77 @@ import ( "go.probo.inc/probo/e2e/internal/testutil" ) +// TestUser_AdminCannotCreateOwner is a non-regression test for the vertical +// privilege escalation where an ADMIN could mint an OWNER membership via +// createUser, bypassing the owner-only set-owner authorization that +// updateMembership enforces. +func TestUser_AdminCannotCreateOwner(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + admin := testutil.NewClientInOrg(t, testutil.RoleAdmin, owner) + + const createUserMutation = ` + mutation($input: CreateUserInput!) { + createUser(input: $input) { + profileEdge { + node { + membership { role } + } + } + } + } + ` + + newUserInput := func(role string) map[string]any { + return map[string]any{ + "input": map[string]any{ + "organizationId": owner.GetOrganizationID().String(), + "emailAddress": factory.SafeEmail(), + "fullName": factory.SafeName("Privesc User"), + "role": role, + "kind": "EMPLOYEE", + "additionalEmailAddresses": []string{}, + }, + } + } + + // An ADMIN must not be able to create an OWNER membership. + _, err := admin.DoConnect(createUserMutation, newUserInput("OWNER")) + testutil.RequireForbiddenError(t, err) + + // The same ADMIN can still create a lower-privileged member. + var adminCreate struct { + CreateUser struct { + ProfileEdge struct { + Node struct { + Membership struct { + Role string `json:"role"` + } `json:"membership"` + } `json:"node"` + } `json:"profileEdge"` + } `json:"createUser"` + } + err = admin.ExecuteConnect(createUserMutation, newUserInput("ADMIN"), &adminCreate) + require.NoError(t, err) + assert.Equal(t, "ADMIN", adminCreate.CreateUser.ProfileEdge.Node.Membership.Role) + + // An OWNER remains able to create an OWNER membership. + var ownerCreate struct { + CreateUser struct { + ProfileEdge struct { + Node struct { + Membership struct { + Role string `json:"role"` + } `json:"membership"` + } `json:"node"` + } `json:"profileEdge"` + } `json:"createUser"` + } + err = owner.ExecuteConnect(createUserMutation, newUserInput("OWNER"), &ownerCreate) + require.NoError(t, err) + assert.Equal(t, "OWNER", ownerCreate.CreateUser.ProfileEdge.Node.Membership.Role) +} + func TestUser_UpdateMembership(t *testing.T) { t.Parallel() owner := testutil.NewClient(t, testutil.RoleOwner) diff --git a/pkg/server/api/connect/v1/profile_resolvers.go b/pkg/server/api/connect/v1/profile_resolvers.go index 0908414a5..7ce17015d 100644 --- a/pkg/server/api/connect/v1/profile_resolvers.go +++ b/pkg/server/api/connect/v1/profile_resolvers.go @@ -26,6 +26,12 @@ func (r *mutationResolver) CreateUser(ctx context.Context, input types.CreateUse return nil, err } + if input.Role == coredata.MembershipRoleOwner { + if _, err := r.authorize(ctx, input.OrganizationID, iam.ActionMembershipRoleSetOwner); err != nil { + return nil, err + } + } + profile, err := r.iam.OrganizationService.CreateUser( ctx, &iam.CreateUserRequest{ diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index 1aea6cab0..2414b14a2 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -2819,6 +2819,12 @@ func (r *Resolver) CreateUserTool(ctx context.Context, req *mcp.CallToolRequest, return nil, types.CreateUserOutput{}, err } + if input.Role == coredata.MembershipRoleOwner { + if _, err := r.Authorize(ctx, input.OrganizationID, iam.ActionMembershipRoleSetOwner); err != nil { + return nil, types.CreateUserOutput{}, err + } + } + var contractStart, contractEnd **time.Time if input.ContractStartDate != nil { contractStart = &input.ContractStartDate