Initial Social Login implementation #268
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const [searchParams] = useSearchParams(); | ||
| const navigate = useNavigate(); | ||
| const onClick = () => { | ||
| signInWithPopup(firebaseAuth, provider) |
There was a problem hiding this comment.
Using then/catch, async/await. Isn't better choose one and go with it?
I find async/wait more clear
| setAccountIdToController({ accountId, networkId }); | ||
|
|
||
| let publicKeyFak: string; | ||
| if (await isPassKeyAvailable()) { |
There was a problem hiding this comment.
I see the code here share a lot of similarities with AuthCallback page. Can we create functions and re-use them to avoid maintaining the same code on two parts?
| @@ -0,0 +1,9 @@ | |||
| export function generateRandomString(length: number) { | |||
There was a problem hiding this comment.
I think we have the UUID library installed for this.
It's also better for collision proof
|
@hcho112 @Pessina Sorry for jumping in, but I'd like to invite you to participate in the Race of Sloths - a fun and gamified open-source contributions program. Consider mentioning @race-of-sloths user in your github comment or PR description to join! See how the flow works here: near/nearcore#11778 |
|
Currently there is an issue with e2e test failures due to smart contract associated on multichain tests. It has nothing to do with this PR. The team is working on resolving the issue |
|
@hcho112 Thank you for your contribution! Your pull request is now a part of the Race of Sloths! Current status: executed
Your contribution is much appreciated with a final score of 2! Another weekly streak completed, well done @hcho112! To keep your weekly streak and get another bonus make pull request next week! Looking forward to see you in race-of-sloths What is the Race of SlothsRace of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! Bot commands
|
| await window.fastAuthController.setAccountId(accountId); | ||
| } | ||
|
|
||
| await window.fastAuthController.claimOidcToken(accessToken); |
There was a problem hiding this comment.
This still duplicated no?
I think it would be good to extract the common logic from AuthCallback and SocialLogin to a separated component
1 - passkeys
2 - claimoidc
3 - call createAccount or signIn
What do you think? Otherwise we have to maintain the same flow on two places.
There was a problem hiding this comment.
Also I've extracted some function in the AuthCallback component in my PR, I think we should look for how to reconcile them.
There was a problem hiding this comment.
I thought previous comment was about refactoring the part that I pull out and replace with setAccountIdToController.
I did notice that @benmalcom also made refactor on this PR, perhaps I will reuse his one. (Either we merge this PR first and once @benmalcom 's PR merged, I refactor or we merge his one first and I will rebase on top of his)
Also the this part is actually different to Authcallback
const isNewUser = shouldCreateAccount(user.metadata.creationTime, user.metadata.lastSignInTime);
let callback;
if (isNewUser) {
callback = onCreateAccount;
} else if (isRecovery) {
callback = onSignIn;
} else {
callback = onCreateAccount;
}
so its not fully refactor-able
There was a problem hiding this comment.
Cool! No problem then, let's work on it after merge both PRs.
| oidcToken: accessToken, | ||
| }); | ||
|
|
||
| const isNewUser = shouldCreateAccount(user.metadata.creationTime, user.metadata.lastSignInTime); |
There was a problem hiding this comment.
Can you use the https://firebase.google.com/docs/reference/js/auth.md#operationtype available on the result from const result = await signInWithPopup(firebaseAuth, provider);? Instead of the custom function to check if it's account creation?
There was a problem hiding this comment.
Unfortunately, it will always return with signIn which it cant used to distinguish if we are using it for create or sign in
| label, | ||
| isRecovery, | ||
| disabled, | ||
| }: SocialButtonProps) { |
There was a problem hiding this comment.
Either way is fine, just sharing knowledge, but you can type a component with:
const SocialButton: React.FC<SocialButtonProps> = (props) => {....}
So we can improve a little bit the type of the function.
| window.location.replace(parsedUrl.href); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| const getProvider = (socialName: string) => { |
There was a problem hiding this comment.
Instead of string you can specify the strings allowed here.
|
🥁 Score it! @Pessina @benmalcom, please score the PR with |
This PR contains initial social login of google and apple (facebook is WIP)
Main difference compared to existing solution is that:
You can review created account on firebase console. (To test creation after you already used your own social login, it would be easier to delete user record on firebase console and try to create again)
Currently I use random string to generate account Id. (This idea was originated from other platform that also uses social login, they also use random string to auto-generate their account id of their user. Intention is to remove extra steps of keying in account id. Assuming that user will always use social login, this is not a bad idea, however I'm happy to replace this after showcase it to rest of the team.)
This mode is currently only available on testnet.
On mainnet:
On testnet:
