Make sure password_reset is a public API

RESOLVED FIXED

Status

Cloud Services
Server: Registration
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: tarek, Assigned: tarek)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

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

Updated

7 years ago
No longer blocks: 624907
(Assignee)

Updated

7 years ago
Blocks: 624907
(Assignee)

Comment 1

7 years ago
Created attachment 503127 [details] [diff] [review]
Make sure the password_reset API is public
Attachment #503127 - Flags: review?(telliott)
(Assignee)

Comment 2

7 years ago
Created attachment 503132 [details] [diff] [review]
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.
Attachment #503132 - Flags: review?(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 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.
Attachment #503127 - Flags: review?(telliott) → review+
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.
Attachment #503132 - Flags: review?(telliott) → review+
(Assignee)

Comment 6

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

Comment 7

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

Comment 8

7 years ago
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
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.