[jitsi-dev] [patch] Inband changing of jabber account password


#1

Hello everyone,

The following patch adds an option to the Jabber account editing
window that allows the change of the account password. It uses the
smack.AccountManager.changePassword() method, which in turn uses the
inband method described in XEP-077 [1].

The interface is a dialog box with just two fields for password and
password confirmation. After a successful change with the server, it
updates the locally stored password if there is one.

Any critique and comments will be appreciated. Particularly, I expect
the following parts to need to be discussed:
  --The part which hides the the new option for gmail.com,
chat.facebook.com and talk.google.com accounts (since none of them
support password changes)
  --The whole user interface
  --The util package imported in
OperationSetChangePasswordJabberImpl.java (should it be
net.java.sip.communicator.util or org.jitsi.util?)

I think the code is mostly straight forward and doesn't need much
explanation, but if you have any questions I'll be happy to explain.

Boris

[1] http://xmpp.org/extensions/xep-0077.html#usecases-changepw

xmpp-change-password.patch (22.2 KB)


#2

Is this going to be released in a update, a download on Jitsi's website, or
just this patch only?

···

On Fri, Aug 3, 2012 at 11:25 AM, Boris Grozev <boris@mustelinae.net> wrote:

Hello everyone,

The following patch adds an option to the Jabber account editing
window that allows the change of the account password. It uses the
smack.AccountManager.changePassword() method, which in turn uses the
inband method described in XEP-077 [1].

The interface is a dialog box with just two fields for password and
password confirmation. After a successful change with the server, it
updates the locally stored password if there is one.

Any critique and comments will be appreciated. Particularly, I expect
the following parts to need to be discussed:
  --The part which hides the the new option for gmail.com,
chat.facebook.com and talk.google.com accounts (since none of them
support password changes)
  --The whole user interface
  --The util package imported in
OperationSetChangePasswordJabberImpl.java (should it be
net.java.sip.communicator.util or org.jitsi.util?)

I think the code is mostly straight forward and doesn't need much
explanation, but if you have any questions I'll be happy to explain.

Boris

[1] http://xmpp.org/extensions/xep-0077.html#usecases-changepw


#3

Hey Boris,

Hello everyone,

The following patch adds an option to the Jabber account editing
window that allows the change of the account password. It uses the
smack.AccountManager.changePassword() method, which in turn uses the
inband method described in XEP-077 [1].

A long awaited feature!

Applied, committed and acked!

The interface is a dialog box with just two fields for password and
password confirmation. After a successful change with the server, it
updates the locally stored password if there is one.

Any critique and comments will be appreciated.

I already mentioned offlist that your patch looks great. Good job!

I've applied a couple of minor changes ... not really worth mentioning
here but you'd need to update your local sandbox.

Particularly, I expect
the following parts to need to be discussed:
  --The part which hides the the new option for gmail.com,
chat.facebook.com and talk.google.com accounts (since none of them
support password changes)

Right. We discussed this offlist and agreed to add a check that sends a
disco info request to the server (like the ones we use to query contact
capabilities) and verifies that it has support for 'jabber:iq:register'.

  --The whole user interface

It's OK. We could add the password strength meter that we use for our
master password but it can wait.

It may also be worth using the password dialog that we generally use for
entering passwords. Alternately you could export the one that we use for
setting a master password and place it in util.

  --The util package imported in
OperationSetChangePasswordJabberImpl.java (should it be
net.java.sip.communicator.util or org.jitsi.util?)

You got it right. The jabber bundle imports the
net.java.sip.communicator.util package so that's what you should use. We
should fix this in the future.

One more thing, in OperationSetChangePasswordJabberImpl.changePassword()
it would be worth making sure that the provider is signed in and
throwing an IllegalStateException if this is not the case.

I think the code is mostly straight forward and doesn't need much
explanation, but if you have any questions I'll be happy to explain.

Thanks again for a job well done!

Emil

···

On 03.08.12, 17:25, Boris Grozev wrote:


#4

Is this going to be released in a update, a download on Jitsi's website, or
just this patch only?

It is going to be integrated. I'll be reviewing it later this week ...
unless someone beats me to it.

Emil

···

On Tue, Aug 7, 2012 at 6:46 AM, Darin Cawthon <dccawthon@gmail.com> wrote:

On Fri, Aug 3, 2012 at 11:25 AM, Boris Grozev <boris@mustelinae.net> wrote:

Hello everyone,

The following patch adds an option to the Jabber account editing
window that allows the change of the account password. It uses the
smack.AccountManager.changePassword() method, which in turn uses the
inband method described in XEP-077 [1].

The interface is a dialog box with just two fields for password and
password confirmation. After a successful change with the server, it
updates the locally stored password if there is one.

Any critique and comments will be appreciated. Particularly, I expect
the following parts to need to be discussed:
  --The part which hides the the new option for gmail.com,
chat.facebook.com and talk.google.com accounts (since none of them
support password changes)
  --The whole user interface
  --The util package imported in
OperationSetChangePasswordJabberImpl.java (should it be
net.java.sip.communicator.util or org.jitsi.util?)

I think the code is mostly straight forward and doesn't need much
explanation, but if you have any questions I'll be happy to explain.

Boris

[1] http://xmpp.org/extensions/xep-0077.html#usecases-changepw


#5

Applied, committed and acked!

Great, thank you!

Right. We discussed this offlist and agreed to add a check that sends a
disco info request to the server (like the ones we use to query contact
capabilities) and verifies that it has support for 'jabber:iq:register'.

OK.

It's OK. We could add the password strength meter that we use for our
master password but it can wait.

It may also be worth using the password dialog that we generally use for
entering passwords. Alternately you could export the one that we use for
setting a master password and place it in util.

OK. I will make these modification when I can.

One more thing, in OperationSetChangePasswordJabberImpl.changePassword()
it would be worth making sure that the provider is signed in and
throwing an IllegalStateException if this is not the case.

That's the current behavior, but I guess it's not explained well.
Smack's changePassword method throws it, and
OperationSetChangePasswordJabberImpl.changePassword just lets it go
through.

Boris

···

On 8/7/12, Emil Ivov <emcho@jitsi.org> wrote:


#6

Hello,

Sorry for the long delay. Here's a new patch that implements some of
what we discussed. The main changes are:

Add a net.java.sip.communicator.util.swing.PasswordChangeDialog class
(based on plugin.securityconfig.masterpassword.MasterPasswordChangeDialog)
that can be extended and used whenever a password change dialog is
required.

Use that dialog for the master password and jabber password changes.

Update the user interface for the jabber password change -- show a
grayed out button by default, only enable it if the server supports
registrations.

Again, any comments are appreciated, and I'm ready to answer any questions.

Boris

password-change-dialog.patch (60 KB)


#7

Hey Boris,

Thanks for taking care of this. Your patch is now applied committed and
acked.

The only (minor) issue that I currently see is about the "change account
password" button state. It basically never changes. So, if I open the
form while my accounts are still loading the button is disabled and then
it doesn't get enabled after they connect.

The same goes for the text that explains why it is disabled. For a Gmail
account for example, it does not say "password change not supported"
unless I close and reopen it.

Sometimes that same text, doesn't show at all. I am not sure when that
happens.

None of the above are particularly urgent though, so you can just have a
look at them when you can.

Cheers and thanks again,
Emil

···

On 19.08.12, 22:57, Boris Grozev wrote:

Hello,

Sorry for the long delay. Here's a new patch that implements some of
what we discussed. The main changes are:

Add a net.java.sip.communicator.util.swing.PasswordChangeDialog class
(based on plugin.securityconfig.masterpassword.MasterPasswordChangeDialog)
that can be extended and used whenever a password change dialog is
required.

Use that dialog for the master password and jabber password changes.

Update the user interface for the jabber password change -- show a
grayed out button by default, only enable it if the server supports
registrations.

Again, any comments are appreciated, and I'm ready to answer any questions.

Boris


#8

Great, thanks.

Boris

···

On Thu, Sep 13, 2012 at 3:40 PM, Emil Ivov <emcho@jitsi.org> wrote:

Hey Boris,

Thanks for taking care of this. Your patch is now applied committed and
acked.