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

FEATURE REQUEST: Add Google/Apple sign in on New Dot, like we have on Old Dot #7079

Closed
mvtglobally opened this issue Jan 7, 2022 · 133 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Design Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Weekly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Jan 7, 2022

Design doc: https://docs.google.com/document/d/1W0hCQbcvcFIH1mlbSElovXKNaP_FK9J3MU3ESzH-y34/edit#

Pre-design: https://expensify.slack.com/archives/C01GTK53T8Q/p1678126175371849

Technical pre-design: https://expensify.slack.com/archives/C01GTK53T8Q/p1680016894091629

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open app
  2. Try to login with Google/Apple sign in

Expected Result:

User should be able to login with Google/Apple sign in

Actual Result:

User can only login with email

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.26-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen Shot 2022-01-07 at 12 52 26 AM
Screen Shot 2022-01-07 at 12 52 14 AM

Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1641509570059600

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Jan 7, 2022
@MelvinBot
Copy link

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jan 7, 2022
@alexpensify alexpensify removed their assignment Jan 7, 2022
@MelvinBot
Copy link

Triggered auto assignment to @marcochavezf (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@marcochavezf
Copy link
Contributor

This issue should be internal because it will require checking the implementation of Google/Apple of OldDot to make it compatible with the corresponding Web-E commands.

@marcochavezf marcochavezf added the Internal Requires API changes or must be handled by Expensify staff label Jan 7, 2022
@MelvinBot
Copy link

Triggered auto assignment to @puneetlath (Internal) because issue was reported by a contributor who needs to be compensated if this issue is fixed

@marcochavezf marcochavezf added Weekly KSv2 and removed Daily KSv2 labels Jan 7, 2022
@marcochavezf
Copy link
Contributor

Update: I did a bit of research of the most maintained third-party libraries for both Google and Apple sign-in functionality and those are:

Also asked in #expensify-open-source for a second opinion and we're ok using those libraries.

@puneetlath puneetlath removed their assignment Jan 12, 2022
@puneetlath
Copy link
Contributor

Would we also use libraries for web/desktop? Or just do it ourselves?

@marcochavezf
Copy link
Contributor

Would we also use libraries for web/desktop? Or just do it ourselves?

Ah that's a good question, I don't know yet, also I'm not sure if we can re-use the native libraries on web/desktop. I will investigate and compare.

@marcochavezf
Copy link
Contributor

marcochavezf commented Jan 29, 2022

Related to using libraries for web/desktop, TL:DR; I found out for Google SignIn is better to implement it ourselves.

I checked out the implementation of https://github.com/anthonyjgrove/react-google-login (which is the most popular library for Google SignIn in regular React) and I found out the implementation is relatively simple and it's better to implement it ourselves because the library contains features that we don't need (like a specific Google button, offline access, Google SignOut) and we should adapt our code to the library (create a new component to wrap the library) when we can create a function and simply hook it up to a regular Button component.

@MelvinBot MelvinBot removed the Overdue label Jan 29, 2022
@marcochavezf
Copy link
Contributor

When I'm testing the Google Sign-in I'm having the error Error 403: restricted_client on iOS because the OAuth consent screen is not set up in Google Cloud Platform:

Screen Shot 2022-02-07 at 19 01 32

Rory (ring3) was trying to help with that but he also doesn't have permission. I'm going to need someone of ring0 to set it up in GCP.

@rafecolton
Copy link
Member

rafecolton commented Feb 9, 2022

Did my best to configure the consent screen but not sure if it's correct, and it says it may need 4-6 weeks for Google to verify before we can publish it. More info in the linked internal issue.

@marcochavezf
Copy link
Contributor

marcochavezf commented Feb 18, 2022

Update:

  • The Google SignIn has been implemented for iOS and Android and it's working fine locally.
  • I implemented the Google SignIn on Desktop using the google-auth-library-nodejs library. I tried using first the same approach used on web, but I was having issues with Electron (https://expensify.slack.com/archives/C01GTK53T8Q/p1644432240338919). Also, I found out the web library hasn't been officially supported for Electron (link 1 and link 2) and there have been situations where all Electron apps using the web library suddenly had issues and needed to wait for an update or change the BrowserWindow.webPreferences.nativeWindowOpen configuration in Electron.

@marcochavezf
Copy link
Contributor

No update this week, I've been focused on an N7 issue also I'm going to be OOO for a few days. I will come back to it next week.

@melvin-bot melvin-bot bot removed the Overdue label Nov 6, 2023
@marcochavezf
Copy link
Contributor

No update

@melvin-bot melvin-bot bot added the Overdue label Nov 13, 2023
@shawnborton
Copy link
Contributor

What's the latest here?

@melvin-bot melvin-bot bot removed the Overdue label Nov 13, 2023
@marcochavezf
Copy link
Contributor

The 2 pending issues and the email wrap-up. I will move this to weekly, since I'm a little swamped with other higher priorities atm

@marcochavezf marcochavezf added Weekly KSv2 and removed Daily KSv2 labels Nov 13, 2023
@marcochavezf
Copy link
Contributor

No update

@JmillsExpensify
Copy link

Any update?

@JmillsExpensify
Copy link

Still not sure where we're at.

@JmillsExpensify
Copy link

Same

@melvin-bot melvin-bot bot added the Overdue label Dec 21, 2023
@marcochavezf
Copy link
Contributor

No update, focused on wave issues. The 2 remaining issues are minor and I think low priority, should be part of a VIP project? So I can give priority to them

@melvin-bot melvin-bot bot removed the Overdue label Dec 21, 2023
@melvin-bot melvin-bot bot added the Overdue label Jan 1, 2024
@marcochavezf
Copy link
Contributor

No update

@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Jan 11, 2024
@JmillsExpensify
Copy link

@marcochavezf What's the latest on this one?

@melvin-bot melvin-bot bot removed the Overdue label Jan 12, 2024
@marcochavezf
Copy link
Contributor

No update here since I've been prioritizing wave7 issues. I think the two remaining issues can be part of #vip-vbs

@quinthar
Copy link

Don't we already have this?
image

@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
@shawnborton
Copy link
Contributor

Friendly bump - what's the latest on this one @marcochavezf ?

@melvin-bot melvin-bot bot removed the Overdue label Jan 29, 2024
@marcochavezf
Copy link
Contributor

Just the wrap-up email is pending but I've been deprioritizing because of wave bugs. I will prioritize it this week for real, sorry for the delay here.

@marcochavezf
Copy link
Contributor

Writing the wrap-up email, aiming to send it out later today

@marcochavezf
Copy link
Contributor

Email sent, closing it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Design Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests