fix(client): remove extra slash when redirecting to client (#42588)
Co-authored-by: Nicholas Carrigan (he/him) <nhcarrigan@gmail.com>
This commit is contained in:
committed by
GitHub
parent
e2ca65c803
commit
92d7ae1725
@ -17,8 +17,8 @@ import { fixCompletedChallengeItem } from '../../common/utils';
|
|||||||
import { getChallenges } from '../utils/get-curriculum';
|
import { getChallenges } from '../utils/get-curriculum';
|
||||||
import {
|
import {
|
||||||
getRedirectParams,
|
getRedirectParams,
|
||||||
getRedirectBase,
|
normalizeParams,
|
||||||
normalizeParams
|
getPrefixedLandingPath
|
||||||
} from '../utils/redirection';
|
} from '../utils/redirection';
|
||||||
|
|
||||||
const log = debug('fcc:boot:challenges');
|
const log = debug('fcc:boot:challenges');
|
||||||
@ -341,7 +341,7 @@ export function createRedirectToCurrentChallenge(
|
|||||||
const { user } = req;
|
const { user } = req;
|
||||||
const { origin, pathPrefix } = getRedirectParams(req, normalizeParams);
|
const { origin, pathPrefix } = getRedirectParams(req, normalizeParams);
|
||||||
|
|
||||||
const redirectBase = getRedirectBase(origin, pathPrefix);
|
const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
|
||||||
if (!user) {
|
if (!user) {
|
||||||
return res.redirect(redirectBase + '/learn');
|
return res.redirect(redirectBase + '/learn');
|
||||||
}
|
}
|
||||||
|
@ -11,9 +11,9 @@ import passportProviders from './passport-providers';
|
|||||||
import { setAccessTokenToResponse } from './utils/getSetAccessToken';
|
import { setAccessTokenToResponse } from './utils/getSetAccessToken';
|
||||||
import {
|
import {
|
||||||
getReturnTo,
|
getReturnTo,
|
||||||
getRedirectBase,
|
getPrefixedLandingPath,
|
||||||
getRedirectParams,
|
getRedirectParams,
|
||||||
isRootPath
|
haveSamePath
|
||||||
} from './utils/redirection';
|
} from './utils/redirection';
|
||||||
import { jwtSecret } from '../../../config/secrets';
|
import { jwtSecret } from '../../../config/secrets';
|
||||||
import { availableLangs } from '../../../config/i18n/all-langs';
|
import { availableLangs } from '../../../config/i18n/all-langs';
|
||||||
@ -100,9 +100,14 @@ export const devLoginRedirect = () => {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
returnTo += isRootPath(getRedirectBase(origin, pathPrefix), returnTo)
|
|
||||||
? '/learn'
|
// if returnTo has a trailing slash, we need to remove it before comparing
|
||||||
: '';
|
// it to the prefixed landing path
|
||||||
|
if (returnTo.slice(-1) === '/') {
|
||||||
|
returnTo = returnTo.slice(0, -1);
|
||||||
|
}
|
||||||
|
const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
|
||||||
|
returnTo += haveSamePath(redirectBase, returnTo) ? '/learn' : '';
|
||||||
return res.redirect(returnTo);
|
return res.redirect(returnTo);
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
@ -142,7 +147,7 @@ we recommend using your email address: ${user.email} to sign in instead.
|
|||||||
const state = req && req.query && req.query.state;
|
const state = req && req.query && req.query.state;
|
||||||
// returnTo, origin and pathPrefix are audited by getReturnTo
|
// returnTo, origin and pathPrefix are audited by getReturnTo
|
||||||
let { returnTo, origin, pathPrefix } = getReturnTo(state, jwtSecret);
|
let { returnTo, origin, pathPrefix } = getReturnTo(state, jwtSecret);
|
||||||
const redirectBase = getRedirectBase(origin, pathPrefix);
|
const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
|
||||||
|
|
||||||
// TODO: getReturnTo could return a success flag to show a flash message,
|
// TODO: getReturnTo could return a success flag to show a flash message,
|
||||||
// but currently it immediately gets overwritten by a second message. We
|
// but currently it immediately gets overwritten by a second message. We
|
||||||
@ -150,7 +155,12 @@ we recommend using your email address: ${user.email} to sign in instead.
|
|||||||
// multiple messages to appear at once.
|
// multiple messages to appear at once.
|
||||||
|
|
||||||
if (user.acceptedPrivacyTerms) {
|
if (user.acceptedPrivacyTerms) {
|
||||||
returnTo += isRootPath(redirectBase, returnTo) ? '/learn' : '';
|
// if returnTo has a trailing slash, we need to remove it before comparing
|
||||||
|
// it to the prefixed landing path
|
||||||
|
if (returnTo.slice(-1) === '/') {
|
||||||
|
returnTo = returnTo.slice(0, -1);
|
||||||
|
}
|
||||||
|
returnTo += haveSamePath(redirectBase, returnTo) ? '/learn' : '';
|
||||||
return res.redirectWithFlash(returnTo);
|
return res.redirectWithFlash(returnTo);
|
||||||
} else {
|
} else {
|
||||||
return res.redirectWithFlash(`${redirectBase}/email-sign-up`);
|
return res.redirectWithFlash(`${redirectBase}/email-sign-up`);
|
||||||
|
@ -50,10 +50,7 @@ function normalizeParams(
|
|||||||
return { returnTo, origin, pathPrefix };
|
return { returnTo, origin, pathPrefix };
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO: tests!
|
function getPrefixedLandingPath(origin, pathPrefix) {
|
||||||
// TODO: ensure origin and pathPrefix validation happens first
|
|
||||||
// (it needs a dedicated function that can be called from here and getReturnTo)
|
|
||||||
function getRedirectBase(origin, pathPrefix) {
|
|
||||||
const redirectPathSegment = pathPrefix ? `/${pathPrefix}` : '';
|
const redirectPathSegment = pathPrefix ? `/${pathPrefix}` : '';
|
||||||
return `${origin}${redirectPathSegment}`;
|
return `${origin}${redirectPathSegment}`;
|
||||||
}
|
}
|
||||||
@ -70,14 +67,14 @@ function getRedirectParams(req, _normalizeParams = normalizeParams) {
|
|||||||
return _normalizeParams({ returnTo: returnUrl.href, origin, pathPrefix });
|
return _normalizeParams({ returnTo: returnUrl.href, origin, pathPrefix });
|
||||||
}
|
}
|
||||||
|
|
||||||
function isRootPath(redirectBase, returnUrl) {
|
function haveSamePath(redirectBase, returnUrl) {
|
||||||
const base = new URL(redirectBase);
|
const base = new URL(redirectBase);
|
||||||
const url = new URL(returnUrl);
|
const url = new URL(returnUrl);
|
||||||
return base.pathname === url.pathname;
|
return base.pathname === url.pathname;
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports.getReturnTo = getReturnTo;
|
module.exports.getReturnTo = getReturnTo;
|
||||||
module.exports.getRedirectBase = getRedirectBase;
|
module.exports.getPrefixedLandingPath = getPrefixedLandingPath;
|
||||||
module.exports.normalizeParams = normalizeParams;
|
module.exports.normalizeParams = normalizeParams;
|
||||||
module.exports.getRedirectParams = getRedirectParams;
|
module.exports.getRedirectParams = getRedirectParams;
|
||||||
module.exports.isRootPath = isRootPath;
|
module.exports.haveSamePath = haveSamePath;
|
||||||
|
@ -36,6 +36,7 @@
|
|||||||
Cypress.Commands.add('login', () => {
|
Cypress.Commands.add('login', () => {
|
||||||
cy.visit('/');
|
cy.visit('/');
|
||||||
cy.contains("Get started (it's free)").click();
|
cy.contains("Get started (it's free)").click();
|
||||||
|
cy.url().should('eq', Cypress.config().baseUrl + '/learn/');
|
||||||
cy.contains('Welcome back');
|
cy.contains('Welcome back');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user