Code

#20541 closed Cleanup/optimization (fixed)

Differenciate user from superuser creation in django.contrib.auth at a signal level

Reported by: antonio@… Owned by: bak1an
Component: contrib.auth Version: 1.5
Severity: Normal Keywords:
Cc: antonbaklanov@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Hi all,

Currently the steps to create a superuser (auth.models.User.create_superuser) are:

1) Create user
2) Save User
3) Assign staff, active, superuser
4) Save again

When the first save happens the post_save signal is raised, but the user is not a superuser yet. The signal will get raised again after a few lines, which is wasteful. My proposal is to implement superuser creation like this:

class UserManager(BaseUserManager):

    def create_user(self, username, email=None, password=None, save=True, **extra_fields):
        """
        Creates and saves a User with the given username, email and password.
        """
        now = timezone.now()
        if not username:
            raise ValueError('The given username must be set')
        email = self.normalize_email(email)
        user = self.model(username=username, email=email,
                          is_staff=False, is_active=True, is_superuser=False,
                          last_login=now, date_joined=now, **extra_fields)

        user.set_password(password)
        if save:
                user.save(using=self._db)
        return user

    def create_superuser(self, username, email, password, **extra_fields):
        u = self.create_user(username, email, password, save=False, **extra_fields)
        u.is_staff = True
        u.is_active = True
        u.is_superuser = True
        u.save(using=self._db)
        return u

Now when post_save is fired, one can check is_superuser, is_staff or is_active and take action based on that.

Attachments (0)

Change History (3)

comment:1 Changed 11 months ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I fully accept that the underlying problem -- that saving a superuser involves 2 saves -- should be solved. I'm just not completely convinced about the solution. The extra save=True argument seems messy to me.

Personally, I'd rather see the 'guts' of create_user factored out into in internal method that is called by both create_user and create_superuser - that way, the public call to create_user would end up looking a lot like create_superuser, but with reversed boolean values.

comment:2 Changed 10 months ago by bak1an

  • Cc antonbaklanov@… added
  • Owner changed from nobody to bak1an
  • Status changed from new to assigned

here is my pull request for this https://github.com/django/django/pull/1305

comment:3 Changed 10 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In cab333cb16656cbb7d2bcfb06b2f7aeab9bac7af:

Fixed #20541 -- don't raise db signals twice when creating superuser

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.