388922 - don't let admins tell bugzilla to construct invalid email addresses using emailsuffix and ensure mail is deliverable during transitions to/from email suffix

Status: NEW

(Bugzilla :: User Accounts, enhancement)

Description

[timeless]

Attachment: sql and stuff

bugs are described by gdr.
except 3 bullet 2 has a typo it says "You can't" but it should read "You can". 

This code basically:
* enable login handling to tolerate users who include emailsuffix
* changes editparams to support setters (I was going to make check_shadowdb use it, but it seem it's harmless, see notes).
* adds a checker for emailsuffix to complain if you're trying to break some of your users
* adds a setter for emailsuffix
  there are three behaviors:
  "mozilla.org" => "users.mozilla.org" this is a promotion, people will have "users." stripped from their accounts, logins change but effective email address does not.
  "users.mozilla.org" => "mozilla.org" this is a demotion, people will have "users." appended to their accounts, logins change but effective email address does not.
  "users.mozilla.org" => "people.mozilla.org" this is a change, logins will not change although the effective does change.

I don't plan to write documentation for this today.

notes:
afaict check_shadowdb has been harmless for ages:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/webtools/bugzilla/Attic&command=DIFF_FRAMESET&file=defparams.pl&rev2=1.107&rev1=1.106

Comment 1

[Max Kanat-Alexander]

Comment on attachment 273093 [details] [diff] [review]
sql and stuff

Described by gdr? What the heck does that mean?

Comment 2

[Max Kanat-Alexander]

Comment on attachment 273093 [details] [diff] [review]
sql and stuff

Wait, does this code actually manually append the emailsuffix to users in the DB, or what?

Also, I see no changes to editparams, so I don't know what you were talking about.

The correct reviewer for params changes like this would be LpSolit. I am the auth guy, it's true, but first I want LpSolit to look at the params side of things.

Comment 3

[Max Kanat-Alexander]

Also, could somebody who understands this actually make a better summary than "fix emailsuffix handling"? Bug descriptions should say *what's broken*, not your suggested resolution or just something ridiculous like "fix broken thing".

Comment 4

[timeless]

gdr is the author of the document in the url field and on the cc list.

> does this code actually manually append the emailsuffix to users in the DB

sometimes.

if the email suffix is "bugzilla.mozilla.org" and you change it to "mozilla.org"

then this code would convert users from "timeless@" to "timeless@bugzilla."

If the email suffix is "bugzilla.mozilla.org" and you change it to "bugs.mozilla.org" then it will not change the users' addresses at all.

mostly, if you change the email suffix from "@bugzilla.org" to "" it will change "timeless" to "[e-mail]".

or if you change the suffix from "" to "@bugzilla.org" it will change "[e-mail]" to "timeless".

note that it will complain if you have [e-mail] and [e-mail] and you try to set *any* suffix because it can't make that work.

that's the general idea anyway.

> Also, I see no changes to editparams

I've changed SetParam which afaiu is part of editparams to call set_emailsuffix which does the dirty work.

Comment 5

[Max Kanat-Alexander]

(In reply to comment #4)
> if the email suffix is "bugzilla.mozilla.org" and you change it to
> "mozilla.org"
> 
> then this code would convert users from "timeless@" to "timeless@bugzilla."

  Ah, okay. That's not the intention of modifying emailsuffix, and it also violates the principle of least surprise.

  I do understand that it's tricky to fix users if you have a change like that, but perhaps instead we should offer a way to mass-change users.(In reply to comment #4)

> note that it will complain if you have [e-mail] and
> [e-mail] and you try to set *any* suffix because it can't make that
> work.

  That's probably good.

Comment 6

[timeless]

i don't think it's a real concern. most people will use emailsuffix of the form "@foo", as such you'll never reach the codepath I wrote.

The only time you would hit such a codepath is if you intentionally make people use logins like "timeless@". Can you seriously imagine people doing that?

Eventually, I'd probably like the code to recognize and allow for changes from @mail.mozilla.org to @mozilla.org (by ignoring a leading @ if it's present in both cases), but that's sorta a future thing.

I don't see how it violates least surprise, if I (a user) create an account "timeless@" with email suffix "bugzilla.org" and you change the email suffix to "org", why is least surprise that I get no mail (because it's sent to timeless@org)?

Comment 7

[Frédéric Buclin]

Comment on attachment 273093 [details] [diff] [review]
sql and stuff

The patch doesn't compile. runtests.pl throws compilation errors.