OneToOneField causes duplicate NULL key errors

[lcary]

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.

[lcary]

Note: the same problem will likely arise for the refresh token model, which declares the access_token field as OneToOne as well:

    access_token = models.OneToOneField(
        oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
        related_name="refresh_token"
    )

[n2ygk]

See #497

[n2ygk]

@phillbaker - can you take a look at this?

[phillbaker]

@n2ygk sorry, I'm pretty tied up in a bunch of things. I might be able to help review a solution, but I don't have time currently to dig into the underlying causes here.

[lcary]

@phillbaker For the record, I was able to dig into the underlying cause already. It looks like the issue was introduced by commit 2e4d15e, which features multiple nullable OneToOneField fields. In one line: if more than one of the OneToOneField values are set to NULL, which happens regularly, there is a fatal "violation of unique key constraint" error due to duplicate NULL key values. I've only tested this on MSSQL, but it likely repros for any database that enforces the unique key constraint. Our fork's fix of using a ForeignKey instead seems to be working fine for our projects, but I'm not sure if it will break anything regarding your work to "Revoke refresh tokens instead of deleting them" from the above-mentioned commit.

[n2ygk]

I believe the reason NULL is allowed is that Refresh Tokens are not always requested/granted, especially for the Implicit grant in which they aren't even allowed. PR coming, Real Soon Now.

[n2ygk]

This is a SQL Server bug. The code is correctly using OneToOneField.

SQL Server does not properly implement the UNIQUE NULL constraint in which no two NULLs are equal. There is a workaround that involves changing the CREATE UNIQUE INDEX to have WHERE _field_ is NOT NULL.

[waheedahmed]

@phillbaker We are also experiencing this problem in openedX instance looks like due to concurrent requests from mobile native apps, any timeline to fix this problem? I was able to reproduce this issue using this script:

2019-05-28 09:28:56,480 ERROR 266 [django.request] [user None] exception.py:135 - Internal Server Error: /oauth2/access_token/
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 249, in _legacy_get_response
    response = self._get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/oauth_dispatch/views.py", line 100, in dispatch
    response = super(AccessTokenView, self).dispatch(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/ratelimit/mixins.py", line 58, in dispatch
    )(super(RatelimitMixin, self).dispatch)(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/ratelimit/decorators.py", line 30, in _wrapped
    return fn(*args, **kw)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/oauth_dispatch/views.py", line 55, in dispatch
    return view(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 67, in _wrapper
    return bound_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 63, in bound_func
    return func.__get__(self, type(self))(*args2, **kwargs2)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/generic/base.py", line 88, in dispatch
    return handler(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 67, in _wrapper
    return bound_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/decorators/debug.py", line 76, in sensitive_post_parameters_wrapper
    return view(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 63, in bound_func
    return func.__get__(self, type(self))(*args2, **kwargs2)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauth2_provider/views/base.py", line 206, in post
    url, headers, body, status = self.create_token_response(request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauth2_provider/views/mixins.py", line 125, in create_token_response
    return core.create_token_response(request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauth2_provider/oauth2_backends.py", line 140, in create_token_response
    headers, extra_credentials)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/base.py", line 64, in wrapper
    return f(endpoint, uri, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/token.py", line 117, in create_token_response
    request, self.default_token_type)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py", line 68, in create_token_response
    self.request_validator.save_token(token, request)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/request_validator.py", line 246, in save_token
    return self.save_bearer_token(token, request, *args, **kwargs)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py", line 83, in save_bearer_token
    super(EdxOAuth2Validator, self).save_bearer_token(token, request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauth2_provider/oauth2_validators.py", line 542, in save_bearer_token
    source_refresh_token=refresh_token_instance,
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/oauth2_provider/oauth2_validators.py", line 574, in _create_access_token
    access_token.save()
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/base.py", line 808, in save
    force_update=force_update, update_fields=update_fields)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/base.py", line 838, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/base.py", line 924, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/base.py", line 963, in _do_insert
    using=using, raw=raw)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/query.py", line 1079, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 1112, in execute_sql
    cursor.execute(sql, params)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 101, in execute
    return self.cursor.execute(query, args)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 206, in execute
    res = self._query(query)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 312, in _query
    db.query(q)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/MySQLdb/connections.py", line 224, in query
    _mysql.connection.query(self, query)
IntegrityError: (1062, "Duplicate entry '47' for key 'source_refresh_token_id'")

CC: @n2ygk

[bradenmacdonald]

@waheedahmed Did you find a fix for this issue? We are seeing it on our Open edX devstacks (django-oauth-toolkit version 1.1.3).

[waheedahmed]

@bradenmacdonald No, this is the only workaround for SQL databases #667 (comment).

[jbaltodano2019]

I've tried to add the where _field_ IS NULL into the index but MSSQL syntax error appear. what is the right syntax to add the WHERE?

[gaboroa14]

I'm using Djongo as my Database Manager, yet I'm getting the same error here. Is there any way to fix this with MongoDB?