diff --git a/apps/console/src/pages/iam/organizations/people/PeoplePage.tsx b/apps/console/src/pages/iam/organizations/people/PeoplePage.tsx index a1df1cd76..00b57a15f 100644 --- a/apps/console/src/pages/iam/organizations/people/PeoplePage.tsx +++ b/apps/console/src/pages/iam/organizations/people/PeoplePage.tsx @@ -29,7 +29,7 @@ export const peoplePageQuery = graphql` organization: node(id: $organizationId) @required(action: THROW) { __typename ... on Organization { - canCreateUser: permission(action: "iam:membership-profile:create") + canCreateUser: permission(action: "iam:membership-profile:create", attributes: { target_role: "VIEWER" }) ...PeopleListFragment @arguments(first: 20, order: { direction: ASC, field: FULL_NAME }) } diff --git a/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx b/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx index 87f51ebb2..92a4445bb 100644 --- a/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx +++ b/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx @@ -48,7 +48,7 @@ const fragment = graphql` membership @required(action: THROW) { id role - canUpdate: permission(action: "iam:membership:update") + canUpdate: permission(action: "iam:membership:update", attributes: { target_role: "VIEWER" }) } lastInvitation: pendingInvitations(first: 1, orderBy: { field: CREATED_AT, direction: DESC }) @required(action: THROW) @@ -134,6 +134,10 @@ export function PeopleListItem(props: { const profile = useFragment(fragment, fKey); const lastInvitation = profile.lastInvitation.edges[0]?.node; + const roleOptions = availableRoles.includes(profile.membership.role) + ? availableRoles + : [...availableRoles, profile.membership.role]; + const isInactive = profile.state === "INACTIVE"; const canSendActivationMail = isInactive && profile.source !== "SCIM" && profile.canInvite; @@ -286,19 +290,19 @@ export function PeopleListItem(props: { value={profile.membership.role} onValueChange={role => void handleUpdateRole(role)} > - {availableRoles.includes("OWNER") && ( + {roleOptions.includes("OWNER") && ( )} - {availableRoles.includes("ADMIN") && ( + {roleOptions.includes("ADMIN") && ( )} - {availableRoles.includes("VIEWER") && ( + {roleOptions.includes("VIEWER") && ( )} - {availableRoles.includes("AUDITOR") && ( + {roleOptions.includes("AUDITOR") && ( )} - {availableRoles.includes("EMPLOYEE") && ( + {roleOptions.includes("EMPLOYEE") && ( )} diff --git a/cmd/probod/CHANGELOG.md b/cmd/probod/CHANGELOG.md index d5b4799b5..51ba6dac6 100644 --- a/cmd/probod/CHANGELOG.md +++ b/cmd/probod/CHANGELOG.md @@ -10,7 +10,7 @@ All notable changes to `probod` (the server, including the bundled `@probo/conso ### Changed -- Consolidated ownership-grant authorization into policy: granting OWNER (via `createUser` or `updateMembership`) is now restricted to organization owners through role-scoped allow policies conditioned on the assigned role, replacing the per-resolver custom checks and the now-removed `iam:membership-role:set-owner` action. The `permission` field gained an optional typed `attributes` argument so the console can refine dry-run checks (e.g. by target role) without loosening the base grants +- Consolidated ownership-grant authorization into policy: granting OWNER (via `createUser` or `updateMembership`) is now restricted to organization owners through role-scoped allow policies conditioned on the assigned role, replacing the per-resolver custom checks and the now-removed `iam:membership-role:set-owner` action. The `permission` field gained an optional generic `attributes` key/value argument so the console can refine dry-run checks (e.g. by target role) without loosening the base grants ## [0.223.3] - 2026-07-06 diff --git a/e2e/console/user_test.go b/e2e/console/user_test.go index 79fc2670b..89a9532f5 100644 --- a/e2e/console/user_test.go +++ b/e2e/console/user_test.go @@ -97,6 +97,92 @@ func TestUser_AdminCannotCreateOwner(t *testing.T) { assert.Equal(t, "OWNER", ownerCreate.CreateUser.ProfileEdge.Node.Membership.Role) } +// TestUser_AdminCannotPromoteToOwner is a non-regression test that an ADMIN +// cannot grant ownership by promoting an existing member to OWNER via +// updateMembership. Granting ownership is owner-only, enforced by policy on the +// assigned (target) role; an ADMIN may still change members between non-owner +// roles. +func TestUser_AdminCannotPromoteToOwner(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + admin := testutil.NewClientInOrg(t, testutil.RoleAdmin, owner) + viewer := testutil.NewClientInOrg(t, testutil.RoleViewer, owner) + + const membershipQuery = ` + query($id: ID!) { + node(id: $id) { + ... on Profile { + membership { id } + } + } + } + ` + + var viewerMembership struct { + Node struct { + Membership struct { + ID string `json:"id"` + } `json:"membership"` + } `json:"node"` + } + + err := owner.ExecuteConnect(membershipQuery, map[string]any{ + "id": viewer.GetProfileID().String(), + }, &viewerMembership) + require.NoError(t, err) + require.NotEmpty(t, viewerMembership.Node.Membership.ID) + + membershipID := viewerMembership.Node.Membership.ID + + const updateMembershipMutation = ` + mutation($input: UpdateMembershipInput!) { + updateMembership(input: $input) { + membership { role } + } + } + ` + + updateInput := func(role string) map[string]any { + return map[string]any{ + "input": map[string]any{ + "organizationId": owner.GetOrganizationID().String(), + "membershipId": membershipID, + "role": role, + }, + } + } + + // An ADMIN must not be able to promote a member to OWNER. + _, err = admin.DoConnect(updateMembershipMutation, updateInput("OWNER")) + testutil.RequireForbiddenError(t, err) + + // The same ADMIN can still change a member between non-owner roles. + var adminUpdate struct { + UpdateMembership struct { + Membership struct { + Role string `json:"role"` + } `json:"membership"` + } `json:"updateMembership"` + } + + err = admin.ExecuteConnect(updateMembershipMutation, updateInput("ADMIN"), &adminUpdate) + require.NoError(t, err) + assert.Equal(t, "ADMIN", adminUpdate.UpdateMembership.Membership.Role) + + // An OWNER remains able to promote a member to OWNER. + var ownerUpdate struct { + UpdateMembership struct { + Membership struct { + Role string `json:"role"` + } `json:"membership"` + } `json:"updateMembership"` + } + + err = owner.ExecuteConnect(updateMembershipMutation, updateInput("OWNER"), &ownerUpdate) + require.NoError(t, err) + assert.Equal(t, "OWNER", ownerUpdate.UpdateMembership.Membership.Role) +} + // TestUser_AdminCannotRemoveMembers is a non-regression test for the broken // access control where an ADMIN could hard-remove members (including OWNERs) // via removeUser because the mutation only checked the weaker diff --git a/pkg/iam/iam_policies.go b/pkg/iam/iam_policies.go index 65ff85ff0..ad0f95517 100644 --- a/pkg/iam/iam_policies.go +++ b/pkg/iam/iam_policies.go @@ -265,6 +265,9 @@ var IAMAdminPolicy = policy.NewPolicy( WithSID("membership-admin-access"). When(policy.Equals("principal.organization_id", "resource.organization_id")), + // Can update memberships, but neither of an existing owner (resource.role) + // nor to grant ownership (resource.target_role); only owner can grant + // ownership. policy.Allow( ActionMembershipUpdate, ). @@ -272,13 +275,13 @@ var IAMAdminPolicy = policy.NewPolicy( When( policy.Equals("principal.organization_id", "resource.organization_id"), policy.NotEquals("resource.role", "OWNER"), + policy.NotEquals("resource.target_role", "OWNER"), ), - // Can view membership profiles (scoped to own organization) + // Can view and manage membership profiles (scoped to own organization) policy.Allow( ActionMembershipProfileGet, ActionMembershipProfileList, - ActionMembershipProfileCreate, ActionMembershipProfileUpdate, ActionMembershipProfileDelete, ActionMembershipProfileActivate, @@ -287,6 +290,15 @@ var IAMAdminPolicy = policy.NewPolicy( WithSID("membership-profile-admin-access"). When(policy.Equals("principal.organization_id", "resource.organization_id")), + // Can create members, but not with the OWNER role (resource.target_role); + // only owner can grant ownership. + policy.Allow(ActionMembershipProfileCreate). + WithSID("membership-profile-admin-create"). + When( + policy.Equals("principal.organization_id", "resource.organization_id"), + policy.NotEquals("resource.target_role", "OWNER"), + ), + // Can view identities of members in the same organization policy.Allow(ActionIdentityGet). WithSID("view-member-identity"). @@ -313,15 +325,6 @@ var IAMAdminPolicy = policy.NewPolicy( policy.Deny(ActionMembershipDelete). WithSID("deny-remove-member"), - // Cannot grant ownership, whether by creating an OWNER member or promoting an - // existing member to OWNER (only owner can grant ownership) - policy.Deny(ActionMembershipProfileCreate). - WithSID("deny-create-owner"). - When(policy.Equals("resource.target_role", "OWNER")), - policy.Deny(ActionMembershipUpdate). - WithSID("deny-promote-owner"). - When(policy.Equals("resource.target_role", "OWNER")), - // Cannot manage SAML configurations (only owner can) policy.Deny( ActionSAMLConfigurationCreate, diff --git a/pkg/server/api/connect/v1/graphql/base.graphql b/pkg/server/api/connect/v1/graphql/base.graphql index 254a2ebaf..89d693031 100644 --- a/pkg/server/api/connect/v1/graphql/base.graphql +++ b/pkg/server/api/connect/v1/graphql/base.graphql @@ -17,6 +17,7 @@ scalar Datetime scalar Upload scalar EmailAddr scalar OAuth2Scope +scalar Map interface Node { id: ID! diff --git a/pkg/server/api/connect/v1/graphql/membership.graphql b/pkg/server/api/connect/v1/graphql/membership.graphql index f58cf4e8a..4b6eecaa2 100644 --- a/pkg/server/api/connect/v1/graphql/membership.graphql +++ b/pkg/server/api/connect/v1/graphql/membership.graphql @@ -16,7 +16,7 @@ type Membership implements Node { lastSession: Session @goField(forceResolver: true) - permission(action: String!): Boolean! + permission(action: String!, attributes: Map): Boolean! @goField(forceResolver: true) @authentication(required: PRESENT) } diff --git a/pkg/server/api/connect/v1/graphql/organization.graphql b/pkg/server/api/connect/v1/graphql/organization.graphql index 9a8b81829..5622b6d03 100644 --- a/pkg/server/api/connect/v1/graphql/organization.graphql +++ b/pkg/server/api/connect/v1/graphql/organization.graphql @@ -39,7 +39,7 @@ type Organization implements Node { viewer: Profile @goField(forceResolver: true) - permission(action: String!): Boolean! + permission(action: String!, attributes: Map): Boolean! @goField(forceResolver: true) @authentication(required: PRESENT) } diff --git a/pkg/server/api/connect/v1/membership_resolvers.go b/pkg/server/api/connect/v1/membership_resolvers.go index 8b06b45fb..6ffca6bd1 100644 --- a/pkg/server/api/connect/v1/membership_resolvers.go +++ b/pkg/server/api/connect/v1/membership_resolvers.go @@ -44,8 +44,8 @@ func (r *membershipResolver) LastSession(ctx context.Context, obj *types.Members } // Permission is the resolver for the permission field. -func (r *membershipResolver) Permission(ctx context.Context, obj *types.Membership, action string) (bool, error) { - return r.Resolver.Permission(ctx, obj, action) +func (r *membershipResolver) Permission(ctx context.Context, obj *types.Membership, action string, attributes map[string]any) (bool, error) { + return r.Resolver.permission(ctx, obj, action, attributes) } // UpdateMembership is the resolver for the updateMembership field. diff --git a/pkg/server/api/connect/v1/organization_resolvers.go b/pkg/server/api/connect/v1/organization_resolvers.go index ea38d1718..c2fd695dd 100644 --- a/pkg/server/api/connect/v1/organization_resolvers.go +++ b/pkg/server/api/connect/v1/organization_resolvers.go @@ -358,8 +358,8 @@ func (r *organizationResolver) Viewer(ctx context.Context, obj *types.Organizati } // Permission is the resolver for the permission field. -func (r *organizationResolver) Permission(ctx context.Context, obj *types.Organization, action string) (bool, error) { - return r.Resolver.Permission(ctx, obj, action) +func (r *organizationResolver) Permission(ctx context.Context, obj *types.Organization, action string, attributes map[string]any) (bool, error) { + return r.Resolver.permission(ctx, obj, action, attributes) } // Organization returns schema.OrganizationResolver implementation. diff --git a/pkg/server/api/connect/v1/resolver.go b/pkg/server/api/connect/v1/resolver.go index 1c86cbac8..1e64ab56a 100644 --- a/pkg/server/api/connect/v1/resolver.go +++ b/pkg/server/api/connect/v1/resolver.go @@ -129,7 +129,18 @@ func NewMux( } func (r *Resolver) Permission(ctx context.Context, obj types.Node, action string) (bool, error) { - _, err := r.authorize(ctx, obj.GetID(), action, authz.WithDryRun()) + return r.permission(ctx, obj, action, nil) +} + +func (r *Resolver) permission(ctx context.Context, obj types.Node, action string, attributes map[string]any) (bool, error) { + opts := []authz.AuthorizeFuncOption{authz.WithDryRun()} + for key, value := range attributes { + if s, ok := value.(string); ok { + opts = append(opts, authz.WithAttr(key, s)) + } + } + + _, err := r.authorize(ctx, obj.GetID(), action, opts...) return err == nil, nil } diff --git a/relay.config.json b/relay.config.json index 618c408d9..c4fbe880b 100644 --- a/relay.config.json +++ b/relay.config.json @@ -39,7 +39,8 @@ "Duration": "string", "BigInt": "number", "EmailAddr": "string", - "OAuth2Scope": "string" + "OAuth2Scope": "string", + "Map": "Record" } }, "trust": {