Skip to content

fix: include a link and more useful error details for 403 response codes #16644

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

Merged
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
1 change: 1 addition & 0 deletions coderd/httpapi/httpapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func ResourceNotFound(rw http.ResponseWriter) {
func Forbidden(rw http.ResponseWriter) {
Write(context.Background(), rw, http.StatusForbidden, codersdk.Response{
Message: "Forbidden.",
Detail: "You don't have permission to view this content. If you believe this is a mistake, please contact your administrator or try signing in with different credentials.",
})
}

Expand Down
2 changes: 1 addition & 1 deletion site/e2e/setup/addUsersAndLicense.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test } from "@playwright/test";
import { API } from "api/api";
import { Language } from "pages/CreateUserPage/CreateUserForm";
import { Language } from "pages/CreateUserPage/Language";
import { coderPort, license, premiumTestsRequired, users } from "../constants";
import { expectUrl } from "../expectUrl";
import { createUser } from "../helpers";
Expand Down
8 changes: 8 additions & 0 deletions site/src/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ export const getErrorDetail = (error: unknown): string | undefined => {
return undefined;
};

export const getErrorStatus = (error: unknown): number | undefined => {
if (isApiError(error)) {
return error.status;
}

return undefined;
};

export class DetailedError extends Error {
constructor(
message: string,
Expand Down
34 changes: 25 additions & 9 deletions site/src/components/Alert/ErrorAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,44 @@
import AlertTitle from "@mui/material/AlertTitle";
import { getErrorDetail, getErrorMessage } from "api/errors";
import { getErrorDetail, getErrorMessage, getErrorStatus } from "api/errors";
import type { FC } from "react";
import { Link } from "../Link/Link";
import { Alert, AlertDetail, type AlertProps } from "./Alert";

export const ErrorAlert: FC<
Omit<AlertProps, "severity" | "children"> & { error: unknown }
> = ({ error, ...alertProps }) => {
const message = getErrorMessage(error, "Something went wrong.");
const detail = getErrorDetail(error);
const status = getErrorStatus(error);

// For some reason, the message and detail can be the same on the BE, but does
// not make sense in the FE to showing them duplicated
const shouldDisplayDetail = message !== detail;

return (
<Alert severity="error" {...alertProps}>
{detail ? (
<>
<AlertTitle>{message}</AlertTitle>
{shouldDisplayDetail && <AlertDetail>{detail}</AlertDetail>}
</>
) : (
message
)}
{
// When the error is a Forbidden response we include a link for the user to
// go back to a known viewable page.
status === 403 ? (
<>
<AlertTitle>{message}</AlertTitle>
<AlertDetail>
{detail}{" "}
<Link href="/workspaces" className="w-fit">
Go to workspaces
</Link>
</AlertDetail>
</>
) : detail ? (
<>
<AlertTitle>{message}</AlertTitle>
{shouldDisplayDetail && <AlertDetail>{detail}</AlertDetail>}
</>
) : (
message
)
}
</Alert>
);
};
13 changes: 1 addition & 12 deletions site/src/pages/CreateUserPage/CreateUserForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,7 @@ import {
onChangeTrimmed,
} from "utils/formUtils";
import * as Yup from "yup";

export const Language = {
emailLabel: "Email",
passwordLabel: "Password",
usernameLabel: "Username",
nameLabel: "Full name",
emailInvalid: "Please enter a valid email address.",
emailRequired: "Please enter an email address.",
passwordRequired: "Please enter a password.",
createUser: "Create",
cancel: "Cancel",
};
import { Language } from "./Language";

export const authMethodLanguage = {
password: {
Expand Down
2 changes: 1 addition & 1 deletion site/src/pages/CreateUserPage/CreateUserPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {
renderWithAuth,
waitForLoaderToBeRemoved,
} from "testHelpers/renderHelpers";
import { Language as FormLanguage } from "./CreateUserForm";
import { CreateUserPage } from "./CreateUserPage";
import { Language as FormLanguage } from "./Language";

const renderCreateUserPage = async () => {
renderWithAuth(<CreateUserPage />, {
Expand Down
11 changes: 11 additions & 0 deletions site/src/pages/CreateUserPage/Language.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export const Language = {
emailLabel: "Email",
passwordLabel: "Password",
usernameLabel: "Username",
nameLabel: "Full name",
emailInvalid: "Please enter a valid email address.",
emailRequired: "Please enter an email address.",
passwordRequired: "Please enter a password.",
createUser: "Create",
cancel: "Cancel",
};
Loading