We’re using Discourse at my company for internal discussion; only employees are allowed to access it. Currently I’m using OAuth2 authentication via Box with an email domain whitelist to ensure only employees can log in, and that’s working fairly well because for most (or maybe all) of our employees, our company is the reason they have a Box account, so their company email is their primary email in their Box account.
However, we recently tried to enable GitHub authentication, and that’s a different story. It doesn’t work well in combination with the email domain whitelist, because for some (or maybe most) of our employees, they have a single GitHub account that they use for all their various contexts (personal use, various organizations) — so their affiliation with our GitHub organization is just one aspect of their GitHub account, and their company email address is just one of maybe a few addresses in their email account, and frequently not the primary.
In that case, if the domain whitelist is enabled, then those employees can’t login to our site via GitHub, because the GitHub integration only retrieves and uses their primary email address from their GitHub account, and that frequently conflicts with the whitelist.
It’d be great to possibly enhance the GitHub integration to support this case a little better.
I can think of two ways to approach this:
Retrieve all the verified email addresses from the user’s GitHub account and display them all in a dropdown in the registration form and allow the user to choose which one they want associated with their Discourse account
Allow me to specify the ID of our GitHub organization in settings, and have the integration verify that the user is indeed a member of that organization, and if so, cool, but if not, then block them. In this case I’d just disable the email domain whitelist.
But of course I’m sure there are other possible approaches.
I have a very similar situation: we have been using a Discourse SSO for authentication, but upstream sources aren’t nearly as reliable as GitHub. I have a “GitHub with Email whitelist” plugin that bridges the gap between the current GithubAuthenticator and the current email whitelist. Now that it’s 95% complete, I’m rethinking the admin experience and wondering whether I should simply submit the changes as a PR for the core Discourse GithubAuthenticator.
Rationale:
With the current existing builtin implementation, if you enable GitHub logins and also set an email whitelist, new logins/signups will often appear to work, only to be followed by failure and an inability to fix the problem. Immediately after you authenticate with GitHub (as a new Discourse user), you see:
… but the “Email” field is not editable, and you’re stuck.
Having multiple email addresses is “native” to GitHub, so there’s no reason not to automatically/always make use of these when the email whitelist is set.
If “GitHub with email whitelist” is a separate plugin, the admin has to specifically disable the builtin GitHub login, or else you’ll end up with two “with GitHub” buttons on the login screen. True, in this case you’ve specifically configured Discourse to use the plugin, so you should know what you’re doing, but it still feels weird. (I’ve started looking into making the plugin simply alter the builtin auth’s handling, but I’m not savvy enough with Ruby to get this working yet.)
So… @erlend_sh (and team), what say you? Should I keep the email-whitelist-savvy GitHub logic isolated to a plugin, or is this something that makes sense in the builtin GitHub authenticator? (And if you’d prefer the former, I’d love it if someone could help me figure out the right incantation to make the “alter the behavior of the builtin authenticator when the plugin is enabled” thing work… I just haven’t figured out how to use add_class_method/define_singelton_method to hook/replace the builtin’s after_authenticate implementation.)
That’s exactly what my currently-a-plugin-but-feels-awkward does… preference goes to the first email address returned by GitHub that passes the EmailValidator check.
I’ll spend a teeny bit more time seeing if I can’t find a way to make this a plugin that simply co-opts the existing GitHub after_authenticate step, but if I can’t figure that out I’ll just make it a PR on discourse proper.
