636513 - Implement the latest SReg spec in Python

Status: RESOLVED FIXED

(Cloud Services :: Server: Registration, defect)

Description

[Tarek Ziadé (:tarek)]

See: https://wiki.mozilla.org/Services/Sync/SRegAPI

Comment 1

[Tarek Ziadé (:tarek)]

Attachment: Refactored sreg w/ the latest infrasec reqs

Comment 2

[Toby Elliott [:telliott]]

Comment on attachment 514838 [details] [diff] [review]
Refactored sreg w/ the latest infrasec reqs

Looks good. Couple notes:

+        # does the user has a valid email ?
+        __, user_email = self.auth.get_user_info(user_id)
+        if user_email is None:
+            raise HTTPJsonBadRequest(WEAVE_NO_EMAIL_ADRESS)

Technically, we're not validating email here, just determining that they have one. I don't know that we care enough to validate, but the option is available to us.


+The ${product} Team

At this point, I'm pretty sure that we can just use "Services". This used to be a variable because we were swapping over from Weave to Sync, but now that this is generic, I think we're set.

Comment 3

[Tarek Ziadé (:tarek)]

(In reply to comment #2)
> Comment on attachment 514838 [details] [diff] [review]
> Refactored sreg w/ the latest infrasec reqs
> 
> Looks good. Couple notes:
> 
> +        # does the user has a valid email ?
> +        __, user_email = self.auth.get_user_info(user_id)
> +        if user_email is None:
> +            raise HTTPJsonBadRequest(WEAVE_NO_EMAIL_ADRESS)
> 
> Technically, we're not validating email here, just determining that they have
> one. I don't know that we care enough to validate, but the option is available
> to us.

Yes that's what I am doing, just checking that the mail is present. Will fix the comment

> 
> 
> +The ${product} Team
> 
> At this point, I'm pretty sure that we can just use "Services". This used to be
> a variable because we were swapping over from Weave to Sync, but now that this
> is generic, I think we're set.

OK for the body, but for the mail subject, I am not sure if  "Resetting your Services password" is very meaningful.

Comment 4

[Tarek Ziadé (:tarek)]

Attachment: New version -- adds TEST_REMOTE support and remove the branding on the team name

Comment 5

[Tarek Ziadé (:tarek)]

http://hg.mozilla.org/services/server-sreg/rev/c879701bb46c

Comment 6

[Mike Connor [:mconnor]]

Can we not take branding out of config?  For _us_ it's fine, but maybe not for some other random deployment (i.e. inside of a corporation).

Comment 7

[Tarek Ziadé (:tarek)]

(In reply to comment #6)
> Can we not take branding out of config?  For _us_ it's fine, but maybe not for
> some other random deployment (i.e. inside of a corporation).

so, as I did it, there are two levels of configuration:
- "product" can be changed in the config file, and is used in the mail subject and in the body.
- the mail body itself is a template file that can be changed - http://hg.mozilla.org/services/server-sreg/file/tip/syncsreg/templates/password_reset_mail.mako

And we don't have anymore "weave" or "sync" word harcoded in the code. 

Granted, this template is kinda treated as a source file in our releases, so it means that corps will have to go there and change the file, and maybe get annoyed on the next update.

So I guess we could also allow the configuration of the path of the template, so people can provide their own version.