Fix: Improve error handling for Server API (#170)

* Added entity checks to provide better errors in API for 'server' access type

* Removed 'ensureUserTeamPermissionExist', changed permissionId type to string in 'ensureUserHasTeamPermission'

* added different error types for user team permission

---------

Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Co-authored-by: Zai Shi <zaishi00@outlook.com>
This commit is contained in:
Fahad Khan 2024-08-10 05:26:07 +05:30 committed by Konstantin Wohlwend
parent 5b9ee575f8
commit 4792aa53f8
9 changed files with 89 additions and 29 deletions

View File

@ -1,4 +1,4 @@
import { ensureUserHasTeamPermission } from "@/lib/request-checks";
import { ensureUserTeamPermissionExists } from "@/lib/request-checks";
import { prismaClient } from "@/prisma-client";
import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
import { adaptSchema, clientOrHigherAuthTypeSchema, teamIdSchema, teamInvitationCallbackUrlSchema, teamInvitationEmailSchema, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields";
@ -30,11 +30,12 @@ export const POST = createSmartRouteHandler({
async handler({ auth, body }) {
await prismaClient.$transaction(async (tx) => {
if (auth.type === "client") {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
userId: auth.user.id,
teamId: body.team_id,
permissionId: "$invite_members"
permissionId: "$invite_members",
errorType: 'required',
});
}
});

View File

@ -1,4 +1,4 @@
import { ensureTeamExist, ensureTeamMembershipExist, ensureUserExist, ensureUserHasTeamPermission } from "@/lib/request-checks";
import { ensureTeamExist, ensureTeamMembershipExists, ensureUserExist, ensureUserTeamPermissionExists } from "@/lib/request-checks";
import { prismaClient } from "@/prisma-client";
import { createCrudHandlers } from "@/route-handlers/crud-handler";
import { getIdFromUserIdOrMe } from "@/route-handlers/utils";
@ -43,14 +43,15 @@ export const teamMemberProfilesCrudHandlers = createLazyProxy(() => createCrudHa
throw new StatusError(StatusError.BadRequest, 'team_id is required for access type client');
}
await ensureTeamMembershipExist(tx, { projectId: auth.project.id, teamId: query.team_id, userId: currentUserId });
await ensureTeamMembershipExists(tx, { projectId: auth.project.id, teamId: query.team_id, userId: currentUserId });
if (userId !== currentUserId) {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: query.team_id,
userId: currentUserId,
permissionId: '$read_members',
errorType: 'required',
});
}
} else {
@ -85,15 +86,16 @@ export const teamMemberProfilesCrudHandlers = createLazyProxy(() => createCrudHa
const userId = getIdFromUserIdOrMe(params.user_id, auth.user);
if (auth.type === 'client' && userId !== auth.user?.id) {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr("Client must be authenticated"),
permissionId: '$read_members',
errorType: 'required',
});
}
await ensureTeamMembershipExist(tx, { projectId: auth.project.id, teamId: params.team_id, userId: userId });
await ensureTeamMembershipExists(tx, { projectId: auth.project.id, teamId: params.team_id, userId: userId });
const db = await tx.teamMember.findUnique({
where: {
@ -122,7 +124,7 @@ export const teamMemberProfilesCrudHandlers = createLazyProxy(() => createCrudHa
throw new StatusError(StatusError.Forbidden, 'Cannot update another user\'s profile');
}
await ensureTeamMembershipExist(tx, {
await ensureTeamMembershipExists(tx, {
projectId: auth.project.id,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr("Client must be authenticated"),

View File

@ -1,4 +1,4 @@
import { ensureTeamExist, ensureTeamMembershipDoesNotExist, ensureUserHasTeamPermission } from "@/lib/request-checks";
import { ensureTeamExist, ensureTeamMembershipExists, ensureTeamMembershipDoesNotExist, ensureUserTeamPermissionExists } from "@/lib/request-checks";
import { isTeamSystemPermission, teamSystemPermissionStringToDBType } from "@/lib/permissions";
import { prismaClient } from "@/prisma-client";
import { createCrudHandlers } from "@/route-handlers/crud-handler";
@ -100,14 +100,21 @@ export const teamMembershipsCrudHandlers = createLazyProxy(() => createCrudHandl
// Users are always allowed to remove themselves from a team
// Only users with the $remove_members permission can remove other users
if (auth.type === 'client' && userId !== auth.user?.id) {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr('auth.user is null'),
permissionId: "$remove_members",
errorType: 'required',
});
}
await ensureTeamMembershipExists(tx, {
projectId: auth.project.id,
teamId: params.team_id,
userId,
});
await tx.teamMember.delete({
where: {
projectId_projectUserId_teamId: {

View File

@ -1,4 +1,5 @@
import { grantTeamPermission, listUserTeamPermissions, revokeTeamPermission } from "@/lib/permissions";
import { ensureTeamMembershipExists, ensureUserTeamPermissionExists } from "@/lib/request-checks";
import { prismaClient } from "@/prisma-client";
import { createCrudHandlers } from "@/route-handlers/crud-handler";
import { getIdFromUserIdOrMe } from "@/route-handlers/utils";
@ -21,6 +22,8 @@ export const teamPermissionsCrudHandlers = createLazyProxy(() => createCrudHandl
}),
async onCreate({ auth, params }) {
return await prismaClient.$transaction(async (tx) => {
await ensureTeamMembershipExists(tx, { projectId: auth.project.id, teamId: params.team_id, userId: params.user_id });
return await grantTeamPermission(tx, {
project: auth.project,
teamId: params.team_id,
@ -31,6 +34,14 @@ export const teamPermissionsCrudHandlers = createLazyProxy(() => createCrudHandl
},
async onDelete({ auth, params }) {
return await prismaClient.$transaction(async (tx) => {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: params.team_id,
userId: params.user_id,
permissionId: params.permission_id,
errorType: 'not-exist',
});
return await revokeTeamPermission(tx, {
project: auth.project,
teamId: params.team_id,

View File

@ -1,4 +1,4 @@
import { ensureTeamExist, ensureTeamMembershipExist, ensureUserHasTeamPermission } from "@/lib/request-checks";
import { ensureTeamExist, ensureTeamMembershipExists, ensureUserTeamPermissionExists } from "@/lib/request-checks";
import { sendTeamCreatedWebhook, sendTeamDeletedWebhook, sendTeamUpdatedWebhook } from "@/lib/webhooks";
import { prismaClient } from "@/prisma-client";
import { createCrudHandlers } from "@/route-handlers/crud-handler";
@ -66,7 +66,7 @@ export const teamsCrudHandlers = createLazyProxy(() => createCrudHandlers(teamsC
onRead: async ({ params, auth }) => {
const db = await prismaClient.$transaction(async (tx) => {
if (auth.type === 'client') {
await ensureTeamMembershipExist(tx, {
await ensureTeamMembershipExists(tx, {
projectId: auth.project.id,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr('auth.user is null'),
@ -94,11 +94,12 @@ export const teamsCrudHandlers = createLazyProxy(() => createCrudHandlers(teamsC
onUpdate: async ({ params, auth, data }) => {
const db = await prismaClient.$transaction(async (tx) => {
if (auth.type === 'client') {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr('auth.user is null'),
permissionId: "$update_team",
errorType: 'required',
});
}
@ -130,13 +131,15 @@ export const teamsCrudHandlers = createLazyProxy(() => createCrudHandlers(teamsC
onDelete: async ({ params, auth }) => {
await prismaClient.$transaction(async (tx) => {
if (auth.type === 'client') {
await ensureUserHasTeamPermission(tx, {
await ensureUserTeamPermissionExists(tx, {
project: auth.project,
teamId: params.team_id,
userId: auth.user?.id ?? throwErr('auth.user is null'),
permissionId: "$delete_team",
errorType: 'required',
});
}
await ensureTeamExist(tx, { projectId: auth.project.id, teamId: params.team_id });
await tx.team.delete({
where: {

View File

@ -1,3 +1,4 @@
import { ensureTeamMembershipExists, ensureUserExist } from "@/lib/request-checks";
import { prismaClient } from "@/prisma-client";
import { createCrudHandlers } from "@/route-handlers/crud-handler";
import { BooleanTrue, Prisma } from "@prisma/client";
@ -197,7 +198,17 @@ export const usersCrudHandlers = createLazyProxy(() => createCrudHandlers(usersC
},
onUpdate: async ({ auth, data, params }) => {
const db = await prismaClient.$transaction(async (tx) => {
await ensureUserExist(tx, { projectId: auth.project.id, userId: params.user_id });
if (data.selected_team_id !== undefined) {
if (data.selected_team_id !== null) {
await ensureTeamMembershipExists(tx, {
projectId: auth.project.id,
teamId: data.selected_team_id,
userId: params.user_id,
});
}
await tx.teamMember.updateMany({
where: {
projectId: auth.project.id,
@ -257,14 +268,18 @@ export const usersCrudHandlers = createLazyProxy(() => createCrudHandlers(usersC
return result;
},
onDelete: async ({ auth, params }) => {
await prismaClient.projectUser.delete({
where: {
projectId_projectUserId: {
projectId: auth.project.id,
projectUserId: params.user_id,
await prismaClient.$transaction(async (tx) => {
await ensureUserExist(tx, { projectId: auth.project.id, userId: params.user_id });
await tx.projectUser.delete({
where: {
projectId_projectUserId: {
projectId: auth.project.id,
projectUserId: params.user_id,
},
},
},
include: fullInclude,
include: fullInclude,
});
});
await sendUserDeletedWebhook({

View File

@ -2,7 +2,7 @@ import { ProxiedOAuthProviderType, StandardOAuthProviderType } from "@prisma/cli
import { KnownErrors } from "@stackframe/stack-shared";
import { ProjectsCrud } from "@stackframe/stack-shared/dist/interface/crud/projects";
import { ProviderType, sharedProviders, standardProviders } from "@stackframe/stack-shared/dist/utils/oauth";
import { TeamSystemPermission, listUserTeamPermissions } from "./permissions";
import { listUserTeamPermissions } from "./permissions";
import { PrismaTransaction } from "./types";
@ -24,7 +24,7 @@ async function _getTeamMembership(
});
}
export async function ensureTeamMembershipExist(
export async function ensureTeamMembershipExists(
tx: PrismaTransaction,
options: {
projectId: string,
@ -77,16 +77,17 @@ export async function ensureTeamExist(
}
}
export async function ensureUserHasTeamPermission(
export async function ensureUserTeamPermissionExists(
tx: PrismaTransaction,
options: {
project: ProjectsCrud["Admin"]["Read"],
teamId: string,
userId: string,
permissionId: TeamSystemPermission,
permissionId: string,
errorType: 'required' | 'not-exist',
}
) {
await ensureTeamMembershipExist(tx, {
await ensureTeamMembershipExists(tx, {
projectId: options.project.id,
teamId: options.teamId,
userId: options.userId,
@ -101,7 +102,11 @@ export async function ensureUserHasTeamPermission(
});
if (result.length === 0) {
throw new KnownErrors.TeamPermissionRequired(options.teamId, options.userId, options.permissionId);
if (options.errorType === 'not-exist') {
throw new KnownErrors.TeamPermissionNotFound(options.teamId, options.userId, options.permissionId);
} else {
throw new KnownErrors.TeamPermissionRequired(options.teamId, options.userId, options.permissionId);
}
}
}

View File

@ -1034,6 +1034,21 @@ const TeamPermissionRequired = createKnownErrorConstructor(
(json) => [json.team_id, json.user_id, json.permission_id] as const,
);
const TeamPermissionNotFound = createKnownErrorConstructor(
KnownError,
"TEAM_PERMISSION_NOT_FOUND",
(teamId, userId, permissionId) => [
401,
`User ${userId} does not have permission ${permissionId} in team ${teamId}.`,
{
team_id: teamId,
user_id: userId,
permission_id: permissionId,
},
] as const,
(json) => [json.team_id, json.user_id, json.permission_id] as const,
);
const InvalidSharedOAuthProviderId = createKnownErrorConstructor(
KnownError,
"INVALID_SHARED_OAUTH_PROVIDER_ID",
@ -1156,6 +1171,7 @@ export const KnownErrors = {
InvalidSharedOAuthProviderId,
InvalidStandardOAuthProviderId,
InvalidAuthorizationCode,
TeamPermissionNotFound,
} satisfies Record<string, KnownErrorConstructor<any, any>>;

View File

@ -185,7 +185,7 @@ export const userIdSchema = yupString().uuid().meta({ openapiField: { descriptio
export const primaryEmailSchema = emailSchema.meta({ openapiField: { description: 'Primary email', exampleValue: 'johndoe@example.com' } });
export const primaryEmailVerifiedSchema = yupBoolean().meta({ openapiField: { description: 'Whether the primary email has been verified to belong to this user', exampleValue: true } });
export const userDisplayNameSchema = yupString().nullable().meta({ openapiField: { description: _displayNameDescription('user'), exampleValue: 'John Doe' } });
export const selectedTeamIdSchema = yupString().meta({ openapiField: { description: 'ID of the team currently selected by the user', exampleValue: 'team-id' } });
export const selectedTeamIdSchema = yupString().uuid().meta({ openapiField: { description: 'ID of the team currently selected by the user', exampleValue: 'team-id' } });
export const profileImageUrlSchema = yupString().meta({ openapiField: { description: _profileImageUrlDescription('user'), exampleValue: 'https://example.com/image.jpg' } });
export const signedUpAtMillisSchema = yupNumber().meta({ openapiField: { description: _signedUpAtMillisDescription, exampleValue: 1630000000000 } });
export const userClientMetadataSchema = jsonSchema.meta({ openapiField: { description: _clientMetaDataDescription('user'), exampleValue: { key: 'value' } } });