Refactor team accounts feature and improve form validation

Updated several components within the team accounts feature to use more specific schema definitions and provide clearer form validation. Several schema files were renamed to better reflect their usage and additional properties were included. The commit also introduces handling for a new accountId property, which provides more accurate user account tracking. Autocomplete was turned off on deletion actions for better security. Changes related to account ownership transfer were also made, including transaction logging and error handling improvements.
This commit is contained in:
giancarlo
2024-03-29 17:39:35 +08:00
parent 9e06d420bd
commit af908ae685
17 changed files with 995 additions and 887 deletions

View File

@@ -147,6 +147,7 @@ async function TeamAccountMembersPage({ params }: Params) {
<AccountMembersTable
userRoleHierarchy={currentUserRoleHierarchy}
currentUserId={user.id}
currentAccountId={account.id}
members={members}
isPrimaryOwner={isPrimaryOwner}
canManageRoles={canManageRoles}

View File

@@ -109,6 +109,7 @@ function DeleteAccountForm() {
<FormControl>
<Input
autoComplete={'off'}
data-test={'delete-account-input-field'}
required
name={'confirmation'}

View File

@@ -25,7 +25,7 @@ import {
import { If } from '@kit/ui/if';
import { Trans } from '@kit/ui/trans';
import { UpdateRoleSchema } from '../../schema/update-role-schema';
import { UpdateMemberRoleSchema } from '../../schema/update-member-role-schema';
import { updateInvitationAction } from '../../server/actions/team-invitations-server-actions';
import { MembershipRoleSelector } from '../members/membership-role-selector';
import { RolesDataProvider } from '../members/roles-data-provider';
@@ -99,7 +99,7 @@ function UpdateInvitationForm({
const form = useForm({
resolver: zodResolver(
UpdateRoleSchema.refine(
UpdateMemberRoleSchema.refine(
(data) => {
return data.role !== userRole;
},

View File

@@ -37,6 +37,7 @@ interface Permissions {
type AccountMembersTableProps = {
members: Members;
currentUserId: string;
currentAccountId: string;
userRoleHierarchy: number;
isPrimaryOwner: boolean;
canManageRoles: boolean;
@@ -45,6 +46,7 @@ type AccountMembersTableProps = {
export function AccountMembersTable({
members,
currentUserId,
currentAccountId,
isPrimaryOwner,
userRoleHierarchy,
canManageRoles,
@@ -60,7 +62,10 @@ export function AccountMembersTable({
canTransferOwnership: isPrimaryOwner,
};
const columns = useGetColumns(permissions, currentUserId);
const columns = useGetColumns(permissions, {
currentUserId,
currentAccountId,
});
const filteredMembers = members.filter((member) => {
const searchString = search.toLowerCase();
@@ -87,7 +92,10 @@ export function AccountMembersTable({
function useGetColumns(
permissions: Permissions,
currentUserId: string,
params: {
currentUserId: string;
currentAccountId: string;
},
): ColumnDef<Members[0]>[] {
const { t } = useTranslation('teams');
@@ -99,7 +107,7 @@ function useGetColumns(
cell: ({ row }) => {
const member = row.original;
const displayName = member.name ?? member.email.split('@')[0];
const isSelf = member.user_id === currentUserId;
const isSelf = member.user_id === params.currentUserId;
return (
<span className={'flex items-center space-x-4 text-left'}>
@@ -168,12 +176,13 @@ function useGetColumns(
<ActionsDropdown
permissions={permissions}
member={row.original}
currentUserId={currentUserId}
currentUserId={params.currentUserId}
accountId={params.currentAccountId}
/>
),
},
],
[permissions, currentUserId, t],
[t, params, permissions],
);
}
@@ -181,10 +190,12 @@ function ActionsDropdown({
permissions,
member,
currentUserId,
accountId,
}: {
permissions: Permissions;
member: Members[0];
currentUserId: string;
accountId: string;
}) {
const [isRemoving, setIsRemoving] = useState(false);
const [isTransferring, setIsTransferring] = useState(false);
@@ -268,7 +279,7 @@ function ActionsDropdown({
isOpen
setIsOpen={setIsTransferring}
targetDisplayName={member.name ?? member.email}
accountId={member.id}
accountId={accountId}
userId={member.user_id}
/>
</If>

View File

@@ -77,25 +77,12 @@ function TransferOrganizationOwnershipForm({
const [pending, startTransition] = useTransition();
const [error, setError] = useState<boolean>();
const onSubmit = () => {
startTransition(async () => {
try {
await transferOwnershipAction({
accountId,
userId,
});
setIsOpen(false);
} catch (error) {
setError(true);
}
});
};
const form = useForm({
resolver: zodResolver(TransferOwnershipConfirmationSchema),
defaultValues: {
confirmation: '',
accountId,
userId,
},
});
@@ -103,7 +90,17 @@ function TransferOrganizationOwnershipForm({
<Form {...form}>
<form
className={'flex flex-col space-y-4 text-sm'}
onSubmit={form.handleSubmit(onSubmit)}
onSubmit={form.handleSubmit((data) => {
startTransition(async () => {
try {
await transferOwnershipAction(data);
setIsOpen(false);
} catch (error) {
setError(true);
}
});
})}
>
<If condition={error}>
<TransferOwnershipErrorAlert />
@@ -129,7 +126,12 @@ function TransferOrganizationOwnershipForm({
</FormLabel>
<FormControl>
<Input type={'text'} required {...field} />
<Input
autoComplete={'off'}
type={'text'}
required
{...field}
/>
</FormControl>
<FormDescription>
@@ -176,11 +178,11 @@ function TransferOwnershipErrorAlert() {
return (
<Alert variant={'destructive'}>
<AlertTitle>
<Trans i18nKey={'teams:transferOrganizationErrorHeading'} />
<Trans i18nKey={'teams:transferTeamErrorHeading'} />
</AlertTitle>
<AlertDescription>
<Trans i18nKey={'teams:transferOrganizationErrorMessage'} />
<Trans i18nKey={'teams:transferTeamErrorMessage'} />
</AlertDescription>
</Alert>
);

View File

@@ -25,7 +25,7 @@ import {
import { If } from '@kit/ui/if';
import { Trans } from '@kit/ui/trans';
import { UpdateRoleSchema } from '../../schema/update-role-schema';
import { RoleSchema } from '../../schema/update-member-role-schema';
import { updateMemberRoleAction } from '../../server/actions/team-members-server-actions';
import { MembershipRoleSelector } from './membership-role-selector';
import { RolesDataProvider } from './roles-data-provider';
@@ -107,7 +107,7 @@ function UpdateMemberForm({
const form = useForm({
resolver: zodResolver(
UpdateRoleSchema.refine(
RoleSchema.refine(
(data) => {
return data.role !== userRole;
},

View File

@@ -189,6 +189,7 @@ function DeleteTeamConfirmationForm({
data-test={'delete-team-input-field'}
required
type={'text'}
autoComplete={'off'}
className={'w-full'}
placeholder={''}
pattern={name}
@@ -315,6 +316,7 @@ function LeaveTeamContainer(props: {
data-test="leave-team-input-field"
type="text"
className="w-full"
autoComplete={'off'}
placeholder=""
pattern="LEAVE"
required

View File

@@ -1,5 +1,5 @@
import { z } from 'zod';
export const DeleteTeamAccountSchema = z.object({
accountId: z.string(),
accountId: z.string().uuid(),
});

View File

@@ -1,10 +1,8 @@
import { z } from 'zod';
type Role = string;
const InviteSchema = z.object({
email: z.string().email(),
role: z.custom<Role>(() => z.string().min(1)),
role: z.string().min(1),
});
export const InviteMembersSchema = z

View File

@@ -0,0 +1,6 @@
import { z } from 'zod';
export const RemoveMemberSchema = z.object({
accountId: z.string().uuid(),
userId: z.string().uuid(),
});

View File

@@ -2,11 +2,8 @@ import { z } from 'zod';
const confirmationString = 'TRANSFER';
export const TransferOwnershipConfirmationSchema = z
.object({
confirmation: z.string(),
})
.refine((data) => data.confirmation === confirmationString, {
message: `Confirmation must be ${confirmationString}`,
path: ['confirmation'],
export const TransferOwnershipConfirmationSchema = z.object({
userId: z.string().uuid(),
confirmation: z.custom((value) => value === confirmationString),
accountId: z.string().uuid(),
});

View File

@@ -0,0 +1,10 @@
import { z } from 'zod';
export const RoleSchema = z.object({
role: z.string().min(1),
});
export const UpdateMemberRoleSchema = RoleSchema.extend({
accountId: z.string().uuid(),
userId: z.string().uuid(),
});

View File

@@ -1,5 +0,0 @@
import { z } from 'zod';
export const UpdateRoleSchema = z.object({
role: z.string().min(1),
});

View File

@@ -1,16 +1,22 @@
'use server';
import { revalidatePath } from 'next/cache';
import { SupabaseClient } from '@supabase/supabase-js';
import { z } from 'zod';
import { Database } from '@kit/supabase/database';
import { getSupabaseServerActionClient } from '@kit/supabase/server-actions-client';
import { RemoveMemberSchema } from '../../schema/remove-member.schema';
import { TransferOwnershipConfirmationSchema } from '../../schema/transfer-ownership-confirmation.schema';
import { UpdateMemberRoleSchema } from '../../schema/update-member-role-schema';
import { AccountMembersService } from '../services/account-members.service';
export async function removeMemberFromAccountAction(params: {
accountId: string;
userId: string;
}) {
export async function removeMemberFromAccountAction(
params: z.infer<typeof RemoveMemberSchema>,
) {
const client = getSupabaseServerActionClient();
const { data, error } = await client.auth.getUser();
@@ -18,21 +24,21 @@ export async function removeMemberFromAccountAction(params: {
throw new Error(`Authentication required`);
}
const { accountId, userId } = RemoveMemberSchema.parse(params);
const service = new AccountMembersService(client);
await service.removeMemberFromAccount({
accountId: params.accountId,
userId: params.userId,
accountId,
userId,
});
return { success: true };
}
export async function updateMemberRoleAction(params: {
accountId: string;
userId: string;
role: string;
}) {
export async function updateMemberRoleAction(
params: z.infer<typeof UpdateMemberRoleSchema>,
) {
const client = getSupabaseServerActionClient();
await assertSession(client);
@@ -48,21 +54,43 @@ export async function updateMemberRoleAction(params: {
return { success: true };
}
export async function transferOwnershipAction(params: {
accountId: string;
userId: string;
}) {
export async function transferOwnershipAction(
params: z.infer<typeof TransferOwnershipConfirmationSchema>,
) {
const client = getSupabaseServerActionClient();
const { accountId, userId } =
TransferOwnershipConfirmationSchema.parse(params);
// assert that the user is authenticated
await assertSession(client);
const service = new AccountMembersService(client);
// assert that the user is the owner of the account
const { data: isOwner, error } = await client.rpc('is_account_owner', {
account_id: accountId,
});
if (error ?? !isOwner) {
throw new Error(
`You must be the owner of the account to transfer ownership`,
);
}
// at this point, the user is authenticated and is the owner of the account
// so we proceed with the transfer of ownership with admin privileges
const service = new AccountMembersService(
getSupabaseServerActionClient({ admin: true }),
);
await service.transferOwnership({
accountId: params.accountId,
userId: params.userId,
accountId,
userId,
confirmation: params.confirmation,
});
// revalidate all pages that depend on the account
revalidatePath('/home/[account]', 'layout');
return { success: true };
}

View File

@@ -1,13 +1,21 @@
import { SupabaseClient } from '@supabase/supabase-js';
import 'server-only';
import { z } from 'zod';
import { Logger } from '@kit/shared/logger';
import { Database } from '@kit/supabase/database';
import { RemoveMemberSchema } from '../../schema/remove-member.schema';
import { TransferOwnershipConfirmationSchema } from '../../schema/transfer-ownership-confirmation.schema';
import { UpdateMemberRoleSchema } from '../../schema/update-member-role-schema';
export class AccountMembersService {
private readonly namespace = 'account-members';
constructor(private readonly client: SupabaseClient<Database>) {}
async removeMemberFromAccount(params: { accountId: string; userId: string }) {
async removeMemberFromAccount(params: z.infer<typeof RemoveMemberSchema>) {
const { data, error } = await this.client
.from('accounts_memberships')
.delete()
@@ -23,11 +31,7 @@ export class AccountMembersService {
return data;
}
async updateMemberRole(params: {
accountId: string;
userId: string;
role: string;
}) {
async updateMemberRole(params: z.infer<typeof UpdateMemberRoleSchema>) {
const { data, error } = await this.client
.from('accounts_memberships')
.update({
@@ -45,21 +49,35 @@ export class AccountMembersService {
return data;
}
async transferOwnership(params: { accountId: string; userId: string }) {
const { data, error } = await this.client
.from('accounts')
.update({
primary_owner_user_id: params.userId,
})
.match({
id: params.accountId,
user_id: params.userId,
});
async transferOwnership(
params: z.infer<typeof TransferOwnershipConfirmationSchema>,
) {
const ctx = {
namespace: this.namespace,
...params,
};
Logger.info(ctx, `Transferring ownership of account`);
const { data, error } = await this.client.rpc(
'transfer_team_account_ownership',
{
target_account_id: params.accountId,
new_owner_id: params.userId,
},
);
if (error) {
Logger.error(
{ ...ctx, error },
`Failed to transfer ownership of account`,
);
throw error;
}
Logger.info(ctx, `Successfully transferred ownership of account`);
return data;
}
}

File diff suppressed because it is too large Load Diff

View File

@@ -314,6 +314,35 @@ with
check (auth.uid () = primary_owner_user_id);
-- Functions
-- Function to transfer team account ownership to another user
create or replace function public.transfer_team_account_ownership (target_account_id uuid, new_owner_id uuid) returns void as $$
begin
if current_user not in('service_role') then
raise exception 'You do not have permission to transfer account ownership';
end if;
-- update the primary owner of the account
update public.accounts
set primary_owner_user_id = new_owner_id
where id = target_account_id and is_personal_account = false;
-- update membership assigning it the hierarchy role
update public.accounts_memberships
set account_role = (
select
name
from
public.roles
where
hierarchy_level = 1
)
where target_account_id = account_id and user_id = new_owner_id;
end;
$$ language plpgsql;
grant execute on function public.transfer_team_account_ownership (uuid, uuid) to service_role;
create function public.is_account_owner (account_id uuid) returns boolean as $$
select
exists(
@@ -326,6 +355,8 @@ create function public.is_account_owner (account_id uuid) returns boolean as $$
and primary_owner_user_id = auth.uid());
$$ language sql;
grant execute on function public.is_account_owner (uuid) to authenticated, service_role;
create
or replace function kit.protect_account_fields () returns trigger as $$
begin