If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Generate livechat_id when user registers (at tiki-register.php)

VERIFIED FIXED in 1.5.2

Status

support.mozilla.org Graveyard
Chat
P2
normal
VERIFIED FIXED
8 years ago
5 years ago

People

(Reporter: paulc, Assigned: paulc)

Tracking

unspecified
1.5.2
Dependency tree / graph

Details

(Whiteboard: sumo_only)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
https://wiki.mozilla.org/Support/Live_Chat/SUMO_login
Part of adding SUMO logins to livechat. See above wiki page for more details.
(Assignee)

Updated

8 years ago
No longer blocks: 503215
(Assignee)

Updated

8 years ago
Blocks: 503215
(Assignee)

Comment 1

8 years ago
Created attachment 417204 [details] [diff] [review]
update tiki-register and userslib

This patch does the stuff mentioned on the wiki:
https://wiki.mozilla.org/Support/Live_Chat/SUMO_login#tiki-register.php_.28bug_534340.29
If livechat_id already exists, the response is as if username must be changed. Seems reasonable?
Attachment #417204 - Flags: review?(james)
Attachment #417204 - Flags: review?(bugs)
(Assignee)

Comment 2

8 years ago
Created attachment 417205 [details] [diff] [review]
SQL to create livechat_id column

Corresponding SQL to attachment 417204 [details] [diff] [review].
Attachment #417205 - Flags: review?(james)
Attachment #417205 - Flags: review?(bugs)
(Assignee)

Comment 3

8 years ago
Question regarding: https://wiki.mozilla.org/Support/Live_Chat/SUMO_login#Database_.28bug_534340.29
It seems to me that tiki-register.php does NOT need access to the OpenFire database, since it creates a unique livechat_id based a the newly registered user login. I'm not sure it would be necessary to check the OpenFire database (what would we look for? potential future migrators?). Am I missing something?
The livechat_id must be unique both on itself and on Openfire's of_user database, to avoid duplicate Openfire account names.  This purpose is only temporary, as no Openfire accounts will exist after the migration.

It may be better to create temporary TikiWiki accounts for all Openfire accounts with name collisions in the automatic migration script, to avoid additional complexity here.  (Openfire accounts without name collisions will be converted automatically.)
(Assignee)

Comment 5

8 years ago
(In reply to comment #4)
So from what I understand, you want to make sure that no two users can be online where one's SUMO livechat_id == the other's Openfire user. Is that right? If so, you can avoid this by appending _of (after sign-in) for Openfire users, and additionally giving users a choice between signing in with Openfire accounts or SUMO livechat IDs (which I'm assuming you are doing anyway, while we are in this migration process). Would this work?

So it looks like we should merge non-conflicting Openfire accounts first (bug 534351).

Additionally, we can add a temporary notification at the top of tiki-register.php: "Already have an Openfire account? Transfer it over using the <link>migration process</link>".

This should be enough, right? Sounds good Matthew?
(In reply to comment #5)
> (In reply to comment #4)
> So from what I understand, you want to make sure that no two users can be
> online where one's SUMO livechat_id == the other's Openfire user. Is that
> right? If so, you can avoid this by appending _of (after sign-in) for Openfire
> users, and additionally giving users a choice between signing in with Openfire
> accounts or SUMO livechat IDs (which I'm assuming you are doing anyway, while
> we are in this migration process). Would this work?

Users won't be given a choice as to which authentication system to use.  Openfire will first check the SUMO database for a valid sign-on.  If this login fails, the server will try Openfire's of_user table.  If both fail, the user will see an authentication failed dialog.

> So it looks like we should merge non-conflicting Openfire accounts first (bug
> 534351).

Yes, this must be done first before SUMO logins will be active.

> Additionally, we can add a temporary notification at the top of
> tiki-register.php: "Already have an Openfire account? Transfer it over using
> the <link>migration process</link>".
> 
> This should be enough, right? Sounds good Matthew?

Yes, linking to that page from tiki-register sounds like a good idea.  Openfire will also send a notice to users who authenticate from the of_users table that they should migrate.
(Assignee)

Comment 7

8 years ago
I still would rather have the Openfire sign-in process handle uniques between the two databases and let users pick, rather than temporarily bloat tiki-register.php.

What do you think James?
(Assignee)

Updated

8 years ago
Blocks: 534350
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> I still would rather have the Openfire sign-in process handle uniques between
> the two databases and let users pick, rather than temporarily bloat
> tiki-register.php.
So if I understand it correctly, this is not a concern anymore. We run the automatic migration script first to help minimize potential conflicts (bug 534351) and if any problems arise with users taking someone else's preferred ID, we can handle them on a case-by-case basis using the admin interface (bug 534350). Hence the patch above should suffice (functionally...) and only the SUMO db needs to be checked against for livechat_id uniqueness.

If I hear no objections I'm going to assume this way is fine.
Openfire will consult the SUMO database first for a valid sign-in, so any conflicts generated would allow an Openfire account to be taken over.

The best way to work around this is probably to have the Openfire plugin compare the creation date of the TikiWiki account to the Openfire account.  If the TikiWiki account is newer and a matching Openfire account exists, then TikiWiki authentication should be skipped.  What do you think?
(Assignee)

Comment 10

8 years ago
That sounds good! One exception is when the TikiWiki account has been created by the migration script. So I guess you will need a way to distinguish between the two cases -- what if the migration script copies over the creationDate from Openfire (ofUser table)?
(In reply to comment #10)
> That sounds good! One exception is when the TikiWiki account has been created
> by the migration script. So I guess you will need a way to distinguish between
> the two cases -- what if the migration script copies over the creationDate from
> Openfire (ofUser table)?

The migration script will delete the user from the ofUser table, so this won't be an issue.
(Assignee)

Comment 12

8 years ago
Oops. Right :)
Comment on attachment 417205 [details] [diff] [review]
SQL to create livechat_id column

WFM.
Attachment #417205 - Flags: review?(james) → review+
Comment on attachment 417204 [details] [diff] [review]
update tiki-register and userslib

Could you change the mb_ereg_replace to have a + operator on it? I'd rather "user%#$name" => "user_name" than to "user___name". Or is that going to cause problems with the uniqueness?
(Assignee)

Comment 15

8 years ago
Created attachment 418286 [details] [diff] [review]
v2, turns all invalid characters into one underscore

Tharrrrr she goes...
Attachment #417204 - Attachment is obsolete: true
Attachment #418286 - Flags: review?(james)
Attachment #417204 - Flags: review?(james)
Attachment #417204 - Flags: review?(bugs)
(Assignee)

Updated

8 years ago
Attachment #417205 - Flags: review?(bugs)
(Assignee)

Comment 16

8 years ago
(In reply to comment #15)
> Created an attachment (id=418286) [details]
> v2, turns all invalid characters into one underscore
We decided that conflicts are not that much more likely so doing this is okay.
Comment on attachment 418286 [details] [diff] [review]
v2, turns all invalid characters into one underscore

Working for me. Matthew, this meets the requirements?
Attachment #418286 - Flags: review?(james) → review+
(Assignee)

Comment 18

8 years ago
According to Matthew, validation should follow these specs: http://xmpp.org/rfcs/rfc3920.html#nodeprep
(Assignee)

Updated

8 years ago
Depends on: 535868
(Assignee)

Comment 19

8 years ago
r58192 (trunk). Will we need this on fennec in case they use live chat before we do the merge back to trunk?
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Target Milestone: 1.5.1 → 1.5.2
Verified FIXED on trunk; Vishal and I actually verified this when we found that our new accounts let us log in :-)
Status: RESOLVED → VERIFIED
Component: Chat → Chat
Product: support.mozilla.org → support.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.