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.
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.
Created attachment 512787 [details] [diff] [review] Make sure we raise an error if the password is missing when needed
Attachment #512787 - Flags: review?(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.
Attachment #512787 - Flags: review?(telliott) → review+
Partially applied, as auth.delete_user now uses the admin auth. http://hg.mozilla.org/services/server-core/rev/1332db2f42b7
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.