Implement the latest SReg spec in Python

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

(1 attachment, 1 obsolete attachment)

(Assignee)

Comment 1

7 years ago
Created attachment 514838 [details] [diff] [review]
Refactored sreg w/ the latest infrasec reqs
Attachment #514838 - Flags: review?(telliott)
(Assignee)

Updated

7 years ago
Blocks: 636514
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.
Attachment #514838 - Flags: review?(telliott) → review+
(Assignee)

Comment 3

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

Comment 4

7 years ago
Created attachment 514846 [details] [diff] [review]
New version -- adds TEST_REMOTE support and remove the branding on the team name
Attachment #514846 - Flags: review?(telliott)
(Assignee)

Updated

7 years ago
Attachment #514838 - Attachment is obsolete: true
Attachment #514846 - Flags: review?(telliott) → review+
(Assignee)

Comment 5

7 years ago
http://hg.mozilla.org/services/server-sreg/rev/c879701bb46c
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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).
(Assignee)

Comment 7

7 years ago
(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.
You need to log in before you can comment on or make changes to this bug.