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

[auth-session] Create built-in providers for Google and Facebook #9361

Merged
merged 31 commits into from
Aug 22, 2020

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Jul 23, 2020

Why

How

  • Introduce the response.authentication property which gets initialized when an auth request finishes with response.params.access_token. This is the same TokenResponse that would be returned from exchanging an auth code for an access token.

  • Created built-in providers:

import * as Google from 'expo-auth-session/providers/Google';
import * as Facebook from 'expo-auth-session/providers/Facebook';

// ...

Google.useAuthRequest({ 
  expoClientId: '',
  iosClientId: '',
  androidClientId: '',
  webClientId: ''
})

Facebook.useAuthRequest({ 
  clientId: '',
})
  • These improve auth by abstracting the discovery, scopes, useProxy, usePKCE, and redirectUri logic away (with ability to overwrite the abstraction). The abstraction helps the docs to focus more on provider setup rather than Expo configuration.

Google

import * as Google from 'expo-auth-session/providers/Google';

const [,response, promptAsync] = Google.useAuthRequest({ 
  expoClientId: '',
  iosClientId: '',
  androidClientId: '',
  webClientId: ''
})

React.useEffect(() => {
  if (response?.type === 'success') {
    console.log(response.authentication);
  }
}, [response])

Google Firebase

Firebase requires the id_token to be properly created. On native this can be achieved using the standard authentication, but on web you can only get the id_token when the response_type=id_token is used. Because of this I've reluctantly created an extension of Google.useAuthRequest which switches the response type on web or when using the proxy service.

const [,response, promptAsync] = Google.useIdTokenAuthRequest({ 
  expoClientId: '',
  iosClientId: '',
  androidClientId: '',
  webClientId: ''
})

React.useEffect(() => {
  if (response?.type === 'success') {
    console.log(response.params.id_token);
  }
}, [response])

Facebook

  • Facebook defaults to using Implicit auth instead of Auth code response type. This is because you cannot exchange a code without a server or the client secret.
  • Default scopes are optimized for Firebase and other providers.
  • Any form of Facebook auth can be used with Firebase afaict.
  • Automatically generate async nonce parameter and add it to the loading hook's pipeline.
import * as Facebook from 'expo-auth-session/providers/Facebook';

const [,response, promptAsync] = Facebook.useAuthRequest({ 
  clientId: '',
})

React.useEffect(() => {
  if (response?.type === 'success') {
    console.log(response.authentication);
  }
}, [response])

General

  • almost of the opinions can be overridden, either by dropping down to AuthSession.useAuthRequest or with other properties.
    • Proxy defaults can be overridden with the second parameter, i.e. Facebook.useAuthRequest({}, { useProxy: true })
    • Auto exchange on Google can be disabled with config.shouldAutoExchangeCode

Test Plan

Updated NCL tests to reflect the changes.

  • Test all response types (implicit, auth code, id-token).
  • Test bare-expo on iOS, and Android.
  • Test NCL in the client on iOS, and Android.
  • Test web NCL.
  • Test Standalone (via custom clients?).
  • Ensure Firebase flows work as expected.

@EvanBacon EvanBacon added AuthSession enhancement New feature or request labels Jul 23, 2020
@EvanBacon EvanBacon self-assigned this Jul 23, 2020
@github-actions
Copy link
Contributor

Native Component List for this branch is ready

*
* - **Application Type**: Android Application
* - **Package name**: Must match the value of `android.package` in your `app.json`.
* - **Signing-certificate fingerprint**: Run `openssl rand -base64 32 | openssl sha1 -c` for the results.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably need something like expo fetch:android:hashes instead.

Copy link
Member

Choose a reason for hiding this comment

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

you may already know this and i'm just misreading the comment but expo fetch:android:hashes does this already, the value you're looking for is "Google Certificate Fingerprint"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean if you run it in any Expo project that isn't built for prod you get:

Configuring credentials for bacon in project native-component-list
There is no valid Keystore defined for this app

Effectively meaning that in dev you need to set it to one value then change it in prod or create another Google key which is worse.

Copy link

@SleeplessByte SleeplessByte Jul 25, 2020

Choose a reason for hiding this comment

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

The current way everything is set-up, íf you want to use Google App Signing AND you want expo builds to have a different key store, you must have two google keys. Google's "recent" restrictions on those keys are weird, as almost all google systems allow for multiple values (all other android key restrictions).

And I figured out today, before seeing this PR, documented in #9391, you will need 3 separate keys anyway -- it's not thát weird to have a different key per bundle/release channel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just opened an issue to make the CLI return a keystore on demand without requiring a build expo/expo-cli#2456 this would effectively cut out one more key upfront for Android auth.

Choose a reason for hiding this comment

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

That's a good idea. I don't think having to have 3-4 keys is that problematic, but the fact that the "openssl" command is cryptic as best makes this a very good win, I think.

Copy link
Contributor Author

@EvanBacon EvanBacon Aug 13, 2020

Choose a reason for hiding this comment

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

I think it's problematic currently because currently you:

  • get fake hash -> register Google key with fake hash -> expo build it -> get actual key -> register new Google key -> update codebase -> expo publish again.

After this you could

  • get future hash -> register Google key with real hash -> expo build it.

Much easier to document, test, and develop.

Copy link

@SleeplessByte SleeplessByte Aug 13, 2020

Choose a reason for hiding this comment

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

Yeah, I think making this process for Google Play Store simpler is much much better. I did exactly what you say in the "before" list, and it's confusing if you don't know what's going on.

But that's not what I meant with it not being problematic. I meant that having a key for expo-non-google builds and google-signed builds does make sense -- so has nothing to do with the process of acquiring that key, but with the fact that we still should be able to support at least 2 keys for Android projects, 1 key for iOS and 1 key for Web.

Why?

If there can only be one Android key, non-google signed builds will no longer work, making the internal testing process far worse, and also make it much harder to have a google signed build for the play store and a expo signed build for all the other stores (amazon, samsung etc.).

Right now the real process is:

  1. expo build (you don't need anything other than the basic project and app.json` -- you don't need to use a fake key at the moment, you can just have this command generate it on the expo servers).
  2. expo fetch:android:hashes to get the hash
  3. Add a new google key with this hash, and update your project accordingly.
  4. expo build to get a working standalone build which won't work with app signing, but will work on non play stores / direct install

Optional for standalones:

  1. Upload the build to the play store to turn on App Signing and retrieve the hashes
  2. Add a new google key with this hash, and update your project accordingly
  3. expo build once more to get a working standalone build, to be used for the store

With your new proposed flow, if I understand correctly, it's:

  1. expo build (you don't need anything other than the basic project and app.json -- you don't need to use a fake key at the moment, you can just have this command generate it on the expo servers).
  2. expo fetch:android:hashes to get the hash
  3. Add a new google key with this hash, and update your project accordingly.
  4. expo build to get a working standalone build which won't work with app signing, but will work on non play stores / direct install

Optional for standalones:

  1. Upload the build to the play store to turn on App Signing and retrieve the hashes
  2. Add a new google key with this hash, and update your project accordingly
  3. expo build once more to get a working standalone build, to be used for the store

Or am I missing something here?

@EvanBacon EvanBacon force-pushed the @evanbacon/auth-session/providers/google-init branch from 01d7c2f to bdbe927 Compare August 13, 2020 01:33
@EvanBacon EvanBacon marked this pull request as ready for review August 15, 2020 05:12
@EvanBacon EvanBacon merged commit 6621638 into master Aug 22, 2020
@EvanBacon EvanBacon deleted the @evanbacon/auth-session/providers/google-init branch August 22, 2020 02:09
EvanBacon added a commit that referenced this pull request Aug 22, 2020
* [WIP] Created basic auth-session providers

* [WIP] Improved default auth flows

* Remove extra features

* Remove fetchUserInfoAsync

* Updated doc blocks

* Updated docs with google provider

* Updated docs with facebook provider

* Updated auth-session docs

* Added getting started links to the doc blocks

* Update authentication.md

* Update CHANGELOG.md

* Added shim for testing Firebase auth

* Disable selectAccount on Facebook

* Added authentication back

* Updated google auth

* Use implicit auth by default

* Update authentication.md

* Delete Firebase auth screen

* Split and share code

* Added comments

* Remove unused files

* Fix casing

* revert casing

* Transform casing

* Fix tests

* Fix unit tests

* fix docs

* use URL directly

* eslint ignore
@doronnahum
Copy link

Hi @EvanBacon ,
the provider folder is not published to the npm package

You can validate that by click on the Create-google-app button in the docs
https://docs.expo.io/guides/authentication/#google

@ottob
Copy link
Contributor

ottob commented Aug 28, 2020

the provider folder is not published to the npm package

I can confirm this on sdk 38. I just did a expo install expo-auth-session and got [email protected]. This functionality will probably be released in sdk 39?

Is it documented somewhere when to use expo-auth-session compared to the expo-facebook package for authentication?

@doronnahum
Copy link

doronnahum commented Aug 28, 2020

I tried to follow this guide:
https://docs.expo.io/guides/authentication/#facebook

import * as Facebook from 'expo-auth-session/providers/facebook';
''''

@ottob
Copy link
Contributor

ottob commented Aug 28, 2020

I tried to follow this guide

I did too :)

But if you look here https://github.com/expo/expo/blob/master/packages/expo-auth-session/CHANGELOG.md the new provider features are in version v1.5.0 and that has not been released yet for the 38 sdk. Im not sure if it can be cherry-picked to the sdk-38 branch or if it depends on changes in the 39 sdk. @EvanBacon probably knows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AuthSession enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Add info on using AuthSession with Firebase
5 participants