devilry / trix2

Next generation Trix. Detailed task control and statistics app for better learning outcome.
BSD 3-Clause "New" or "Revised" License
2 stars 3 forks source link

`AssertionError` from logins #128

Closed torgeirl closed 11 months ago

torgeirl commented 1 year ago

It seems many users experience a HTTP 500 error the first time they log in using the federated authentication.

It is caused by an AssertionError triggered by attempting to insert their email. It happens when trix/trix_auth /allauth_adapter.py's save_user function uses allauth/socialaccount/models.py's save function which uses allauth/account/utils.py's setup_user_email. Trace:

  1. allauth/account/utils.py in setup_user_email at line 266
  2. allauth/socialaccount/models.py in save at line 253
  3. trix/trix_auth/allauth_adapter.py in save_user at line 25
  4. allauth/socialaccount/helpers.py in _process_signup at line 47
  5. allauth/socialaccount/helpers.py in _complete_social_login at line 181
  6. allauth/socialaccount/helpers.py in complete_social_login at line 160
  7. allauth/socialaccount/providers/oauth2/views.py in dispatch at line 158
  8. allauth/socialaccount/providers/oauth2/views.py in view at line 81
  9. django/core/handlers/base.py in _get_response at line 181
  10. django/core/handlers/exception.py in inner at line 47

Line 266 in allauth/account/utils.py reads: assert not EmailAddress.objects.filter(user=user).exists()

When it yields an exception, the values it receives (among other things) an user object (<User: username@uio.no>) and an addresses object ([ <EmailAddress: username@uio.no>]) where username in both cases is a valid UiO username.

It looks to me that AssertionError is triggered due to some failed check: SELECT (1) AS "a" FROM "account_emailaddress" WHERE "account_emailaddress"."user_id" = %s LIMIT 1.

For some reason this doesn't happen in our test environment so unclear if we have some unintended differences between test and production, or if its simply «a numbers game».

torgeirl commented 11 months ago

The release notes of Django Allauth v0.55.0 mentions some changes that might be related:

Backwards incompatible changes

  • Data model changes: when ACCOUNT_UNIQUE_EMAIL=True (the default), there was a unique constraint on set on the email field of the EmailAddress model. This constraint has been relaxed, now there is a unique constraint on the combination of email and verified=True. Migrations are in place to automatically transition, but if you have a lot of accounts, you may need to take special care using CREATE INDEX CONCURRENTLY.
  • The method allauth.utils.email_address_exists() has been removed.
  • (...)
torgeirl commented 11 months ago

Hopefully solved with dbbd5bd64736447bf628b4a057fa3e9e2237eb44.