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.

Sign up for GitHub

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

Jump to bottom

Fixes for Twitter and Google login on Android #121

Closed
wants to merge 4 commits into from
Closed

Fixes for Twitter and Google login on Android #121

wants to merge 4 commits into from

Conversation

gvillenave
Copy link

No description provided.

@gvillenave
Fix scope string format
a0dd695 
According to the OAuth spec, the scopes have to be in a space separated string. Fixing this resolves failing Google login.
@gvillenave
Fix OAuth token parsing and add token secret
db7ae5e 
The underlying ScribeJava library already does the parsing of the OAuth tokens, so just use that instead of relying on JSON parsing (especially when the response is not JSON!). Also add the access token secret in the response since it's needed for Twitter.
@gvillenave gvillenave mentioned this pull request Jun 13, 2017
Open
@gvillenave
Fix for OAuth 2 scopes
8b3f862 
Scopes should be space separated, not comma
@kyle-ssg
Copy link

kyle-ssg commented Aug 6, 2017

Any chance of this getting merged ? Android twitter auth basically doesn't work without this from what I can see.

@stewartlord
Copy link

👍 this should be merged!

@hanalaydrus
Copy link

hanalaydrus commented Aug 15, 2017 •
edited
Loading

@gvillenave would you mind fixing the conflicts? I hope this will be merged after the conflicts solved.

@gvillenave
Copy link
Author

Done!

@f4z3k4s
Copy link

f4z3k4s commented Sep 4, 2017

This definitely should be merged. Although, line 410: String oauthTokenSecret = (String) accessTokenMap.get("oauth_token_secret"); in android/src/main/java/io/fullstack/oauth/OAuthManagerModule.java should also be dealt with. It casues me a build error(error: cannot find symbol pointing to this line) in case of android release builds, rebased to package version 2.2.0. Apart from that, Twitter finally redirects me to my app. Thanks.

@vvnull vvnull mentioned this pull request Sep 5, 2017
Open
@desktp
Copy link

desktp commented Sep 6, 2017

This also fixes crashing on logging in with GitHub. +1 for merging!

@desktp desktp mentioned this pull request Sep 6, 2017
Open
@joncursi
Copy link

joncursi commented Oct 3, 2017

The only issue I see with this is that the location of the authorized key is different on iOS than android. On iOS, you have to check:

if (accessTokenData.response.authorized)

whereas on android, you have to use:

if (accessTokenData.authorized)

The API should be the same across these platforms.

SailingSteve
SailingSteve reviewed Oct 3, 2017
View reviewed changes
Copy link
Contributor

@SailingSteve SailingSteve left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Line 410 of OAuthManagerModule needs to be changed to
String oauthTokenSecret = (String) accessToken.getParameter("token_secret");

It is working for me on Android after that one change.

@MBach
Copy link

MBach commented Nov 16, 2017

Nice catch @SailingSteve
This PR should really be merged into master!

@alejandrosanabriav
Copy link

alejandrosanabriav commented Nov 23, 2017 •
edited
Loading

Please merge this, or please maybe if you doesn't has the time to maintain this package pass it to some one how can like react-native-community or some contributor... Thanks for all the effort. I said cause I doesn't seem changes for 4 months ago.

@SailingSteve SailingSteve mentioned this pull request Nov 27, 2017
Merged
@negebauer
Copy link

This should also be deleted

https://github.com/gvillenave/react-native-oauth/blob/4f34e87e99863ba04fe0a11646712ef2b16a28f2/android/src/main/java/io/fullstack/oauth/OAuthManagerModule.java#L410

And also this

https://github.com/gvillenave/react-native-oauth/blob/4f34e87e99863ba04fe0a11646712ef2b16a28f2/android/src/main/java/io/fullstack/oauth/OAuthManagerModule.java#L421

negebauer pushed a commit to negebauer/push-for-github-mobile that referenced this pull request Jul 19, 2018
feat: oauth
30de5c0 
TODO: create fork from react-native-oauth with fixes
- fullstackreact/react-native-oauth#121
- fullstackreact/react-native-oauth#157
- fullstackreact/react-native-oauth#158
- fullstackreact/react-native-oauth#208 ^
@negebauer negebauer mentioned this pull request Jul 23, 2018
Closed
@gvillenave gvillenave closed this by deleting the head repository Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SailingSteve SailingSteve SailingSteve left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.