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

Fix authentication again #873

Merged
merged 7 commits into from Aug 1, 2018
Merged

Fix authentication again #873

merged 7 commits into from Aug 1, 2018

Conversation

@shana
Copy link
Contributor

shana commented Jul 19, 2018

While spelunking in the code I noticed that there were a bunch of problems with the authentication:

  • Account dropdown not showing if the repository is not initialized
  • We were blindly taking the first connection on the list to sign out instead of the connection that matches the repository that we're on (which might explain why it sometimes failed)
  • There were a bunch of subtle mismatches between what the connection file had and what we were looking for (which was reported before but never really fixed properly - like the file having "github.com" for the host and we looking up "https://github.com" (and failing))

I tried to fix these as I found them and simplify the code, especially around running octorun and setting environment variables there (the user is never used by octorun so I killed that).

I was also annoyed by the fact that zip does not create deterministic zips - i.e. the exact same content produces different zip files because zip stores the file timestamps (including access time ???), and those can be anything, so I added a packaging submodule with a typescript thing to produce deterministic zips. I haven't tested this particular step on Windows yet - it only requires node to be installed in the system and nothing else, so it should work fine?

Copy link
Member

StanleyGoldman left a comment

Last I tested this did not work when I attempted to login with my email address.

Andreia Gaita and others added 4 commits Jul 18, 2018
Fix signin/signout not available if there's no repo. Fix signout of the
active connection. Fix mismatches between what's in the connections file
to what we're trying to signout or load the keychain for.
- Make sure we save the token before verifying the token,
	and save the new username after verifying it
- Minimize how many times we query the cred manager
@shana shana force-pushed the fix-authentication-again branch from 6eb9dfa to 43828e6 Jul 30, 2018
shana and others added 2 commits Jul 30, 2018
@StanleyGoldman StanleyGoldman dismissed their stale review Aug 1, 2018

Email authentication works

@StanleyGoldman StanleyGoldman merged commit 60f8233 into master Aug 1, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@StanleyGoldman StanleyGoldman deleted the fix-authentication-again branch Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.