Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detail error when permission validation error occurs #2767

Merged
merged 5 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions backend/src/ee/services/permission/permission-types.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import picomatch from "picomatch";
import { z } from "zod";

export enum PermissionConditionOperators {
$IN = "$in",
$ALL = "$all",
$REGEX = "$regex",
$EQ = "$eq",
$NEQ = "$ne",
$GLOB = "$glob"
}
import { PermissionConditionOperators } from "@app/lib/casl";

export const PermissionConditionSchema = {
[PermissionConditionOperators.$IN]: z.string().trim().min(1).array(),
Expand Down
4 changes: 2 additions & 2 deletions backend/src/ee/services/permission/project-permission.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { AbilityBuilder, createMongoAbility, ForcedSubject, MongoAbility } from "@casl/ability";
import { z } from "zod";

import { conditionsMatcher } from "@app/lib/casl";
import { conditionsMatcher, PermissionConditionOperators } from "@app/lib/casl";
import { UnpackedPermissionSchema } from "@app/server/routes/santizedSchemas/permission";

import { PermissionConditionOperators, PermissionConditionSchema } from "./permission-types";
import { PermissionConditionSchema } from "./permission-types";

export enum ProjectPermissionActions {
Read = "read",
Expand Down
9 changes: 9 additions & 0 deletions backend/src/lib/casl/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,12 @@ export const isAtLeastAsPrivileged = (permissions1: MongoAbility, permissions2:

return set1.size >= set2.size;
};

export enum PermissionConditionOperators {
$IN = "$in",
$ALL = "$all",
$REGEX = "$regex",
$EQ = "$eq",
$NEQ = "$ne",
$GLOB = "$glob"
}
10 changes: 8 additions & 2 deletions backend/src/server/plugins/error-handler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ForbiddenError } from "@casl/ability";
import { ForbiddenError, PureAbility } from "@casl/ability";
import fastifyPlugin from "fastify-plugin";
import jwt from "jsonwebtoken";
import { ZodError } from "zod";
Expand Down Expand Up @@ -63,7 +63,13 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider
void res.status(HttpStatusCodes.Forbidden).send({
statusCode: HttpStatusCodes.Forbidden,
error: "PermissionDenied",
message: `You are not allowed to ${error.action} on ${error.subjectType} - ${JSON.stringify(error.subject)}`
message: `You are not allowed to ${error.action} on ${error.subjectType}`,
details: (error.ability as PureAbility).rulesFor(error.action as string, error.subjectType).map((el) => ({
action: el.action,
inverted: el.inverted,
subject: el.subject,
conditions: el.conditions
}))
});
} else if (error instanceof ForbiddenRequestError) {
void res.status(HttpStatusCodes.Forbidden).send({
Expand Down
1 change: 1 addition & 0 deletions backend/src/server/routes/sanitizedSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const DefaultResponseErrorsSchema = {
403: z.object({
statusCode: z.literal(403),
message: z.string(),
details: z.any().optional(),
error: z.string()
}),
500: z.object({
Expand Down
16 changes: 12 additions & 4 deletions frontend/src/components/notifications/Notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { Id, toast, ToastContainer, ToastOptions, TypeOptions } from "react-toas
export type TNotification = {
title?: string;
text: ReactNode;
children?: ReactNode;
};

export const NotificationContent = ({ title, text }: TNotification) => {
export const NotificationContent = ({ title, text, children }: TNotification) => {
return (
<div className="msg-container">
{title && <div className="text-md mb-1 font-medium">{title}</div>}
<div className={title ? "text-sm" : "text-md"}>{text}</div>
<div className={title ? "text-sm text-neutral-400" : "text-md"}>{text}</div>
{children && <div className="mt-2">{children}</div>}
</div>
);
};
Expand All @@ -23,7 +25,13 @@ export const createNotification = (
position: "bottom-right",
...toastProps,
theme: "dark",
type: myProps?.type || "info",
type: myProps?.type || "info"
});

export const NotificationContainer = () => <ToastContainer pauseOnHover toastClassName="border border-mineshaft-500" style={{ width: "400px" }} />;
export const NotificationContainer = () => (
<ToastContainer
pauseOnHover
toastClassName="border border-mineshaft-500"
style={{ width: "400px" }}
/>
);
9 changes: 9 additions & 0 deletions frontend/src/context/ProjectPermissionContext/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ export enum PermissionConditionOperators {
$GLOB = "$glob"
}

export const formatedConditionsOperatorNames: { [K in PermissionConditionOperators]: string } = {
[PermissionConditionOperators.$EQ]: "equal to",
[PermissionConditionOperators.$IN]: "contains",
[PermissionConditionOperators.$ALL]: "contains all",
[PermissionConditionOperators.$NEQ]: "not equal to",
[PermissionConditionOperators.$GLOB]: "matches glob pattern",
[PermissionConditionOperators.$REGEX]: "matches regex pattern"
};

export type TPermissionConditionOperators = {
[PermissionConditionOperators.$IN]: string[];
[PermissionConditionOperators.$ALL]: string[];
Expand Down
8 changes: 7 additions & 1 deletion frontend/src/hooks/api/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { PureAbility } from "@casl/ability";
import { ZodIssue } from "zod";

export type { TAccessApprovalPolicy } from "./accessApproval/types";
Expand Down Expand Up @@ -52,9 +53,14 @@ export type TApiErrors =
| {
error: ApiErrorTypes.ValidationError;
message: ZodIssue[];
statusCode: 401;
}
| {
error: ApiErrorTypes.ForbiddenError;
message: string;
details: PureAbility["rules"];
statusCode: 403;
}
| { error: ApiErrorTypes.ForbiddenError; message: string; statusCode: 401 }
| {
statusCode: 400;
message: string;
Expand Down
166 changes: 145 additions & 21 deletions frontend/src/reactQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,42 +3,166 @@ import axios from "axios";

import { createNotification } from "@app/components/notifications";

// akhilmhdh: doing individual imports to avoid cyclic import error
import { Button } from "./components/v2/Button";
import { Modal, ModalContent, ModalTrigger } from "./components/v2/Modal";
import { Table, TableContainer, TBody, Td, Th, THead, Tr } from "./components/v2/Table";
import {
formatedConditionsOperatorNames,
PermissionConditionOperators
} from "./context/ProjectPermissionContext/types";
import { ApiErrorTypes, TApiErrors } from "./hooks/api/types";

// this is saved in react-query cache
export const SIGNUP_TEMP_TOKEN_CACHE_KEY = ["infisical__signup-temp-token"];
export const MFA_TEMP_TOKEN_CACHE_KEY = ["infisical__mfa-temp-token"];
export const AUTH_TOKEN_CACHE_KEY = ["infisical__auth-token"];

const camelCaseToSpaces = (input: string) => {
return input.replace(/([a-z])([A-Z])/g, "$1 $2");
};

export const queryClient = new QueryClient({
mutationCache: new MutationCache({
onError: (error) => {
if (axios.isAxiosError(error)) {
const serverResponse = error.response?.data as TApiErrors;
if (serverResponse?.error === ApiErrorTypes.ValidationError) {
createNotification({
title: "Validation Error",
type: "error",
text: (
<div>
{serverResponse.message?.map(({ message, path }) => (
<div className="flex space-y-2" key={path.join(".")}>
<div>
Field <i>{path.join(".")}</i> {message.toLowerCase()}
</div>
</div>
))}
</div>
)
});
createNotification(
{
title: "Validation Error",
type: "error",
text: "Please check the input and try again.",
children: (
<Modal>
<ModalTrigger>
<Button variant="outline_bg" size="xs">
Show more
</Button>
</ModalTrigger>
<ModalContent title="Validation Error Details">
<TableContainer>
<Table>
<THead>
<Tr>
<Th>Field</Th>
<Th>Issue</Th>
</Tr>
</THead>
<TBody>
{serverResponse.message?.map(({ message, path }) => (
<Tr key={path.join(".")}>
<Td>{path.join(".")}</Td>
<Td>{message.toLowerCase()}</Td>
</Tr>
))}
</TBody>
</Table>
</TableContainer>
</ModalContent>
</Modal>
)
},
{ closeOnClick: false }
);
return;
}
if (serverResponse.statusCode === 401) {
createNotification({
title: "Forbidden Access",
type: "error",
text: serverResponse.message
});
if (serverResponse?.error === ApiErrorTypes.ForbiddenError) {
createNotification(
{
title: "Forbidden Access",
type: "error",
text: serverResponse.message,
children: serverResponse?.details?.length ? (
<Modal>
<ModalTrigger>
<Button variant="outline_bg" size="xs">
Show more
</Button>
</ModalTrigger>
<ModalContent
title="Validation Rules"
subTitle="Please review the allowed rules below."
>
<div className="flex flex-col gap-2">
{serverResponse.details?.map((el, index) => {
const hasConditions = Object.keys(el.conditions || {}).length;
return (
<div
key={`Forbidden-error-details-${index + 1}`}
className="rounded-md border border-gray-600 p-4"
>
<div>
{el.inverted ? "Cannot" : "Can"}{" "}
<span className="text-yellow-600">
{el.action.toString().replaceAll(",", ", ")}
</span>{" "}
{el.subject.toString()} {hasConditions && "with conditions:"}
</div>
{hasConditions && (
<ul className="flex list-disc flex-col gap-1 pl-5 pt-2 text-sm">
{Object.keys(el.conditions || {}).flatMap((field, fieldIndex) => {
const operators = (
el.conditions as Record<
string,
| string
| { [K in PermissionConditionOperators]: string | string[] }
>
)[field];

const formattedFieldName = camelCaseToSpaces(field).toLowerCase();
if (typeof operators === "string") {
return (
<li
key={`Forbidden-error-details-${index + 1}-${
fieldIndex + 1
}`}
>
<span className="font-bold capitalize">
{formattedFieldName}
</span>{" "}
<span className="text-mineshaft-200">equal to</span>{" "}
<span className="text-yellow-600">{operators}</span>
</li>
);
}

return Object.keys(operators).map((operator, operatorIndex) => (
<li
key={`Forbidden-error-details-${index + 1}-${
fieldIndex + 1
}-${operatorIndex + 1}`}
>
<span className="font-bold capitalize">
{formattedFieldName}
</span>{" "}
<span className="text-mineshaft-200">
{
formatedConditionsOperatorNames[
operator as PermissionConditionOperators
]
}
</span>{" "}
<span className="text-yellow-600">
{operators[
operator as PermissionConditionOperators
].toString()}
</span>
</li>
));
})}
</ul>
)}
</div>
);
})}
</div>
</ModalContent>
</Modal>
) : undefined
},
{ closeOnClick: false }
);
return;
}
createNotification({ title: "Bad Request", type: "error", text: serverResponse.message });
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ html {
@apply rounded-md;
}

.Toastify__toast-body {
@apply items-start;
}

.Toastify__toast-icon {
@apply w-4 pt-1;
}

.rdp-day,
.rdp-nav_button {
@apply rounded-md hover:text-mineshaft-500;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export const CreateSecretForm = ({
isMulti
name="tagIds"
isDisabled={!canReadTags}
isLoading={isTagsLoading}
isLoading={isTagsLoading && canReadTags}
options={projectTags?.map((el) => ({ label: el.slug, value: el.id }))}
value={field.value}
onChange={field.onChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ export const SecretOverviewPage = () => {
<div className="thin-scrollbar mt-4">
<TableContainer
onScroll={(e) => setScrollOffset(e.currentTarget.scrollLeft)}
className="thin-scrollbar"
className="thin-scrollbar rounded-b-none"
>
<Table>
<THead>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export const CreateSecretForm = ({ secretPath = "/", getSecretByKey, onClose }:
isMulti
name="tagIds"
isDisabled={!canReadTags}
isLoading={isTagsLoading}
isLoading={isTagsLoading && canReadTags}
options={projectTags?.map((el) => ({ label: el.slug, value: el.id }))}
value={field.value}
onChange={field.onChange}
Expand Down
Loading