Skip to content

Commit

Permalink
fix: Redirect to '?redirect' query parameter after successful login (#…
Browse files Browse the repository at this point in the history
…307)

Fixes #304 
Unblocks #298 

After logging in, the login flow should redirect to a whatever path is specified by the `?redirect` query parameter. This is important for cases like #298 - where we need to set `?redirect=%2Fcli_auth`, but also really any case where the user is linked and might have to go back to the login screen.

The fix is simple - just check if the `redirect` query parameter is set, and if it is, use that as the path to redirect to on success. Also adds a test case - we had one checking that we redirect to the default (root `/`) url, but not one of the `?redirect` param
  • Loading branch information
bryphe-coder committed Feb 17, 2022
1 parent d436993 commit c2ad91b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
24 changes: 23 additions & 1 deletion site/components/SignIn/SignInForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,29 @@ describe("SignInForm", () => {
act(() => elem.click())

// Then
// Should redirect because login was successfully
// Should redirect because login was successful
await waitFor(() => expect(singletonRouter).toMatchObject({ asPath: "/" }))
})

it("respects ?redirect query parameter when complete", async () => {
// Given
const loginHandler = (_email: string, _password: string) => Promise.resolve()
// Set a path to redirect to after login is successful
mockRouter.setCurrentUrl("/login?redirect=%2Fsome%2Fother%2Fpath")

// When
// Render the component
const { container } = render(<SignInForm loginHandler={loginHandler} />)
// Set user / password
const inputs = container.querySelectorAll("input")
fireEvent.change(inputs[0], { target: { value: "test@coder.com" } })
fireEvent.change(inputs[1], { target: { value: "password" } })
// Click sign-in
const elem = await screen.findByText("Sign In")
act(() => elem.click())

// Then
// Should redirect to /some/other/path because ?redirect was specified and login was successful
await waitFor(() => expect(singletonRouter).toMatchObject({ asPath: "/some/other/path" }))
})
})
16 changes: 14 additions & 2 deletions site/components/SignIn/SignInForm.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { makeStyles } from "@material-ui/core/styles"
import { FormikContextType, useFormik } from "formik"
import { useRouter } from "next/router"
import { NextRouter, useRouter } from "next/router"
import React from "react"
import { useSWRConfig } from "swr"
import * as Yup from "yup"
Expand All @@ -9,6 +9,7 @@ import { Welcome } from "./Welcome"
import { FormTextField } from "../Form"
import * as API from "./../../api"
import { LoadingButton } from "./../Button"
import { firstOrItem } from "../../util/array"

/**
* BuiltInAuthFormValues describes a form using built-in (email/password)
Expand Down Expand Up @@ -61,7 +62,9 @@ export const SignInForm: React.FC<SignInProps> = ({
await loginHandler(email, password)
// Tell SWR to invalidate the cache for the user endpoint
await mutate("/api/v2/users/me")
await router.push("/")

const redirect = getRedirectFromRouter(router)
await router.push(redirect)
} catch (err) {
helpers.setFieldError("password", "The username or password is incorrect.")
}
Expand Down Expand Up @@ -117,3 +120,12 @@ export const SignInForm: React.FC<SignInProps> = ({
</>
)
}

const getRedirectFromRouter = (router: NextRouter) => {
const defaultRedirect = "/"
if (router.query?.redirect) {
return firstOrItem(router.query.redirect, defaultRedirect)
} else {
return defaultRedirect
}
}

0 comments on commit c2ad91b

Please sign in to comment.