Skip to content

Commit

Permalink
feat: /account/login?redirect_to=/final/url should redirect to /final…
Browse files Browse the repository at this point in the history
…/url after authn (#1950)

Motivation:
* Current if you're logged out, then try to access a page that requires
auth like /account/payments, you'll be redirected through the
authentication process and end up at /account (not the destination you
originally wanted, always /account)

What this does
* /account/login?redirect_to=/final/url should redirect to /final/url
after authn
* /account/payment page getStaticProps redirectTo redirects to
`/account/login?redirect_uri=/account/payment` so the /pricing page can
link to /account/payment
  • Loading branch information
gobengo committed Sep 29, 2022
1 parent 5bd350d commit 2bbe6de
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 41 deletions.
10 changes: 8 additions & 2 deletions packages/website/lib/magic.js
Expand Up @@ -49,11 +49,17 @@ export async function isLoggedIn() {
* Login with email
*
* @param {string} email
* @param {string} [finalRedirectHref] - href of final url that end-user should
* be redirected to after successful authentication
*/
export async function loginEmail(email) {
export async function loginEmail(email, finalRedirectHref) {
const redirectUri = new URL('/callback/', window.location.origin);
if (finalRedirectHref) {
redirectUri.searchParams.set('redirect_uri', finalRedirectHref);
}
const didToken = await getMagic().auth.loginWithMagicLink({
email: email,
redirectURI: new URL('/callback/', window.location.origin).href,
redirectURI: redirectUri.href,
});

if (didToken) {
Expand Down
2 changes: 1 addition & 1 deletion packages/website/pages/account/payment.js
Expand Up @@ -193,7 +193,7 @@ export function getStaticProps() {
props: {
title: 'Payment',
isRestricted: true,
redirectTo: '/login/',
redirectTo: '/login/?redirect_uri=/account/payment',
stripePublishableKey,
},
};
Expand Down
15 changes: 9 additions & 6 deletions packages/website/pages/callback/index.js
Expand Up @@ -2,8 +2,8 @@ import { useEffect } from 'react';
import { useRouter } from 'next/router';
import { useQueryClient } from 'react-query';

import Loading from 'components/loading/loading';
import { redirectMagic, redirectSocial } from 'lib/magic.js';
import { redirectMagic, redirectSocial } from '../../lib/magic.js';
import Loading from '../../components/loading/loading.js';

export function getStaticProps() {
return {
Expand All @@ -16,13 +16,15 @@ export function getStaticProps() {
const Callback = () => {
const router = useRouter();
const queryClient = useQueryClient();

const redirectUriQuery = router.query.redirect_uri;
const redirectUri =
(redirectUriQuery && Array.isArray(redirectUriQuery) ? redirectUriQuery[0] : redirectUriQuery) ?? '/account';
useEffect(() => {
const finishSocialLogin = async () => {
try {
await redirectSocial();
await queryClient.invalidateQueries('magic-user');
router.push('/account');
router.push(redirectUri);
} catch (err) {
console.error(err);
await queryClient.invalidateQueries('magic-user');
Expand All @@ -33,7 +35,8 @@ const Callback = () => {
try {
await redirectMagic();
await queryClient.invalidateQueries('magic-user');
router.push('/account');

router.push(redirectUri);
} catch (err) {
console.error(err);
await queryClient.invalidateQueries('magic-user');
Expand All @@ -46,7 +49,7 @@ const Callback = () => {
if (router.query.provider) {
finishSocialLogin();
}
}, [router, router.query, queryClient]);
}, [router, router.query, queryClient, redirectUri]);

// TODO handle errors
return (
Expand Down
21 changes: 17 additions & 4 deletions packages/website/pages/login/index.js
Expand Up @@ -14,12 +14,24 @@ const LoginType = {
EMAIL: 'email',
};

/**
* Get the first thing
* @template T
* @param {T | T[] | undefined} thingOrThings
*/
function first(thingOrThings) {
if (Array.isArray(thingOrThings)) {
return thingOrThings[0];
}
return thingOrThings;
}

const Login = () => {
// App query client
const queryClient = useQueryClient();

// App wide methods
const { push } = useRouter();
const { push, query } = useRouter();

// Error states
const [errors, setErrors] = useState(/** @type {{email?: string}} */ ({}));
Expand All @@ -36,10 +48,11 @@ const Login = () => {
const onLoginWithEmail = useCallback(async () => {
setErrors({ email: undefined });
setIsLoggingIn(LoginType.EMAIL);
const finalRedirectUri = first(query.redirect_uri) ?? '/account';
try {
await loginEmail(email || '');
await loginEmail(email || '', finalRedirectUri);
await queryClient.invalidateQueries('magic-user');
push('/account');
push(finalRedirectUri);
} catch (error) {
setIsLoggingIn('');

Expand All @@ -48,7 +61,7 @@ const Login = () => {
// @ts-ignore Catch clause variable type annotation must be 'any' or 'unknown' if specified.ts(1196)
setErrors({ email: error.message });
}
}, [email, push, queryClient]);
}, [email, push, queryClient, query.redirect_uri]);

// Callback for github login logic
const onGithubLogin = useCallback(async () => {
Expand Down
56 changes: 28 additions & 28 deletions packages/website/tests/accountPayment.e2e.spec.ts
Expand Up @@ -10,14 +10,21 @@ test.beforeEach(async ({ page }) => {
});

test.describe('/account/payment', () => {
test('redirects to /login/ when not authenticated', async ({ page }, testInfo) => {
await Promise.all([
page.waitForNavigation({
waitUntil: 'networkidle',
}),
page.goto('/account/payment'),
]);
await expect(page).toHaveURL('/login/');
test('redirects through login and back when not initially authenticated', async ({ page }, testInfo) => {
const accountPaymentPathname = '/account/payment';
// try to go to page that requires authn
await page.goto(accountPaymentPathname, { waitUntil: 'networkidle' });
// wait for redirect to a Log in page
await page.locator('text=Log in').waitFor();
const pageUrl = new URL(page.url());
expect(pageUrl.pathname).toEqual('/login/');
expect(pageUrl.searchParams.get('redirect_uri')).toEqual(accountPaymentPathname);
// fill login
await LoginTester().login(page, {
email: MAGIC_SUCCESS_EMAIL,
});
// should be back at our initial target destination
expect(new URL(page.url()).pathname).toEqual(accountPaymentPathname + '/');
await page.screenshot({
fullPage: true,
path: await E2EScreenshotPath(testInfo, `accountPayment-noauth`),
Expand All @@ -27,35 +34,33 @@ test.describe('/account/payment', () => {
page.on('pageerror', err => {
console.error('pageerror', err);
});
const goToPayment = (page: Page) =>
Promise.all([
page.waitForNavigation({
waitUntil: 'networkidle',
}),
page.goto('/account/payment'),
]);
await goToPayment(page);
await LoginTester().login(page, {
const tester = LoginTester();
await page.goto(tester.loginHref);
await tester.login(page, {
email: MAGIC_SUCCESS_EMAIL,
});
// @todo - this should redirect you back to where you wanted to go: /account/payment
await expect(page).toHaveURL('/account/');
await goToPayment(page);
await page.goto('/account/payment/');
await expect(page).toHaveURL('/account/payment/');
await page.screenshot({
fullPage: true,
path: await E2EScreenshotPath(testInfo, `accountPayment`),
});
});
test('can enter credit card details', async ({ page }) => {
await LoginTester().login(page, { email: MAGIC_SUCCESS_EMAIL });
const tester = LoginTester();
await page.goto(tester.loginHref);
await tester.login(page, { email: MAGIC_SUCCESS_EMAIL });
await page.goto(AccountPaymentTester().url);
await AccountPaymentTester().fillCreditCardDetails(page);
await AccountPaymentTester().clickAddCardButton(page);
});
});

function LoginTester() {
function LoginTester({
loginHref = '/login/',
}: {
loginHref?: string;
} = {}) {
async function fillLoginForm(page: Page, email: string) {
await page.locator('.login-email').fill(email);
}
Expand All @@ -67,20 +72,15 @@ function LoginTester() {
async function login(
page: Page,
{
url = '/login/',
email,
}: {
url?: string;
email: string;
}
) {
if (new URL(page.url()).pathname !== url) {
await page.goto(url);
}
await fillLoginForm(page, email);
await submitLoginForm(page);
}
return { login, submitLoginForm };
return { login, loginHref, submitLoginForm };
}

function AccountPaymentTester() {
Expand Down

0 comments on commit 2bbe6de

Please sign in to comment.