Download & Extend

OpenID throws an error when two accounts try to use the same OpenID

Project:Drupal core
Version:8.x-dev
Component:openid.module
Category:bug report
Priority:normal
Assigned:wojtha
Status:needs work
Issue tags:needs backport to D6, needs backport to D7, Needs tests

Issue Summary

Instead of cleanly rejecting the attempt at using the openid on two distinct accounts, the module returns a MySQL error:

user warning: Duplicate entry '(some openid url)' for key 2 query: INSERT INTO www_authmap (uid, authname, module) VALUES (1, '(some open id url)','openid') in /somepath/www/modules/openid/openid.pages.inc on line 38.

then confirms "Successfully added (some openid url)", although it was obviously not added.

Comments

#1

Subscribing.

#2

Version:6.9» 7.x-dev

Still there in 6.10 and 7.0 UNSTABLE 5

#3

Status:active» needs review

This patch catches the duplicate key exception and displays an error message. Also, it doesn't check the identity prior to contacting the OpenID Provider, because the actual claimed_id may different from the one supplied by the user. E.g. if the user just enters "yahoo.com", Yahoo responds with a claimed id that looks something like this: https://me.yahoo.com/a/15_jkHfcSwQSxcvmOujhdYhdsnwW-#a73Px

AttachmentSizeStatusTest resultOperations
openid-duplicate-key-1.patch6.87 KBIdleFailed: Failed to apply patch.View details | Re-test

#4

I updated the test to add a query string argument instead of a dummy fragment, because the fragment should be stripped according to section 7.2 of OpenID Authentication 2.0 (though they are currently not).

AttachmentSizeStatusTest resultOperations
openid-duplicate-key-2.patch7.02 KBIdleFailed: Failed to apply patch.View details | Re-test

#5

Status:needs review» needs work

The last submitted patch failed testing.

#6

Status:needs work» needs review

Reroll.

AttachmentSizeStatusTest resultOperations
openid-duplicate-key-3.patch7.11 KBIdlePassed: 12153 passes, 0 fails, 0 exceptionsView details | Re-test

#7

Status:needs review» needs work

The last submitted patch failed testing.

#8

Status:needs work» needs review

Test bot glitch.

#9

Status:needs review» needs work

The last submitted patch failed testing.

#10

Status:needs work» needs review

Bot was failed

#11

Status:needs review» needs work

The last submitted patch failed testing.

#12

Status:needs work» needs review

trying out a new testing bot that apparently wasn't working

#13

Set #473982: OpenID not checked for duplicity a duplicate of this one.

#14

Tested #6 looks fine but needs another review to rtbc

#15

Status:needs review» needs work

$exception->errorInfo[0] == 23000 is not a reliable way to detect key conflicts across databases - see #505812: PDO uses inconsistant error codes between DBs.

#16

Subscribing. In D6.15 a similar warning is issued when trying to add the same OpenID twice (within the same Drupal user account).

#17

When trying to add the same Google OpenID (https://www.google.com/accounts/o8/id) on two different accounts, both attempts create an identity using the currently logged in Google account. Of course, if one wants to have different Google OpenIDs on two different Drupal accounts, one must log out of one Google account after having created the first OpenID, then log into a second Google account before creating the second OpenID, in which case twice entering https://www.google.com/accounts/o8/id will work (both attempts will lead to different OpenID keys). Obvious, yes, but to OpenID newbies this might be confusing -- perhaps the error message might be tweaked to help users understand how OpenID works.

#18

Subscribing

#19

Assigned to:Anonymous» wojtha

Slightly different approach. I'm leaving the validation as it is, becuase it could catch some of the OpenID before whole discovery and authorization process begins and in worst case cost us only one select in DB but should save more DB requests and server load. And I'm not using PDO Exception which is not same for all databases according to #15, but the same approach like in form validation handler.

AttachmentSizeStatusTest resultOperations
openid_duplicate_key_364348.patch1.87 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch openid_duplicate_key_364348.patch. See the log in the details link for more information.View details | Re-test

#20

Status:needs work» needs review

#21

Test passed, but drupal.org doesn't respond this time because of network trouble, so the test status wasn't updated.

#22

#19: openid_duplicate_key_364348.patch queued for re-testing.

#23

Version:7.x-dev» 8.x-dev
Status:needs review» needs work
Issue tags:+needs backport to D7, +Needs tests

Sent #19 for re-test against D7. However, this needs to be re-rolled against D8 either way.

Should also be covered by tests.

#24

Suppose we should decide on a way of collision detection - I filed #1376778: Consistent 'duplicate key' detection in core

nobody click here