Logging into github breaks dartpad (possible race condition?)

[jakemac53]

trafficstars

What happened?

I noticed that dartpad now has github integration which is great, so I attempted to log in. When I clicked the button to log in, it eventually refreshed the page and showed an error that the "gists scope had not been requested" or something along those lines.

The UI for dartpad was also broken, and I could no longer run any dartpad scripts.

I saw this error in the console as well:

playground.dart.js:455 Uncaught Exception: ERROR - decryptAuthTokenFromReturnedSecureAuthToken() general decryption exception.
    at Object.a (https://dartpad.dev/scripts/playground.dart.js:455:3)
    at pg.om (https://dartpad.dev/scripts/playground.dart.js:13770:100)
    at pg.lk (https://dartpad.dev/scripts/playground.dart.js:13557:17)
    at https://dartpad.dev/scripts/playground.dart.js:4171:3
    at w0.a (https://dartpad.dev/scripts/playground.dart.js:1637:63)
    at w0.$2 (https://dartpad.dev/scripts/playground.dart.js:7290:14)
    at vy.$1 (https://dartpad.dev/scripts/playground.dart.js:7284:21)
    at mA.hr (https://dartpad.dev/scripts/playground.dart.js:8182:23)
    at uQ.$0 (https://dartpad.dev/scripts/playground.dart.js:7702:11)
    at Object.fQ (https://dartpad.dev/scripts/playground.dart.js:1719:39)

However, once I went back to dartpad.dev and tried to log in again, it works now (I removed the query params). I think that there is probably some race condition (maybe an unawaited future or something, where the github auth hasn't actually gone through yet?).

Steps to reproduce problem

I can consistently reproduce the error message about the github scopes any time I log in/out of github, but it no longer seems to completely break the UI.

Browser

Browser: Chrome

Version: 102.0.5005.125

Are you using any extensions/plugins that affect website behavior (particularly those that affect iframes, such as ad blockers)? No

Are there any warnings or errors in your browser's JavaScript console? If so, paste them below: (pasted above)

Machine

Operating system: Chrome OS

Version: 102.0.5005.125

[timmaffett]

By switching to the beta or old channel's before selecting the GitHub login menu I was able to reproduce the lack of login completion after returning from the GitHub OAuth screen. (and also that removing the query params and reloading did complete the login).
In the case I what I am seeing I did not see any exceptions, but I did see the query params get garbled with %3F appearing in the url (which is a second '?' that is getting encoded). I will be investigating further and getting to the root of this problem, but I wanted to ask @jakemac53 if there was any chance he was using the non stable channel when he was seeing the problem.

[jakemac53]

I don't know for sure, but I don't generally mess with the channel selection. It currently has stable selected.

However, I think I was on https://dartpad.dartlang.org/, which I noticed then redirects you to https://dartpad.dev/ after logging in. I wonder if that has anything to do with it? I think I go to the former one just because its what chrome auto completes for me, I would expect it to always redirect to https://dartpad.dev/ though

[timmaffett]

@domesticmouse You set up the OAuth apps for the GitHub login support right ? I was wondering if for the beta, dev and old channel versions if it was possible that you set the return url's to include the '?channel=XXX' appended onto the end ? Like 'https://dartpad.dev?channel=beta' instead of 'https://dartpad.dev/'

The reason I am asking is that it appears GitHub always appends '?gh=XYZ&scope=gist' to the URL.. it appears that when using the alternate channels (other than stable) we get back to dartpad with garbled URLs, like https://dartpad.dev/?channel=beta?gh=XYZ&scope=gist

Within dartpad it gets the 'channel' query parameter being 'beta?gh=XYZ&scope=gist' - it then sees no 'gh' or 'scope' parameters, so it does not even see that it is returning from the GitHub OAuth..

Appending the channel information is not necessary because the GitHub login code automatically saves all query parameters before going to GitHub - and then restores them on return - so the channel information will be restored that way. The GitHub OAuth code does not appear to be smart when appending query parameters to a URL that already contains query parameters. All four dartpad OAuth endpoints should have the same 'https://dartpad.dev' set for the return URL. (And @domesticmouse it is possible that you already DO have all of the return url's as 'https://dartpad.dev' and the garbling is happening with dartpad code - it is a little tricky here because I am debugging with an instrumented localhost version, but the redirect happens to dartpad.dev and I manually have to change the url to localhost:8000. This question stems from my best guess looking at the info I have so far.)

[timmaffett]

@jakemac53 - GitHub requires the OAuth endpoints to return to a 'static' url (to which is appends the gh and scope query parameter string. I am storing current URL state information (before login) into localstorage - I did not ever consider the case of starting from 'https://dartpad.dartlang.org'. I am imagining that in this case the pre-login information is being stored in the local storage for https://dartpad.dartlang.org and then upon returning to https://dartpad.dev it will not be able to find the pre-login state information.. I will look into this case - I believe I can fix this case by having the code change to https://dartpad.dev before storing the state information, and then proceeding with the login, so then the expected information would be found in localstorage on the return from GitHub OAuth.

This is a great bug (or couple bugs?) Thank you @jakemac53 for find and reporting this so it can be fixed!

[jakemac53]

Fwiw I think it would make a lot of sense to just have dartpad.dartlang.org always redirect to dartpad.dev, which would solve this problem with conflicting domains? It also can hurt SEO and other things to have both around.

This is a great bug (or couple bugs?) Thank you @jakemac53 for find and reporting this so it can be fixed!

Of course!

[domesticmouse]

I think it would make a lot of sense to just have dartpad.dartlang.org always redirect to dartpad.dev

Hey @mit-mit, can you point me at the dartlang.org domain file? I think redirecting dartpad.dartlang.org to dartpad.dev at DNS lookup probably is the easiest fix here.

[domesticmouse]

@domesticmouse You set up the OAuth apps for the GitHub login support right ? I was wondering if for the beta, dev and old channel versions if it was possible that you set the return url's to include the '?channel=XXX' appended onto the end ?

Here's Stable DartPad configuration:

And here is Beta DartPad:

How do you want me to update this configuration?

[timmaffett]

@domesticmouse - OK, after digging through the code I realized that the place where we need to fix it is not in the OAuth settings but in the K_GITHUB_OAUTH_RETURN_TO_APP_URL google cloud run environmental variable for each stable/dev/beta/old dart-services. I think these must have the ?channel=XYZ appended onto the K_GITHUB_OAUTH_RETURN_TO_APP_URL urls. If you could check the K_GITHUB_OAUTH_RETURN_TO_APP_URL environmental variable, and see if they have a ?channel=xyz appended to it, and remove it... for the the dev, beta and old channel versions.

Additional Minor suggestion follows: One change that I might suggest for the OAuth settings for each app is to change the application names to lead with 'DartPad' - ie. something like 'DartPad - Beta Channel', 'DartPad - Dev Channel', etc

The reason I mention this is that GitHub presents these in alphabetical order, and it seemed a little more logical to have DartPad under 'D' and grouped together. While debugging this I had to repeatedly search for 4 different places (under Beta DartPad, Dev DartPad, Old DartPad and Stable DartPad to find each version. Again this is minor, but it seems like leading the application name with DartPad will seem more logical and natural to our users. You can just change these in the settings and 'apply' and they change will instantly.

[domesticmouse]

I've updated and redeployed beta, dev and old.

[timmaffett]

Nice! That was fast @domesticmouse ! I have verified that fixes the GitHub logins starting from the non stable channels. (and the updated OAuth application names are nice as well!)

Once the dartpad.dartlang.org redirects to dartpad.dev then I believe that @jakemac53 's original bug will be fixed as well. (I understand that this is a whole different thing, and changing the dns entries will take some time to accomplish, as well as propagate.)

[mit-mit]

@domesticmouse for the DNS question, that's in dartlang/db.dartlang

[domesticmouse]

@mit-mit I'm confused, there is no reference to dartpad.dartlang.org in dartlang/db.dartlang.

[domesticmouse]

Heh, it's worse than initially thought. DartPad serves on the following URLs:

• dartpad.dev (the canonical URL)
• www.dartpad.dev (CNAME)
• try.dartlang.org (CNAME)
• dartpad.dartlang.org (CNAME)

We should probably redirect the CNAMEs in the browser, so as not to lose the Gist ID on the original URL. Or bounce the user to the canonical URL before attempting to log the user into GitHub. I can't see how to solve this in DNS without breaking old links by losing Gist information.

[timmaffett]

@domesticmouse - I had created code that does the bounce to dartpad.dev when the url was not dartpad.dev (when a user selects login to github). I can finish testing this (also from those new urls you list) and create a pull request for you to take a look at.

[timmaffett]

I have submitted https://github.com/dart-lang/dart-pad/pull/2330 which initiates a bounce if we are not sitting at dartpad.dev when a github login is initiated.