Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error in send_mail still writes a user, triggers duplicate error; needs txn? #414

Open
dradetsky opened this issue Mar 4, 2018 · 0 comments

Comments

@dradetsky
Copy link

I do the following:

  1. run the bare-minimum user registration app more-or-less copied from the tutorial (can make example repo if useful).
  2. send a registration message to server
  3. error raised during sending of confirmation email (because I forgot to enable console email backend)
  4. enable console email backend & restart
  5. send same registration message to server

Expected:

A user is registered and an email is printed to console

Actual:

% http $u/ra/reg/ < res/reg0.json
HTTP/1.1 400 Bad Request
Allow: POST, OPTIONS
Content-Length: 125
Content-Type: application/json
Date: Sun, 04 Mar 2018 17:11:09 GMT
Server: WSGIServer/0.2 CPython/3.6.4
Vary: Accept, Cookie
X-Frame-Options: SAMEORIGIN

{
    "email": [
        "A user is already registered with this e-mail address."
    ],
    "username": [
        "A user with that username already exists."
    ]
}

In my view, this is a bug, or at least a bad design. This may only be me messing around with this stuff on my laptop, but it's a real issue that could come up in production.

The problem is that the register method (rest_auth.registration.views.RegisterView.perform_create) writes the user and then calls complete_signup, which could error for any number of reasons, including transient errors. If it does, the user might experience the following:

  1. submits signup form
  2. sees "sorry, an error occurred"
  3. tries to signup again
  4. sees "sorry, this email is registered"
  5. tries to login
  6. sees "sorry, this account is disabled"
  7. curses repeatedly, leaves forever

I would suggest something like this:

  1. on entering perform_create we begin a db transaction.
  2. on completing everything successfully, we write the txn, creating the new registration.
  3. if there is an unhandled exception, we roll back the txn (possibly, only if ROLL_BACK_REGISTRATION_ON_ERROR is set).

I made a stab at a PR for this (which I'll submit in a sec). I haven't used django in about 5 years, so it may not be good style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant