search

python-social-auth / social-app-django

Python Social Auth - Application - Django
BSD 3-Clause "New" or "Revised" License
2.03k stars 380 forks source link

Accidental incompatible behavior change in version 5.4.1 #568

Closed vainu-arto closed 6 months ago

vainu-arto commented 6 months ago

Expected behaviour

We have a client implementation for an OAuth2 provider that returns the user id as an integer instead of a string. Before commit 31c3e0c7edb187004d8abbde7e9c4f7ef9098138 this worked seamlessly.

Actual behaviour

After that commit the fact that the user id is a duplicate isn't detected until actually attempting to save it to the database (as a violation of the unique constraint).

What are the steps to reproduce this issue?

Input a duplicate user_id as an integer.

Any logs, error output, etc?

  File "/usr/local/lib/python3.11/dist-packages/social_core/actions.py", line 49, in do_complete
    user = backend.complete(user=user, redirect_name=redirect_name, *args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/base.py", line 39, in complete
    return self.auth_complete(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/utils.py", line 253, in wrapper
    return func(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/oauth.py", line 427, in auth_complete
    return self.do_auth(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/utils.py", line 253, in wrapper
    return func(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/oauth.py", line 440, in do_auth
    return self.strategy.authenticate(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_django/strategy.py", line 104, in authenticate
    return authenticate(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/views/decorators/debug.py", line 42, in sensitive_variables_wrapper
    return func(*func_args, **func_kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/contrib/auth/__init__.py", line 77, in authenticate
    user = backend.authenticate(request, **credentials)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/base.py", line 83, in authenticate
    return self.pipeline(pipeline, *args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/base.py", line 86, in pipeline
    out = self.run_pipeline(pipeline, pipeline_index, *args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/backends/base.py", line 118, in run_pipeline
    result = func(*args, **out) or {}
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_core/pipeline/social_auth.py", line 36, in associate_user
    social = backend.strategy.storage.user.create_social_auth(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/social_django/storage.py", line 147, in create_social_auth
    social_auth = cls.objects.create(user=user, uid=uid, provider=provider)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/query.py", line 658, in create
    obj.save(force_insert=True, using=self.db)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/base.py", line 814, in save
    self.save_base(
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/base.py", line 877, in save_base
    updated = self._save_table(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/base.py", line 1020, in _save_table
    results = self._do_insert(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/base.py", line 1061, in _do_insert
    return manager._insert(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/query.py", line 1805, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/models/sql/compiler.py", line 1822, in execute_sql
    cursor.execute(sql, params)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/django/db/backends/sqlite3/base.py", line 328, in execute
    return super().execute(query, params)
    ^^^^^^^^^^^^^^^^^
django.db.utils.IntegrityError: UNIQUE constraint failed: social_auth_usersocialauth.provider, social_auth_usersocialauth.uid

Any other comments?

Is the correct solution to restore the type conversion to the library code (perhaps just by doing str(uid) in get_social_auth()), or is it to codify that BaseAuth.get_user_id() must always return a string?

nijel commented 6 months ago

As user ID is stored as a string in the database, it should really be a string. I think https://github.com/python-social-auth/social-core/pull/905 would be the way to address this.

vainu-arto commented 6 months ago

As user ID is stored as a string in the database, it should really be a string. I think python-social-auth/social-core#905 would be the way to address this.

That looks good to me, a nicer developer experience than requiring that each backend does that itself.