-
Notifications
You must be signed in to change notification settings - Fork 819
Description
Line L262 appears to be causing us some bugs. Having a source_refresh_token on the AccessToken as a OneToOneField seems to cause duplicate NULL key errors. Why isn't this a ForeignKey?
With any version newer than 1.1 that I have tested against MS SQL (perhaps this only appears on MS SQL because it enforces the constraint more rigidly than other databases?) we see the following error when adding or updating tokens:
('23000', "[23000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Violation of UNIQUE KEY constraint 'UQ__oauth2_p__A1C69CA6C6465E5D'. Cannot insert duplicate key in object 'dbo.oauth2_provider_accesstoken'. The duplicate key value is (<NULL>). (2627) (SQLExecDirectW)")
At first I thought it was inserting NULL tokens, but then I looked a bit deeper and noticed that the insert statements looked fine from the token perspective: they all had non-null tokens. However, the source_refresh_token_id is NULL in our case:
[07/Nov/2018 20:18:43+0000] DEBUG [django.db.backends:111] (0.015) QUERY = 'SET NOCOUNT ON INSERT INTO [oauth2_provider_accesstoken] ([user_id], [source_refresh_token_id], [token], [application_id], [expires], [scope], [created], [updated]) VALUES (%s, %s, %s, %s, %s, %s, %s, %s); SELECT CAST(SCOPE_IDENTITY() AS bigint)' - PARAMS = (2034, None, '<redacted>', None, datetime.datetime(2018, 11, 7, 22, 3, 33), '<redacted>, datetime.datetime(2018, 11, 7, 20, 18, 43, 670891), datetime.datetime(2018, 11, 7, 20, 18, 43, 670906)); args=(2034, None, '<redacted>', None, datetime.datetime(2018, 11, 7, 22, 3, 33), '<redacted>', datetime.datetime(2018, 11, 7, 20, 18, 43, 670891), datetime.datetime(2018, 11, 7, 20, 18, 43, 670906))
Looking deeper, I noticed that there was an extra unique constraint at the database level, at which point I noticed in the migrations that OneToOneField is used, which enforces that unique constraint. This appears to have been added in commit 2e4d15e. That commit explains why I started seeing this issue only after upgrading beyond version 1.1.0 of the library. I don't think it makes sense to enforce the unique=True rule (which occurs by default for OneToOneField fields) if the field also allows NULL value. To be clear, the code that's causing the issue is:
source_refresh_token = models.OneToOneField(
# unique=True implied by the OneToOneField
oauth2_settings.REFRESH_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
related_name="refreshed_access_token"
)
For now, I'm working around this by forking the library internally, and changing the OneToOneField to a ForeignKey, which seems to work fine.