627882 - Update email and update password take an optional password

Status: RESOLVED FIXED

(Cloud Services :: Server: Core, defect)

Description

[Toby Elliott [:telliott]]

def update_email(self, user_id, email, password=None):
        if password is None:
            return False   # we need a password


This is dangerous. It's a silent failure that pretty much requires sticking prints into the code. Is there a case for this? We should do one of two things:

1) At the very least, log something somewhere that this was called without a password. 

2) make password non-optional so that it breaks if you don't provide it. This seems like a good thing to do in all these functions.

Comment 1

[Tarek Ziadé (:tarek)]

the optional password is useful only for the ldap backend. The goal is to be able to use the user auth as an alternative bind, rather than the admin.

I agree that it should not return False here, and it should do like in update_password: http://hg.mozilla.org/services/server-core/file/58b2a967aa6f/services/auth/ldapsql.py#l419

Other backend should ignore it.

Comment 2

[Tarek Ziadé (:tarek)]

Attachment: Make sure we raise an error if the password is missing when needed

Comment 3

[Toby Elliott [:telliott]]

Comment on attachment 512787 [details] [diff] [review]
Make sure we raise an error if the password is missing when needed

Your comment on the return value for one of the functions refers to a NoneImplementedError, but that's pretty minor. Rest looks good.

Comment 4

[Tarek Ziadé (:tarek)]

Partially applied, as auth.delete_user now uses the admin auth.

http://hg.mozilla.org/services/server-core/rev/1332db2f42b7