Switching from GitHub auth to OAuth2


#1

Hey folks, we’re running hosted Discourse using only GitHub authentication. We’ve successfully added functionality to our own platform to delegate authentication to our system using OAuth2, and our goal is to switch from GitHub auth to OAuth2, and hoped that users logging in using OAuth2 with the same email address that’s associated with an existing account would be logged into that account.

My testing in a self-hosted Discourse so far goes like this. I’ve got a few accounts with the same email address in both GitHub and in our external system:

  1. I create an account in our test instance using GitHub authentication.
  2. sign out of that account in the test instance
  3. attempt to log in using OAuth2 and an account in our system with the same email address

For these accounts when I perform step 3 I’m prompted to create a new account instead of being logged into the existing account, and if I try to complete that process I’m told the email already exists (as one would expect). Here’s a screenshot of one account combination:

The login attempt is on the right; on the left is the admin view of the corresponding account.

Oddly, with another account (the initial admin account) I can log in using GitHub or OAuth2 interchangeably without any difficulty.

What are the correct expectations for this functionality? Should these accounts be recognized as the same? Is there somewhere in the database that I can look to see why some accounts work as hoped and others do not? Is there a mechanism or configuration that I’m missing that will make this work as hoped? Any guidance appreciated!


(David Taylor) #2

Have you enabled the “oauth2_email_verified” setting? If not, then Discourse won’t trust the email addresses from the OAuth provider, and will display the create account dialog instead.


#3

Yep we did find that setting:

All of the testing I listed was performed with that setting on.


(David Taylor) #4

It is surprising that the error message is “Please enter a valid email address”. That suggests that the email address is invalid, rather than matching an existing account.

To debug, I would suggest comparing the email address in the OAuth2 debug logs, to the emails in the database. If it works for some emails, but not others, it suggests that there is something different about the email addresses. Maybe some unusual special character, a leading/trailing space, or something like that.

Please feel free to contact us directly if you’d like help with this on your discourse.org hosted site.


#5

Thanks for the reply!

Sorry, I think that email address error must have been an error from a previous iteration of testing. After a hard refresh and another attempt, this is what I see (and have seen in other cases):

Note that the username is automagically-appended with a number to make it into an available one.

Here’s what I see when I query the database using the email address from the OAuth payload from the logs pane:

So I feel fairly confident that the email address is correct and matches in the important particulars. That’s the correct username for the account I expect to log in as, for instance, and the user_id also matches.

Interestingly, here’s what I see when I grab the OAuth payload email for an account that works as expected:

So the token in the OAuth payload from that account made it into the database just fine at some point, but the token returned for the failing user ins’t making it into the database.

Thanks also for the offer for help on our hosted instance. However, before we turn this on in that instance I want to make sure I understand what to expect and whether the solution we’re hoping to leverage is actually going to solve the problem we want to solve. I’d like to give our community members some forewarning and if achieving our goals of having everyone authenticate on our platform means that we have to merge ~1000+ accounts that’s probably not going to work (especially since we don’t have access to the command-line-only merge tool).

Anything else I can look at? If it would be easier to puzzle out I can set up a hosted trial and configure it there - I’m using self-hosted to avoid having a trial expire (and because I hope to do some work on the Slack plugin, for which a stand-alone instance seems required).


(David Taylor) #6

The logic for matching by email address can be found here:

You can access mappings from oauth2 “UID” to discourse accounts using the query

SELECT * from plugin_store_rows where plugin_name = "oauth2_basic"

(I tend to use the data explorer plugin, but a raw psql shell will also work)

It’s also worth checking the email addresses which are being interpreted by the OAuth plugin. You can check the verbose logs at <your-domain>/logs.


#7

Ok, I think I see where the problem is, at least for some of these records; my test plan has been:

  1. sign up in Discourse with new users from the OAuth process in each of 3 different scenarios (our platform has a few different signin scenarios that I wanted to test)
  2. delete those users using the Admin UI
  3. Use the same platform accounts, sign up for new users in Discourse using GitHub logins for each of the three accounts
  4. Log out and attempt to sign into those accounts using the OAuth process

Steps 3-4 were intended to replicate logging into existing (GitHub-based) Discourse account using the matching accounts from our platform - the migration path we’re hoping will work.

I think the problem is that when deleting the users from the Admin UI, it doesn’t remove the corresponding rows from the plugin_store_rows table, so when I come back in using the same OAuth token from our platform, current_user is populated at discourse-oauth2-basic/plugin.rb at master · discourse/discourse-oauth2-basic · GitHub, but User.where(id: current_info[:user_id]).first doesn’t find the corresponding user (because it’s been deleted and the user_id no longer matches the right record). Deleting the “offending” row from the plugin_store_rows table makes that account work.

I don’t know if the non-deletion of plugin_store_rows rows is a legitimate bug or if this is just an artifact of my testing strategy (because it’s at least that).


(Jeff Atwood) #8

Thanks for the detailed repro! :female_detective:

Would that be considered a bug @david?


(David Taylor) #9

Yes, I think so. Although it is fairly unlikely to occur in a “real” environment. It will certainly be fixed when we migrate the plugin to ManagedAuthenticator.