Skip to content

Commit

Permalink
Fix/asub 8551 apple signin redirect (#2184)
Browse files Browse the repository at this point in the history 
* add localstorage to redirect url

* add test for using localhost redirect

* fix tests

* add location.assign

* update tests for login link

* revision updates
  • Loading branch information
@accbjt
accbjt authored Jul 29, 2024
1 parent 14dee61 commit 236fba9
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 42 deletions.
31 changes: 23 additions & 8 deletions blocks/identity-block/components/login/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ const useLogin = ({
const [isAppleAuthSuccess, setIsAppleAuthSuccess] = useState(false);
const { loginByOIDC } = useOIDCLogin();

const setRedirectUrl = (url) => {
setCurrentRedirectToURL(url);
sessionStorage.setItem('ArcXP_redirectUrl', url);
};

const getRedirectURL = () => {
const localStorageRedirectUrl = sessionStorage.getItem('ArcXP_redirectUrl');

return redirectQueryParam || localStorageRedirectUrl || currentRedirectToURL;
};

useEffect(() => {
const askForloginWithApple = async (code) => {
Expand Down Expand Up @@ -55,16 +65,17 @@ const useLogin = ({

if (redirectToPreviousPage && document?.referrer) {
const redirectUrlLocation = new URL(document.referrer);
let newRedirectUrl = redirectUrlLocation.pathname.includes('/pagebuilder/')
? redirectURL
: `${redirectUrlLocation.pathname}${redirectUrlLocation.search}`;

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

setCurrentRedirectToURL(newRedirectUrl);
setRedirectUrl(newRedirectUrl);
}

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

Expand All @@ -88,7 +99,11 @@ const useLogin = ({
if (isOIDC) {
loginByOIDC();
} else {
window.location = redirectQueryParam || validatedLoggedInPageLoc;
const localStorageRedirectUrl = sessionStorage.getItem('ArcXP_redirectUrl');
const validatedLocalRedirectURL = validateURL(localStorageRedirectUrl);
const newRedirectUrl = redirectQueryParam || validatedLocalRedirectURL || validatedLoggedInPageLoc;

window.location = newRedirectUrl;
}
}
};
Expand All @@ -98,7 +113,7 @@ const useLogin = ({
}, [Identity, redirectQueryParam, loggedInPageLocation, isAdmin, loginByOIDC, isOIDC, isAppleAuthSuccess]);

return {
loginRedirect: redirectQueryParam || currentRedirectToURL,
loginRedirect: getRedirectURL(),
};
};

Expand Down
79 changes: 61 additions & 18 deletions blocks/identity-block/components/login/index.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,18 @@ const Test = (props) => {
);
};

const windowLocationValues = {
origin: 'http://localhost',
href: 'http://localhost',
search: '',
pathname: '/',
}

describe("useLogin()", () => {
beforeEach(() => {
Object.defineProperty(window, "location", {
writable: true,
value: {
origin: 'http://localhost',
href: 'http://localhost',
search: '',
pathname: '/',
}
value: windowLocationValues,
});
useIdentity.mockImplementation(() => ({
isInitialized: true,
Expand All @@ -64,6 +66,7 @@ describe("useLogin()", () => {

it("uses redirect query", async () => {
Object.defineProperty(window, "location", {
...windowLocationValues,
writable: true,
value: {
search: "?test=123&redirect=/new-account/",
Expand Down Expand Up @@ -97,6 +100,7 @@ describe("useLogin()", () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
...windowLocationValues,
origin: 'http://localhost',
href: 'http://localhost',
search: '?reset_password=true',
Expand All @@ -118,13 +122,15 @@ describe("useLogin()", () => {
},
}));
await render(<Test />);
expect(window.location).toBe(`http://localhost${defaultParams.loggedInPageLocation}`);

expect(window.location).toBe(`http://localhost${defaultParams.redirectURL}`);
});

it("replaces potentially unsafe URLs in query param", async () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
...windowLocationValues,
search: "?test=123&redirect=https://somewhere.com",
pathname: "/",
},
Expand All @@ -134,21 +140,23 @@ describe("useLogin()", () => {
expect(window.location).toBe("/");
});

it("replaces potentially unsafe URLs in redirectURL parameter", async () => {
await render(<Test redirectURL="https://somewhere.com" />);
it("replaces potentially unsafe URLs in query param", async () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
...windowLocationValues,
search: "",
pathname: "/",
},
});
await render(<Test loggedInPageLocation="https://somewhere.com" />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/");
});

it("replaces potentially unsafe URLs in loggedInPageLocation parameter", async () => {
useIdentity.mockImplementation(() => ({
isInitialized: true,
Identity: {
isLoggedIn: jest.fn(() => true),
getConfig: jest.fn(() => ({})),
},
}));
await render(<Test loggedInPageLocation="https://somewhere.com" />);
it("replaces potentially unsafe URLs in redirectURL parameter", async () => {
await render(<Test redirectURL="https://somewhere.com" />);
fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/");
});

Expand All @@ -161,6 +169,7 @@ describe("useLogin()", () => {
Object.defineProperty(window, "location", {
writable: true,
value: {
...windowLocationValues,
origin: 'http://localhost',
href: 'http://localhost',
search: '',
Expand All @@ -172,4 +181,38 @@ describe("useLogin()", () => {
expect(window.location).toBe("/account/");
delete document.referrer;
});

it("should use redirectUrl from sessionStorage", async () => {
const referrerURL = "http://localhost/featured-articles/";
Object.defineProperty(document, "referrer", {
value: referrerURL,
configurable: true,
});
Object.defineProperty(window, "location", {
writable: true,
value: {
...windowLocationValues,
origin: 'http://localhost',
href: 'http://localhost',
search: '',
pathname: '/'
}
});

useIdentity.mockImplementation(() => ({
isInitialized: true,
Identity: {
isLoggedIn: jest.fn(() => true),
getConfig: jest.fn(() => ({})),
},
}));

await render(<Test />);

expect(sessionStorage.getItem("ArcXP_redirectUrl")).toBe('/featured-articles/');

fireEvent.click(screen.getByRole("button"));
expect(window.location).toBe("/featured-articles/");
delete document.referrer;
});
});
26 changes: 13 additions & 13 deletions blocks/identity-block/components/social-sign-on/_children/AppleSignIn.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ function AppleSignIn({ socialSignOnIn, className }) {
const { Identity } = useIdentity();

return (
<Button
id="apple-btn"
variant="secondary-reverse"
onClick={() => Identity.initAppleSignOn()}
iconLeft={AppleIcon}
className={`${className}__Apple`}
>
{socialSignOnIn !== SIGN_UP ? (
<span>{phrases.t("identity-block.social-signOn-apple-login")}</span>
) : (
<span>{phrases.t("identity-block.social-signOn-apple-signUp")}</span>
)}
</Button>
<Button
id="apple-btn"
variant="secondary-reverse"
onClick={() => Identity.initAppleSignOn()}
iconLeft={AppleIcon}
className={`${className}__Apple`}
>
{socialSignOnIn !== SIGN_UP ? (
<span>{phrases.t("identity-block.social-signOn-apple-login")}</span>
) : (
<span>{phrases.t("identity-block.social-signOn-apple-signUp")}</span>
)}
</Button>
);
}

Expand Down
4 changes: 2 additions & 2 deletions blocks/identity-block/components/social-sign-on/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import AppleSignIn from "./_children/AppleSignIn";
import useSocialSignIn from "./utils/useSocialSignIn";

const SocialSignOn = ({ className, onError, redirectURL, isOIDC, socialSignOnIn }) => {
const { facebookAppId, googleClientId, appleTeamId, appleKeyId, appleUrlToReceiveAuthToken} = useSocialSignIn(redirectURL, isOIDC, socialSignOnIn, onError);
const { facebookAppId, googleClientId, appleTeamId, appleKeyId, appleUrlToReceiveAuthToken } = useSocialSignIn(redirectURL, isOIDC, socialSignOnIn, onError);
return (
<section className={className}>
{googleClientId ? <GoogleSignIn onError={onError} redirectURL={redirectURL} socialSignOnIn={socialSignOnIn} className={className} /> : null}
{facebookAppId ? <FacebookSignIn socialSignOnIn={socialSignOnIn}/> : null}
{facebookAppId ? <FacebookSignIn socialSignOnIn={socialSignOnIn} /> : null}
{appleTeamId && appleKeyId && appleUrlToReceiveAuthToken ? <AppleSignIn socialSignOnIn={socialSignOnIn} className={className} /> : null}
</section>
);
Expand Down
2 changes: 1 addition & 1 deletion blocks/identity-block/features/social-sign-on/default.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const SocialSignOnBlock = ({ customFields }) => {

return (
<Stack className={BLOCK_CLASS_NAME} data-testid="social-sign-on-container">
{!hideDiv ? <div className={`${BLOCK_CLASS_NAME}__dividerWithText`}>{phrases.t("identity-block.or")}</div> : null}
{!hideDiv ? <div className={`${BLOCK_CLASS_NAME}__dividerWithText`}>{phrases.t("identity-block.or")}</div> : null}
<GoogleSignInProvider>
<SocialSignOn
className={`${BLOCK_CLASS_NAME}__button-container`}
Expand Down
1 change: 1 addition & 0 deletions blocks/identity-block/utils/validate-redirect-url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const validateURL = (url) => {
return url;
}

sessionStorage.setItem("ArcXP_redirectUrl", "/");
return "/";
};

Expand Down

0 comments on commit 236fba9

Please sign in to comment.