Handle membership deletions and role updates in iam policies

Signed-off-by: Émile Ré <emile@getprobo.com>
This commit is contained in:
Émile Ré
2026-01-16 16:09:04 +04:00
committed by Bryan Frimin
parent 00eb3cde5e
commit 3669df3f8a
6 changed files with 66 additions and 28 deletions

View File

@@ -10,7 +10,7 @@ import {
useConfirm, useConfirm,
} from "@probo/ui"; } from "@probo/ui";
import clsx from "clsx"; import clsx from "clsx";
import { use, useState } from "react"; import { useState } from "react";
import { useFragment } from "react-relay"; import { useFragment } from "react-relay";
import { graphql } from "relay-runtime"; import { graphql } from "relay-runtime";
import type { MemberListItemFragment$key } from "/__generated__/iam/MemberListItemFragment.graphql"; import type { MemberListItemFragment$key } from "/__generated__/iam/MemberListItemFragment.graphql";
@@ -18,7 +18,6 @@ import { useMutationWithToasts } from "/hooks/useMutationWithToasts";
import { useOrganizationId } from "/hooks/useOrganizationId"; import { useOrganizationId } from "/hooks/useOrganizationId";
import { sprintf } from "@probo/helpers"; import { sprintf } from "@probo/helpers";
import { EditMemberDialog } from "./EditMemberDialog"; import { EditMemberDialog } from "./EditMemberDialog";
import { CurrentUser } from "/providers/CurrentUser";
const fragment = graphql` const fragment = graphql`
fragment MemberListItemFragment on Membership { fragment MemberListItemFragment on Membership {
@@ -62,19 +61,15 @@ export function MemberListItem(props: {
const [dialogOpen, setDialogOpen] = useState(false); const [dialogOpen, setDialogOpen] = useState(false);
const membership = useFragment<MemberListItemFragment$key>(fragment, fKey); const membership = useFragment<MemberListItemFragment$key>(fragment, fKey);
const { role } = use(CurrentUser);
const isInactive = membership.state === "INACTIVE"; const isInactive = membership.state === "INACTIVE";
// Only OWNER can edit OWNER members
const canEditThisRole = membership.role === "OWNER" ? role === "OWNER" : true;
const [removeMembership, isRemoving] = useMutationWithToasts( const [removeMembership, isRemoving] = useMutationWithToasts(
removeMemberMutation, removeMemberMutation,
{ {
successMessage: __("Member removed successfully"), successMessage: __("Member removed successfully"),
errorMessage: __("Failed to remove member"), errorMessage: __("Failed to remove member"),
} },
); );
const handleRemove = async () => { const handleRemove = async () => {
@@ -93,9 +88,9 @@ export function MemberListItem(props: {
{ {
message: sprintf( message: sprintf(
__("Are you sure you want to remove %s?"), __("Are you sure you want to remove %s?"),
membership.profile.fullName membership.profile.fullName,
), ),
} },
); );
}; };
@@ -104,7 +99,7 @@ export function MemberListItem(props: {
<Tr <Tr
className={clsx( className={clsx(
isRemoving && "opacity-60 pointer-events-none", isRemoving && "opacity-60 pointer-events-none",
isInactive && "opacity-50" isInactive && "opacity-50",
)} )}
> >
<Td> <Td>
@@ -129,7 +124,7 @@ export function MemberListItem(props: {
className="flex gap-2 justify-end" className="flex gap-2 justify-end"
onClick={(e) => e.stopPropagation()} onClick={(e) => e.stopPropagation()}
> >
{membership.canUpdate && canEditThisRole && ( {membership.canUpdate && (
<Button <Button
variant="secondary" variant="secondary"
onClick={() => setDialogOpen(true)} onClick={() => setDialogOpen(true)}
@@ -141,9 +136,7 @@ export function MemberListItem(props: {
{isRemoving ? ( {isRemoving ? (
<Spinner size={16} /> <Spinner size={16} />
) : ( ) : (
membership.canDelete && membership.canDelete && (
canEditThisRole &&
membership.source !== "SCIM" && (
<Button <Button
variant="danger" variant="danger"
onClick={handleRemove} onClick={handleRemove}

View File

@@ -245,8 +245,10 @@ LEFT JOIN
func (m *Membership) AuthorizationAttributes(ctx context.Context, conn pg.Conn) (map[string]string, error) { func (m *Membership) AuthorizationAttributes(ctx context.Context, conn pg.Conn) (map[string]string, error) {
q := ` q := `
SELECT SELECT
identity_id identity_id,
, organization_id organization_id,
role,
source
FROM FROM
iam_memberships iam_memberships
WHERE WHERE
@@ -256,7 +258,14 @@ LIMIT 1;
var identityID gid.GID var identityID gid.GID
var organizationID gid.GID var organizationID gid.GID
if err := conn.QueryRow(ctx, q, m.ID).Scan(&identityID, &organizationID); err != nil { var role MembershipRole
var source MembershipSource
if err := conn.QueryRow(ctx, q, m.ID).Scan(
&identityID,
&organizationID,
&role,
&source,
); err != nil {
if errors.Is(err, pgx.ErrNoRows) { if errors.Is(err, pgx.ErrNoRows) {
return nil, ErrResourceNotFound return nil, ErrResourceNotFound
} }
@@ -266,6 +275,8 @@ LIMIT 1;
return map[string]string{ return map[string]string{
"identity_id": identityID.String(), "identity_id": identityID.String(),
"organization_id": organizationID.String(), "organization_id": organizationID.String(),
"role": role.String(),
"source": source.String(),
}, nil }, nil
} }
@@ -449,7 +460,7 @@ JOIN
LEFT JOIN LEFT JOIN
iam_membership_profiles mp ON mp.membership_id = mbr.id iam_membership_profiles mp ON mp.membership_id = mbr.id
WHERE WHERE
%s %s
` `
query = fmt.Sprintf(query, scope.SQLFragment(), cursor.SQLFragment()) query = fmt.Sprintf(query, scope.SQLFragment(), cursor.SQLFragment())

View File

@@ -112,12 +112,15 @@ func (a *Authorizer) authorize(ctx context.Context, conn pg.Conn, params Authori
} }
// Only set principal.organization_id if they have a role in this org // Only set principal.organization_id if they have a role in this org
var principalOrgID string var scopedPrincipalAttrs map[string]string
if membership != nil && role != "" { if membership != nil && role != "" {
principalOrgID = membership.OrganizationID.String() scopedPrincipalAttrs = map[string]string{
"organization_id": membership.OrganizationID.String(),
"role": membership.Role.String(),
}
} }
principalAttrs, err := a.buildPrincipalAttributes(ctx, conn, params.Principal, principalOrgID) principalAttrs, err := a.buildPrincipalAttributes(ctx, conn, params.Principal, scopedPrincipalAttrs)
if err != nil { if err != nil {
return fmt.Errorf("cannot build principal attributes: %w", err) return fmt.Errorf("cannot build principal attributes: %w", err)
} }
@@ -176,12 +179,12 @@ func (a *Authorizer) buildPrincipalAttributes(
ctx context.Context, ctx context.Context,
conn pg.Conn, conn pg.Conn,
principalID gid.GID, principalID gid.GID,
organizationID string, defaultAttrs map[string]string,
) (map[string]string, error) { ) (map[string]string, error) {
attrs := map[string]string{ attrs := map[string]string{
"id": principalID.String(), "id": principalID.String(),
"organization_id": organizationID,
} }
maps.Copy(attrs, defaultAttrs)
if entity, ok := coredata.NewEntityFromID(principalID); ok { if entity, ok := coredata.NewEntityFromID(principalID); ok {
if attributer, ok := entity.(AuthorizationAttributer); ok { if attributer, ok := entity.(AuthorizationAttributer); ok {

View File

@@ -48,6 +48,9 @@ const (
ActionMembershipUpdate = "iam:membership:update" ActionMembershipUpdate = "iam:membership:update"
ActionMembershipDelete = "iam:membership:delete" ActionMembershipDelete = "iam:membership:delete"
// Membership role actions
ActionMembershipRoleSetOwner = "iam:membership-role:set-owner"
// Membership Profile actions // Membership Profile actions
ActionMembershipProfileGet = "iam:membership-profile:get" ActionMembershipProfileGet = "iam:membership-profile:get"

View File

@@ -123,9 +123,23 @@ var IAMOwnerPolicy = policy.NewPolicy(
WithSID("full-org-access"). WithSID("full-org-access").
When(policy.Equals("principal.organization_id", "resource.organization_id")), When(policy.Equals("principal.organization_id", "resource.organization_id")),
// Full access to member management (scoped to own organization) // Full access to member management (scoped to own organization), except deletion on SCIM sourced memberships
policy.Allow("iam:membership:*"). policy.Allow(
WithSID("full-membership-access"). ActionMembershipGet,
ActionMembershipList,
ActionMembershipUpdate,
).
WithSID("membership-owner-access").
When(policy.Equals("principal.organization_id", "resource.organization_id")),
policy.Allow(
ActionMembershipDelete,
).
WithSID("membership-deletion-owner-access").
When(policy.NotEquals("resource.source", "SCIM")),
// Can set other members OWNER
policy.Allow(ActionMembershipRoleSetOwner).
WithSID("membership-role-owner-access").
When(policy.Equals("principal.organization_id", "resource.organization_id")), When(policy.Equals("principal.organization_id", "resource.organization_id")),
// Full access to membership profiles (scoped to own organization) // Full access to membership profiles (scoped to own organization)
@@ -185,11 +199,19 @@ var IAMAdminPolicy = policy.NewPolicy(
// Can manage memberships (scoped to own organization) // Can manage memberships (scoped to own organization)
policy.Allow( policy.Allow(
ActionMembershipGet, ActionMembershipGet,
ActionMembershipUpdate,
). ).
WithSID("membership-admin-access"). WithSID("membership-admin-access").
When(policy.Equals("principal.organization_id", "resource.organization_id")), When(policy.Equals("principal.organization_id", "resource.organization_id")),
policy.Allow(
ActionMembershipUpdate,
).
WithSID("membership-role-admin-access").
When(
policy.Equals("principal.organization_id", "resource.organization_id"),
policy.NotEquals("resource.role", "OWNER"),
),
// Can view membership profiles (scoped to own organization) // Can view membership profiles (scoped to own organization)
policy.Allow(ActionMembershipProfileGet). policy.Allow(ActionMembershipProfileGet).
WithSID("membership-profile-admin-access"). WithSID("membership-profile-admin-access").

View File

@@ -950,6 +950,12 @@ func (r *mutationResolver) UpdateMembership(ctx context.Context, input types.Upd
return nil, err return nil, err
} }
if input.Role == coredata.MembershipRoleOwner {
if err := r.authorize(ctx, input.MembershipID, iam.ActionMembershipRoleSetOwner); err != nil {
return nil, err
}
}
membership, err := r.iam.OrganizationService.UpdateMempership(ctx, input.OrganizationID, input.MembershipID, input.Role) membership, err := r.iam.OrganizationService.UpdateMempership(ctx, input.OrganizationID, input.MembershipID, input.Role)
if err != nil { if err != nil {
r.logger.ErrorCtx(ctx, "cannot update membership", log.Error(err)) r.logger.ErrorCtx(ctx, "cannot update membership", log.Error(err))