Compare commits
2 Commits
dependabot
...
fix/login-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fd85704489 | ||
|
|
a0138531ec |
@@ -121,6 +121,82 @@ describe("LoginPage", () => {
|
||||
await screen.findByText("Setup");
|
||||
});
|
||||
|
||||
it("performs a hard reload after successful password login", async () => {
|
||||
// Given - user is NOT signed in
|
||||
let loggedIn = false;
|
||||
server.use(
|
||||
http.get("/api/v2/users/me", () => {
|
||||
if (!loggedIn) {
|
||||
return HttpResponse.json(
|
||||
{ message: "no user here" },
|
||||
{ status: 401 },
|
||||
);
|
||||
}
|
||||
return HttpResponse.json(MockUserOwner);
|
||||
}),
|
||||
http.post("/api/v2/users/login", () => {
|
||||
loggedIn = true;
|
||||
return HttpResponse.json({
|
||||
session_token: "test-session-token",
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
// Spy on window.location.href
|
||||
const originalLocation = window.location;
|
||||
const locationHrefSpy = vi.fn();
|
||||
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: {
|
||||
...originalLocation,
|
||||
origin: originalLocation.origin,
|
||||
set href(url: string) {
|
||||
locationHrefSpy(url);
|
||||
},
|
||||
get href() {
|
||||
return originalLocation.href;
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// When
|
||||
renderWithRouter(
|
||||
createMemoryRouter(
|
||||
[
|
||||
{
|
||||
path: "/login",
|
||||
element: <LoginPage />,
|
||||
},
|
||||
],
|
||||
{ initialEntries: ["/login"] },
|
||||
),
|
||||
);
|
||||
|
||||
await waitForLoaderToBeRemoved();
|
||||
|
||||
const email = screen.getByLabelText(/Email/);
|
||||
const password = screen.getByLabelText(/Password/);
|
||||
|
||||
await userEvent.type(email, "test@coder.com");
|
||||
await userEvent.type(password, "password");
|
||||
|
||||
const signInButton = await screen.findByText("Sign In");
|
||||
fireEvent.click(signInButton);
|
||||
|
||||
// Then - it should hard reload to "/" so the server re-renders
|
||||
// HTML with fresh metadata (userAppearance, etc.).
|
||||
await waitFor(() => {
|
||||
expect(locationHrefSpy).toHaveBeenCalledWith("/");
|
||||
});
|
||||
|
||||
// Cleanup
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: originalLocation,
|
||||
});
|
||||
});
|
||||
|
||||
it("redirects to /oauth2/authorize via server-side redirect when signed in", async () => {
|
||||
// Given - user is signed in
|
||||
server.use(
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
import { type FC, useEffect } from "react";
|
||||
import { useQuery } from "react-query";
|
||||
import { Navigate, useLocation, useNavigate } from "react-router";
|
||||
import { Navigate, useLocation } from "react-router";
|
||||
import { buildInfo } from "#/api/queries/buildInfo";
|
||||
import { authMethods } from "#/api/queries/users";
|
||||
import { useAuthContext } from "#/contexts/auth/AuthProvider";
|
||||
import { useEmbeddedMetadata } from "#/hooks/useEmbeddedMetadata";
|
||||
import { getApplicationName } from "#/utils/appearance";
|
||||
import { retrieveRedirect } from "#/utils/redirect";
|
||||
import { retrieveRedirect, sanitizeRedirect } from "#/utils/redirect";
|
||||
import { sendDeploymentEvent } from "#/utils/telemetry";
|
||||
import { LoginPageView } from "./LoginPageView";
|
||||
|
||||
@@ -24,7 +24,6 @@ const LoginPage: FC = () => {
|
||||
const authMethodsQuery = useQuery(authMethods());
|
||||
const redirectTo = retrieveRedirect(location.search);
|
||||
const applicationName = getApplicationName();
|
||||
const navigate = useNavigate();
|
||||
const { metadata } = useEmbeddedMetadata();
|
||||
const buildInfoQuery = useQuery(buildInfo(metadata["build-info"]));
|
||||
let redirectError: Error | null = null;
|
||||
@@ -57,8 +56,7 @@ const LoginPage: FC = () => {
|
||||
// use `<Navigate>`, react would handle the redirect itself and never
|
||||
// request the page from the backend.
|
||||
if (isApiRouteRedirect) {
|
||||
const sanitizedUrl = new URL(redirectTo, window.location.origin);
|
||||
window.location.href = sanitizedUrl.pathname + sanitizedUrl.search;
|
||||
window.location.href = sanitizeRedirect(redirectTo);
|
||||
// Setting the href should immediately request a new page. Show an
|
||||
// error state if it doesn't.
|
||||
redirectError = new Error("unable to redirect");
|
||||
@@ -87,7 +85,11 @@ const LoginPage: FC = () => {
|
||||
isSigningIn={isSigningIn}
|
||||
onSignIn={async ({ email, password }) => {
|
||||
await signIn(email, password);
|
||||
navigate("/");
|
||||
// Use a hard reload instead of React Router navigation
|
||||
// so the server re-renders the HTML with all metadata
|
||||
// tags populated (userAppearance, user, permissions,
|
||||
// etc.) using the new session cookie.
|
||||
window.location.href = sanitizeRedirect(redirectTo);
|
||||
}}
|
||||
redirectTo={redirectTo}
|
||||
/>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { embedRedirect, retrieveRedirect } from "./redirect";
|
||||
import { embedRedirect, retrieveRedirect, sanitizeRedirect } from "./redirect";
|
||||
|
||||
describe("redirect helper functions", () => {
|
||||
describe("embedRedirect", () => {
|
||||
@@ -17,4 +17,20 @@ describe("redirect helper functions", () => {
|
||||
expect(result).toEqual("/workspaces");
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeRedirect", () => {
|
||||
it("is a no-op for a relative path", () => {
|
||||
expect(sanitizeRedirect("/bar/baz")).toEqual("/bar/baz");
|
||||
});
|
||||
it("removes the origin from url", () => {
|
||||
expect(sanitizeRedirect("http://www.evil.com/bar/baz")).toEqual(
|
||||
"/bar/baz",
|
||||
);
|
||||
});
|
||||
it("preserves search params", () => {
|
||||
expect(
|
||||
sanitizeRedirect("https://www.example.com/bar?baz=1&quux=2"),
|
||||
).toEqual("/bar?baz=1&quux=2");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,3 +21,11 @@ export const retrieveRedirect = (search: string): string => {
|
||||
const redirect = searchParams.get("redirect");
|
||||
return redirect ? redirect : defaultRedirect;
|
||||
};
|
||||
|
||||
/**
|
||||
* Ensures the redirect is not an open redirect, aka it's relative
|
||||
*/
|
||||
export const sanitizeRedirect = (redirectTo: string) => {
|
||||
const sanitizedUrl = new URL(redirectTo, window.location.origin);
|
||||
return sanitizedUrl.pathname + sanitizedUrl.search;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user