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

How to get AdditionalUserInfo ? #23

Closed
go-u opened this issue Sep 19, 2019 · 14 comments
Closed

How to get AdditionalUserInfo ? #23

go-u opened this issue Sep 19, 2019 · 14 comments

Comments

@go-u
Copy link

go-u commented Sep 19, 2019

Hi! Thank you for very nice app !!

I want to get below info from twitter login user .
screen_name / name / description / profile_image_url_https / profile_banner_url

I think result.additionalUserInfo.profile contain these info. How can I get this ?

Maybe here, this result contain result.additionalUserInfo.profile.

plugin.signIn({providerId}).then((result :TwitterSignInResult) => {

But only return userCredential.user here ? right ?

observer.next(userCredential.user);

@go-u
Copy link
Author

go-u commented Sep 23, 2019

Hi. I think that is better to just return (result :TwitterSignInResult) itself here.
This result may contain not only AdditionalUserInfo, but also credential and so on.

plugin.signIn({providerId}).then((result :TwitterSignInResult) => {

And here, at cfaSignIn method, we could use SignInOptions.

export const cfaSignIn = (providerId: string, data?: SignInOptions): Observable<firebase.User> => {

New Pull Request ?

So If this is not bad idea, Do you need new pull request ?

If that so, I will write code for user options like this way. what do you think about this way?
cfaSign('twitter.com', { result: true})

(Sorry, I am not professional )

@go-u
Copy link
Author

go-u commented Sep 23, 2019

Hi !

As I'm not professional and new to typescript, for now, I simply added below code for get result.

https://github.com/go-u/capacitor-firebase-auth/commit/591f359f07d937e3753c29ec50bf7e06ca9992a5

This work fine for me.
I get result.idToken, result.secret from result,
and I get additional user info from twitter rest api with those token and secret.

If you can advise me for improve my code, I update my code and send you pull request.

@baumblatt
What do you think ?

@baumblatt
Copy link
Owner

Hi Go-U,

I understand your needs, but in my opinion, it’s better to keep the facade equals for every authentication provider.

But please, keep in my that this is only the facade, you can use the plugin directly exactly in the same way as the facade do.

In other words, your version of cfaSignInTwitter can be placed anywhere inside your project, even with the same name (resolving on import), or renaming it to gouSignInTwitter.

Anyway, take a look at this answer in stackoverflow about getting user info after user sign in.

Please let me know if you have any questions, or we can close this issue.

Best regards,
Bernardo Baumblatt

@go-u
Copy link
Author

go-u commented Sep 30, 2019

Thank you for your very polite reply.

I now think that there is no need to add functionality for "AdditionalUserInfo".

However, I think we need a way to handle Twitter user's access token and secret from Javascript.

Now, TwitterSignInResult already contains an access token and secret, but this plugin doesn't seem to have the ability to receive it in Javascript.

Let me explain a specific example.

Example

I am currently using a method similar to the one you introduced,
In other words, I get Twitter user information from the Twitter API endpoint.

There are "app auth" and "user auth" for Twitter API endpoint authentication.
For example, let's assume you want to create a Twitter app that only gets a Follower list of logged-in user.

This is done using the GET followers / list endpoint.
https://developer.twitter.com/en/docs/basics/rate-limits

If this endpoint is used with "app auth", the rate limit will be reached immediately and the app will not function.
"app auth" can use this endpoint up to 15 times in 15 minutes.
But if 16 users use it at the same time, problems arise.

On the other hand, "user auth" can use this endpoint up to 15 times in 15 minutes, but Rate Limit is calculated for each user.

I think that it is common to use "user auth" when making a Twitter application.
And "user auth" requires a Twitter user's access token and secret.

Question

In this way, most Twitter apps seem to require a Twitter user's access token and secret,
Is there a way to receive user's access token and secret already included in TwitterSignInResult?

plugin.signIn({providerId}).then((result :TwitterSignInResult) => {

Thank you again for a very polite reply.
Thank you very much.

@baumblatt
Copy link
Owner

Hi Go-U,

The answer is yes, there is, but the point of doubt is:

Should we add this feature to the Javascript facade of the plugin?

On my opinion, we should keep it simple on behalf of the major of plugin users.

But is very simple to implement your own call to the plugin, that as you mention already return the TwitterSignInResult from the roots.

You can put this snippet anywhere in your App, without any change in the plugin itself.

import {TwitterSignInResult} from 'capacitor-firebase-auth';
import {CapacitorFirebaseAuthPlugin} from 'capacitor-firebase-auth/dist/esm/definitions';

// @ts-ignore
const plugin: CapacitorFirebaseAuthPlugin = Plugins.CapacitorFirebaseAuth;

/**
 * Call the Twitter sign in method on native and sign in on web layer with retrieved credentials and 
 * return the twitter credentials and the firebase user.
 */
export const myCfaSignInTwitter = (): Observable<firebase.User> => {
	return new Observable(observer => {
		// get the provider id
		const providerId = firebase.auth.TwitterAuthProvider.PROVIDER_ID;

		// native sign in
		plugin.signIn({providerId}).then((result :TwitterSignInResult) => {
			// create the credentials
			const credential = firebase.auth.TwitterAuthProvider.credential(result.idToken, result.secret);

			// web sign in
			firebase.app().auth().signInWithCredential(credential)
				.then((userCredential: firebase.auth.UserCredential) => {
					observer.next({
						credentials: {idToken: result.idToken, secret: result.secret}
						user: userCredential.user,
					});
					observer.complete();
				})
				.catch((reject: any) => observer.error(reject));

		}).catch(reject => observer.error(reject));
	});
};

In the next release I will expose the CapacitorFirebaseAuthPlugin, in a way that the deeper import could be avoided.

Finally, if you still thing that the facade’s plugin should do the job, I’m willing to expose the method above with a better name.

I’m looking forward to hearing from you.

Best regards,
Bernardo Baumblatt

@go-u
Copy link
Author

go-u commented Oct 1, 2019

Thank you for the detailed explanation with code !

Should we add this feature to the Javascript facade of the plugin?

Finally, if you still thing that the facade ’s plugin should do the job, I ’m willing to expose the method above with a better name.

If the meaning of this explanation is to make it easy to receive the “Twitter's user's access token and secret ” in Javascript, I think that is a great feature.

Reasons

The reason why I think it is great feature to receive "Twitter's user's access token and secret" in Javascript is as follows.
・ Vuex (Vue.js) is a little incompatible with TypeScript, so Javascript is the mainstream.
・ Quasar, a Vue.js-based framework, does not yet support TypeScript officially.
・ Firebase Auth + Twitter Login + Twitter API (use "user's access token and secret") is a golden combination for developers who I know.

Supplement

I live in Japan, where Twitter is very popular and maintains a world record of tweets per second. 143,199TPS (143K Tweet / Sec.)
https://blog.twitter.com/engineering/en_us/a/2013/new-tweets-per-second-record-and-how.html

Twitter's user's access token and secret (user auth) is required for the above reasons.
Even if you can log in via email or Facebook using the Firebase login function of the app, there is a great demand for additional integration with the Twitter API.

My opinion

For the above reasons, at least Japanese developers I know want to easily receive “Twitter user's access token and secret” in Javascript.

Finally, thanks for writing a great plugin.
Thank you for the wonderful tool !

@baumblatt
Copy link
Owner

baumblatt commented Oct 1, 2019

Hi Go-U,

Ok, let's do it!

Last two questions before start coding:

  1. Should we provide this feature in your special method only, with exactly the code proposed in this thread or provide it though cfaSignIn or cfaSignInTwitter with SignInOptions as suggested by you?

In my opinion, we can’t use the already existent method because we will need to change the cfaSignIn and cfaSignInTwitter return type from Observable<firebase.User> to Observable<firebase.User> | Observable<{user: firebase.User, credentials: {idToken: string, secret: string}}>

  1. If we choose the special method, do you have a suggested name for that?

Best regards,
Bernardo Baumblatt.

@go-u
Copy link
Author

go-u commented Oct 2, 2019

Hi baumblatt, thanks your reply !

Ok, let's do it!

Thank you! Super great news!

Should we provide this feature in your special method only, with exactly the code proposed in this thread or provide it though cfaSignIn or cfaSignInTwitter with SignInOptions as suggested by you?

I am not a professional programmer.
I am not familiar with TypeScript, so think of this as a little reference.

plug-in users use case

  • Twitter is strong in some countries.
  • Developers first create an app prototype specifically for Twitter. Dedicated functions to accelerate share in Twitter (use user's token / secret)
  • Developers want to add Facebook login and share functionality for FB once they are successful with a prototype for Twitter.

Implementation points for Twitter API and Facebook API

Twitter (I have experienced myself)

Need user's access token and secret included in TwitterSignInResult

Facebook

Need Only access token included in FacebookSignInResult.
( Early Airbnb and Uber worked with Facebook to read face photos, etc., and increased the mutual trust of users in the service. Globally, FB integration will be more important than Twitter integration. )

Key points

When developing dedicated functions using Twitter or Facebook APIs within the app, separate implementations are required for each API.

Conclusion (if I choose one of the dedicated methods or options)

Developers need to implement separate code for Facebook and Twitter APIs.
Therefore, even if it is provided as a dedicated method or provided as an option of the existing method, it seems that there is no big difference for developers using this plugin.
(The only difference is whether conditional branching is a little faster or slower)

If that is the case, I think that the convenience of the person developing the Plugin is important.
I think that it should be considered in terms of ease of code implementation and ease of maintenance of the code in the future.

In terms of ease of code maintenance, I think it is best to implement it in an optional form.
I have little knowledge of TypeScript, so I believe in judgment based on your experience.

Supplementary conclusion (if there is a third option)

The Raw Mode : Returning a SignInResult itself

As the direction of the current story, I think that what is returned is tokens.

Specifically, the contents are as follows.

  • In case of Twitter, user's Access Token and Secret
  • For Facebook, only user's Access Token

How about the idea of ​​returning a SignInResult itself containing these Tokens as they are?

The SignInResult type is further divided into TwitterSignInResult etc.

Looking at the JAVA code of the current TwitterSignInResult part, only idToken and secret are included.

For example, additionalUserInfo that Firebase returned is not included.
So returning SignInResult itself is doesn't make sense at this time.

However, future development will be easier.
For example, in case we need to get more information than just tokens.

In other words, this is a Raw Mode.
Raw mode is often seen as a method option.

It seems that it is easy for users to understand that there is raw mode.
Raw mode can receiving the SignInResult itself, and user can use raw data as they wish.

The idea is that those who need additional information, such as tokens, can use raw mode.

Method name

If we choose the special method, do you have a suggested name for that?

In case of token
cfaSignInGetToken

In case of raw mode-like
CfaSignInRawMode

To be honest, both names seem ugly. That may be a sign that these should be implemented as options.

Thanks !

baumblatt added a commit that referenced this issue Oct 11, 2019
baumblatt added a commit that referenced this issue Oct 11, 2019
@go-u
Copy link
Author

go-u commented Oct 14, 2019

Hi baumblatt, thank you very much for adding the code!

Currently trying out new code. New code works great !!

But small import error occur.
The import part looks something like this. I'm using javascript. Not typescript.

import { cfaSignInTwitter } from 'capacitor-firebase-auth/alternative'

And error is this.

This dependency was not found:
* capacitor-firebase-auth/alternative in ./src/store/auth/actions.js
To install it, you can run: npm install --save capacitor-firebase-auth/alternative

Module not found: Error: Can't resolve 'capacitor-firebase-auth/alternative' in......

And I tried above notice command npm install --save capacitor-firebase-auth/alternative.
Then, this message shown....

npm ERR! Error while executing:
npm ERR! /usr/bin/git ls-remote -h -t ssh://git@github.com/capacitor-firebase-auth/alternative.git
npm ERR!
npm ERR! ERROR: Repository not found.
npm ERR! fatal: Could not read from remote repository.
npm ERR!
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.
npm ERR!
npm ERR! exited with error code: 128

And I change import part like this.
import { cfaSignInTwitter } from 'capacitor-firebase-auth/dist/esm/alternative'

Then, no error. Your plugin works perfectly !!
So, I want to close this issue later.

But do you need more information on the above import errors?
Or do I need to close this issue thread as soon as possible?

Finally, thank you again!

@baumblatt
Copy link
Owner

baumblatt commented Oct 14, 2019 via email

@baumblatt
Copy link
Owner

Hi Go-u,

The version 0.2.4 is out there with a little typo fixed that maybe solve the import issue.

I looking forward to hearing about your tests.

Best regards,
Bernardo Baumblatt.

@go-u
Copy link
Author

go-u commented Oct 21, 2019

Hi baumblatt, thank you for your update !

The update works almost perfectly! (Tried 0.2.4 from npm)
The import error has been resolved!

However, a very small problem remains. (Or is this my mistake?)
I think that the return values of "userCredential" and "result" are swapped.
Details are below.

My code

import { cfaSignOut } from 'capacitor-firebase-auth'
import { cfaSignInTwitter } from 'capacitor-firebase-auth/alternative'

cfaSignIn('twitter.com')
  .subscribe(({ userCredential, result }) => {
    console.log(userCredential)
    console.log(result)
  })

Then, console.log(userCredential) displayed this
( containing additionalUserInfo, so this is "result", not credential.)

 {user: Q, credential: sg, additionalUserInfo: dg, operationType: "signIn"}

Then, console.log(result) displayed this
( containing token and secret, so this is "credential", not result. )

  {callbackId: "89xxxxxx", providerId: "twitter.com", idToken: "100000000000000000-axxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", secret: "abcxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"}

I tried both cfaSignIn ('twitter.com') method and cfaSignInTwitter method.
I also changed the order of the return values, but the values remained reversed.

Since version 0.2.4 is not released on Github, I could not find out the cause.
Maybe my usage is bad?

Thanks !

@go-u
Copy link
Author

go-u commented Oct 21, 2019

This is a postscript

I'm sorry. In Firebase, I found that the following is officially called UserCredential.

   type UserCredential = {
     additionalUserInfo ?: firebase.auth.AdditionalUserInfo | null;
     credential: firebase.auth.AuthCredential | null;
     operationType ?: string | null;
     user: firebase.User | null;
   };

So the above comment was my mistake. I'm sorry.
Your code is Perfect !!!

I will close this issue as soon as version 0.2.4 is added on Github.
Thank you for the wonderful update !

baumblatt added a commit that referenced this issue Oct 21, 2019
@baumblatt
Copy link
Owner

Hi Go-U,

Sorry about, I forgot to push the last changes. Now, it’s all there.

Best regards,
Bernardo Baumblatt.

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

No branches or pull requests

2 participants