🪟 🎉 Enable OAuth login #15414

timroes · 2022-08-08T17:58:11Z

What

Closes https://github.com/airbytehq/airbyte-cloud/issues/1429

This implements OAuth login for our cloud application.

This does not require any backend changes, since Firebase gives us the same JWTs that already work on the backend. We have on frontend-dev a Google and GitHub dev application enabled, so this can fully tested against frontend-dev. Also LaunchDarkly currently has both login mechanism enabled on LaunchDarkly.

Some notes around behavior/function:

OAuth doesn't make a difference between login and sign up so the buttons on the login and signup page do exactly the same.
When logging in via Google to an existing account, this will work and log you into your existing account (since Google does know that your email is valid, and Firebase is a Google product). If you'd try to login using GitHub into an existing account with the same mail it will fail with a "auth/account-exists-with-different-credential" error code, which we show a warning for to log in using email/password.
You can perfectly fine request a new password for a mail that was originally logged into using OAuth, so no special handling needed there. If you will use the link you get via mail to set a new password it will convert the login from a GitHub account into a mail/password login (and after that you can't login using GitHub anymore, the same as mentioned above). In case of a Google account it will just be a "hybrid" account and you can login using either mail/password or sign in via google.
I've tried to comment everything in code as well as possible, also see my github comments for some implementation specific details.
The TOS checkbox was replaced by a pure disclaimer text below the login/signup buttons as discussion with Andy.
The invite user flow is still requiring users to register with a username/password afterwards. This is not an easy fix, since the way firebase handles invites via mail, the actual invite already is the login, and we just change the password. Addressing this will be postponed to a later PR (especially since users can always use Google to login later into the account they created this way).

timroes · 2022-08-08T17:59:56Z

airbyte-webapp/src/packages/cloud/lib/auth/GoogleAuthService.ts

 } from "firebase/auth";

 import { Provider } from "config";
 import { FieldError } from "packages/cloud/lib/errors/FieldError";
 import { EmailLinkErrorCodes, ErrorCodes } from "packages/cloud/services/auth/types";

-interface AuthService {


ℹ️ We never exported this type, and implementing the interface in TypeScript does not even help not needing to respecify the types in the GoogleAuthService anyway. Thus this interface is not really fulfilling any purpose (besides needing to arbitrarily type the method signature twice) and I removed it.

timroes · 2022-08-08T18:01:00Z

airbyte-webapp/src/packages/cloud/services/auth/AuthService.tsx

 interface AuthContextApi {
  user: User | null;
  inited: boolean;
  emailVerified: boolean;
  isLoading: boolean;
  loggedOut: boolean;
  login: AuthLogin;
+  loginWithOAuth: (provider: OAuthProviders) => Observable<OAuthLoginState>;


ℹ️ This method returns an Obersvable, since we need to know in the UI, when the middle of the method starts (the user selected an account, and we start loading the user), to show the loading spinner. Thus we can't express that with a Promise.

timroes · 2022-08-08T18:02:09Z

airbyte-webapp/src/packages/cloud/services/auth/AuthService.tsx

+      name: user.name,
+      email: user.email,
+      // Which login provider was used, e.g. "password", "google.com", "github.com"
+      provider: fbUser.providerData[0]?.providerId,


ℹ️ There is a chance a firebase user has multiple providers linked (e.g. mail and google as mentioned in the PR description). But that shouldn't be the case when the user is just created, in which case they should only have one provider linked, thus we can simply take the first provider here for sending the event.

edmundito

Initial feedback. I have not tested the changes locally.

airbyte-webapp/src/packages/cloud/services/auth/AuthService.tsx

edmundito · 2022-08-09T13:13:36Z

airbyte-webapp/src/packages/cloud/views/auth/OAuthLogin/OAuthLogin.module.scss

+
+  &:hover,
+  &:focus {
+    box-shadow: 0 1px 3px rgba(53, 53, 66, 0.2), 0 1px 2px rgba(53, 53, 66, 0.12), 0 1px 1px rgba(53, 53, 66, 0.14);


Where did these drop shadow colors come from?

This shadow is from the regular button components. Most likely better to exctract it into a SASS variable. Any preference for which file?

airbyte-webapp/src/scss/_variables.scss seems the most appropriate, though a new file might be better. Not sure it's worth making a new file.

@edmundito Any preferences for a file we should pack this into?

airbyte-webapp/src/packages/cloud/views/auth/OAuthLogin/OAuthLogin.tsx

edmundito · 2022-08-10T13:42:56Z

@timroes looks like the most recent changes include removing the checkbox to agree to the TOS and the PP. Could you update the title/description of this PR?

timroes · 2022-08-10T17:20:43Z

@timroes looks like the most recent changes include removing the checkbox to agree to the TOS and the PP. Could you update the title/description of this PR?

@edmundito updated it in the description!

edmundito · 2022-08-10T19:32:51Z

Did some local testing and found a couple of issues:

The scroll needs to be fixed for smaller screen heights:

The "interested in self hosting" CTA seems too close to the sign up actions:

If I try to sign in with google, close the popup, and then try with github, I see an error when I shouldn't:

The sign up button should only be enabled if I filled out information, but it's enabled when I load the page:

airbyte-webapp/src/packages/cloud/locales/en.json

airbyte-webapp/src/packages/cloud/views/users/AccountSettingsView/AccountSettingsView.tsx

Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com>

timroes · 2022-08-10T20:08:39Z

The sign up button should only be enabled if I filled out information, but it's enabled when I load the page:

This is the prior and desired behavior at the moment (we'll later make this disabled by default, for users coming from GDPR affected countries).

The scroll needs to be fixed for smaller screen heights:

This issue is also on master, and since it might touch more code again (e.g. the scaling of the iframe might need to be addressed as well?) I'd prefer if we don't mix the fix into this PR.

The "interested in self hosting" CTA seems too close to the sign up actions:

This was also present beforehand, and it makes sense to change this whole page to use flexbox or grid. So same here: if possible I'd prefer moving that into a separate PR, if it would be okay for you?

krishnaglick · 2022-08-12T19:00:47Z

This appears to be working as expected locally 👍

timroes · 2022-08-15T11:47:31Z

Created #15650 to track the scroll issues on the login/sign-up page.

* Enable OAuth login * Style buttons * Make sure to hide error wrapper without error * Extract OAuthProviders type * Make google login button outline more visible * Add provider to segment identify call * Switch TOS checkbox by disclaimer * Address review feedback * Hide password change section for OAuth accounts * Update airbyte-webapp/src/packages/cloud/locales/en.json Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com> * Address review feedback * Add additional flags to disable on sign-up * Adding more tests * Review feedback * Fix broken linting Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com>

Tim Roes added 4 commits August 8, 2022 11:55

Enable OAuth login

e0d6c42

Style buttons

368a35d

Make sure to hide error wrapper without error

2ad7b8e

Extract OAuthProviders type

e8676df

timroes requested a review from a team as a code owner August 8, 2022 17:58

timroes added the area/frontend Related to the Airbyte webapp label Aug 8, 2022

github-actions bot added the area/platform issues related to the platform label Aug 8, 2022

timroes added ui/auth and removed area/platform issues related to the platform labels Aug 8, 2022

timroes commented Aug 8, 2022

View reviewed changes

Merge branch 'master' into tim/oauth

996cec0

github-actions bot added the area/platform issues related to the platform label Aug 8, 2022

Tim Roes added 2 commits August 9, 2022 15:12

Make google login button outline more visible

7ba2a85

Add provider to segment identify call

14cd79e

edmundito reviewed Aug 9, 2022

View reviewed changes

Tim Roes added 4 commits August 10, 2022 02:24

Switch TOS checkbox by disclaimer

be7177a

Address review feedback

3ba46ad

Merge branch 'master' into tim/oauth

c2754d1

Hide password change section for OAuth accounts

734fc2d

timroes requested a review from edmundito August 10, 2022 13:25

edmundito requested a review from a team August 10, 2022 13:39

edmundito reviewed Aug 10, 2022

View reviewed changes

airbyte-webapp/src/packages/cloud/locales/en.json Outdated Show resolved Hide resolved

airbyte-webapp/src/packages/cloud/views/users/AccountSettingsView/AccountSettingsView.tsx Outdated Show resolved Hide resolved

Update airbyte-webapp/src/packages/cloud/locales/en.json

50eea48

Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com>

Address review feedback

84ff5a3

Review feedback

6fd5a83

Fix broken linting

e21e0e1

timroes changed the title ~~🪟 Enable OAuth login~~ 🪟 🎉 Enable OAuth login Aug 15, 2022

krishnaglick approved these changes Aug 15, 2022

View reviewed changes

timroes requested a review from a team as a code owner August 15, 2022 17:51

github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Aug 15, 2022

octavia-squidington-iii added connectors/destination/postgres connectors/destination/postgres-strict-encrypt connectors/source/postgres labels Aug 15, 2022

timroes temporarily deployed to more-secrets August 15, 2022 17:53 Inactive

Merge branch 'master' into tim/oauth

23cef5b

timroes force-pushed the tim/oauth branch from bfe84da to 23cef5b Compare August 15, 2022 17:54

timroes removed the request for review from a team August 15, 2022 17:54

github-actions bot removed area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Aug 15, 2022

timroes removed area/platform issues related to the platform connectors/source/postgres connectors/destination/postgres connectors/destination/postgres-strict-encrypt labels Aug 15, 2022

edmundito approved these changes Aug 16, 2022

View reviewed changes

Merge branch 'master' into tim/oauth

36bbf80

github-actions bot added the area/platform issues related to the platform label Aug 17, 2022

timroes merged commit 8c79e4f into master Aug 17, 2022

timroes deleted the tim/oauth branch August 17, 2022 13:15

octavia-squidington-iii mentioned this pull request Aug 19, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1-alpha #15775

Closed

evantahler mentioned this pull request Aug 22, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1 #15857

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎉 Enable OAuth login #15414

🪟 🎉 Enable OAuth login #15414

timroes commented Aug 8, 2022 •

edited

Loading

timroes Aug 8, 2022

timroes Aug 8, 2022

timroes Aug 8, 2022

edmundito left a comment

edmundito Aug 9, 2022

timroes Aug 9, 2022

krishnaglick Aug 12, 2022

timroes Aug 12, 2022

edmundito commented Aug 10, 2022

timroes commented Aug 10, 2022

edmundito commented Aug 10, 2022

timroes commented Aug 10, 2022

krishnaglick commented Aug 12, 2022

timroes commented Aug 15, 2022

🪟 🎉 Enable OAuth login #15414

🪟 🎉 Enable OAuth login #15414

Conversation

timroes commented Aug 8, 2022 • edited Loading

What

timroes Aug 8, 2022

Choose a reason for hiding this comment

timroes Aug 8, 2022

Choose a reason for hiding this comment

timroes Aug 8, 2022

Choose a reason for hiding this comment

edmundito left a comment

Choose a reason for hiding this comment

edmundito Aug 9, 2022

Choose a reason for hiding this comment

timroes Aug 9, 2022

Choose a reason for hiding this comment

krishnaglick Aug 12, 2022

Choose a reason for hiding this comment

timroes Aug 12, 2022

Choose a reason for hiding this comment

edmundito commented Aug 10, 2022

timroes commented Aug 10, 2022

edmundito commented Aug 10, 2022

timroes commented Aug 10, 2022

krishnaglick commented Aug 12, 2022

timroes commented Aug 15, 2022

timroes commented Aug 8, 2022 •

edited

Loading