Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix social login #291

Merged
merged 3 commits into from
Jul 24, 2023
Merged

Fix social login #291

merged 3 commits into from
Jul 24, 2023

Conversation

serjonya-trili
Copy link
Contributor

Proposed changes

Task link

I fix a few issues:

  • G Auth: it didn't work at all neither for dev electron build, nor for the prod build. I upgraded the CustomAuth package version and updated the code for redirection here
  • G Auth: it couldn't work while you're on ghost net, now it works. The reason was (I presume so at least) that the web3ClientId we have is not setup for the testnet. Since all we need is the sk, pk and the pkh there is no need to go to the testnet anyway for the ghostnet authentication & authorisation because the address and the sk will remain the same (as with the mnemonics or ledger accounts)
  • I fixed electron:package:mac:debug command. the && break things and NODE_ENV is going to always be production when you build thanks to react create app
  • devTools are disabled. And "this is the way" because the env variable can always be overridden like that (on Mac) ENABLE_DEV_TOOLS=true open -a "Umami"
  • we don't need to setup a label for the account, we just take the email that we get from OAuth
  • a little bit better G logo (still waiting for the redesign to complete it)
  • web3AuthClientId was taken from Kukai since we are using their account

Types of changes

Put an x in the boxes that apply

  • Bugfix
  • New feature
  • Refactor
  • Documentation Update

Steps to reproduce

  • in GoogleAuth.tsx change baseUrl: "https://umamiwallet.com/auth/v2/", to baseUrl: "https://umamiwallet-imdk0e12n-trili-tech.vercel.app/auth/v2/" since it hasn't been deployed
  • yarn electron:start and sign up and/or sign a transactions
  • yarn build && yarn electron:package:mac:debug then install the app from the dist directory and repeat the steps above

Screenshots

dev build:
ghost net operation sign
https://github.com/trilitech/umami-v2/assets/129749432/3529738c-2d61-46d5-a2af-c1301800bf4b
sign up
https://github.com/trilitech/umami-v2/assets/129749432/ebe746ba-490b-482f-9d90-01d9a9b082d5
ghost net sign up
https://github.com/trilitech/umami-v2/assets/129749432/c8b61412-7252-412a-be38-fadc037b0051

prod build:
sign up
https://github.com/trilitech/umami-v2/assets/129749432/50bc106f-adac-4be4-a192-43076f6a84d5
ghost net operation sign
https://github.com/trilitech/umami-v2/assets/129749432/0e967493-68d0-45d3-a4ee-2684977e7b24

account create toast
Screenshot 2023-07-23 at 17 27 46

Checklist

  • Tests that prove my fix is effective or that my feature works have been added
  • Documentation has been added (if appropriate)
  • Screenshots are added (if any UI changes have been made)
  • All TODOs have a corresponding task created (and the link is attached to it)

README.md Outdated

Runs the electron app in the development mode.
In order to get the dev tools work please make sure to set the `ENABLE_DEV_TOOLS` env variable to `true`
In order to get the dev tools work please make sure to `devTools` to `true` in the `webPreferences`
Copy link
Contributor

@ryutamago ryutamago Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set the devTools :)

redirectToOpener: true,
uxMode: "popup",
network: toTorusNetwork(umamiNetwork),
});

const authenticate = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the logic to prevent authenticating multiple times here as well. this way we don't have to rely on the button UI

if(loading) {
   return;
}


const onSuccessfulSocialAuth = async (sk: string, email: string) => {
const { pk, pkh } = await getPkAndPkhFromSk(sk);
restoreSocial(pk, pkh, email);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we handle the case when this fails? e.g. ) the social account already exists

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it throws, the same way as for other accounts

Copy link
Contributor

@ryutamago ryutamago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

@serjonya-trili serjonya-trili merged commit 95967a7 into main Jul 24, 2023
1 check passed
@serjonya-trili serjonya-trili deleted the fix-social-login branch July 24, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants