-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add Twitter as an IDP #268
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
Conversation
@@ -80,5 +80,9 @@ | |||
</intent-filter> | |||
</activity> | |||
|
|||
<meta-data | |||
android:name="io.fabric.ApiKey" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually necessary? In the Firebase Auth quickstart I was able to get Twitter Sign In to work with only the consumer key/secret and didn't need the app ID or any meta-data:
https://github.com/firebase/quickstart-android/blob/master/auth/app/src/main/AndroidManifest.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it doesn't appear to be necessary. Removed it.
@amandle this looks really good! We do need to wait for iOS to release but I think we can still do work like this on a |
@@ -229,6 +245,17 @@ private boolean isFacebookConfigured() { | |||
} | |||
|
|||
@MainThread | |||
private boolean isTwitterConfigured() { | |||
List<String> twitterConfigs = Arrays.asList( | |||
getResources().getString(R.string.twitter_app_id), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed one reference to twitter_app_id
LGTM, thanks! |
Add Twitter as a provider in response to issue #157
I am not entirely sure how this will fit in with our feature parity plans with other platforms, we may have to wait for iOS to implement it before we can actually release.
Also, the Twitter Client Secret is stored in a resources file, which is not ideal but without writing a custom back-end server to coordinate with I'm not sure there is a better alternative (just obfuscation techniques) though I'm open to any suggestions.