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

metamask-mobile not accepting cookies from github login flow on gitcoin #1273

Closed
owocki opened this issue Jan 13, 2020 · 55 comments
Closed

metamask-mobile not accepting cookies from github login flow on gitcoin #1273

owocki opened this issue Jan 13, 2020 · 55 comments
Labels
browser Related to web browser functionality, e.g. HTTP CORS, bookmarking, etc. dapp-compatibility

Comments

@owocki
Copy link

owocki commented Jan 13, 2020 •
edited

Hi from Gitcoin.

weve got a few reports from users that metamask mobile is not accepting cookies set in the browser from github login flow, preventing users from logging in on gitcoin, and creating a 500 on the Gitcoin login flow.

For example i got this from a user..

Screenshot_20200110-215041_MetaMask (1)

The objective of this bounty is to get Gitcoin login working on metamask mobile

Suggested fix path is to clone metamask-mobile, get it running on your local, and then try to login to gitcoin. The browser will receive a set of cookies/session variables back from Gitcoin.. which are not stored in the same manner that Chrome/Opera store them.. thus causing logins on gitcoin to fail on metamask mobile.

Please only apply to the bounty if you can get this done within 2 weeks; and if you have strong independent debugging skills.
@owocki
Copy link
Author

owocki commented Jan 13, 2020

related : #1244

@csplawn918794
Copy link

Thanks for a Swift professional response.
Same issue as I referenced
Working on getting gas to submit work
Hoping faucet will work

@csplawn918794 csplawn918794 mentioned this issue Jan 21, 2020
Closed
@csplawn918794
Copy link

Referenced metamask Mobile
Error 500

@owocki owocki changed the title metamask not accepting cookies from github login flow on gitcoin metamask-mobile not accepting cookies from github login flow on gitcoin Jan 30, 2020
@owocki owocki mentioned this issue Jan 30, 2020
Closed
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 1.3 ETH (236.23 USD @ $181.72/ETH) attached to it.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Workers have applied to start work.

These users each claimed they can complete the work by 1 month from now.
Please review their action plans below:

1) mul1sh has applied to start work (Funders only: approve worker | reject worker).

This is caused by a long standing bug in react native (facebook/react-native#19958). So there are a couple of available fixes to it and I will be able to get it tested & fixed over the weekend without any delays.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Workers have applied to start work.

These users each claimed they can complete the work by 3 weeks, 4 days from now.
Please review their action plans below:

1) mul1sh has applied to start work (Funders only: approve worker | reject worker).

This is caused by a long standing bug in react native (facebook/react-native#19958). So there are a couple of available fixes to it and I will be able to get it tested & fixed over the weekend without any delays.
2) adriancova has applied to start work (Funders only: approve worker | reject worker).

I've done some research on the problem and believe I'm fully capable of solving it the terms you're requiring. I'll get the project running on my local machine and will reproduce and fix the bug, testing it in both virtual and physical devices Android and iOS. Timewise since It'd be the only bounty I would be working on and I'm pretty sure I can get it done by next monday, but will keep you informed of any important development or setback I may have.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 3 weeks, 3 days from now.
Please review their action plans below:

1) mul1sh has been approved to start work.

This is caused by a long standing bug in react native (facebook/react-native#19958). So there are a couple of available fixes to it and I will be able to get it tested & fixed over the weekend without any delays.
2) adriancova has applied to start work (Funders only: approve worker | reject worker).

I've done some research on the problem and believe I'm fully capable of solving it the terms you're requiring. I'll get the project running on my local machine and will reproduce and fix the bug, testing it in both virtual and physical devices Android and iOS. Timewise since It'd be the only bounty I would be working on and I'm pretty sure I can get it done by next monday, but will keep you informed of any important development or setback I may have.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

@mul1sh Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@csplawn918794
Copy link

csplawn918794 commented Feb 7, 2020 via email

@mul1sh
Copy link

mul1sh commented Feb 9, 2020

@csplawn918794 so im I, want to collab on this or something.

@gkapkowski
Copy link

Please take a look at my investigations in #1280, metamask webview code is doing duplicate requests where one request does not contain all headers (eg. cookies). In my case (also oauth) this behavior is expiring one time token from url but is rejected because of lack of cookies. The second request containing cookies previously set by the server is rejected because one time token from url is already expired/used.
This might not be the root cause but it's definitely interfering with oauth flows.

@rekmarks rekmarks added the browser Related to web browser functionality, e.g. HTTP CORS, bookmarking, etc. label Feb 11, 2020
@mul1sh
Copy link

mul1sh commented Feb 12, 2020

@gkapkowski Great observation, thanks.

Though still for gitcoin, the cookies are set server-side by django but for some reason the react-native webview is not storing them, or atleast the current version of react-native used by metamask is not setting them. Some earlier and later versions of react-native are working fine, which is weird.

Anyway thanks for the heads up, appreciate it.

@mul1sh mul1sh mentioned this issue Feb 15, 2020
Closed
3 tasks
@mul1sh
Copy link

mul1sh commented Feb 21, 2020

@gkapkowski just confirm what path does your site store the cookies in? the root path?

@gkapkowski
Copy link

@mul1sh Yes, they are in root / path

@mul1sh
Copy link

mul1sh commented Feb 24, 2020

@gkapkowski ok thanks, funny thing is that I can get metamask mobile to now work with any site, so long as the cookies are set in any other path apart from the root, which is weird. Any ideas what might cause this?

@gkapkowski
Copy link

@mul1sh Weird, no, haven't encountered any code that would indicate this behavior.

@mul1sh
Copy link

mul1sh commented Feb 27, 2020 •
edited

Hmm ok, i'm still getting this behavior with my local build after adding the react-native-cookies lib so i'm investigating it further.

@mul1sh
Copy link

mul1sh commented Mar 9, 2020 •
edited

Final update after testing on many different versions of react-native, i'll use the react-native-cookies lib to sort this issue. It seems to be the most straightforward solution, even in RN versions that support cookies well right out of the box.

So wrapping this up in the next few days 🙂

@owocki
Copy link
Author

owocki commented Mar 9, 2020 via email

@mul1sh
Copy link

mul1sh commented Mar 18, 2020

Almost done with iOS just a bit..

@owocki
Copy link
Author

owocki commented Mar 18, 2020 via email

@mul1sh
Copy link

mul1sh commented Mar 21, 2020

iOS is done, testing my changes again and then this will be finished

@jacintogonzalez
Copy link

Almost there with the Android X fix...

Looking forward to the new beta release.

@owocki
Copy link
Author

owocki commented Apr 22, 2020

hi from gitcoin - we get emails about this every week. how goes with the android fix?

@mul1sh
Copy link

mul1sh commented Apr 25, 2020

@owocki Sorry for the delay, the android fix is coming long great. Kindly give me till Tuesday i'll have this working + an apk too.

@owocki
Copy link
Author

owocki commented Apr 26, 2020

@mul1sh thanks! pls keep me posted

@jacintogonzalez
Copy link

@mul1sh Hello! I see that version 0.2.15 has been released today, but we still have a problem logging in. Other wallets have no such problem.

@octavioamu
Copy link

@mul1sh why you don't put a WIP pr? that can be helpful.

@guardian939
Copy link 

@gitcoinbot

@guardian939
Copy link

https://t.me/greennetvpn/65

@guardian939
Copy link

@gitcoinbot

@guardian939
Copy link

app = ::firebase::App::Create(::firebase::AppOptions(), jni_env, activity);

@owocki
Copy link
Author

owocki commented Sep 8, 2020

@mul1sh did this ever get done? i think i may cancel the bounty if not; as it doesnt seem to be going anywhere

@mul1sh
Copy link

mul1sh commented Sep 8, 2020

@owocki yes I kinda had it working on my end like 3 months ago but to be honest I got sidetracked alot with other task/issues so I did not fully wrap it up.

Nonetheless I'll resume working on it from the start of this coming week and have it ready by the end of the week with a testable apk and also do a WIP PR as I work on it.

@owocki
Copy link
Author

owocki commented Sep 8, 2020 via email

@mul1sh
Copy link

mul1sh commented Sep 9, 2020

@owocki definitely will alert you and thanks for your patience 🙂

@mul1sh mul1sh mentioned this issue Sep 21, 2020
Closed
1 task
@mul1sh mul1sh mentioned this issue Oct 12, 2020
Closed
@deal27
Copy link

deal27 commented Nov 22, 2020

CODE_OF_CONDUCT.md``

@mul1sh
Copy link

mul1sh commented Nov 24, 2020

@owocki an update, this issue was automatically fixed when the app was updated to use the official version of the react-native-webview dependency instead of the forked version. The forked version that was being used before had a bug in the handling of SSL and also setting the session cookies, but the bug was already fixed in the official repo, sometime in July, I think.

I've been testing this thoroughly for the past 1 week and needless to say, my workaround which involved using the react-native-cookies lib to force the cookies to be saved is not needed anymore.

So i'm sure this issue can now be closed, unless of course another regression occurs.

@owocki
Copy link
Author

owocki commented Nov 24, 2020 via email

@mul1sh
Copy link

mul1sh commented Nov 25, 2020

@owocki i think the latest version is out, i've been testing with the google play version all day and so far no issues 🙂

@ibrahimtaveras00
Copy link
Contributor

Seems like this no longer is an issue, correct @owocki and @mul1sh?

@ibrahimtaveras00
Copy link
Contributor

This can be reopened if issue arises again

@mul1sh
Copy link

mul1sh commented Dec 18, 2020

@ibrahimtaveras00 yes correct i've tested thoroughly, this is no longer an issue as far as I can tell.

@x0m-i0x
Copy link

x0m-i0x commented Jan 10, 2021

Privacy Badger (www.eff.org/privacybadger) is a browser extension that automatically learns to block invisible trackers. Privacy Badger is made by the Electronic Frontier Foundation, a nonprofit that fights for your rights online.

Privacy Badger blocked 2 potential trackers on medium.com:

app.link
www.google-analytics.com

@owocki
Copy link
Author

owocki commented Jan 15, 2021

im going to close this bounty now since its already fixed on metamask side. if you feel you contributed to it being fixed, then pls DM + i can give u some ETH

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Cancelled

The funding of 1.3 ETH (1544.84 USD @ $1188.33/ETH) attached to this issue has been cancelled by the bounty submitter

@x0m-i0x
Copy link

x0m-i0x commented Jan 17, 2021 via email

@voanhcung
Copy link

0xB98ceE834EF70C4F67CD5a7dCdBa89ed4948e0c5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
browser Related to web browser functionality, e.g. HTTP CORS, bookmarking, etc. dapp-compatibility
Projects
None yet
Milestone
No milestone