Skip to content

Commit

Permalink
Bug/asub 8456 reset password login redirect issue (#2138)
Browse files Browse the repository at this point in the history
* add redirect to reset password

* add test for reset password redirect

* refactor code

* added back in logic for rendering reset password

* fix test

* add removing of nonce if reset is submitted and successful

* add list for whitelisted items

* add tests for validating redirect url

* updated validateUrl to return absolute path

* add code to pagebuilder redirect to last page logic
  • Loading branch information
accbjt authored Jun 20, 2024
1 parent 063d668 commit 9ee574e
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 27 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
Atomics: "readonly",
SharedArrayBuffer: "readonly",
Fusion: "readonly",
history: "readonly"
},
ignorePatterns: [
"**/features/ad-taboola/default.jsx",
Expand Down
33 changes: 21 additions & 12 deletions blocks/identity-block/components/login/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,31 @@ const useLogin = ({

const { Identity } = useIdentity();
const validatedRedirectURL = validateURL(redirectURL);
const [redirectToURL, setRedirectToURL] = useState(validatedRedirectURL);
const [currentRedirectToURL, setCurrentRedirectToURL] = useState(validatedRedirectURL);
const [redirectQueryParam, setRedirectQueryParam] = useState(null);
const [isAppleAuthSuccess, setIsAppleAuthSuccess] = useState(false);
const { loginByOIDC } = useOIDCLogin();

useEffect(()=>{

useEffect(() => {
const askForloginWithApple = async (code) => {
await Identity.appleSignOn(code);
const isLoggedIn = await Identity.isLoggedIn();
if(isLoggedIn){

if (isLoggedIn) {
setIsAppleAuthSuccess(true);
}
};

if(Identity && appleCode){
if (Identity && appleCode) {
askForloginWithApple(appleCode);
}
},[appleCode, Identity]);
}, [appleCode, Identity]);

useEffect(() => {
if (window?.location?.search) {
const searchParams = new URLSearchParams(window.location.search.substring(1));
const searchParams = new URLSearchParams(window.location.search.substring(1));

if (window?.location?.search) {
// redirectURL could have additional params
const params = ["paymentMethodID"];
const aditionalParams = params.filter((p) => {
Expand All @@ -53,11 +54,19 @@ const useLogin = ({
}

if (redirectToPreviousPage && document?.referrer) {
const redirectUrl = new URL(document.referrer);
const redirectUrlLocation = new URL(document.referrer);

setRedirectToURL(`${redirectUrl.pathname}${redirectUrl.search}`);
if (searchParams.has('reset_password')) {
setCurrentRedirectToURL(`${redirectURL}${redirectUrlLocation.search}`);
} else {
const newRedirectUrl = redirectUrlLocation.pathname.includes('/pagebuilder/')
? redirectURL
: `${redirectUrlLocation.pathname}${redirectUrlLocation.search}`;

setCurrentRedirectToURL(newRedirectUrl);
}
}
}, [redirectQueryParam, redirectToPreviousPage]);
}, [redirectQueryParam, redirectToPreviousPage, redirectURL]);

useEffect(() => {
const getConfig = async () => {
Expand Down Expand Up @@ -89,7 +98,7 @@ const useLogin = ({
}, [Identity, redirectQueryParam, loggedInPageLocation, isAdmin, loginByOIDC, isOIDC, isAppleAuthSuccess]);

return {
loginRedirect: redirectQueryParam || redirectToURL,
loginRedirect: redirectQueryParam || currentRedirectToURL,
};
};

Expand Down
56 changes: 52 additions & 4 deletions blocks/identity-block/components/login/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ jest.mock("@wpmedia/arc-themes-components");
const defaultParams = {
isAdmin: false,
redirectURL: "/account/",
resetPasswordURL: "/reset-password/",
redirectToPreviousPage: true,
loggedInPageLocation: "/account-2/",
};
Expand Down Expand Up @@ -36,8 +37,10 @@ describe("useLogin()", () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
origin: 'http://localhost',
href: 'http://localhost',
search: ''
search: '',
pathname: '/',
}
});
useIdentity.mockImplementation(() => ({
Expand All @@ -56,19 +59,21 @@ describe("useLogin()", () => {
it("uses the passed in redirect URL", async () => {
await render(<Test />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe(defaultParams.redirectURL);
expect(window.location).toBe(`http://localhost${defaultParams.redirectURL}`);
});

it("uses redirect query", async () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
search: "?test=123&redirect=/new-account/",
origin: "http://localhost",
pathname: "/",
},
});
await render(<Test />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/new-account/");
expect(window.location).toBe("http://localhost/new-account/");
});

it("uses document referrer", async () => {
Expand All @@ -83,6 +88,27 @@ describe("useLogin()", () => {
delete document.referrer;
});

it("uses redirectURL when referrer is the reset password page", async () => {
const referrerURL = "http://localhost/reset-password/";
Object.defineProperty(document, "referrer", {
value: referrerURL,
configurable: true,
});
Object.defineProperty(window, "location", {
writable: true,
value: {
origin: 'http://localhost',
href: 'http://localhost',
search: '?reset_password=true',
pathname: '/',
}
});
await render(<Test />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/account/");
delete document.referrer;
});

it("redirects when already logged in", async () => {
useIdentity.mockImplementation(() => ({
isInitialized: true,
Expand All @@ -92,14 +118,15 @@ describe("useLogin()", () => {
},
}));
await render(<Test />);
expect(window.location).toBe(defaultParams.loggedInPageLocation);
expect(window.location).toBe(`http://localhost${defaultParams.loggedInPageLocation}`);
});

it("replaces potentially unsafe URLs in query param", async () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
search: "?test=123&redirect=https://somewhere.com",
pathname: "/",
},
});
await render(<Test />);
Expand All @@ -124,4 +151,25 @@ describe("useLogin()", () => {
await render(<Test loggedInPageLocation="https://somewhere.com" />);
expect(window.location).toBe("/");
});

it("should use redirectUrl from customFields if coming from /pagebulder/", async () => {
const referrerURL = "http://localhost/pagebuilder/";
Object.defineProperty(document, "referrer", {
value: referrerURL,
configurable: true,
});
Object.defineProperty(window, "location", {
writable: true,
value: {
origin: 'http://localhost',
href: 'http://localhost',
search: '',
pathname: '/'
}
});
await render(<Test />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/account/");
delete document.referrer;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function useSocialSignIn(redirectURL, isOIDC, socialSignOnIn, onError = () => {}
}
});
}
}, [config.googleClientId, Identity, isGoogleLoaded, isOIDC, loginByOIDC, redirectURL, socialSignOnIn ]);
}, [config.googleClientId, Identity, isGoogleLoaded, isOIDC, loginByOIDC, redirectURL, socialSignOnIn]);

useEffect(() => {
const initializeFacebook = async () => {
Expand Down
18 changes: 14 additions & 4 deletions blocks/identity-block/features/login/default.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,21 @@ export function definedMessageByCode(code) {
}

const Login = ({ customFields }) => {
const { redirectURL, redirectToPreviousPage, loggedInPageLocation, OIDC, termsAndPrivacyURL } =
customFields;
const {
redirectURL,
redirectToPreviousPage,
loggedInPageLocation,
OIDC,
termsAndPrivacyURL,
} = customFields;

const urlString = window.location.href;
const url = new URL(urlString);
let urlString = '';
let url = '';

if (window?.location?.href) {
urlString = window.location.href;
url = new URL(urlString);
}

const { isAdmin, arcSite } = useFusionContext();
const { locale } = getProperties(arcSite);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ describe("Identity One Time Password Request Form - Arc Block", () => {
delete window.location;
window.location = {
href: "http://localhost/onetimeaccess/?ota_nonce=123",
pathname: '/onetimeaccess/',
origin: "http://localhost",
};

useIdentity.mockImplementation(() => ({
Expand All @@ -147,7 +149,7 @@ describe("Identity One Time Password Request Form - Arc Block", () => {
render(<OneTimePassword customFields={customFields} />);
expect(window.location.href).toBe('http://localhost/onetimeaccess/?ota_nonce=123');

await waitFor(() => expect(window.location.href).toBe(`${customFields.loggedInPageLocation}`));
await waitFor(() => expect(window.location.href).toBe(`http://localhost${customFields.loggedInPageLocation}`));
});

it("Should fail when trying to redeem the nonce", async () => {
Expand Down
30 changes: 27 additions & 3 deletions blocks/identity-block/features/reset-password/default.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ export const ResetPasswordPresentation = ({ isAdmin = false, phrases, successAct

// eslint doesn't handle globalThis yet, but this is appropriate
/* global globalThis */
const nonce = new URLSearchParams(globalThis.location.search).get("nonce");

let nonce = '';

if (globalThis?.location?.search) {
nonce = new URLSearchParams(globalThis.location.search).get("nonce");
}

useEffect(() => {
const getConfig = async () => {
Expand All @@ -50,6 +55,26 @@ export const ResetPasswordPresentation = ({ isAdmin = false, phrases, successAct
}
}, [Identity]);

useEffect(() => {
if (submitted) {
const url = new URL(window.location.href);

url.searchParams.delete('nonce');

window.history.replaceState(window.history.state, '', url.href);
}
}, [submitted])

const getRedirectUrl = () => {
const redirect = validateURL(successActionURL);

if (redirect.includes('?')) {
return `${redirect}&reset_password=true`
}

return `${redirect}?reset_password=true`;
}

const {
pwLowercase = 0,
pwMinLength = 0,
Expand Down Expand Up @@ -102,8 +127,7 @@ export const ResetPasswordPresentation = ({ isAdmin = false, phrases, successAct
headline={phrases.t("identity-block.reset-password-headline-submitted")}
buttonLabel={phrases.t("identity-block.reset-password-submit-submitted")}
onSubmit={() => {
const redirect = validateURL(successActionURL);
window.location.assign(redirect);
window.location.assign(getRedirectUrl());
}}
>
<Paragraph>{phrases.t("identity-block.reset-password-instruction-submitted")}</Paragraph>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ describe("Identity Password Reset Feature", () => {
}));
Object.defineProperty(window, "location", {
value: {
origin: "http://localhost",
...window.location,
assign: assignMock,
},
Expand Down
8 changes: 8 additions & 0 deletions blocks/identity-block/utils/validate-redirect-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@ const validateURL = (url) => {
if (!url) return null;
const validationRegEx = /^\/[^/].*$/;
const valid = validationRegEx.test(url);

if (valid) {
return `${window.location.origin}${url}`;
}

const urlLocation = new URL(url);

if (urlLocation.origin === window.location.origin) {
return url;
}

return "/";
};

Expand Down
13 changes: 11 additions & 2 deletions blocks/identity-block/utils/validate-redirect-url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,21 @@ describe("validateURL()", () => {
it("reutrns URL when it is a page on the current site", () => {
const url = "/redirect-here";
const result = validateURL(url);
expect(result).toBe(url);

expect(result).toBe('http://localhost/redirect-here');
});

it("reutrns the root URL when potentially unsafe", () => {
it("returns the root URL when potentially unsafe", () => {
const url = "https://www.unkown.com/redirect-here";
const result = validateURL(url);

expect(result).toBe("/");
});

it("returns concats contextPath if defined", () => {
const url = "/redirect-here/";
const result = validateURL(url);

expect(result).toBe('http://localhost/redirect-here/');
});
});

0 comments on commit 9ee574e

Please sign in to comment.