Fix/asub 8551 apple signin redirect #2184
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
blocks/identity-block/components/social-sign-on/utils/useSocialSignIn.js
Fixed
Show fixed
Hide fixed
blocks/identity-block/components/social-sign-on/utils/useSocialSignIn.js
Fixed
Show fixed
Hide fixed
blocks/identity-block/components/social-sign-on/utils/useSocialSignIn.js
Fixed
Show fixed
Hide fixed
blocks/identity-block/components/social-sign-on/utils/useSocialSignIn.js
Fixed
Show fixed
Hide fixed
| setCurrentRedirectToURL(`${redirectURL}${redirectUrlLocation.search}`); | ||
| const newRedirectUrl = `${redirectURL}${redirectUrlLocation.search}`; | ||
|
|
||
| setRedirectUrl(newRedirectUrl); |
There was a problem hiding this comment.
@accbjt I think we can define a let newRedirectUrl and move this line out from the if-else statement.
There was a problem hiding this comment.
@LauraPinilla Okay, I made those updates.
| @@ -13,6 +13,7 @@ const validateURL = (url) => { | |||
| return url; | |||
| } | |||
|
|
|||
| sessionStorage.setItem("ArcXP_redirectUrl", "/"); | |||
There was a problem hiding this comment.
Hi @accbjt, why are you setting ArcXP_redirectUrl to "/". Sounds like the value we are setting here should depend on the custom fields or the page in which the user is.
There was a problem hiding this comment.
@LauraPinilla The original validateURL return '/' if the url is not valid so I was just following what they set in that function. I think if the url is not a valid url then it is okay to set it as /. That is the original logic and I don't want to change that and make it more complicated.
accbjt
added
the
ready for review
There was a problem hiding this comment.
@accbjt very small update removing newlines in multiple components. Everything else is look good
| @@ -21,6 +21,7 @@ function useSocialSignIn(redirectURL, isOIDC, socialSignOnIn, onError = () => {} | |||
| loginByOIDC(); | |||
| } else { | |||
| const validatedURL = validateURL(redirectURL); | |||
|
|
|||
There was a problem hiding this comment.
@accbjt Can you please revert this new line
| @@ -51,6 +52,7 @@ function useSocialSignIn(redirectURL, isOIDC, socialSignOnIn, onError = () => {} | |||
| loginByOIDC(); | |||
| } else { | |||
| const validatedURL = validateURL(redirectURL); | |||
|
|
|||
| @@ -43,6 +43,7 @@ function AccountManagement({ customFields }) { | |||
| const checkLoggedInStatus = () => | |||
| Identity.isLoggedIn().then((isLoggedIn) => { | |||
| if (!isLoggedIn) { | |||
|
|
|||
| @@ -90,6 +90,7 @@ const Login = ({ customFields }) => { | |||
| loginByOIDC(); | |||
| } else { | |||
| const validatedURL = validateURL(loginRedirect); | |||
|
|
|||
|
@malavikakoppula Okay, I made those changes. Thanks |
…ple-signin-redirect
Be sure to run
npm test,npm run lint, and write detailed test steps before requesting reviewersDescription
Jira Ticket: THEMES-
Information about what you changed for this PR. Include any dependencies, companion PRs, relevant screenshots, or config changes.
Test Steps
Add detailed test steps a reviewer must complete to test this PR
Video recording of testing this locally. I created this so you don't have to set this up on your local machine. This depends heavily on the Apple sign on implementation on subssinglesite and is decent amount of work to get it setup locally.
https://www.loom.com/share/2b1bbc5bd69348c9ac3e05b1c09a4f4a?sid=194460fd-1cee-46d5-8ea8-1c2dd085e698
Here is another example on an article page.
https://www.loom.com/share/c0a83222672a4ab885ff7a4d1b26a278?sid=0608528b-9acf-42c0-a87c-41f2b5b0cf71