Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

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
Member

@shana 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
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

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

Andreia Gaita and others added 4 commits July 30, 2018 16:37
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 Compare July 30, 2018 14:46
@StanleyGoldman StanleyGoldman dismissed their stale review August 1, 2018 16:37

Email authentication works

@StanleyGoldman StanleyGoldman merged commit 60f8233 into master Aug 1, 2018
@StanleyGoldman StanleyGoldman deleted the fix-authentication-again branch August 1, 2018 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants