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

Link GitHub identity to existing account #1519

Merged
merged 2 commits into from
Jul 17, 2018

Conversation

outoftime
Copy link
Contributor

@outoftime outoftime commented Jul 11, 2018

Adds a Link GitHub button to the user menu which is available when you are logged into an account that does not have an associated GitHub identity. Clicking this button will initiate a popup-based GitHub account linking flow.

If the account linking is successful, the application will be in a fully valid state, allowing the user to use their GitHub credential to e.g. export gists/repos.

Closes #1514

To Do

  • Load all credentials when logging in to account with multiple identities
  • Load all credentials when logged in at application bootstrap time
  • Error handling for account linking process
  • Tests
  • More nuanced handling of mismatch between linked identities and stored credentials

@outoftime outoftime force-pushed the link-github branch 4 times, most recently from 5c95b1c to 7f02c4f Compare July 17, 2018 02:01
Adds a **Link GitHub** button to the user menu which is available when
you are logged into an account that does not have an associated GitHub
identity. Clicking this button will initiate a popup-based GitHub
account linking flow.

If the account linking is successful, the application will be in a fully
valid state, allowing the user to use their GitHub credential to e.g.
export gists/repos.

However, while the GitHub credentials _are_ saved to Firebase, they’re
not currently loaded properly for future sessions. This affects both
reloading the page (thus inheriting a preexisting Firebase session) and
logging in to an account that already has a linked GitHub identity.

There is also currently no error handling.
Previously, when passing on the results of an authentication-related
event, the Firebase client always exposed an object conforming to the
Firebase `UserCredential` type. This contains exactly one user and one
credential. In many cases the object itself was synthesized using
credentials that we stored ourselves, but the shape of the data was
consistent with what Firebase returns when a user successfully logs in.

However, this only works if the user has only linked a single provider
identity. In the general case, we always want access to all credentials
we have for the user—e.g. if the user logs in with Google, we still want
access to the stored GitHub credential if there is one. Similarly if the
user is initially logged in, logs in in another tab, etc.

So, the client ditches the `UserCredential` interface altogether,
instead always exposing an object whose properties are `user` and
`credentials`. The latter is an array of credential objects. The
various consumers of this data are updated to handle a collection of
credentials.

Note that this also removes the storage of `additionalUserInfo`, i.e.
raw profile data from the identity providers. Previously it was
necessary to go spelunking in there to find a suitable display name for
a user in the case that their account did not have an associated real
name. However, since all user accounts will soon be Google-linked, we
should be able to guarantee a good display name.
@outoftime outoftime merged commit 13e5298 into popcodeorg:master Jul 17, 2018
@outoftime outoftime deleted the link-github branch July 17, 2018 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant