Update email and update password take an optional password

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: telliott, Assigned: tarek)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
(Assignee)

Comment 1

8 years ago
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.
(Assignee)

Comment 2

8 years ago
Created attachment 512787 [details] [diff] [review]
Make sure we raise an error if the password is missing when needed
Attachment #512787 - Flags: review?(telliott)
(Reporter)

Comment 3

8 years ago
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+
(Assignee)

Comment 4

8 years ago
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.