search

idvoretskyi / horizon-cloud

An operations service to deploy, manage, and scale Horizon apps
0 stars 0 forks source link

Make Horizon use GitHub IDs instead of UUIDs when it creates new users. #150

Open mlucy opened 8 years ago

mlucy commented 8 years ago

It's extremely helpful for a variety of reasons to always refer to a user by whatever Horizon uses as the id field in the users table. (It makes it easy to assert that a user is allowed to do a particular operation just by checking the JWT they provide, it removes the need to handle the error case where a document references a name that refers to two users, etc.)

The problem with this is that showing UUIDs to people is bad, so whenever a document contains one or more users we need to issue additional queries on the frontend to get their display names. This increases both latency and read load by a lot.

It would also be really really bad if we had two people with different Horizon IDs but the same display name, because they could trick people into letting them onto a project. We could trivially prevent this case if the display name were identical with the Horizon ID.

We need to make some changes to Horizon to implement this (specifically we'll have to move the part of the hzc-web-sync logic that makes a GitHub API request into Horizon), but I think it's worth it.

For now I'm just going to write everything using the UUIDs in anticipation of the switch.

dalanmiller commented 8 years ago

Just a random 2 cents, but is it wise to tightly couple Horizon Cloud to Github IDs? Not sure if you're using the id as a unique identifier in some way and adding more services would cause collisions or not.

Sooner or later we will want to support both Gitlab and Atlasssian Stash, (also maybe AWS CodeCommit and GCP Cloud Source), Github from my experience, has not been as popular in the enterprise because it is so insanely expensive compared to other offerings.

mlucy commented 8 years ago

I think we want the property that two users can't have the same display name, because someone could pretend to be another user and ask to get added to a project or something. So when we add support for other OAuth providers, we'll need to check for duplicate names and have some sort of flow for resolving it if there's a conflict.