{"action":"created","issue":{"url":"https://api.github.com/repos/getsentry/freight/issues/25","labels_url":"https://api.github.com/repos/getsentry/freight/issues/25/labels{/name}","comments_url":"https://api.github.com/repos/getsentry/freight/issues/25/comments","events_url":"https://api.github.com/repos/getsentry/freight/issues/25/events","html_url":"https://github.com/getsentry/freight/pull/25","id":93033304,"number":25,"title":"GitHub authentication","user":{"login":"nickbruun","id":336765,"avatar_url":"https://avatars.githubusercontent.com/u/336765?v=3","gravatar_id":"","url":"https://api.github.com/users/nickbruun","html_url":"https://github.com/nickbruun","followers_url":"https://api.github.com/users/nickbruun/followers","following_url":"https://api.github.com/users/nickbruun/following{/other_user}","gists_url":"https://api.github.com/users/nickbruun/gists{/gist_id}","starred_url":"https://api.github.com/users/nickbruun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickbruun/subscriptions","organizations_url":"https://api.github.com/users/nickbruun/orgs","repos_url":"https://api.github.com/users/nickbruun/repos","events_url":"https://api.github.com/users/nickbruun/events{/privacy}","received_events_url":"https://api.github.com/users/nickbruun/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-07-04T17:41:53Z","updated_at":"2015-07-07T08:50:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/getsentry/freight/pulls/25","html_url":"https://github.com/getsentry/freight/pull/25","diff_url":"https://github.com/getsentry/freight/pull/25.diff","patch_url":"https://github.com/getsentry/freight/pull/25.patch"},"body":"We rely on GitHub authentication for most developer-related services, so I thought it would be nice if Freight also support it (great job, btw!) This pull request in its current form adds rudimentary support for GitHub authentication, limiting the user to either a specific team ID or organization ID, depending on the granularity desired.\r\n\r\nTo select the authentication backend used, I've added a configuration variable `AUTH_BACKEND` which defaults to `google` for Google authentication, and can also be `github` for Github authentication. I've also added some initialization time checks for the validity of the configuration of `AUTH_BACKEND` and Github authentication-specific configuration variables. For now, I've defaulted to `RuntimeError`, but I don't know if you have a preferred exception in this case.\r\n\r\nI've also subclassed `OAuth2WebServerFlow` for the case of GitHub, as fetching both the user and team after the code/access token exchange has completed, is necessary. Lastly, I've added a check after the exchange to make sure that the user is limited to whatever team or organization is specified.\r\n\r\nA few notes/questions:\r\n\r\n- GitHub's only properly stable user identifier is the user ID. Right now, I've continued to use e-mail addresses as the identification, but in an ideal world, one would rely on the user ID. What are your thoughts on this? One possibility I can think of is to add a table with many-to-one relationship to `User` for a hypothetical number of authentication backends.\r\n- Given the different checks, I'd propose a basic class-based abstraction of an authentication backend. What are your thoughts on this? Mostly, these backends would abstract the OAuth flow but also perform access checks (domain check in the case of Google, or team/organization check in the case of GitHub) but would be a good structural component in my opinion. I'd gladly add that as part of this PR if you agree.\r\n\r\nI purposefully haven't done any documentation updates yet, so consider this PR a \"for your consideration\" at the moment. I'll do proper test coverage and documentation once the details are settled, of course :)\r\n\r\nLet me know what you think!"},"comment":{"url":"https://api.github.com/repos/getsentry/freight/issues/comments/119126500","html_url":"https://github.com/getsentry/freight/pull/25#issuecomment-119126500","issue_url":"https://api.github.com/repos/getsentry/freight/issues/25","id":119126500,"user":{"login":"nickbruun","id":336765,"avatar_url":"https://avatars.githubusercontent.com/u/336765?v=3","gravatar_id":"","url":"https://api.github.com/users/nickbruun","html_url":"https://github.com/nickbruun","followers_url":"https://api.github.com/users/nickbruun/followers","following_url":"https://api.github.com/users/nickbruun/following{/other_user}","gists_url":"https://api.github.com/users/nickbruun/gists{/gist_id}","starred_url":"https://api.github.com/users/nickbruun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickbruun/subscriptions","organizations_url":"https://api.github.com/users/nickbruun/orgs","repos_url":"https://api.github.com/users/nickbruun/repos","events_url":"https://api.github.com/users/nickbruun/events{/privacy}","received_events_url":"https://api.github.com/users/nickbruun/received_events","type":"User","site_admin":false},"created_at":"2015-07-07T08:50:16Z","updated_at":"2015-07-07T08:50:16Z","body":"Alright, abstracted the providers. Let me know, what you think. If we're good, I'll add a bunch of tests to make sure everything works as expected."}}