Looking at the commit, the code looks ok. I do have issue with the fact that you're renaming the error message "invalid_login_or_password" to "invalid_username_or_password" but still leaving a call to the former name in Bugzilla/Auth.pm (line 188).

When I try to use the feature, I'm sent to github.cgi which displays the following error:

DBD::Pg::db do failed: ERREUR: erreur de syntaxe sur ou près de « DUPLICATE »
LINE 1: ...token_data (token, extra_data) VALUES ($1, $2) ON DUPLICATE ...
^ [for Statement "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON DUPLICATE KEY UPDATE extra_data = ?"] at Bugzilla/DB.pm line 52.
Bugzilla::DB::ANON(Bugzilla::DB::Pg=HASH(0x56350b182010), "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON D"..., undef, "KK1TSdc21g", "{"type":"github_login","target_uri":"http://localhost/bugzill"..., "{"type":"github_login","target_uri":"http://localhost/bugzill"...) called at Bugzilla/Token.pm line 478
Bugzilla::Token::set_token_extra_data("KK1TSdc21g", HASH(0x56350b234290)) called at /var/www/html/bugzilla/github.cgi line 55

(Sorry for the french but that's the way my system is set up.)

The first bit (Auth.pm line 188) is something I missed when reworking it against 5.2, I'll fix shortly.

For the second, are you using Postgres? If so, try:

mistotebe@88b0540

@eseyman

The DB error appears to be from a mysql-ism that crept into BMO:

http://www.mysqltutorial.org/mysql-insert-or-update-on-duplicate-key-update/

@eseyman

In fact, this looks like a divergence between the two implementations. Is there something that can be keyed off of to indicate mysql/mariadb vs postgres?

https://en.wikipedia.org/wiki/Merge_(SQL)#Other_non-standard_implementations

@eseyman

In fact, how does this look to you?

+  if (Bugzilla->dbh->isa('Bugzilla::DB::Mysql')) {
+    Bugzilla->dbh->do(
+      "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON DUPLICATE KEY UPDATE extra_data = ?",
+      undef, $token, $data, $data);
+  } else {
+    Bugzilla->dbh->do(
+      "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON CONFLICT (token) DO UPDATE SET extra_data = ?",
+      undef, $token, $data, $data);
   }

FTR, I'm using Bugzilla on CentOS using the shipped version of PostgreSQL.

I'm not a huge fan of mysql-isms (other than in Bugzilla::DB::MySQL) so I would prefer having one SQL statement that works everywhere but I'll defer to @dylanwh here.

@eseyman Looking at the BMO tree, it looks like @globau introduced this mysqlism. Perhaps they have an alternate solution that's not SQL version dependent.

@dylanwh says on IRC:

so my suggestion since there are competing syntaxes is that we add a bz_upsert() method to Bugzilla::DB which returns the proper upsert syntax for MySQL (REPLACE INTO), PostgreSQL (the chat above), etc.

Work in progress in #90

There is a small change also needed in the near future - changing how we use the oauth token.

I've tested this patch and it seems to work:
mistotebe@2851ea7

Where is the BMO codebase so it can be merged there?

@mistotebe Well, BMO is an internal bugzilla version for Mozilla. We just want this patched in mainline Bugzilla. ;)