625001 - Make sure password_reset is a public API

Status: RESOLVED FIXED

(Cloud Services :: Server: Registration, defect)

Description

[Tarek Ziadé (:tarek)]

The Password reset process needs this API to be public.

We also need to add a test in Hudson that will exercise this in dev & stage

Comment 1

[Tarek Ziadé (:tarek)]

Attachment: Make sure the password_reset API is public

Comment 2

[Tarek Ziadé (:tarek)]

Attachment: Make sure the Hudson functional test suite exercise the password_reset API

The test is not complete because:
- we don't have a capability page to know if the server supports , so we make some assumptions
- we don't call yet the REAL UI which is another PHP web page -- once this is available I'll adapt the test

Although this test is good enough to exercise the server to make sure the password_reset API fails on unknown user *and* works on an existing user.

I've also started to clean up the way the users are deleted to avoid leaving users in the DB if the test fails.

Comment 3

[Toby Elliott [:telliott]]

Comment on attachment 503127 [details] [diff] [review]
Make sure the password_reset API is public

+        user_name = request.sync_info['username']
+        user_id = self.auth.get_user_id(user_name)
+        user_name, user_email = self.auth.get_user_info(user_id)

This trio is a little weirdly loopy, though I understand why. Perhaps we should just add a get_user_email into auth.

Of course, eventually that should all go away anyway. Other than that, this seems fine.

Comment 4

[Toby Elliott [:telliott]]

Comment on attachment 503127 [details] [diff] [review]
Make sure the password_reset API is public

+        user_name = request.sync_info['username']
+        user_id = self.auth.get_user_id(user_name)
+        user_name, user_email = self.auth.get_user_info(user_id)

This trio is a little weirdly loopy, though I understand why. Perhaps we should just add a get_user_email into auth.

Of course, eventually that should all go away anyway. Other than that, this seems fine.

Comment 5

[Toby Elliott [:telliott]]

Comment on attachment 503132 [details] [diff] [review]
Make sure the Hudson functional test suite exercise the password_reset API

+        # unknown user
+        req = urllib2.Request("%s/xxx/password_reset" % root_url)

Are we certain there won't be an actual user called xxx? It's unlikely, but just checking.

Other than that (and random newlines at the end, but whatever), all good.

Comment 6

[Tarek Ziadé (:tarek)]

(In reply to comment #4)
> Comment on attachment 503127 [details] [diff] [review]
> Make sure the password_reset API is public
> 
> +        user_name = request.sync_info['username']
> +        user_id = self.auth.get_user_id(user_name)
> +        user_name, user_email = self.auth.get_user_info(user_id)
> 
> This trio is a little weirdly loopy, though I understand why. Perhaps we should
> just add a get_user_email into auth.
> 

This is not optimal for sure. 

Another solution would be to add an optional "fields" argument to get_user_info() ala ldapsearch, to filter out the fields we want to get back:

  user_email = self.auth.get_user_info(user_id, fields=['email'])

This can be useful as long as we're able to define a generic list of fields for the user, every backend can translate to its own internal representation.

Comment 7

[Tarek Ziadé (:tarek)]

(In reply to comment #5)
> Comment on attachment 503132 [details] [diff] [review]
> Make sure the Hudson functional test suite exercise the password_reset API
> 
> +        # unknown user
> +        req = urllib2.Request("%s/xxx/password_reset" % root_url)
> 
> Are we certain there won't be an actual user called xxx? It's unlikely, but
> just checking.

I'll make it a random name + check its presence so we're safe on this.

Comment 8

[Tarek Ziadé (:tarek)]

Commited in :
- http://hg.mozilla.org/services/server-reg/rev/f61da9cb3871
- http://hg.mozilla.org/services/server-full/rev/bfc5f2ae3c08
- http://hg.mozilla.org/services/server-full/rev/883f221161bc

Added Bug 625347 to discuss the auth interface